Don't implicitly keep default constructors for keepclassmembers.
This change also disallows optimizing implicitly kept initializers
which was also an oversight as the callsite info for them is
incomplete.
Bug: b/248473941
Bug: b/247054688
Change-Id: I80d5afde0642d7061684551b30a295b0b5fcde93
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 90b9dfb..2cce5b4 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -137,6 +137,7 @@
import com.android.tools.r8.shaking.EnqueuerWorklist.TraceStaticFieldWriteAction;
import com.android.tools.r8.shaking.GraphReporter.KeepReasonWitness;
import com.android.tools.r8.shaking.KeepInfoCollection.MutableKeepInfoCollection;
+import com.android.tools.r8.shaking.KeepMethodInfo.Joiner;
import com.android.tools.r8.shaking.RootSetUtils.ConsequentRootSet;
import com.android.tools.r8.shaking.RootSetUtils.ConsequentRootSetBuilder;
import com.android.tools.r8.shaking.RootSetUtils.RootSet;
@@ -943,9 +944,19 @@
if (clazz.hasDefaultInitializer()) {
ProgramMethod defaultInitializer = clazz.getProgramDefaultInitializer();
if (forceProguardCompatibility) {
- workList.enqueueMarkMethodKeptAction(
- defaultInitializer,
- graphReporter.reportCompatKeepDefaultInitializer(defaultInitializer));
+ Joiner joiner = KeepMethodInfo.newEmptyJoiner();
+ for (ProguardKeepRuleBase rule : rules) {
+ if (!rule.getType().equals(ProguardKeepRuleType.KEEP_CLASS_MEMBERS)) {
+ joiner.addRule(rule);
+ }
+ }
+ if (!joiner.getRules().isEmpty()) {
+ workList.enqueueMarkMethodKeptAction(
+ defaultInitializer,
+ graphReporter.reportCompatKeepDefaultInitializer(defaultInitializer));
+ applyMinimumKeepInfoWhenLiveOrTargeted(
+ defaultInitializer, joiner.disallowOptimization());
+ }
}
if (clazz.isExternalizable(appView)) {
workList.enqueueMarkMethodLiveAction(defaultInitializer, defaultInitializer, witness);
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/KeepClassMembersDefaultCtorTest.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/KeepClassMembersDefaultCtorTest.java
index 1acd95b..bdb4da5 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/KeepClassMembersDefaultCtorTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/KeepClassMembersDefaultCtorTest.java
@@ -3,6 +3,9 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.shaking.forceproguardcompatibility.defaultctor;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
+import static org.hamcrest.MatcherAssert.assertThat;
+
import com.android.tools.r8.ProguardVersion;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
@@ -40,16 +43,17 @@
@Test
public void testCompatR8() throws Exception {
- // R8 retains the default constructor but inserts throw null.
- // TODO(b/248473941): R8 should strip the constructor instead.
- run(testForR8Compat(parameters.getBackend()))
- .assertFailureWithErrorThatThrows(NullPointerException.class);
+ run(testForR8Compat(parameters.getBackend()));
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ run(testForR8(parameters.getBackend()));
}
@Test
public void testPG() throws Exception {
- run(testForProguard(ProguardVersion.getLatest()).addDontWarn(getClass()))
- .assertFailureWithErrorThatThrows(NoSuchMethodException.class);
+ run(testForProguard(ProguardVersion.getLatest()).addDontWarn(getClass()));
}
private TestRunResult<?> run(TestShrinkerBuilder<?, ?, ?, ?, ?> builder) throws Exception {
@@ -58,7 +62,12 @@
.addKeepRules("-keepclassmembers class * { <fields>; }")
.addKeepClassAndMembersRules(TestClass.class)
.addDontObfuscate()
- .run(parameters.getRuntime(), TestClass.class);
+ .run(parameters.getRuntime(), TestClass.class)
+ .inspectFailure(
+ inspector -> {
+ assertThat(inspector.clazz(A.class).init(), isAbsent());
+ })
+ .assertFailureWithErrorThatThrows(NoSuchMethodException.class);
}
static class A {
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/KeepClassesWithMembersDefaultCtorTest.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/KeepClassesWithMembersDefaultCtorTest.java
new file mode 100644
index 0000000..54ee56f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/KeepClassesWithMembersDefaultCtorTest.java
@@ -0,0 +1,112 @@
+// Copyright (c) 2022, 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.shaking.forceproguardcompatibility.defaultctor;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.ProguardVersion;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.TestRunResult;
+import com.android.tools.r8.TestShrinkerBuilder;
+import com.android.tools.r8.utils.StringUtils;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class KeepClassesWithMembersDefaultCtorTest extends TestBase {
+
+ static final String EXPECTED = StringUtils.lines("A()");
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDefaultCfRuntime().build();
+ }
+
+ public KeepClassesWithMembersDefaultCtorTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testReference() throws Exception {
+ testForRuntime(parameters)
+ .addInnerClasses(KeepClassesWithMembersDefaultCtorTest.class)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test
+ public void testCompatR8() throws Exception {
+ checkInitKept(run(testForR8Compat(parameters.getBackend())));
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ checkInitNotKept(run(testForR8(parameters.getBackend())));
+ }
+
+ @Test
+ public void testPG() throws Exception {
+ checkInitKept(run(testForProguard(ProguardVersion.getLatest()).addDontWarn(getClass())));
+ }
+
+ private TestRunResult<?> run(TestShrinkerBuilder<?, ?, ?, ?, ?> builder) throws Exception {
+ return builder
+ .addInnerClasses(KeepClassesWithMembersDefaultCtorTest.class)
+ .addKeepRules("-keepclasseswithmembers class * { <fields>; }")
+ .addKeepClassAndMembersRules(TestClass.class)
+ .addDontObfuscate()
+ .run(parameters.getRuntime(), TestClass.class);
+ }
+
+ private TestRunResult<?> checkInitKept(TestRunResult<?> result) throws Exception {
+ return result
+ .inspect(inspector -> assertThat(inspector.clazz(A.class).init(), isPresent()))
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ private TestRunResult<?> checkInitNotKept(TestRunResult<?> result) throws Exception {
+ return result
+ .inspectFailure(inspector -> assertThat(inspector.clazz(A.class).init(), isAbsent()))
+ .assertFailureWithErrorThatThrows(NoSuchMethodException.class);
+ }
+
+ static class A {
+
+ public long x = System.nanoTime();
+
+ public A() {
+ System.out.println("A()");
+ }
+ }
+
+ static class TestClass {
+
+ public static A getA() {
+ // Since TestClass.A is hard kept, the shrinker can't assume anything about the return value.
+ return null;
+ }
+
+ public static void main(String[] args) throws Exception {
+ String name = args.length == 0 ? "A" : null;
+ Class<?> clazz =
+ Class.forName(
+ TestClass.class.getPackage().getName()
+ + ".KeepClassesWithMembersDefaultCtorTest$"
+ + name);
+ Object obj = clazz.getConstructor().newInstance();
+ if (args.length > 0) {
+ // Use the field so we are sure that the keep rule triggers.
+ A a = getA();
+ System.out.println(a.x);
+ }
+ }
+ }
+}
