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();
+    }
+  }
+}