Mark bridge methods on interfaces as live
Otherwise they will be remove during the first round of treeshaking
and instantiated lambdaes may no longer keep their implementation
because we cannot trace through a bridge that no longer exists.
Bug: 133457361
Change-Id: Ie7498a3c1b01213a4cd98b5c755451306a33ab59
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
index a9773f8..6122236 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
@@ -363,15 +363,31 @@
}
}
+ Consumer<DexEncodedMethod> addIfNotAbstract =
+ m -> {
+ if (!m.accessFlags.isAbstract()) {
+ result.add(m);
+ }
+ };
+ // Default methods are looked up when looking at a specific subtype that does not override them.
+ // Otherwise, we would look up default methods that are actually never used. However, we have to
+ // add bridge methods, otherwise we can remove a bridge that will be used.
+ Consumer<DexEncodedMethod> addIfNotAbstractAndBridge =
+ m -> {
+ if (!m.accessFlags.isAbstract() && m.accessFlags.isBridge()) {
+ result.add(m);
+ }
+ };
+
Set<DexType> set = subtypes(method.holder);
for (DexType type : set) {
DexClass clazz = definitionFor(type);
- // Default methods are looked up when looking at a specific subtype that does not
- // override them, so we ignore interfaces here. Otherwise, we would look up default methods
- // that are factually never used.
- if (!clazz.isInterface()) {
+ if (clazz.isInterface()) {
+ ResolutionResult targetMethods = resolveMethodOnInterface(type, method);
+ targetMethods.forEachTarget(addIfNotAbstractAndBridge);
+ } else {
ResolutionResult targetMethods = resolveMethodOnClass(type, method);
- targetMethods.forEachTarget(result::add);
+ targetMethods.forEachTarget(addIfNotAbstract);
}
}
return result;
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 02ee0b5..ed7c25a 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -8,6 +8,7 @@
import static com.android.tools.r8.naming.IdentifierNameStringUtils.isReflectionMethod;
import static com.android.tools.r8.shaking.AnnotationRemover.shouldKeepAnnotation;
import static com.android.tools.r8.shaking.EnqueuerUtils.toImmutableSortedMap;
+import static com.google.common.base.Predicates.or;
import com.android.tools.r8.Diagnostic;
import com.android.tools.r8.dex.IndexedItemCollection;
@@ -1380,7 +1381,9 @@
private boolean isInstantiatedOrHasInstantiatedSubtype(DexType type) {
return instantiatedTypes.contains(type)
- || appInfo.subtypes(type).stream().anyMatch(instantiatedTypes::contains);
+ || instantiatedLambdas.contains(type)
+ || appInfo.subtypes(type).stream()
+ .anyMatch(or(instantiatedTypes::contains, instantiatedLambdas::contains));
}
private void markInstanceFieldAsReachable(DexField field, KeepReason reason) {
@@ -1502,7 +1505,8 @@
continue;
}
- if (instantiatedTypes.contains(possibleTarget.holder)) {
+ if (instantiatedTypes.contains(possibleTarget.holder)
+ || instantiatedLambdas.contains(possibleTarget.holder)) {
markVirtualMethodAsLive(
encodedPossibleTarget, KeepReason.reachableFromLiveType(possibleTarget.holder));
} else {
diff --git a/src/test/java/com/android/tools/r8/shaking/interfacebridge/LambdaAbstractMethodErrorTest.java b/src/test/java/com/android/tools/r8/shaking/interfacebridge/LambdaAbstractMethodErrorTest.java
new file mode 100644
index 0000000..48576f5
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/interfacebridge/LambdaAbstractMethodErrorTest.java
@@ -0,0 +1,48 @@
+// 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.interfacebridge;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+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 LambdaAbstractMethodErrorTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDexRuntimes().build();
+ }
+
+ public LambdaAbstractMethodErrorTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test_b133457361() throws ExecutionException, CompilationFailedException, IOException {
+ testForR8(parameters.getBackend())
+ .addProgramClassesAndInnerClasses(Main.class)
+ .addProgramClassesAndInnerClasses(Task.class, OuterClass.class)
+ .addKeepMainRule(Main.class)
+ .addOptionsModification(
+ internalOptions -> {
+ internalOptions.enableInlining = false;
+ internalOptions.enableClassInlining = false;
+ internalOptions.enableVerticalClassMerging = false;
+ })
+ .setMinApi(parameters.getRuntime())
+ .run(Main.class.getTypeName())
+ .assertSuccessWithOutput("FOO");
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/interfacebridge/Main.java b/src/test/java/com/android/tools/r8/shaking/interfacebridge/Main.java
new file mode 100644
index 0000000..5c3fb27
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/interfacebridge/Main.java
@@ -0,0 +1,12 @@
+// 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.interfacebridge;
+
+public class Main {
+
+ public static void main(String[] args) {
+ OuterClass.startTask(result -> System.out.print(result));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/interfacebridge/OuterClass.java b/src/test/java/com/android/tools/r8/shaking/interfacebridge/OuterClass.java
new file mode 100644
index 0000000..af0b8bc
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/interfacebridge/OuterClass.java
@@ -0,0 +1,29 @@
+// 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.interfacebridge;
+
+public class OuterClass {
+
+ public interface Callback extends Task.OnTaskFinishedListener<String> {
+ @Override
+ void onTaskFinished(String result);
+ }
+
+ private static class ConcreteTask extends Task<String> {
+
+ public ConcreteTask(Callback listener) {
+ super(listener);
+ }
+
+ @Override
+ public String doInBackground() {
+ return "FOO";
+ }
+ }
+
+ public static void startTask(Callback callback) {
+ new ConcreteTask(callback).execute();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/interfacebridge/Task.java b/src/test/java/com/android/tools/r8/shaking/interfacebridge/Task.java
new file mode 100644
index 0000000..98204ef
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/interfacebridge/Task.java
@@ -0,0 +1,30 @@
+// 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.interfacebridge;
+
+public abstract class Task<Result> {
+
+ public interface OnTaskFinishedListener<Result> {
+ void onTaskFinished(Result result);
+ }
+
+ public abstract Result doInBackground();
+
+ public void execute() {
+ onPostExecute(doInBackground());
+ }
+
+ OnTaskFinishedListener<Result> mListener;
+
+ Task(OnTaskFinishedListener<Result> listener) {
+ mListener = listener;
+ }
+
+ public void onPostExecute(Result result) {
+ if (mListener != null) {
+ mListener.onTaskFinished(result);
+ }
+ }
+}
