Version 1.3.39
Merge: Mark more lambdas as instantiated
CL: https://r8-review.googlesource.com/c/r8/+/31180
Bug: 119899698
Change-Id: Ie18cf50b5029839b1f43b09a6eebb305e2a45a16
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 3393210..f905944 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.3.38";
+ public static final String LABEL = "1.3.39";
private Version() {
}
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 150e516..fd04436 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
@@ -3,16 +3,15 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
-import com.android.tools.r8.Diagnostic;
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.ir.desugar.LambdaDescriptor;
import com.android.tools.r8.origin.Origin;
-import com.android.tools.r8.utils.Reporter;
-import com.android.tools.r8.utils.StringDiagnostic;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
+import java.util.ArrayDeque;
import java.util.Collections;
+import java.util.Deque;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.List;
@@ -243,32 +242,19 @@
* <p>The returned set of methods all have {@code callSite.methodName} as the method name.
*
* @param callSite Call site to resolve.
- * @param reporter Reporter used when an unknown metafactory is encountered.
* @return Methods implemented by the lambda expression that created the {@code callSite}.
*/
- public Set<DexEncodedMethod> lookupLambdaImplementedMethods(
- DexCallSite callSite, Reporter reporter) {
+ public Set<DexEncodedMethod> lookupLambdaImplementedMethods(DexCallSite callSite) {
List<DexType> callSiteInterfaces = LambdaDescriptor.getInterfaces(callSite, this);
- if (callSiteInterfaces == null) {
- if (!isStringConcat(callSite.bootstrapMethod)) {
- if (reporter != null) {
- Diagnostic message =
- new StringDiagnostic("Unknown bootstrap method " + callSite.bootstrapMethod);
- reporter.warning(message);
- }
- }
+ if (callSiteInterfaces == null || callSiteInterfaces.isEmpty()) {
return Collections.emptySet();
}
Set<DexEncodedMethod> result = new HashSet<>();
- for (DexType iface : callSiteInterfaces) {
+ Deque<DexType> worklist = new ArrayDeque<>(callSiteInterfaces);
+ Set<DexType> visited = Sets.newIdentityHashSet();
+ while (!worklist.isEmpty()) {
+ DexType iface = worklist.removeFirst();
if (iface.isUnknown()) {
- if (reporter != null) {
- StringDiagnostic message =
- new StringDiagnostic(
- "Lambda expression implements missing library interface "
- + iface.toSourceString());
- reporter.warning(message);
- }
// Skip this interface. If the lambda only implements missing library interfaces and not any
// program interfaces, then minification and tree shaking are not interested in this
// DexCallSite anyway, so skipping this interface is harmless. On the other hand, if
@@ -279,21 +265,26 @@
// anyway.
continue;
}
+ if (!visited.add(iface)) {
+ // Already visited previously. May happen due to "diamond shapes" in the interface
+ // hierarchy.
+ continue;
+ }
assert iface.isInterface();
DexClass clazz = definitionFor(iface);
if (clazz != null) {
- clazz.forEachMethod(
- method -> {
- if (method.method.name == callSite.methodName && method.accessFlags.isAbstract()) {
- result.add(method);
- }
- });
+ for (DexEncodedMethod method : clazz.virtualMethods()) {
+ if (method.method.name == callSite.methodName && method.accessFlags.isAbstract()) {
+ result.add(method);
+ }
+ }
+ Collections.addAll(worklist, clazz.interfaces.values);
}
}
return result;
}
- private boolean isStringConcat(DexMethodHandle bootstrapMethod) {
+ public boolean isStringConcat(DexMethodHandle bootstrapMethod) {
return bootstrapMethod.type.isInvokeStatic()
&& (bootstrapMethod.asMethod() == dexItemFactory.stringConcatWithConstantsMethod
|| bootstrapMethod.asMethod() == dexItemFactory.stringConcatMethod);
diff --git a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
index 6b109c5..df11ba9 100644
--- a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
@@ -264,7 +264,7 @@
// Don't report errors, as the set of call sites is a conservative estimate, and can
// refer to interfaces which has been removed.
Set<DexEncodedMethod> implementedMethods =
- appInfo.lookupLambdaImplementedMethods(callSite, null);
+ appInfo.lookupLambdaImplementedMethods(callSite);
if (implementedMethods.isEmpty()) {
return;
}
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 dbf5338..f43dab5 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -524,9 +524,22 @@
public void registerCallSite(DexCallSite callSite) {
callSites.add(callSite);
super.registerCallSite(callSite);
- for (DexEncodedMethod method :
- appInfo.lookupLambdaImplementedMethods(callSite, options.reporter)) {
- markLambdaInstantiated(method.method.holder, currentMethod);
+
+ List<DexType> directInterfaces = LambdaDescriptor.getInterfaces(callSite, appInfo);
+ if (directInterfaces != null) {
+ for (DexType lambdaInstantiatedInterface : directInterfaces) {
+ markLambdaInstantiated(lambdaInstantiatedInterface, currentMethod);
+ }
+ } else {
+ if (!appInfo.isStringConcat(callSite.bootstrapMethod)) {
+ if (options.reporter != null) {
+ Diagnostic message =
+ new StringDiagnostic(
+ "Unknown bootstrap method " + callSite.bootstrapMethod,
+ appInfo.originFor(currentMethod.method.holder));
+ options.reporter.warning(message);
+ }
+ }
}
LambdaDescriptor descriptor = LambdaDescriptor.tryInfer(callSite, appInfo);
@@ -570,7 +583,6 @@
// and implement all lambda interfaces.
ScopedDexMethodSet seen = new ScopedDexMethodSet();
- List<DexType> directInterfaces = LambdaDescriptor.getInterfaces(callSite, appInfo);
if (directInterfaces == null) {
return;
}
@@ -1019,7 +1031,31 @@
}
private void markLambdaInstantiated(DexType itf, DexEncodedMethod method) {
- instantiatedLambdas.add(itf, KeepReason.instantiatedIn(method));
+ DexClass clazz = appInfo.definitionFor(itf);
+ if (clazz == null) {
+ if (options.reporter != null) {
+ StringDiagnostic message =
+ new StringDiagnostic(
+ "Lambda expression implements missing interface `" + itf.toSourceString() + "`",
+ appInfo.originFor(method.method.holder));
+ options.reporter.warning(message);
+ }
+ return;
+ }
+ if (!clazz.isInterface()) {
+ if (options.reporter != null) {
+ StringDiagnostic message =
+ new StringDiagnostic(
+ "Lambda expression expected to implement an interface, but found "
+ + "`" + itf.toSourceString() + "`",
+ appInfo.originFor(method.method.holder));
+ options.reporter.warning(message);
+ }
+ return;
+ }
+ if (clazz.isProgramClass()) {
+ instantiatedLambdas.add(itf, KeepReason.instantiatedIn(method));
+ }
}
private void markDirectStaticOrConstructorMethodAsLive(
@@ -2309,6 +2345,9 @@
: Iterables.concat(
ImmutableList.of(refinedReceiverType), subtypes(refinedReceiverType));
for (DexType type : subTypesToExplore) {
+ if (instantiatedLambdas.contains(type)) {
+ return null;
+ }
if (pinnedItems.contains(type)) {
// For kept classes we cannot ensure a single target.
return null;
diff --git a/src/test/java/com/android/tools/r8/shaking/InstantiatedLambdaReceiverTest.java b/src/test/java/com/android/tools/r8/shaking/InstantiatedLambdaReceiverTest.java
new file mode 100644
index 0000000..225ddd6
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/InstantiatedLambdaReceiverTest.java
@@ -0,0 +1,71 @@
+// Copyright (c) 2018, 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.Assume.assumeTrue;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+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 InstantiatedLambdaReceiverTest extends TestBase {
+
+ private Backend backend;
+
+ private static final String expectedOutput = "In C.m()";
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public InstantiatedLambdaReceiverTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void jvmTest() throws Exception {
+ assumeTrue(
+ "JVM test independent of Art version - only run when testing on latest",
+ ToolHelper.getDexVm().getVersion().isLatest());
+ testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expectedOutput);
+ }
+
+ @Test
+ public void dexTest() throws Exception {
+ testForR8(backend)
+ .addInnerClasses(InstantiatedLambdaReceiverTest.class)
+ .addKeepMainRule(TestClass.class)
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput);
+ }
+
+ interface I {
+ void m();
+ }
+
+ interface II extends I {}
+
+ static class C implements I {
+
+ @Override
+ public void m() {
+ System.out.print("In C.m()");
+ }
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ I i = new C();
+ II x = i::m; // This should mark II as being instantiated!
+ x.m();
+ }
+ }
+}