Mark library method overrides starting from the instantiated type.
This CL fixes an existing issue where interface methods might not be marked and
documents an outstanding issue where lambda are simply not considered.
Bug: 149976493
Change-Id: I8aa3843a3c68518c283ef8f70c06e1583bf4a8c8
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 2b5cdde..4dd45b3 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1842,25 +1842,20 @@
markVirtualMethodAsLive(
clazz, target, KeepReason.isLibraryMethod(clazz, libraryOrClasspathClass.type));
}
- markOverridesAsLibraryMethodOverrides(clazz, target);
+ markOverridesAsLibraryMethodOverrides(instantiatedClass, target.method);
}
private void markOverridesAsLibraryMethodOverrides(
- DexProgramClass holder, DexEncodedMethod libraryMethodOverride) {
- assert holder.type == libraryMethodOverride.holder();
- if (libraryMethodOverride.isLibraryMethodOverride().isTrue()) {
- return;
- }
- libraryMethodOverride.setLibraryMethodOverride(OptionalBool.TRUE);
+ DexProgramClass instantiatedClass, DexMethod libraryMethodOverride) {
WorkList<DexType> worklist = WorkList.newIdentityWorkList();
- holder.forEachImmediateSupertype(worklist::addIfNotSeen);
+ worklist.addIfNotSeen(instantiatedClass.type);
while (worklist.hasNext()) {
DexType type = worklist.next();
DexProgramClass clazz = getProgramClassOrNull(type);
if (clazz == null) {
continue;
}
- DexEncodedMethod override = clazz.lookupVirtualMethod(libraryMethodOverride.method);
+ DexEncodedMethod override = clazz.lookupVirtualMethod(libraryMethodOverride);
if (override != null) {
if (override.isLibraryMethodOverride().isTrue()) {
continue;
diff --git a/src/test/java/com/android/tools/r8/shaking/LibraryMethodOverrideInInterfaceMarkingTest.java b/src/test/java/com/android/tools/r8/shaking/LibraryMethodOverrideInInterfaceMarkingTest.java
new file mode 100644
index 0000000..bff8192
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/LibraryMethodOverrideInInterfaceMarkingTest.java
@@ -0,0 +1,111 @@
+// 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.shaking;
+
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexType;
+import java.util.AbstractList;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class LibraryMethodOverrideInInterfaceMarkingTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public LibraryMethodOverrideInInterfaceMarkingTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(LibraryMethodOverrideInInterfaceMarkingTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addOptionsModification(
+ options -> options.testing.enqueuerInspector = this::verifyLibraryOverrideInformation)
+ .enableMergeAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("true", "true");
+ }
+
+ private void verifyLibraryOverrideInformation(AppInfoWithLiveness appInfo, Enqueuer.Mode mode) {
+ DexItemFactory dexItemFactory = appInfo.dexItemFactory();
+ verifyIsEmptyMarkedAsOverridingLibraryMethod(
+ appInfo, dexItemFactory.createType(descriptor(A.class)));
+ verifyIsEmptyMarkedAsOverridingLibraryMethod(
+ appInfo, dexItemFactory.createType(descriptor(I.class)));
+ }
+
+ private void verifyIsEmptyMarkedAsOverridingLibraryMethod(
+ AppInfoWithLiveness appInfo, DexType type) {
+ DexProgramClass clazz = appInfo.definitionFor(type).asProgramClass();
+ DexEncodedMethod method =
+ clazz.lookupVirtualMethod(m -> m.method.name.toString().equals("isEmpty"));
+ assertTrue(method.isLibraryMethodOverride().isTrue());
+ }
+
+ static class TestClass {
+
+ static void onA(A a) {
+ System.out.println(a.isEmpty());
+ }
+
+ static void onI(I i) {
+ System.out.println(i.isEmpty());
+ }
+
+ public static void main(String[] args) {
+ Object object = args.length == 42 ? null : new B();
+ onA((A) object);
+ onI((I) object);
+ }
+ }
+
+ @NeverMerge
+ abstract static class A extends AbstractList<Object> {
+
+ @Override
+ public boolean isEmpty() {
+ return true;
+ }
+
+ @Override
+ public int size() {
+ return 0;
+ }
+
+ @Override
+ public A get(int index) {
+ return null;
+ }
+ }
+
+ @NeverMerge
+ interface I {
+
+ boolean isEmpty();
+ }
+
+ @NeverMerge
+ static class B extends A implements I {
+ // Intentionally empty.
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/LibraryMethodOverrideInLambdaMarkingTest.java b/src/test/java/com/android/tools/r8/shaking/LibraryMethodOverrideInLambdaMarkingTest.java
new file mode 100644
index 0000000..e3925ca
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/LibraryMethodOverrideInLambdaMarkingTest.java
@@ -0,0 +1,110 @@
+// 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.shaking;
+
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.shaking.Enqueuer.Mode;
+import java.util.Iterator;
+import java.util.Spliterator;
+import java.util.function.Consumer;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class LibraryMethodOverrideInLambdaMarkingTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public LibraryMethodOverrideInLambdaMarkingTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(LibraryMethodOverrideInLambdaMarkingTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addOptionsModification(
+ options -> options.testing.enqueuerInspector = this::verifyLibraryOverrideInformation)
+ .enableMergeAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("null", "null");
+ }
+
+ private void verifyLibraryOverrideInformation(AppInfoWithLiveness appInfo, Mode mode) {
+ DexItemFactory dexItemFactory = appInfo.dexItemFactory();
+ verifyIteratorMethodMarkedAsOverridingLibraryMethod(
+ appInfo, dexItemFactory.createType(descriptor(I.class)));
+ verifyIteratorMethodMarkedAsOverridingLibraryMethod(
+ appInfo, dexItemFactory.createType(descriptor(J.class)));
+ }
+
+ private void verifyIteratorMethodMarkedAsOverridingLibraryMethod(
+ AppInfoWithLiveness appInfo, DexType type) {
+ DexProgramClass clazz = appInfo.definitionFor(type).asProgramClass();
+ DexEncodedMethod method =
+ clazz.lookupVirtualMethod(m -> m.method.name.toString().equals("iterator"));
+ // TODO(b/149976493): Mark library overrides from lambda instances.
+ assertTrue(method.isLibraryMethodOverride().isFalse());
+ }
+
+ static class TestClass {
+
+ public static void onI(I i) {
+ System.out.println(i.iterator());
+ }
+
+ public static void onJ(J j) {
+ System.out.println(j.iterator());
+ }
+
+ public static void main(String[] args) {
+ Object f = ((I & J) () -> null);
+ onI((I) f);
+ onJ((J) f);
+ }
+ }
+
+ @FunctionalInterface
+ @NeverMerge
+ interface I {
+
+ Iterator<Object> iterator();
+ }
+
+ @FunctionalInterface
+ @NeverMerge
+ interface J extends Iterable<Object> {
+
+ @Override
+ Iterator<Object> iterator();
+
+ @Override
+ default void forEach(Consumer<? super Object> action) {
+ // Intentionally empty.
+ }
+
+ @Override
+ default Spliterator<Object> spliterator() {
+ return null;
+ }
+ }
+}
