Add reproduction for minifier renaming pp-dependent classes
This CL adds a reproduction of the package-name minifier not
maintaining package-private overrides and assuming they will be
publicized. To ensure correct behavior, the correct behavior of the
ClassAndMemberPublicizer is disabled until fixed.
Bug: 172496438
Change-Id: I2a4dd68200758743915a9f6eaa5cb9d00e9f7672
diff --git a/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java b/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
index 0922f50..30d9b7d 100644
--- a/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
+++ b/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
@@ -176,7 +176,8 @@
// and there is therefore no need to check the hierarchy above.
MemberPool<DexMethod> memberPool = methodPoolCollection.get(method.getHolder());
Wrapper<DexMethod> methodKey = MethodSignatureEquivalence.get().wrap(method.getReference());
- if (memberPool.hasSeenStrictlyBelow(methodKey)) {
+ if (memberPool.hasSeenStrictlyBelow(methodKey)
+ && appView.options().enablePackagePrivateAwarePublicization) {
return false;
}
doPublicize(method);
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index dd85ad0..48fb234 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -262,6 +262,8 @@
public boolean encodeChecksums = false;
public BiPredicate<String, Long> dexClassChecksumFilter = (name, checksum) -> true;
public boolean cfToCfDesugar = false;
+ // TODO(b/172496438): Temporarily enable publicizing package-private overrides.
+ public boolean enablePackagePrivateAwarePublicization = false;
public int callGraphLikelySpuriousCallEdgeThreshold = 50;
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/PackagePrivateOverridePublicizerTest.java b/src/test/java/com/android/tools/r8/accessrelaxation/PackagePrivateOverridePublicizerTest.java
index ccd0b21..630ff62 100644
--- a/src/test/java/com/android/tools/r8/accessrelaxation/PackagePrivateOverridePublicizerTest.java
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/PackagePrivateOverridePublicizerTest.java
@@ -50,6 +50,10 @@
.enableInliningAnnotations()
.enableNeverClassInliningAnnotations()
.allowAccessModification()
+ .addOptionsModification(
+ options -> {
+ options.enablePackagePrivateAwarePublicization = true;
+ })
.run(parameters.getRuntime(), Main.class)
.apply(this::assertSuccessOutput);
}
diff --git a/src/test/java/com/android/tools/r8/naming/PackagePrivateAllowAccessModificationPackageTest.java b/src/test/java/com/android/tools/r8/naming/PackagePrivateAllowAccessModificationPackageTest.java
new file mode 100644
index 0000000..473df64
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/PackagePrivateAllowAccessModificationPackageTest.java
@@ -0,0 +1,106 @@
+// Copyright (c) 2020, 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 com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoHorizontalClassMerging;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+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 PackagePrivateAllowAccessModificationPackageTest extends TestBase {
+
+ private final TestParameters parameters;
+ private final String[] EXPECTED = new String[] {"B::foo", "ASub::foo", "A::foo"};
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public PackagePrivateAllowAccessModificationPackageTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testRuntime() throws Exception {
+ testForRuntime(parameters)
+ .addProgramClasses(A.class, ASub.class, B.class, Main.class)
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(A.class, ASub.class, B.class, Main.class)
+ .addKeepMainRule(Main.class)
+ .addKeepClassAndMembersRules(A.class)
+ .allowAccessModification()
+ .enableNoHorizontalClassMergingAnnotations()
+ .enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .addOptionsModification(
+ options -> {
+ options.enablePackagePrivateAwarePublicization = true;
+ })
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject clazz = inspector.clazz(ASub.class);
+ assertThat(clazz, isPresentAndRenamed());
+ })
+ .run(parameters.getRuntime(), Main.class)
+ // TODO(b/172496438): This should not fail.
+ .assertFailureWithErrorThatThrows(IllegalAccessError.class);
+ }
+
+ @NeverClassInline
+ public static class A {
+
+ @NeverInline
+ void foo() {
+ System.out.println("A::foo");
+ }
+ }
+
+ public static class ASub extends A {
+
+ @Override
+ void foo() {
+ System.out.println("ASub::foo");
+ super.foo();
+ }
+ }
+
+ @NeverClassInline
+ @NoHorizontalClassMerging
+ public static class B {
+
+ @NeverInline
+ void foo() {
+ System.out.println("B::foo");
+ ((A) new ASub()).foo();
+ }
+ }
+
+ public static class Main {
+
+ public static void main(String[] args) {
+ new B().foo();
+ }
+ }
+}
