Version 1.5.27
Cherry-pick: Keep properties of non-member classes.
CL: https://r8-review.googlesource.com/c/r8/+/38440
Cherry-pick: Add tests for minifying non-member classes.
CL: https://r8-review.googlesource.com/c/r8/+/38200
Bug: 132460884, 132128436
Change-Id: I91ab716fbaf5a2acb9131216a0b66870f866fb7f
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index a4d3221..3ca15f2 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.26";
+ public static final String LABEL = "1.5.27";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
index d707108..1b50601 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -25,7 +25,6 @@
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.InternalOptions.PackageObfuscationMode;
import com.android.tools.r8.utils.Timing;
-import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
@@ -115,18 +114,9 @@
timing.begin("rename-classes");
for (DexClass clazz : classes) {
if (!renaming.containsKey(clazz.type)) {
- boolean wasAnonymous = clazz.isAnonymousClass();
- // TreePruner already removed inner-class / enclosing-method attributes for local classes.
- assert !clazz.isLocalClass();
clazz.annotations = clazz.annotations.keepIf(this::isNotKotlinMetadata);
DexString renamed = computeName(clazz.type);
renaming.put(clazz.type, renamed);
- // Then-anonymous class is no longer anonymous after minification. Remaining attributes
- // may make the computation of simple name fail on JVM prior to JDK 9.
- if (wasAnonymous) {
- clazz.removeEnclosingMethod(Predicates.alwaysTrue());
- clazz.removeInnerClasses(attr -> attr.getInner() == clazz.type);
- }
// If the class is a member class and it has used $ separator, its renamed name should have
// the same separator (as long as inner-class attribute is honored).
assert !keepInnerClassStructure
@@ -234,13 +224,6 @@
if (clazz == null) {
return null;
}
- // We do not need to preserve the names for local or anonymous classes, as they do not result
- // in a member type declaration and hence cannot be referenced as nested classes in
- // method signatures.
- // See https://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.5.
- if (clazz.getEnclosingMethod() != null) {
- return null;
- }
// For DEX inputs this could result in returning the outer class of a local class since we
// can't distinguish it from a member class based on just the enclosing-class annotation.
// We could filter out the local classes by looking for a corresponding entry in the
@@ -250,27 +233,25 @@
if (attribute == null) {
return null;
}
- return attribute.getOuter();
- }
-
- private DexString getInnerNameForType(DexType type) {
- // This util is used only after the corresponding outer-class is recognized.
- // Therefore, the definition for the type and its inner-class attribute should be found.
- DexClass clazz = appView.definitionFor(type);
- assert clazz != null;
- InnerClassAttribute attribute = clazz.getInnerClassAttributeForThisClass();
- assert attribute != null;
- return attribute.getInnerName();
+ return attribute.getLiveContext(appView.appInfo());
}
private DexString computeName(DexType type) {
Namespace state = null;
if (keepInnerClassStructure) {
- // When keeping the nesting structure of inner classes, we have to insert the name
- // of the outer class for the $ prefix.
+ // When keeping the nesting structure of inner classes, bind this type to the live context.
+ // Note that such live context might not be always the enclosing class. E.g., a local class
+ // does not have a direct enclosing class, but we use the holder of the enclosing method here.
DexType outerClass = getOutClassForType(type);
if (outerClass != null) {
- String separator = computeInnerClassSeparator(outerClass, type, getInnerNameForType(type));
+ DexClass clazz = appView.definitionFor(type);
+ assert clazz != null;
+ InnerClassAttribute attribute = clazz.getInnerClassAttributeForThisClass();
+ assert attribute != null;
+ // Note that, to be consistent with the way inner-class attribute is written via minifier
+ // lense, we are using attribute's outer-class, not the live context.
+ String separator =
+ computeInnerClassSeparator(attribute.getOuter(), type, attribute.getInnerName());
if (separator == null) {
separator = String.valueOf(INNER_CLASS_SEPARATOR);
}
diff --git a/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java b/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
index f14e456..f1dd450 100644
--- a/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
+++ b/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
@@ -77,7 +77,6 @@
attribute.getOuter(), innerType, attribute.getInnerName());
if (separator == null) {
separator = String.valueOf(DescriptorUtils.INNER_CLASS_SEPARATOR);
-
}
int index = innerTypeMapped.lastIndexOf(separator);
if (index < 0) {
diff --git a/src/main/java/com/android/tools/r8/shaking/TreePruner.java b/src/main/java/com/android/tools/r8/shaking/TreePruner.java
index 1671197..fa2212d 100644
--- a/src/main/java/com/android/tools/r8/shaking/TreePruner.java
+++ b/src/main/java/com/android/tools/r8/shaking/TreePruner.java
@@ -17,7 +17,6 @@
import com.android.tools.r8.graph.PresortedComparable;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.utils.InternalOptions;
-import com.google.common.base.Predicates;
import com.google.common.collect.Sets;
import java.util.ArrayList;
import java.util.Collection;
@@ -119,17 +118,6 @@
if (reachableStaticFields != null) {
clazz.setStaticFields(reachableStaticFields);
}
- // If the class is local, it'll become an ordinary class by renaming.
- // Invalidate its inner-class / enclosing-method attributes early.
- if (appView.options().isMinifying()
- && appView.rootSet().mayBeMinified(clazz.type, appView)
- && clazz.isLocalClass()) {
- assert clazz.getInnerClassAttributeForThisClass() != null;
- clazz.removeEnclosingMethod(Predicates.alwaysTrue());
- InnerClassAttribute innerClassAttribute =
- clazz.getInnerClassAttributeForThisClass();
- clazz.removeInnerClasses(attr -> attr == innerClassAttribute);
- }
clazz.removeInnerClasses(this::isAttributeReferencingPrunedType);
clazz.removeEnclosingMethod(this::isAttributeReferencingPrunedItem);
usagePrinter.visited();
diff --git a/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java b/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
index 725b435..a91cc8b 100644
--- a/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
@@ -311,7 +311,9 @@
public static String computeInnerClassSeparator(
DexType outerClass, DexType innerClass, DexString innerName) {
- if (innerName == null) {
+ assert innerClass != null;
+ // Filter out non-member classes ahead.
+ if (outerClass == null || innerName == null) {
return String.valueOf(INNER_CLASS_SEPARATOR);
}
return computeInnerClassSeparator(
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetSimpleNameTest.java b/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetSimpleNameTest.java
index de5dd77..ee1edd0 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetSimpleNameTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetSimpleNameTest.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.R8TestRunResult;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestRunResult;
+import com.android.tools.r8.TestRuntime.CfVm;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
@@ -134,7 +135,11 @@
"Outer$Inner",
"Outer$Inner"
);
- private static final String RENAMED_OUTPUT = StringUtils.lines(
+ // JDK8 computes the simple name differently: some assumptions about non-member classes,
+ // e.g., 1 or more digits (followed by the simple name if it's local).
+ // Since JDK9, the simple name is computed by stripping off the package name.
+ // See b/132808897 for more details.
+ private static final String RENAMED_OUTPUT_JDK8 = StringUtils.lines(
"d",
"a",
"a",
@@ -144,6 +149,16 @@
"a",
"a"
);
+ private static final String RENAMED_OUTPUT = StringUtils.lines(
+ "d",
+ "a",
+ "a",
+ "a",
+ "c[][][]",
+ "[][][]",
+ "a",
+ "a"
+ );
private static final Class<?> MAIN = ClassGetSimpleName.class;
public GetSimpleNameTest(TestParameters parameters, boolean enableMinification) throws Exception {
@@ -245,7 +260,12 @@
.addOptionsModification(this::configure)
.run(parameters.getRuntime(), MAIN);
if (enableMinification) {
- result.assertSuccessWithOutput(RENAMED_OUTPUT);
+ if (parameters.isCfRuntime()
+ && parameters.getRuntime().asCf().getVm().lessThanOrEqual(CfVm.JDK8)) {
+ result.assertSuccessWithOutput(RENAMED_OUTPUT_JDK8);
+ } else {
+ result.assertSuccessWithOutput(RENAMED_OUTPUT);
+ }
} else {
result.assertSuccessWithOutput(JAVA_OUTPUT);
}
diff --git a/src/test/java/com/android/tools/r8/naming/NonMemberClassTest.java b/src/test/java/com/android/tools/r8/naming/NonMemberClassTest.java
new file mode 100644
index 0000000..04c39b4
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/NonMemberClassTest.java
@@ -0,0 +1,199 @@
+// 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;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestRuntime.CfVm;
+import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.util.Collection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class NonMemberClassTest extends TestBase {
+
+ static class Enclosing {
+ @NeverInline
+ void foo() {
+ class Local {
+ Local() {
+ if (this.getClass().getEnclosingClass() != null) {
+ System.out.println("I'm local.");
+ }
+ }
+ }
+ new Local();
+
+ Runnable r = new Runnable() {
+ @NeverInline
+ @Override
+ public void run() {
+ if (this.getClass().isAnonymousClass()) {
+ System.out.println("I'm anonymous.");
+ }
+ }
+ };
+ r.run();
+ }
+ }
+
+ static class TestMain {
+ public static void main(String... args) {
+ new Enclosing().foo();
+ }
+ }
+
+ enum TestConfig {
+ KEEP_INNER_CLASSES,
+ KEEP_ALLOW_MINIFICATION,
+ NO_KEEP_NO_MINIFICATION,
+ NO_KEEP_MINIFICATION;
+
+ public String getKeepRules() {
+ switch (this) {
+ case KEEP_INNER_CLASSES:
+ return "-keep class " + Enclosing.class.getName() + "$*";
+ case KEEP_ALLOW_MINIFICATION:
+ return "-keep,allowobfuscation class " + Enclosing.class.getName() + "$*";
+ case NO_KEEP_NO_MINIFICATION:
+ return "-dontobfuscate";
+ case NO_KEEP_MINIFICATION:
+ return "";
+ default:
+ throw new Unreachable();
+ }
+ }
+
+ public String getExpectedOutput(TestParameters parameters, boolean isFullMode) {
+ switch (this) {
+ case KEEP_INNER_CLASSES:
+ return JVM_OUTPUT;
+ case KEEP_ALLOW_MINIFICATION:
+ if (parameters.isCfRuntime()
+ && parameters.getRuntime().asCf().getVm().lessThanOrEqual(CfVm.JDK8)) {
+ return MINIFIED_OUTPUT_JDK8;
+ }
+ return JVM_OUTPUT;
+ case NO_KEEP_NO_MINIFICATION:
+ // In full mode, we remove all attributes since nothing pinned, i.e., no reflection uses.
+ return isFullMode ? "" : JVM_OUTPUT;
+ case NO_KEEP_MINIFICATION:
+ // In full mode, we remove all attributes since nothing pinned, i.e., no reflection uses.
+ if (isFullMode) {
+ return "";
+ }
+ if (parameters.isCfRuntime()
+ && parameters.getRuntime().asCf().getVm().lessThanOrEqual(CfVm.JDK8)) {
+ return MINIFIED_OUTPUT_JDK8;
+ }
+ return JVM_OUTPUT;
+ default:
+ throw new Unreachable();
+ }
+ }
+
+ public void inspect(boolean isFullMode, CodeInspector inspector) {
+ int expectedNumberOfNonMemberInnerClasses;
+ switch (this) {
+ case KEEP_INNER_CLASSES:
+ case KEEP_ALLOW_MINIFICATION:
+ expectedNumberOfNonMemberInnerClasses = 2;
+ break;
+ case NO_KEEP_NO_MINIFICATION:
+ case NO_KEEP_MINIFICATION:
+ expectedNumberOfNonMemberInnerClasses = isFullMode ? 0 : 2;
+ break;
+ default:
+ throw new Unreachable();
+ }
+ assertEquals(
+ expectedNumberOfNonMemberInnerClasses,
+ inspector.allClasses().stream().filter(classSubject ->
+ classSubject.getDexClass().isLocalClass()
+ || classSubject.getDexClass().isAnonymousClass()).count());
+ }
+ }
+
+ private static final Class<?> MAIN = TestMain.class;
+
+ // Since JDK9, a class is determined as anonymous if the inner-name in the associated inner-class
+ // attribute is empty.
+ // JDK8 determines an anonymous class differently: checking if a simple name is empty.
+ // Moreover, it computes the simple name differently: some assumptions about non-member classes,
+ // e.g., 1 or more digits (followed by the simple name if it's local).
+ // Since JDK9, the simple name is computed by stripping off the package name.
+ // See b/132808897 for more details.
+ private static final String MINIFIED_OUTPUT_JDK8 = StringUtils.lines(
+ "I'm local."
+ );
+ private static final String JVM_OUTPUT = StringUtils.lines(
+ "I'm local.",
+ "I'm anonymous."
+ );
+
+ private final TestParameters parameters;
+ private final TestConfig config;
+
+ @Parameterized.Parameters(name = "{0} {1}")
+ public static Collection<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimes().withAllApiLevels().build(), TestConfig.values());
+ }
+
+ public NonMemberClassTest(TestParameters parameters, TestConfig config) {
+ this.parameters = parameters;
+ this.config = config;
+ }
+
+ @Test
+ public void testJVMOutput() throws Exception {
+ assumeTrue(
+ "Only run JVM reference on CF runtimes",
+ parameters.isCfRuntime() && config == TestConfig.NO_KEEP_NO_MINIFICATION);
+ testForJvm()
+ .addTestClasspath()
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(JVM_OUTPUT);
+ }
+
+ @Test
+ public void testR8Compat() throws Exception {
+ testForR8Compat(parameters.getBackend())
+ .addInnerClasses(NonMemberClassTest.class)
+ .addKeepMainRule(MAIN)
+ .addKeepRules(config.getKeepRules())
+ .addKeepAttributes("Signature", "InnerClasses", "EnclosingMethod")
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .addOptionsModification(options -> options.enableClassInlining = false)
+ .compile()
+ .inspect(inspector -> config.inspect(false, inspector))
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(config.getExpectedOutput(parameters, false));
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(NonMemberClassTest.class)
+ .addKeepMainRule(MAIN)
+ .addKeepRules(config.getKeepRules())
+ .addKeepAttributes("Signature", "InnerClasses", "EnclosingMethod")
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .addOptionsModification(options -> options.enableClassInlining = false)
+ .compile()
+ .inspect(inspector -> config.inspect(true, inspector))
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(config.getExpectedOutput(parameters, true));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/b132460884/LocalClassRenamingTest.java b/src/test/java/com/android/tools/r8/naming/b132460884/LocalClassRenamingTest.java
new file mode 100644
index 0000000..4d82789
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/b132460884/LocalClassRenamingTest.java
@@ -0,0 +1,98 @@
+// 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.b132460884;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+class TestMap<K, V> {
+ private Map<K, V> map;
+
+ private TestMap() {
+ this.map = new HashMap<>();
+ }
+
+ public static <K, V> TestMap of(K k, V v) {
+ TestMap<K, V> m = new TestMap<>();
+ m.map.put(k, v);
+ return m;
+ }
+
+ public Set<Map.Entry<K, V>> entrySet() {
+ class LocalEntrySet extends HashSet<Map.Entry<K, V>> {
+ @Override
+ public Iterator<Map.Entry<K, V>> iterator() {
+ return map.entrySet().iterator();
+ }
+ }
+ return map.isEmpty() ? new HashSet<>() : new LocalEntrySet();
+ }
+}
+
+class TestMain {
+ public static void main(String... args) {
+ TestMap<String, Integer> map = TestMap.of("R", 8);
+ Iterator<Map.Entry<String, Integer>> it = map.entrySet().iterator();
+ while (it.hasNext()) {
+ Map.Entry<String, Integer> entry = it.next();
+ System.out.println(entry.getKey() + " -> " + entry.getValue());
+ }
+ }
+}
+
+@RunWith(Parameterized.class)
+public class LocalClassRenamingTest extends TestBase {
+ private static final String EXPECTED_OUTPUT = StringUtils.lines("R -> 8");
+
+ private final TestParameters parameters;
+ private final boolean enableMinification;
+
+ @Parameterized.Parameters(name = "{0} minification: {1}")
+ public static Collection<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimes().withAllApiLevels().build(),
+ BooleanUtils.values());
+ }
+
+ public LocalClassRenamingTest(TestParameters parameters, boolean enableMinification) {
+ this.parameters = parameters;
+ this.enableMinification = enableMinification;
+ }
+
+ @Test
+ public void b132460884() throws Exception {
+ testForR8Compat(parameters.getBackend())
+ .addProgramClassesAndInnerClasses(TestMap.class)
+ .addProgramClasses(TestMain.class)
+ .addKeepMainRule(TestMain.class)
+ .addKeepAttributes("Signature", "InnerClasses")
+ .noTreeShaking()
+ .minification(enableMinification)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(inspector -> {
+ ClassSubject local = inspector.clazz(TestMap.class.getName() + "$1LocalEntrySet");
+ assertThat(local, isPresent());
+ assertEquals(enableMinification, local.isRenamed());
+ })
+ .run(parameters.getRuntime(), TestMain.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingAnnotationremovalTest.java b/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingAnnotationremovalTest.java
index 0a21b8c..52a756b 100644
--- a/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingAnnotationremovalTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingAnnotationremovalTest.java
@@ -96,12 +96,12 @@
Assert.assertFalse(inner.isLocalClass());
ClassSubject anonymous = inspector.clazz("annotationremoval.OuterClass$1");
Assert.assertTrue(anonymous.isPresent());
- Assert.assertEquals(!getMinify().isMinify(), anonymous.isAnonymousClass());
+ Assert.assertTrue(anonymous.isAnonymousClass());
Assert.assertFalse(anonymous.isMemberClass());
Assert.assertFalse(anonymous.isLocalClass());
ClassSubject local = inspector.clazz("annotationremoval.OuterClass$1LocalMagic");
Assert.assertTrue(local.isPresent());
- Assert.assertEquals(!getMinify().isMinify(), local.isLocalClass());
+ Assert.assertTrue(local.isLocalClass());
Assert.assertFalse(local.isMemberClass());
Assert.assertFalse(local.isAnonymousClass());
}
