Class merging across nests
- Disable Vertical class merger when merging
across nests or from nest to non-nest.
- Disable Horizontal class merger when merging
a class with private methods in a nest.
- Add tests for all cases
Bug:132670472
Change-Id: Id73d28c5179d22a0b30148d4d38ca98321b5c856
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index 3f3577a..9a1ea4c 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -855,8 +855,10 @@
if (isNestMember()) {
return nestHost.getNestHost();
}
- assert isNestHost();
- return type;
+ if (isNestHost()) {
+ return type;
+ }
+ return null;
}
public NestHostClassAttribute getNestHostClassAttribute() {
diff --git a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
index b526695..3940537 100644
--- a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
@@ -273,6 +273,10 @@
if (!clazz.virtualMethods().stream().allMatch(DexEncodedMethod::isPrivateMethod)) {
return MergeGroup.DONT_MERGE;
}
+ if (clazz.isInANest()) {
+ // Breaks nest access control, abort merging.
+ return MergeGroup.DONT_MERGE;
+ }
if (Streams.stream(clazz.methods())
.anyMatch(
method ->
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 085acee..3e667a2 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -131,6 +131,7 @@
ALWAYS_INLINE,
CONFLICT,
ILLEGAL_ACCESS,
+ MERGE_ACROSS_NESTS,
NATIVE_METHOD,
NO_SIDE_EFFECTS,
PINNED_SOURCE,
@@ -161,6 +162,9 @@
case ILLEGAL_ACCESS:
message = "it could lead to illegal accesses";
break;
+ case MERGE_ACROSS_NESTS:
+ message = "cannot merge across nests, or from nest to non-nest";
+ break;
case NATIVE_METHOD:
message = "it has a native method";
break;
@@ -407,6 +411,15 @@
}
return false;
}
+ DexClass targetClass = appView.definitionFor(singleSubtype);
+ // We abort class merging when merging across nests or from a nest to non-nest.
+ // Without nest this checks null == null.
+ if (targetClass.getNestHost() != clazz.getNestHost()) {
+ if (Log.ENABLED) {
+ AbortReason.MERGE_ACROSS_NESTS.printLogMessageForClass(clazz);
+ }
+ return false;
+ }
return true;
}
diff --git a/src/test/examplesJava11/nestHostExample/BasicNestHostClassMerging.java b/src/test/examplesJava11/nestHostExample/BasicNestHostClassMerging.java
index 0f01e9a..727973b 100644
--- a/src/test/examplesJava11/nestHostExample/BasicNestHostClassMerging.java
+++ b/src/test/examplesJava11/nestHostExample/BasicNestHostClassMerging.java
@@ -19,6 +19,7 @@
public static class InnerMost extends MiddleInner {
+ @NeverInline
public String getFields() {
return ((BasicNestHostClassMerging) this).field
+ ((MiddleOuter) this).field
diff --git a/src/test/examplesJava11/nestHostExample/BasicNestHostTreePruning.java b/src/test/examplesJava11/nestHostExample/BasicNestHostTreePruning.java
index 7c0ca32..286010a 100644
--- a/src/test/examplesJava11/nestHostExample/BasicNestHostTreePruning.java
+++ b/src/test/examplesJava11/nestHostExample/BasicNestHostTreePruning.java
@@ -6,6 +6,7 @@
public static class NotPruned extends BasicNestHostTreePruning {
+ @NeverInline
public String getFields() {
return ((BasicNestHostTreePruning) this).field;
}
diff --git a/src/test/examplesJava11/nestHostExample/NestHostInlining.java b/src/test/examplesJava11/nestHostExample/NestHostInlining.java
new file mode 100644
index 0000000..97a4398
--- /dev/null
+++ b/src/test/examplesJava11/nestHostExample/NestHostInlining.java
@@ -0,0 +1,31 @@
+package nestHostExample;
+
+public class NestHostInlining {
+
+ private String field = "inlining";
+
+ public static class InnerWithPrivAccess {
+ public String access(NestHostInlining host) {
+ return host.field;
+ }
+ }
+
+ public static class InnerNoPrivAccess {
+ public String print() {
+ return "InnerNoPrivAccess";
+ }
+ }
+
+ public abstract static class EmptyNoPrivAccess {}
+
+ public abstract static class EmptyWithPrivAccess {
+ public String access(NestHostInlining host) {
+ return host.field;
+ }
+ }
+
+ public static void main(String[] args) {
+ System.out.println(new InnerWithPrivAccess().access(new NestHostInlining()));
+ System.out.println(new InnerNoPrivAccess().print());
+ }
+}
diff --git a/src/test/examplesJava11/nestHostExample/NestHostInliningSubclasses.java b/src/test/examplesJava11/nestHostExample/NestHostInliningSubclasses.java
new file mode 100644
index 0000000..fd31297
--- /dev/null
+++ b/src/test/examplesJava11/nestHostExample/NestHostInliningSubclasses.java
@@ -0,0 +1,22 @@
+package nestHostExample;
+
+public class NestHostInliningSubclasses {
+
+ public static class InnerWithPrivAccess extends NestHostInlining.InnerWithPrivAccess {
+ public String accessSubclass(NestHostInlining host) {
+ return super.access(host) + "Subclass";
+ }
+ }
+
+ public static class InnerNoPrivAccess extends NestHostInlining.InnerNoPrivAccess {
+ @NeverInline
+ public String printSubclass() {
+ return super.print() + "Subclass";
+ }
+ }
+
+ public static void main(String[] args) {
+ System.out.println(new InnerWithPrivAccess().accessSubclass(new NestHostInlining()));
+ System.out.println(new InnerNoPrivAccess().printSubclass());
+ }
+}
diff --git a/src/test/examplesJava11/nestHostExample/NeverInline.java b/src/test/examplesJava11/nestHostExample/NeverInline.java
new file mode 100644
index 0000000..809b9e7
--- /dev/null
+++ b/src/test/examplesJava11/nestHostExample/NeverInline.java
@@ -0,0 +1,10 @@
+// Copyright (c) 2018, 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 nestHostExample;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD})
+public @interface NeverInline {}
diff --git a/src/test/examplesJava11/nestHostExample/OutsideInliningNoAccess.java b/src/test/examplesJava11/nestHostExample/OutsideInliningNoAccess.java
new file mode 100644
index 0000000..deb0548
--- /dev/null
+++ b/src/test/examplesJava11/nestHostExample/OutsideInliningNoAccess.java
@@ -0,0 +1,8 @@
+package nestHostExample;
+
+public class OutsideInliningNoAccess extends NestHostInlining.EmptyNoPrivAccess {
+
+ public static void main(String[] args) {
+ System.out.println("OutsideInliningNoAccess");
+ }
+}
diff --git a/src/test/examplesJava11/nestHostExample/OutsideInliningWithAccess.java b/src/test/examplesJava11/nestHostExample/OutsideInliningWithAccess.java
new file mode 100644
index 0000000..92f8ca8
--- /dev/null
+++ b/src/test/examplesJava11/nestHostExample/OutsideInliningWithAccess.java
@@ -0,0 +1,9 @@
+package nestHostExample;
+
+public class OutsideInliningWithAccess extends NestHostInlining.EmptyWithPrivAccess {
+
+ public static void main(String[] args) {
+ System.out.println("OutsideInliningNoAccess");
+ System.out.println(new OutsideInliningWithAccess().access(new NestHostInlining()));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/R8TestBuilder.java b/src/test/java/com/android/tools/r8/R8TestBuilder.java
index 86e3121..a587685 100644
--- a/src/test/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -174,11 +174,15 @@
}
public T enableInliningAnnotations() {
+ return enableInliningAnnotations(NeverInline.class.getPackage().getName());
+ }
+
+ public T enableInliningAnnotations(String annotationPackageName) {
if (!enableInliningAnnotations) {
enableInliningAnnotations = true;
addInternalKeepRules(
- "-forceinline class * { @com.android.tools.r8.ForceInline *; }",
- "-neverinline class * { @com.android.tools.r8.NeverInline *; }");
+ "-forceinline class * { @" + annotationPackageName + ".ForceInline *; }",
+ "-neverinline class * { @" + annotationPackageName + ".NeverInline *; }");
}
return self();
}
diff --git a/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/FullNestOnProgramPathTest.java b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/FullNestOnProgramPathTest.java
index 21ee28e..b2adfce 100644
--- a/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/FullNestOnProgramPathTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/FullNestOnProgramPathTest.java
@@ -156,18 +156,22 @@
// Interface method desugaring may add extra classes
assertTrue(NUMBER_OF_TEST_CLASSES <= inspector.allClasses().size());
ImmutableList<String> outerClassNames = MAIN_CLASSES.values().asList();
+ ImmutableList<String> nonNestClasses =
+ ImmutableList.of("NeverInline", "OutsideInliningNoAccess", "OutsideInliningWithAccess");
inspector.forAllClasses(
classSubject -> {
DexClass dexClass = classSubject.getDexClass();
- assertTrue(dexClass.isInANest());
- if (outerClassNames.contains(dexClass.type.getName())) {
- assertNull(dexClass.getNestHostClassAttribute());
- assertFalse(dexClass.getNestMembersClassAttributes().isEmpty());
- } else {
- assertTrue(dexClass.getNestMembersClassAttributes().isEmpty());
- assertTrue(
- outerClassNames.contains(
- dexClass.getNestHostClassAttribute().getNestHost().getName()));
+ if (!nonNestClasses.contains(dexClass.type.getName())) {
+ assertTrue(dexClass.isInANest());
+ if (outerClassNames.contains(dexClass.type.getName())) {
+ assertNull(dexClass.getNestHostClassAttribute());
+ assertFalse(dexClass.getNestMembersClassAttributes().isEmpty());
+ } else {
+ assertTrue(dexClass.getNestMembersClassAttributes().isEmpty());
+ assertTrue(
+ outerClassNames.contains(
+ dexClass.getNestHostClassAttribute().getNestHost().getName()));
+ }
}
});
}
diff --git a/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAccessControlTestUtils.java b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAccessControlTestUtils.java
index 9d99c5b..45a6195 100644
--- a/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAccessControlTestUtils.java
+++ b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAccessControlTestUtils.java
@@ -43,6 +43,16 @@
"BasicNestHostTreePruning",
"BasicNestHostTreePruning$Pruned",
"BasicNestHostTreePruning$NotPruned",
+ "NestHostInlining",
+ "NestHostInlining$InnerWithPrivAccess",
+ "NestHostInlining$InnerNoPrivAccess",
+ "NestHostInlining$EmptyNoPrivAccess",
+ "NestHostInlining$EmptyWithPrivAccess",
+ "NestHostInliningSubclasses",
+ "NestHostInliningSubclasses$InnerWithPrivAccess",
+ "NestHostInliningSubclasses$InnerNoPrivAccess",
+ "OutsideInliningNoAccess",
+ "OutsideInliningWithAccess",
"NestHostExample",
"NestHostExample$NestMemberInner",
"NestHostExample$NestMemberInner$NestMemberInnerInner",
@@ -65,6 +75,8 @@
.put("all", "NestHostExample")
.put("merge", "BasicNestHostClassMerging")
.put("prune", "BasicNestHostTreePruning")
+ .put("inlining", "NestHostInlining")
+ .put("inliningSub", "NestHostInliningSubclasses")
.build();
public static final String ALL_RESULT_LINE =
String.join(
diff --git a/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAttributesUpdateTest.java b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAttributesUpdateTest.java
index 06f0a01..3239a4a 100644
--- a/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAttributesUpdateTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestAttributesUpdateTest.java
@@ -6,6 +6,7 @@
import static com.android.tools.r8.desugar.nestaccesscontrol.NestAccessControlTestUtils.PACKAGE_NAME;
import static com.android.tools.r8.desugar.nestaccesscontrol.NestAccessControlTestUtils.classesMatching;
+import static junit.framework.TestCase.assertEquals;
import static junit.framework.TestCase.assertNotNull;
import static junit.framework.TestCase.assertSame;
import static junit.framework.TestCase.assertTrue;
@@ -49,49 +50,65 @@
@Test
public void testClassMergingNestMemberRemoval() throws Exception {
- testNestAttributesCorrect(MERGING_OUTER_CLASS, MERGING_OUTER_CLASS, MERGING_EXPECTED_RESULT);
+ testNestAttributesCorrect(MERGING_OUTER_CLASS, MERGING_OUTER_CLASS, MERGING_EXPECTED_RESULT, 3);
}
@Test
public void testClassMergingNestHostRemoval() throws Exception {
testNestAttributesCorrect(
- MERGING_OUTER_CLASS + "$MiddleOuter", MERGING_OUTER_CLASS, MERGING_EXPECTED_RESULT);
+ MERGING_OUTER_CLASS + "$MiddleOuter", MERGING_OUTER_CLASS, MERGING_EXPECTED_RESULT, 2);
}
@Test
public void testTreePruningNestMemberRemoval() throws Exception {
- testNestAttributesCorrect(PRUNING_OUTER_CLASS, PRUNING_OUTER_CLASS, PRUNING_EXPECTED_RESULT);
+ testNestAttributesCorrect(PRUNING_OUTER_CLASS, PRUNING_OUTER_CLASS, PRUNING_EXPECTED_RESULT, 2);
}
@Test
public void testTreePruningNestHostRemoval() throws Exception {
testNestAttributesCorrect(
- PRUNING_OUTER_CLASS + "$Pruned", PRUNING_OUTER_CLASS, PRUNING_EXPECTED_RESULT);
+ PRUNING_OUTER_CLASS + "$Pruned", PRUNING_OUTER_CLASS, PRUNING_EXPECTED_RESULT, 1);
}
public void testNestAttributesCorrect(
- String mainClassName, String outerNestName, String expectedResult) throws Exception {
- testNestAttributesCorrect(mainClassName, outerNestName, expectedResult, true);
- testNestAttributesCorrect(mainClassName, outerNestName, expectedResult, false);
+ String mainClassName, String outerNestName, String expectedResult, int expectedNumClassesLeft)
+ throws Exception {
+ testNestAttributesCorrect(
+ mainClassName, outerNestName, expectedResult, true, expectedNumClassesLeft);
+ testNestAttributesCorrect(
+ mainClassName, outerNestName, expectedResult, false, expectedNumClassesLeft);
}
public void testNestAttributesCorrect(
- String mainClassName, String outerNestName, String expectedResult, boolean minification)
+ String mainClassName,
+ String outerNestName,
+ String expectedResult,
+ boolean minification,
+ int expectedNumClassesLeft)
throws Exception {
String actualMainClassName = PACKAGE_NAME + mainClassName;
testForR8(parameters.getBackend())
.addKeepMainRule(actualMainClassName)
.minification(minification)
.setMinApi(parameters.getApiLevel())
+ .addOptionsModification(
+ options -> {
+ // Disable optimizations else additional classes are removed since they become unused.
+ options.enableValuePropagation = false;
+ options.enableClassInlining = false;
+ })
.addProgramFiles(classesMatching(outerNestName))
- .addOptionsModification(options -> options.enableNestBasedAccessDesugaring = true)
.compile()
- .inspect(this::assertNestAttributesCorrect)
+ .inspect(
+ inspector -> {
+ assertEquals(expectedNumClassesLeft, inspector.allClasses().size());
+ assertNestAttributesCorrect(inspector);
+ })
.run(parameters.getRuntime(), actualMainClassName)
.assertSuccessWithOutput(expectedResult);
}
- private void assertNestAttributesCorrect(CodeInspector inspector) {
+ public static void assertNestAttributesCorrect(CodeInspector inspector) {
assertTrue(inspector.allClasses().size() > 0);
for (FoundClassSubject classSubject : inspector.allClasses()) {
DexClass clazz = classSubject.getDexClass();
diff --git a/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestClassMergingTest.java b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestClassMergingTest.java
new file mode 100644
index 0000000..abc22b7
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestClassMergingTest.java
@@ -0,0 +1,109 @@
+package com.android.tools.r8.desugar.nestaccesscontrol;
+
+import static com.android.tools.r8.desugar.nestaccesscontrol.NestAccessControlTestUtils.PACKAGE_NAME;
+import static com.android.tools.r8.desugar.nestaccesscontrol.NestAccessControlTestUtils.classesMatching;
+
+import com.android.tools.r8.R8FullTestBuilder;
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.TestRuntime.CfVm;
+import com.android.tools.r8.utils.StringUtils;
+import java.nio.file.Path;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class NestClassMergingTest extends TestBase {
+
+ public NestClassMergingTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ private final TestParameters parameters;
+
+ private final String NEST_MAIN_CLASS = PACKAGE_NAME + "NestHostInlining";
+ private final String NEST_SUBCLASS_MAIN_CLASS = PACKAGE_NAME + "NestHostInliningSubclasses";
+ private final String OUTSIDE_WITH_ACCESS_MAIN_CLASS = PACKAGE_NAME + "OutsideInliningWithAccess";
+ private final String OUTSIDE_NO_ACCESS_MAIN_CLASS = PACKAGE_NAME + "OutsideInliningNoAccess";
+ private final String NEST_MAIN_EXPECTED_RESULT =
+ StringUtils.lines("inlining", "InnerNoPrivAccess");
+ private final String NEST_SUBCLASS_MAIN_EXPECTED_RESULT =
+ StringUtils.lines("inliningSubclass", "InnerNoPrivAccessSubclass");
+ private final String OUTSIDE_WITH_ACCESS_MAIN_EXPECTED_RESULT =
+ StringUtils.lines("OutsideInliningNoAccess", "inlining");
+ private final String OUTSIDE_NO_ACCESS_MAIN_EXPECTED_RESULT =
+ StringUtils.lines("OutsideInliningNoAccess");
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters()
+ .withCfRuntimesStartingFromIncluding(CfVm.JDK11)
+ .withAllApiLevels()
+ .build();
+ }
+
+ @Test
+ public void testClassMergeAcrossTwoNests() throws Exception {
+ // Potentially merge classes from one nest with classes from another nest.
+ testClassMergeAcrossNest(
+ new String[] {NEST_MAIN_CLASS}, new String[] {NEST_MAIN_EXPECTED_RESULT});
+ testClassMergeAcrossNest(
+ new String[] {NEST_SUBCLASS_MAIN_CLASS}, new String[] {NEST_SUBCLASS_MAIN_EXPECTED_RESULT});
+ testClassMergeAcrossNest(
+ new String[] {NEST_MAIN_CLASS, NEST_SUBCLASS_MAIN_CLASS},
+ new String[] {NEST_MAIN_EXPECTED_RESULT, NEST_SUBCLASS_MAIN_EXPECTED_RESULT});
+ }
+
+ @Test
+ public void testClassMergeAcrossNestAndNonNest() throws Exception {
+ // Potentially merge classes from a nest with non nest classes.
+ testClassMergeAcrossNest(
+ new String[] {
+ NEST_MAIN_CLASS, OUTSIDE_NO_ACCESS_MAIN_CLASS, OUTSIDE_WITH_ACCESS_MAIN_CLASS
+ },
+ new String[] {
+ NEST_MAIN_EXPECTED_RESULT,
+ OUTSIDE_NO_ACCESS_MAIN_EXPECTED_RESULT,
+ OUTSIDE_WITH_ACCESS_MAIN_EXPECTED_RESULT
+ });
+ testClassMergeAcrossNest(
+ new String[] {OUTSIDE_NO_ACCESS_MAIN_CLASS},
+ new String[] {OUTSIDE_NO_ACCESS_MAIN_EXPECTED_RESULT});
+ testClassMergeAcrossNest(
+ new String[] {OUTSIDE_WITH_ACCESS_MAIN_CLASS},
+ new String[] {OUTSIDE_WITH_ACCESS_MAIN_EXPECTED_RESULT});
+ }
+
+ public void testClassMergeAcrossNest(String[] mainClasses, String[] expectedResults)
+ throws Exception {
+ List<Path> bothNestsAndOutsideClassToCompile = classesMatching("Inlining");
+ R8FullTestBuilder r8FullTestBuilder = testForR8(parameters.getBackend());
+ for (String clazz : mainClasses) {
+ r8FullTestBuilder.addKeepMainRule(clazz);
+ }
+ R8TestCompileResult compileResult =
+ r8FullTestBuilder
+ .addOptionsModification(
+ options -> {
+ // Disable optimizations else additional classes are removed since they become
+ // unused.
+ options.enableValuePropagation = false;
+ options.enableClassInlining = false;
+ })
+ .enableInliningAnnotations("nestHostExample")
+ .setMinApi(parameters.getApiLevel())
+ .addProgramFiles(bothNestsAndOutsideClassToCompile)
+ .compile()
+ .inspect(NestAttributesUpdateTest::assertNestAttributesCorrect);
+ for (int i = 0; i < mainClasses.length; i++) {
+ compileResult
+ .run(parameters.getRuntime(), mainClasses[i])
+ .assertSuccessWithOutput(expectedResults[i]);
+ }
+ }
+}
