Merge "Temporarily disable assertion that checks for builder in run_on_as_app"
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 95e3156..8069844 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -276,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(
@@ -352,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();
@@ -609,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 fd8508b..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
@@ -188,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/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/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/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/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);
