Mark interfaces as instantiated in compat mode.
Bug: 140471200
Change-Id: I3f9f4bca356a8f6d2b917ab37afcddda1eacd9d9
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 8cca825..e1650fc 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -261,11 +261,11 @@
private final Set<DexType> instantiatedAppServices = Sets.newIdentityHashSet();
/**
- * Set of interface types for which a lambda expression can be reached. These never have a single
- * interface implementation.
+ * Set of interface types for which there may be instantiations, such as lambda expressions or
+ * explicit keep rules.
*/
- private final SetWithReason<DexProgramClass> instantiatedLambdas =
- new SetWithReason<>(this::registerClass);
+ private final SetWithReason<DexProgramClass> instantiatedInterfaceTypes =
+ new SetWithReason<>(this::registerInterface);
/** A queue of items that need processing. Different items trigger different actions. */
private final EnqueuerWorklist workList;
@@ -435,8 +435,7 @@
DexProgramClass clazz = item.asDexClass().asProgramClass();
KeepReasonWitness witness = graphReporter.reportKeepClass(precondition, rules, clazz);
if (clazz.isInterface() && !clazz.accessFlags.isAnnotation()) {
- // TODO(zerny): This marking is due to a keep rule, so why add to the instantiatedLambdas?
- markInterfaceAsInstantiatedByLambda(clazz, witness);
+ markInterfaceAsInstantiated(clazz, witness);
} else {
workList.enqueueMarkInstantiatedAction(clazz, witness);
if (clazz.hasDefaultInitializer()) {
@@ -466,11 +465,10 @@
pinnedItems.add(item.toReference());
}
- // TODO(zerny): Why is this "ByLambda"?
- private void markInterfaceAsInstantiatedByLambda(DexProgramClass clazz, KeepReason reason) {
+ private void markInterfaceAsInstantiated(DexProgramClass clazz, KeepReason reason) {
assert clazz.isInterface() && !clazz.accessFlags.isAnnotation();
- if (!instantiatedLambdas.add(clazz, reason)) {
+ if (!instantiatedInterfaceTypes.add(clazz, reason)) {
return;
}
populateInstantiatedTypesCache(clazz);
@@ -911,7 +909,7 @@
if (clazz != null) {
KeepReason reason = KeepReason.methodHandleReferencedIn(currentMethod);
if (clazz.isInterface() && !clazz.accessFlags.isAnnotation()) {
- markInterfaceAsInstantiatedByLambda(clazz, reason);
+ markInterfaceAsInstantiated(clazz, reason);
} else {
markInstantiated(clazz, reason);
}
@@ -1020,7 +1018,7 @@
DexType baseType = type.toBaseType(appView.dexItemFactory());
if (baseType.isClassType()) {
DexProgramClass baseClass = getProgramClassOrNull(baseType);
- if (baseClass != null && !baseClass.isInterface()) {
+ if (baseClass != null) {
// Don't require any constructor, see b/112386012.
markClassAsInstantiatedWithCompatRule(baseClass);
} else {
@@ -1692,7 +1690,8 @@
return;
}
if (clazz.isProgramClass()) {
- if (instantiatedLambdas.add(clazz.asProgramClass(), KeepReason.instantiatedIn(method))) {
+ KeepReason reason = KeepReason.instantiatedIn(method);
+ if (instantiatedInterfaceTypes.add(clazz.asProgramClass(), reason)) {
populateInstantiatedTypesCache(clazz.asProgramClass());
}
}
@@ -1855,7 +1854,7 @@
}
if (instantiatedTypes.contains(clazz)
- || instantiatedLambdas.contains(clazz)
+ || instantiatedInterfaceTypes.contains(clazz)
|| pinnedItems.contains(clazz.type)) {
markVirtualMethodAsLive(
clazz, encodedPossibleTarget, KeepReason.reachableFromLiveType(possibleTarget.holder));
@@ -2108,7 +2107,8 @@
Collections.emptySet(),
Collections.emptyMap(),
Collections.emptyMap(),
- SetUtils.mapIdentityHashSet(instantiatedLambdas.getItems(), DexProgramClass::getType));
+ SetUtils.mapIdentityHashSet(
+ instantiatedInterfaceTypes.getItems(), DexProgramClass::getType));
appInfo.markObsolete();
return appInfoWithLiveness;
}
@@ -2436,6 +2436,10 @@
private void markClassAsInstantiatedWithCompatRule(DexProgramClass clazz) {
ProguardKeepRule rule = ProguardConfigurationUtils.buildDefaultInitializerKeepRule(clazz);
KeepReason reason = KeepReason.dueToProguardCompatibilityKeepRule(rule);
+ if (clazz.isInterface() && !clazz.accessFlags.isAnnotation()) {
+ markInterfaceAsInstantiated(clazz, reason);
+ return;
+ }
proguardCompatibilityWorkList.enqueueMarkInstantiatedAction(clazz, reason);
if (clazz.hasDefaultInitializer()) {
proguardCompatibilityWorkList.enqueueMarkReachableDirectAction(
@@ -3093,6 +3097,14 @@
return registerEdge(getClassGraphNode(type), reason);
}
+ private KeepReasonWitness registerInterface(DexProgramClass iface, KeepReason reason) {
+ assert iface.isInterface();
+ if (skipReporting(reason)) {
+ return KeepReasonWitness.INSTANCE;
+ }
+ return registerEdge(getClassGraphNode(iface.type), reason);
+ }
+
private KeepReasonWitness registerClass(DexProgramClass clazz, KeepReason reason) {
if (skipReporting(reason)) {
return KeepReasonWitness.INSTANCE;
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/keepinterface/CompatKeepInterfaceAsInstantiatedTest.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/keepinterface/CompatKeepInterfaceAsInstantiatedTest.java
new file mode 100644
index 0000000..7f4606c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/keepinterface/CompatKeepInterfaceAsInstantiatedTest.java
@@ -0,0 +1,138 @@
+// 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.shaking.forceproguardcompatibility.keepinterface;
+
+import static org.hamcrest.CoreMatchers.containsString;
+
+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;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class CompatKeepInterfaceAsInstantiatedTest extends TestBase {
+
+ public static class TestCast {
+
+ public static void main(String[] args) throws Exception {
+ Object bar =
+ Class.forName(
+ TestCast.class.getPackage().getName()
+ + ".CompatKeepInterfaceAsInstantiatedTest$Ba"
+ + (args.length == 42 ? "" : "r"))
+ .getDeclaredConstructor()
+ .newInstance();
+ System.out.println(
+ "It was a Foo! "
+ // This cast should trigger a compatibility keep for Foo.
+ + ((Foo) bar).getClass().getName());
+ }
+ }
+
+ public static class TestInstanceOf {
+
+ public static void main(String[] args) throws Exception {
+ Object bar =
+ Class.forName(
+ TestInstanceOf.class.getPackage().getName()
+ + ".CompatKeepInterfaceAsInstantiatedTest$Ba"
+ + (args.length == 42 ? "" : "r"))
+ .getDeclaredConstructor()
+ .newInstance();
+ // This instance-of causes Foo to be kept.
+ if (bar instanceof Foo) {
+ System.out.println("It was a Foo! " + bar.getClass().getName());
+ } else {
+ System.out.println("Who knows what it is...");
+ }
+ }
+ }
+
+ // Foo interface referenced in the above test classes.
+ public interface Foo {}
+
+ // Bar implementation of Foo, but not visible at compile time, reflectively created.
+ public static class Bar implements Foo {}
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withCfRuntimes().build();
+ }
+
+ private static final String expected =
+ StringUtils.lines("It was a Foo! " + Bar.class.getTypeName());
+
+ private final TestParameters parameters;
+
+ public CompatKeepInterfaceAsInstantiatedTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testCastReference() throws Exception {
+ testReference(TestCast.class);
+ }
+
+ @Test
+ public void testCastPG() throws Exception {
+ testCast(testForProguard()).assertSuccessWithOutput(expected);
+ }
+
+ @Test
+ public void testCastR8() throws Exception {
+ testCast(testForR8Compat(parameters.getBackend())).assertSuccessWithOutput(expected);
+ }
+
+ @Test
+ public void testInstanceOfReference() throws Exception {
+ testReference(TestInstanceOf.class);
+ }
+
+ @Test
+ public void testInstanceOfPG() throws Exception {
+ testInstanceOf(testForProguard()).assertSuccessWithOutput(expected);
+ }
+
+ @Test
+ public void testInstanceOfR8() throws Exception {
+ testInstanceOf(testForR8Compat(parameters.getBackend()))
+ // TODO(b/140471200): The compatibility behavior should keep Foo.
+ .assertFailureWithErrorThatMatches(containsString("NoClassDefFoundError"));
+ }
+
+ private TestRunResult<?> testCast(TestShrinkerBuilder<?, ?, ?, ?, ?> builder) throws Exception {
+ return testShrinker(builder, TestCast.class);
+ }
+
+ private TestRunResult<?> testInstanceOf(TestShrinkerBuilder<?, ?, ?, ?, ?> builder)
+ throws Exception {
+ return testShrinker(builder, TestInstanceOf.class);
+ }
+
+ private void testReference(Class<?> main) throws Exception {
+ testForJvm()
+ .addProgramClasses(main, Foo.class, Bar.class)
+ .run(parameters.getRuntime(), main)
+ .assertSuccessWithOutput(expected);
+ }
+
+ private TestRunResult<?> testShrinker(TestShrinkerBuilder<?, ?, ?, ?, ?> builder, Class<?> main)
+ throws Exception {
+ return builder
+ // Add -dontwarn to avoid PG failing since this test runner class is not present.
+ .addKeepRules("-dontwarn " + CompatKeepInterfaceAsInstantiatedTest.class.getTypeName())
+ .noMinification()
+ .addProgramClasses(main, Foo.class)
+ .addKeepMainRule(main)
+ .compile()
+ .addRunClasspathClasses(Bar.class)
+ .run(parameters.getRuntime(), main);
+ }
+}
