Merge "Test inlining of virtual interface methods"
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 4755dca..8069844 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -63,6 +63,7 @@
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.CfgPrinter;
+import com.android.tools.r8.utils.CollectionUtils;
import com.android.tools.r8.utils.ExceptionUtils;
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.InternalOptions;
@@ -275,6 +276,7 @@
RootSet rootSet;
String proguardSeedsData = null;
timing.begin("Strip unused code");
+ Set<DexType> classesToRetainInnerClassAttributeFor = null;
try {
Set<DexType> missingClasses = appView.appInfo().getMissingClasses();
missingClasses = filterMissingClasses(
@@ -351,7 +353,14 @@
new AbstractMethodRemover(appView.appInfo().withLiveness()).run();
}
- new AnnotationRemover(appView.appInfo().withLiveness(), appView.graphLense(), options)
+ classesToRetainInnerClassAttributeFor =
+ AnnotationRemover.computeClassesToRetainInnerClassAttributeFor(
+ appView.appInfo().withLiveness(), options);
+ new AnnotationRemover(
+ appView.appInfo().withLiveness(),
+ appView.graphLense(),
+ options,
+ classesToRetainInnerClassAttributeFor)
.ensureValid(compatibility)
.run();
@@ -530,6 +539,9 @@
new SourceFileRewriter(appView.appInfo(), options).run();
timing.end();
+ // Collect the already pruned types before creating a new app info without liveness.
+ Set<DexType> prunedTypes = appView.withLiveness().appInfo().getPrunedTypes();
+
if (!options.mainDexKeepRules.isEmpty()) {
appView.setAppInfo(new AppInfoWithSubtyping(application));
// No need to build a new main dex root set
@@ -593,7 +605,9 @@
appViewWithLiveness.setAppInfo(
appViewWithLiveness
.appInfo()
- .prunedCopyFrom(application, pruner.getRemovedClasses()));
+ .prunedCopyFrom(
+ application,
+ CollectionUtils.mergeSets(prunedTypes, pruner.getRemovedClasses())));
// Print reasons on the application after pruning, so that we reflect the actual result.
if (whyAreYouKeepingConsumer != null) {
@@ -603,7 +617,12 @@
}
}
// Remove annotations that refer to types that no longer exist.
- new AnnotationRemover(appView.appInfo().withLiveness(), appView.graphLense(), options)
+ assert classesToRetainInnerClassAttributeFor != null;
+ new AnnotationRemover(
+ appView.appInfo().withLiveness(),
+ appView.graphLense(),
+ options,
+ classesToRetainInnerClassAttributeFor)
.run();
if (!mainDexClasses.isEmpty()) {
// Remove types that no longer exists from the computed main dex list.
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index b0cef90..f35021b 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.5.9-dev";
+ public static final String LABEL = "1.5.10-dev";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
index e55fc6e..20f4bc2 100644
--- a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
+++ b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
@@ -161,6 +161,9 @@
public DexType parsedTypeName(String name) {
DexType type = appInfo.dexItemFactory.createType(getDescriptorFromClassBinaryName(name));
type = appView.graphLense().lookupType(type);
+ if (appInfo.wasPruned(type)) {
+ type = appInfo.dexItemFactory.objectType;
+ }
DexString renamedDescriptor = renaming.getOrDefault(type, type.descriptor);
renamedSignature.append(getClassBinaryNameFromDescriptor(renamedDescriptor.toString()));
return type;
@@ -185,8 +188,16 @@
// Pick the renamed inner class from the fully renamed binary name.
String fullRenamedBinaryName =
getClassBinaryNameFromDescriptor(renamedDescriptor.toString());
- renamedSignature.append(
- fullRenamedBinaryName.substring(enclosingRenamedBinaryName.length() + 1));
+ int innerClassPos = enclosingRenamedBinaryName.length() + 1;
+ if (innerClassPos < fullRenamedBinaryName.length()) {
+ renamedSignature.append(fullRenamedBinaryName.substring(innerClassPos));
+ } else {
+ reporter.warning(
+ new StringDiagnostic(
+ "Should have retained InnerClasses attribute of " + type + ".",
+ appView.appInfo().originFor(type)));
+ renamedSignature.append(name);
+ }
} else {
// Did not find the class - keep the inner class name as is.
// TODO(110085899): Warn about missing classes in signatures?
diff --git a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
index 3fa9db5..25e0832 100644
--- a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
+++ b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.shaking;
+import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.DexAnnotation;
import com.android.tools.r8.graph.DexAnnotationElement;
@@ -18,7 +19,12 @@
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
+import java.util.Collections;
+import java.util.IdentityHashMap;
import java.util.List;
+import java.util.Map;
+import java.util.Set;
public class AnnotationRemover {
@@ -26,12 +32,18 @@
private final GraphLense lense;
private final ProguardKeepAttributes keep;
private final InternalOptions options;
+ private final Set<DexType> classesToRetainInnerClassAttributeFor;
- public AnnotationRemover(AppInfoWithLiveness appInfo, GraphLense lense, InternalOptions options) {
+ public AnnotationRemover(
+ AppInfoWithLiveness appInfo,
+ GraphLense lense,
+ InternalOptions options,
+ Set<DexType> classesToRetainInnerClassAttributeFor) {
this.appInfo = appInfo;
this.lense = lense;
this.keep = options.getProguardConfiguration().getKeepAttributes();
this.options = options;
+ this.classesToRetainInnerClassAttributeFor = classesToRetainInnerClassAttributeFor;
}
/**
@@ -140,6 +152,79 @@
return this;
}
+ private static boolean hasGenericEnclosingClass(
+ DexProgramClass clazz,
+ Map<DexType, DexProgramClass> enclosingClasses,
+ Set<DexProgramClass> genericClasses) {
+ while (true) {
+ DexProgramClass enclosingClass = enclosingClasses.get(clazz.type);
+ if (enclosingClass == null) {
+ return false;
+ }
+ if (genericClasses.contains(enclosingClass)) {
+ return true;
+ }
+ clazz = enclosingClass;
+ }
+ }
+
+ private static boolean hasSignatureAnnotation(DexProgramClass clazz, DexItemFactory itemFactory) {
+ for (DexAnnotation annotation : clazz.annotations.annotations) {
+ if (DexAnnotation.isSignatureAnnotation(annotation, itemFactory)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ public static Set<DexType> computeClassesToRetainInnerClassAttributeFor(
+ AppInfoWithLiveness appInfo, InternalOptions options) {
+ // In case of minification for certain inner classes we need to retain their InnerClass
+ // attributes because their minified name still needs to be in hierarchical format
+ // (enclosing$inner) otherwise the GenericSignatureRewriter can't produce the correct,
+ // renamed signature.
+
+ // More precisely:
+ // - we're going to retain the InnerClass attribute that refers to the same class as 'inner'
+ // - for live, inner, nonstatic classes
+ // - that are enclosed by a class with a generic signature.
+
+ // In compat mode we always keep all InnerClass attributes (if requested).
+ // If not requested we never keep any. In these cases don't compute eligible classes.
+ if (options.forceProguardCompatibility
+ || !options.getProguardConfiguration().getKeepAttributes().innerClasses) {
+ return Collections.emptySet();
+ }
+
+ // Build lookup table and set of the interesting classes.
+ // enclosingClasses.get(clazz) gives the enclosing class of 'clazz'
+ Map<DexType, DexProgramClass> enclosingClasses = new IdentityHashMap<>();
+ Set<DexProgramClass> genericClasses = Sets.newIdentityHashSet();
+
+ Iterable<DexProgramClass> programClasses = appInfo.classes();
+ for (DexProgramClass clazz : programClasses) {
+ if (hasSignatureAnnotation(clazz, options.itemFactory)) {
+ genericClasses.add(clazz);
+ }
+ for (InnerClassAttribute innerClassAttribute : clazz.getInnerClasses()) {
+ if ((innerClassAttribute.getAccess() & Constants.ACC_STATIC) == 0
+ && innerClassAttribute.getOuter() == clazz.type) {
+ enclosingClasses.put(innerClassAttribute.getInner(), clazz);
+ }
+ }
+ }
+
+ Set<DexType> result = Sets.newIdentityHashSet();
+ for (DexProgramClass clazz : programClasses) {
+ if (clazz.getInnerClassAttributeForThisClass() != null
+ && appInfo.liveTypes.contains(clazz.type)
+ && hasGenericEnclosingClass(clazz, enclosingClasses, genericClasses)) {
+ result.add(clazz.type);
+ }
+ }
+ return result;
+ }
+
public void run() {
for (DexProgramClass clazz : appInfo.classes()) {
stripAttributes(clazz);
@@ -208,6 +293,15 @@
return false;
}
+ private static boolean hasInnerClassesFromSet(DexProgramClass clazz, Set<DexType> innerClasses) {
+ for (InnerClassAttribute attr : clazz.getInnerClasses()) {
+ if (attr.getOuter() == clazz.type && innerClasses.contains(attr.getInner())) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private void stripAttributes(DexProgramClass clazz) {
// If [clazz] is mentioned by a keep rule, it could be used for reflection, and we therefore
// need to keep the enclosing method and inner classes attributes, if requested. In Proguard
@@ -215,15 +309,40 @@
// To ensure reflection from both inner to outer and and outer to inner for kept classes - even
// if only one side is kept - keep the attributes is any class mentioned in these attributes
// is kept.
- if (appInfo.isPinned(clazz.type)
- || enclosingMethodPinned(clazz)
- || innerClassPinned(clazz)
- || options.forceProguardCompatibility) {
+ boolean keptAnyway =
+ appInfo.isPinned(clazz.type)
+ || enclosingMethodPinned(clazz)
+ || innerClassPinned(clazz)
+ || options.forceProguardCompatibility;
+ boolean keepForThisInnerClass = false;
+ boolean keepForThisEnclosingClass = false;
+ if (!keptAnyway) {
+ keepForThisInnerClass = classesToRetainInnerClassAttributeFor.contains(clazz.type);
+ keepForThisEnclosingClass =
+ hasInnerClassesFromSet(clazz, classesToRetainInnerClassAttributeFor);
+ }
+ if (keptAnyway || keepForThisInnerClass || keepForThisEnclosingClass) {
if (!keep.enclosingMethod) {
clazz.clearEnclosingMethod();
}
if (!keep.innerClasses) {
clazz.clearInnerClasses();
+ } else if (!keptAnyway) {
+ // We're keeping this only because of classesToRetainInnerClassAttributeFor.
+ final boolean finalKeepForThisInnerClass = keepForThisInnerClass;
+ final boolean finalKeepForThisEnclosingClass = keepForThisEnclosingClass;
+ clazz.removeInnerClasses(
+ ica -> {
+ if (finalKeepForThisInnerClass && ica.getInner() == clazz.type) {
+ return false;
+ }
+ if (finalKeepForThisEnclosingClass
+ && ica.getOuter() == clazz.type
+ && classesToRetainInnerClassAttributeFor.contains(ica.getInner())) {
+ return false;
+ }
+ return true;
+ });
}
} else {
// These attributes are only relevant for reflection, and this class is not used for
@@ -232,5 +351,4 @@
clazz.clearInnerClasses();
}
}
-
}
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 07023b5..c0cebeb 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -60,6 +60,7 @@
import com.android.tools.r8.shaking.RootSetBuilder.ConsequentRootSet;
import com.android.tools.r8.shaking.RootSetBuilder.IfRuleEvaluator;
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
+import com.android.tools.r8.utils.CollectionUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.Timing;
@@ -2255,7 +2256,7 @@
this.prunedTypes =
removedClasses == null
? previous.prunedTypes
- : mergeSets(previous.prunedTypes, removedClasses);
+ : CollectionUtils.mergeSets(previous.prunedTypes, removedClasses);
this.switchMaps = previous.switchMaps;
this.ordinalsMaps = previous.ordinalsMaps;
assert Sets.intersection(instanceFieldReads.keySet(), staticFieldReads.keySet()).isEmpty();
@@ -2310,7 +2311,8 @@
// after second tree shaking.
this.callSites = previous.callSites;
this.brokenSuperInvokes = lense.rewriteMethodsConservatively(previous.brokenSuperInvokes);
- this.prunedTypes = rewriteItems(previous.prunedTypes, lense::lookupType);
+ // Don't rewrite pruned types - the removed types are identified by their original name.
+ this.prunedTypes = previous.prunedTypes;
this.mayHaveSideEffects =
rewriteReferenceKeys(previous.mayHaveSideEffects, lense::lookupReference);
this.noSideEffects = rewriteReferenceKeys(previous.noSideEffects, lense::lookupReference);
@@ -2561,13 +2563,6 @@
return instantiatedLambdas.contains(type);
}
- private static <T> Set<T> mergeSets(Collection<T> first, Collection<T> second) {
- ImmutableSet.Builder<T> builder = ImmutableSet.builder();
- builder.addAll(first);
- builder.addAll(second);
- return builder.build();
- }
-
@Override
public boolean hasLiveness() {
return true;
@@ -2608,6 +2603,10 @@
return prunedTypes.contains(type);
}
+ public Set<DexType> getPrunedTypes() {
+ return prunedTypes;
+ }
+
public DexEncodedMethod lookup(Type type, DexMethod target, DexType invocationContext) {
DexType holder = target.getHolder();
if (!holder.isClassType()) {
diff --git a/src/main/java/com/android/tools/r8/utils/CollectionUtils.java b/src/main/java/com/android/tools/r8/utils/CollectionUtils.java
new file mode 100644
index 0000000..6584f77
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/utils/CollectionUtils.java
@@ -0,0 +1,18 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.utils;
+
+import com.google.common.collect.ImmutableSet;
+import java.util.Collection;
+import java.util.Set;
+
+public class CollectionUtils {
+ public static <T> Set<T> mergeSets(Collection<T> first, Collection<T> second) {
+ ImmutableSet.Builder<T> builder = ImmutableSet.builder();
+ builder.addAll(first);
+ builder.addAll(second);
+ return builder.build();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/NestedStringBuilderTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/NestedStringBuilderTest.java
new file mode 100644
index 0000000..a73b766
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/NestedStringBuilderTest.java
@@ -0,0 +1,65 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.ir.optimize.string;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.ForceInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestCompileResult;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.Streams;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+class NestedStringBuilders {
+
+ public static void main(String... args) {
+ System.out.println(concat("one", args[0]) + "two");
+ }
+
+ @ForceInline
+ public static String concat(String one, String two) {
+ return one + two;
+ }
+}
+
+@RunWith(Parameterized.class)
+public class NestedStringBuilderTest extends TestBase {
+ private static final Class<?> MAIN = NestedStringBuilders.class;
+ private final Backend backend;
+
+ @Parameterized.Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public NestedStringBuilderTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ private void test(TestCompileResult result) throws Exception {
+ CodeInspector codeInspector = result.inspector();
+ ClassSubject mainClass = codeInspector.clazz(MAIN);
+ MethodSubject main = mainClass.mainMethod();
+ long count = Streams.stream(main.iterateInstructions(instructionSubject ->
+ instructionSubject.isNewInstance(StringBuilder.class.getTypeName()))).count();
+ // TODO(b/113859361): should be 1 after merging StringBuilder's
+ assertEquals(2, count);
+ }
+
+ @Test
+ public void b113859361() throws Exception {
+ TestCompileResult result = testForR8(backend)
+ .addProgramClasses(MAIN)
+ .enableInliningAnnotations()
+ .addKeepMainRule(MAIN)
+ .compile();
+ test(result);
+ }
+
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringToStringTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringToStringTest.java
index a9cf86e..bd7bb01 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringToStringTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringToStringTest.java
@@ -20,7 +20,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Streams;
import java.util.List;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -104,7 +103,6 @@
}
@Test
- @Ignore("b/119399513")
public void testD8() throws Exception {
assumeTrue("Only run D8 for Dex backend", backend == Backend.DEX);
@@ -124,7 +122,6 @@
}
@Test
- @Ignore("b/119399513")
public void testR8() throws Exception {
TestRunResult result = testForR8(backend)
.addProgramClasses(CLASSES)
diff --git a/src/test/java/com/android/tools/r8/naming/b126592786/B126592786.java b/src/test/java/com/android/tools/r8/naming/b126592786/B126592786.java
new file mode 100644
index 0000000..0971e8c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/b126592786/B126592786.java
@@ -0,0 +1,111 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.naming.b126592786;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.FieldSubject;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.util.Collection;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class B126592786 extends TestBase {
+
+ private final Backend backend;
+ private final boolean minify;
+
+ @Parameterized.Parameters(name = "Backend: {0} minify: {1}")
+ public static Collection<Object[]> data() {
+ return buildParameters(Backend.values(), BooleanUtils.values());
+ }
+
+ public B126592786(Backend backend, boolean minify) {
+ this.backend = backend;
+ this.minify = minify;
+ }
+
+ public void runTest(boolean genericTypeLive) throws Exception {
+ Class<?> mainClass = genericTypeLive ? MainGenericTypeLive.class : MainGenericTypeNotLive.class;
+ testForR8(backend)
+ .minification(minify)
+ .addProgramClasses(A.class, GenericType.class, mainClass)
+ .addKeepMainRule(mainClass)
+ .addKeepRules(
+ "-keepclassmembers @" + Marker.class.getTypeName() + " class * {",
+ " <fields>;",
+ "}",
+ "-keepattributes InnerClasses,EnclosingMethod,Signature ")
+ .compile()
+ .inspect(inspector -> {
+ String genericTypeDescriptor = "Ljava/lang/Object;";
+ if (genericTypeLive) {
+ ClassSubject genericType = inspector.clazz(GenericType.class);
+ assertThat(genericType, isRenamed(minify));
+ genericTypeDescriptor = genericType.getFinalDescriptor();
+ }
+ String expectedSignature = "Ljava/util/List<" + genericTypeDescriptor + ">;";
+ FieldSubject list = inspector.clazz(A.class).uniqueFieldWithName("list");
+ assertThat(list, isPresent());
+ assertThat(list.getSignatureAnnotation(), isPresent());
+ assertEquals(expectedSignature, list.getSignatureAnnotationValue());
+ })
+ .run(mainClass)
+ .assertSuccess();
+ }
+
+ @Test
+ public void testGenericClassNotLive() throws Exception {
+ runTest(false);
+ }
+
+ @Test
+ public void testGenericClassLive() throws Exception {
+ runTest(true);
+ }
+}
+
+@Target(ElementType.TYPE)
+@Retention(RetentionPolicy.RUNTIME)
+@interface Marker {
+}
+
+@Marker
+class A {
+
+ List<GenericType> list;
+}
+
+@Marker
+class GenericType {
+
+}
+
+class MainGenericTypeNotLive {
+
+ public static void main(String[] args) {
+ System.out.println(A.class);
+ }
+}
+
+class MainGenericTypeLive {
+
+ public static void main(String[] args) {
+ System.out.println(A.class);
+ System.out.println(GenericType.class);
+ }
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/naming/signature/GenericSignatureRenamingTest.java b/src/test/java/com/android/tools/r8/naming/signature/GenericSignatureRenamingTest.java
new file mode 100644
index 0000000..f599f4d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/signature/GenericSignatureRenamingTest.java
@@ -0,0 +1,176 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.naming.signature;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.R8TestBuilder;
+import com.android.tools.r8.TestBase;
+import java.io.IOException;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.lang.reflect.TypeVariable;
+import org.junit.Test;
+
+public class GenericSignatureRenamingTest extends TestBase {
+
+ @Test
+ public void testJVM() throws IOException, CompilationFailedException {
+ testForJvm().addTestClasspath().run(Main.class).assertSuccess();
+ }
+
+ @Test
+ public void testR8Dex() throws IOException, CompilationFailedException {
+ test(testForR8(Backend.DEX));
+ }
+
+ @Test
+ public void testR8CompatDex() throws IOException, CompilationFailedException {
+ test(testForR8Compat(Backend.DEX));
+ }
+
+ @Test
+ public void testR8DexNoMinify() throws IOException, CompilationFailedException {
+ test(testForR8(Backend.DEX).addKeepRules("-dontobfuscate"));
+ }
+
+ @Test
+ public void testR8Cf() throws IOException, CompilationFailedException {
+ test(testForR8(Backend.CF));
+ }
+
+ @Test
+ public void testR8CfNoMinify() throws IOException, CompilationFailedException {
+ test(testForR8(Backend.CF).addKeepRules("-dontobfuscate"));
+ }
+
+ @Test
+ public void testD8() throws IOException, CompilationFailedException {
+ testForD8()
+ .addProgramClasses(Main.class)
+ .addProgramClassesAndInnerClasses(A.class, B.class, CY.class, CYY.class)
+ .setMode(CompilationMode.RELEASE)
+ .compile()
+ .assertNoMessages()
+ .run(Main.class)
+ .assertSuccess();
+ }
+
+ private void test(R8TestBuilder builder) throws IOException, CompilationFailedException {
+ builder
+ .addKeepRules("-dontoptimize")
+ .addKeepRules("-keepattributes InnerClasses,EnclosingMethod,Signature")
+ .addKeepMainRule(Main.class)
+ .addProgramClasses(Main.class)
+ .addProgramClassesAndInnerClasses(A.class, B.class, CY.class, CYY.class)
+ .setMode(CompilationMode.RELEASE)
+ .compile()
+ .assertNoMessages()
+ .run(Main.class)
+ .assertSuccess();
+ }
+}
+
+class A<T> {
+ class Y {
+
+ class YY {}
+
+ class ZZ extends YY {
+ public YY yy;
+
+ YY newYY() {
+ return new YY();
+ }
+ }
+
+ ZZ zz() {
+ return new ZZ();
+ }
+ }
+
+ class Z extends Y {}
+
+ static class S {}
+
+ Y newY() {
+ return new Y();
+ }
+
+ Z newZ() {
+ return new Z();
+ }
+
+ Y.ZZ newZZ() {
+ return new Y().zz();
+ }
+}
+
+class B<T extends A<T>> {}
+
+class CY<T extends A<T>.Y> {}
+
+class CYY<T extends A<T>.Y.YY> {}
+
+class Main {
+
+ private static void check(boolean b, String message) {
+ if (!b) {
+ throw new RuntimeException("Check failed: " + message);
+ }
+ }
+
+ public static void main(String[] args) {
+ A.Z z = new A().newZ();
+ A.Y.YY yy = new A().newZZ().yy;
+
+ B b = new B();
+ CY cy = new CY();
+
+ CYY cyy = new CYY();
+ A.S s = new A.S();
+
+ // Check if names of Z and ZZ shows A as a superclass.
+ Class classA = new A().getClass();
+ String nameA = classA.getName();
+
+ TypeVariable[] v = classA.getTypeParameters();
+ check(v != null && v.length == 1, classA + " expected to have 1 type parameter.");
+
+ Class classZ = new A().newZ().getClass();
+ String nameZ = classZ.getName();
+ check(nameZ.startsWith(nameA + "$"), nameZ + " expected to start with " + nameA + "$.");
+
+ Class classZZ = new A().newZZ().getClass();
+ String nameZZ = classZZ.getName();
+ check(nameZZ.startsWith(nameA + "$"), nameZZ + " expected to start with " + nameA + "$.");
+
+ // Check that the owner of the superclass of Z is A.
+ Class ownerClassOfSuperOfZ = getEnclosingClass(classZ.getGenericSuperclass());
+
+ check(
+ ownerClassOfSuperOfZ == A.class,
+ ownerClassOfSuperOfZ + " expected to be equal to " + A.class);
+
+ // Check that the owner-owner of the superclass of Z is A.
+ Class ownerOfownerOfSuperOfZZ =
+ getEnclosingClass(classZZ.getGenericSuperclass()).getEnclosingClass();
+
+ check(
+ ownerOfownerOfSuperOfZZ == A.class,
+ ownerOfownerOfSuperOfZZ + " expected to be equal to " + A.class);
+ }
+
+ private static Class getEnclosingClass(Type type) {
+ if (type instanceof ParameterizedType) {
+ // On the JVM it's a ParameterizedType.
+ return (Class) ((ParameterizedType) ((ParameterizedType) type).getOwnerType()).getRawType();
+ } else {
+ // On the ART it's Class.
+ check(type instanceof Class, type + " expected to be a ParameterizedType or Class.");
+ return ((Class) type).getEnclosingClass();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
index 3f8cd81..20da5d0 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
@@ -90,4 +90,9 @@
public String getFinalSignatureAttribute() {
return null;
}
+
+ @Override
+ public AnnotationSubject annotation(String name) {
+ return new AbsentAnnotationSubject();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
index 93a1c37..94a779b 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
@@ -193,6 +193,12 @@
}
@Override
+ public boolean isNewInstance(String type) {
+ return isNewInstance()
+ && ((CfNew) instruction).getType().toString().equals(type);
+ }
+
+ @Override
public boolean isCheckCast() {
return instruction instanceof CfCheckCast;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
index c9b7ee5..60ad731 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
@@ -303,6 +303,12 @@
}
@Override
+ public boolean isNewInstance(String type) {
+ return isNewInstance()
+ && ((NewInstance) instruction).getType().toString().equals(type);
+ }
+
+ @Override
public boolean isCheckCast() {
return instruction instanceof CheckCast;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
index df23da9..0d63e38 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.utils.codeinspector;
+import com.android.tools.r8.graph.DexAnnotation;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexValue;
import com.android.tools.r8.naming.MemberNaming;
@@ -128,6 +129,14 @@
}
@Override
+ public AnnotationSubject annotation(String name) {
+ DexAnnotation annotation = codeInspector.findAnnotation(name, dexField.annotations);
+ return annotation == null
+ ? new AbsentAnnotationSubject()
+ : new FoundAnnotationSubject(annotation);
+ }
+
+ @Override
public String toString() {
return dexField.toSourceString();
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
index dd7cd85..4d7c0fc 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
@@ -288,5 +288,4 @@
? new AbsentAnnotationSubject()
: new FoundAnnotationSubject(annotation);
}
-
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
index 0528185..7a272c7 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
@@ -68,6 +68,8 @@
boolean isNewInstance();
+ boolean isNewInstance(String type);
+
boolean isCheckCast();
boolean isCheckCast(String type);
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
index af1fe14..a5cca23 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
@@ -4,6 +4,9 @@
package com.android.tools.r8.utils.codeinspector;
+import com.android.tools.r8.graph.DexAnnotationElement;
+import com.android.tools.r8.graph.DexValue;
+import com.android.tools.r8.graph.DexValue.DexValueString;
import com.android.tools.r8.naming.MemberNaming.Signature;
public abstract class MemberSubject extends Subject {
@@ -48,6 +51,34 @@
return finalSignature == null ? null : finalSignature.name;
}
+ public abstract AnnotationSubject annotation(String name);
+
+ public AnnotationSubject getSignatureAnnotation() {
+ return annotation("dalvik.annotation.Signature");
+ }
+
+ public String getSignatureAnnotationValue() {
+ AnnotationSubject annotation = getSignatureAnnotation();
+ if (!annotation.isPresent()) {
+ return null;
+ }
+
+ assert annotation.getAnnotation().elements.length == 1;
+ DexAnnotationElement element = annotation.getAnnotation().elements[0];
+ assert element.name.toString().equals("value");
+ assert element.value instanceof DexValue.DexValueArray;
+ DexValue.DexValueArray array = (DexValue.DexValueArray) element.value;
+ StringBuilder builder = new StringBuilder();
+ for (DexValue value : array.getValues()) {
+ if (value instanceof DexValueString) {
+ builder.append(((DexValueString) value).value);
+ } else {
+ builder.append(value.toString());
+ }
+ }
+ return builder.toString();
+ }
+
public FieldSubject asFieldSubject() {
return null;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
index 1575372..8fb4001 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
@@ -72,6 +72,4 @@
public boolean isMethodSubject() {
return true;
}
-
- public abstract AnnotationSubject annotation(String name);
}
diff --git a/tools/archive.py b/tools/archive.py
index c842c69..5c0c231 100755
--- a/tools/archive.py
+++ b/tools/archive.py
@@ -92,8 +92,9 @@
def Main():
(options, args) = ParseOptions()
- if not utils.is_bot() and not options.dry_run:
- raise Exception('You are not a bot, don\'t archive builds')
+ # TODO(126871526): Fix the is_bot check.
+ # if not utils.is_bot() and not options.dry_run:
+ # raise Exception('You are not a bot, don\'t archive builds')
if utils.is_old_bot():
print("Archiving is disabled on old bots.")
diff --git a/tools/run_on_as_app.py b/tools/run_on_as_app.py
index d95453f..aa7ed29 100755
--- a/tools/run_on_as_app.py
+++ b/tools/run_on_as_app.py
@@ -511,8 +511,9 @@
apk_dest = os.path.join(out_dir, unsigned_apk_name)
as_utils.MoveFile(unsigned_apk, apk_dest, quiet=options.quiet)
- assert ('r8' not in shrinker
- or CheckIsBuiltWithExpectedR8(apk_dest, temp_dir, shrinker, options))
+ # TODO(mkroghj) Re-enable this assertion when fix works in Golem.
+ # assert ('r8' not in shrinker
+ # or CheckIsBuiltWithExpectedR8(apk_dest, temp_dir, shrinker, options))
profile_dest_dir = os.path.join(out_dir, 'profile')
as_utils.MoveProfileReportTo(profile_dest_dir, stdout, quiet=options.quiet)