Apply interface invocation desugaring to renamed core libraries. Fix invokespecial invocations for core interfaces.

RELNOTES: None.

PiperOrigin-RevId: 186404206

4 files changed, 151 insertions, 67 deletions

diff --git a/src/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java b/src/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java
index d52ef7821e..853dbbe3b1 100644
--- a/src/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java
+++ b/src/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java
@@ -92,7 +92,7 @@ public class CoreLibrarySupportTest {
   }
 
   @Test
-  public void testIsEmulatedCoreLibraryInvocation() throws Exception {
+  public void testGetCoreInterfaceRewritingTarget_emulatedDefaultMethod() throws Exception {
     CoreLibrarySupport support =
         new CoreLibrarySupport(
             new CoreLibraryRewriter(""),
@@ -100,29 +100,7 @@ public class CoreLibrarySupportTest {
             ImmutableList.of(),
             ImmutableList.of("java/util/Collection"));
     assertThat(
-            support.isEmulatedCoreLibraryInvocation(
-                Opcodes.INVOKEINTERFACE,
-                "java/util/Collection",
-                "removeIf",
-                "(Ljava/util/function/Predicate;)Z",
-                true))
-        .isTrue(); // true for default method
-    assertThat(
-            support.isEmulatedCoreLibraryInvocation(
-                Opcodes.INVOKEINTERFACE, "java/util/Collection", "size", "()I", true))
-        .isFalse(); // false for abstract method
-  }
-
-  @Test
-  public void testGetEmulatedCoreLibraryInvocationTarget_defaultMethod() throws Exception {
-    CoreLibrarySupport support =
-        new CoreLibrarySupport(
-            new CoreLibraryRewriter(""),
-            Thread.currentThread().getContextClassLoader(),
-            ImmutableList.of(),
-            ImmutableList.of("java/util/Collection"));
-    assertThat(
-            support.getEmulatedCoreLibraryInvocationTarget(
+            support.getCoreInterfaceRewritingTarget(
                 Opcodes.INVOKEINTERFACE,
                 "java/util/Collection",
                 "removeIf",
@@ -130,7 +108,7 @@ public class CoreLibrarySupportTest {
                 true))
         .isEqualTo(Collection.class);
     assertThat(
-            support.getEmulatedCoreLibraryInvocationTarget(
+            support.getCoreInterfaceRewritingTarget(
                 Opcodes.INVOKEVIRTUAL,
                 "java/util/ArrayList",
                 "removeIf",
@@ -138,9 +116,9 @@ public class CoreLibrarySupportTest {
                 false))
         .isEqualTo(Collection.class);
     assertThat(
-            support.getEmulatedCoreLibraryInvocationTarget(
+            support.getCoreInterfaceRewritingTarget(
                 Opcodes.INVOKEINTERFACE,
-                "com/google/common/collect/ImmutableList",
+                "com/google/HypotheticalListInterface",
                 "removeIf",
                 "(Ljava/util/function/Predicate;)Z",
                 true))
@@ -148,7 +126,7 @@ public class CoreLibrarySupportTest {
   }
 
   @Test
-  public void testGetEmulatedCoreLibraryInvocationTarget_abstractMethod() throws Exception {
+  public void testGetCoreInterfaceRewritingTarget_abstractMethod() throws Exception {
     CoreLibrarySupport support =
         new CoreLibrarySupport(
             new CoreLibraryRewriter(""),
@@ -156,7 +134,7 @@ public class CoreLibrarySupportTest {
             ImmutableList.of(),
             ImmutableList.of("java/util/Collection"));
     assertThat(
-            support.getEmulatedCoreLibraryInvocationTarget(
+            support.getCoreInterfaceRewritingTarget(
                 Opcodes.INVOKEINTERFACE,
                 "java/util/Collection",
                 "size",
@@ -164,7 +142,7 @@ public class CoreLibrarySupportTest {
                 true))
         .isNull();
     assertThat(
-            support.getEmulatedCoreLibraryInvocationTarget(
+            support.getCoreInterfaceRewritingTarget(
                 Opcodes.INVOKEVIRTUAL,
                 "java/util/ArrayList",
                 "size",
@@ -174,7 +152,7 @@ public class CoreLibrarySupportTest {
   }
 
   @Test
-  public void testGetEmulatedCoreLibraryInvocationTarget_defaultOverride() throws Exception {
+  public void testGetCoreInterfaceRewritingTarget_emulatedDefaultOverride() throws Exception {
     CoreLibrarySupport support =
         new CoreLibrarySupport(
             new CoreLibraryRewriter(""),
@@ -182,7 +160,7 @@ public class CoreLibrarySupportTest {
             ImmutableList.of(),
             ImmutableList.of("java/util/Map"));
     assertThat(
-            support.getEmulatedCoreLibraryInvocationTarget(
+            support.getCoreInterfaceRewritingTarget(
                 Opcodes.INVOKEINTERFACE,
                 "java/util/Map",
                 "putIfAbsent",
@@ -190,7 +168,7 @@ public class CoreLibrarySupportTest {
                 true))
         .isEqualTo(Map.class);
     assertThat(
-            support.getEmulatedCoreLibraryInvocationTarget(
+            support.getCoreInterfaceRewritingTarget(
                 Opcodes.INVOKEINTERFACE,
                 "java/util/concurrent/ConcurrentMap",
                 "putIfAbsent",
@@ -198,7 +176,7 @@ public class CoreLibrarySupportTest {
                 true))
         .isNull(); // putIfAbsent is default in Map but abstract in ConcurrentMap
     assertThat(
-            support.getEmulatedCoreLibraryInvocationTarget(
+            support.getCoreInterfaceRewritingTarget(
                 Opcodes.INVOKEINTERFACE,
                 "java/util/concurrent/ConcurrentMap",
                 "getOrDefault",
@@ -208,7 +186,7 @@ public class CoreLibrarySupportTest {
   }
 
   @Test
-  public void testGetEmulatedCoreLibraryInvocationTarget_staticInterfaceMethod() throws Exception {
+  public void testGetCoreInterfaceRewritingTarget_staticInterfaceMethod() throws Exception {
     CoreLibrarySupport support =
         new CoreLibrarySupport(
             new CoreLibraryRewriter(""),
@@ -216,7 +194,7 @@ public class CoreLibrarySupportTest {
             ImmutableList.of(),
             ImmutableList.of("java/util/Comparator"));
     assertThat(
-            support.getEmulatedCoreLibraryInvocationTarget(
+            support.getCoreInterfaceRewritingTarget(
                 Opcodes.INVOKESTATIC,
                 "java/util/Comparator",
                 "reverseOrder",
@@ -225,8 +203,79 @@ public class CoreLibrarySupportTest {
         .isEqualTo(Comparator.class);
   }
 
+  /**
+   * Tests that call sites of renamed core libraries are treated like call sites in regular
+   * {@link InterfaceDesugaring}.
+   */
+  @Test
+  public void testGetEmulatedCoreLibraryInvocationTarget_renamed() throws Exception {
+    CoreLibrarySupport support =
+        new CoreLibrarySupport(
+            new CoreLibraryRewriter(""),
+            Thread.currentThread().getContextClassLoader(),
+            ImmutableList.of("java/util/"),
+            ImmutableList.of());
+
+    // regular invocations of default methods: ignored
+    assertThat(
+            support.getCoreInterfaceRewritingTarget(
+                Opcodes.INVOKEINTERFACE,
+                "java/util/Collection",
+                "removeIf",
+                "(Ljava/util/function/Predicate;)Z",
+                true))
+        .isNull();
+    assertThat(
+            support.getCoreInterfaceRewritingTarget(
+                Opcodes.INVOKEVIRTUAL,
+                "java/util/ArrayList",
+                "removeIf",
+                "(Ljava/util/function/Predicate;)Z",
+                false))
+        .isNull();
+
+    // abstract methods: ignored
+    assertThat(
+            support.getCoreInterfaceRewritingTarget(
+                Opcodes.INVOKEINTERFACE,
+                "java/util/Collection",
+                "size",
+                "()I",
+                true))
+        .isNull();
+
+    // static interface method
+    assertThat(
+            support.getCoreInterfaceRewritingTarget(
+                Opcodes.INVOKESTATIC,
+                "java/util/Comparator",
+                "reverseOrder",
+                "()Ljava/util/Comparator;",
+                true))
+        .isEqualTo(Comparator.class);
+
+    // invokespecial for default methods: find nearest definition
+    assertThat(
+            support.getCoreInterfaceRewritingTarget(
+                Opcodes.INVOKESPECIAL,
+                "java/util/List",
+                "removeIf",
+                "(Ljava/util/function/Predicate;)Z",
+                true))
+        .isEqualTo(Collection.class);
+    // invokespecial on a class: ignore (even if there's an inherited default method)
+    assertThat(
+            support.getCoreInterfaceRewritingTarget(
+                Opcodes.INVOKESPECIAL,
+                "java/util/ArrayList",
+                "removeIf",
+                "(Ljava/util/function/Predicate;)Z",
+                false))
+        .isNull();
+  }
+
   @Test
-  public void testGetEmulatedCoreLibraryInvocationTarget_ignoreRenamed() throws Exception {
+  public void testGetCoreInterfaceRewritingTarget_ignoreRenamedInvokeInterface() throws Exception {
     CoreLibrarySupport support =
         new CoreLibrarySupport(
             new CoreLibraryRewriter(""),
@@ -234,7 +283,7 @@ public class CoreLibrarySupportTest {
             ImmutableList.of("java/util/concurrent/"),  // should return null for these
             ImmutableList.of("java/util/Map"));
     assertThat(
-            support.getEmulatedCoreLibraryInvocationTarget(
+            support.getCoreInterfaceRewritingTarget(
                 Opcodes.INVOKEINTERFACE,
                 "java/util/Map",
                 "getOrDefault",
@@ -242,7 +291,7 @@ public class CoreLibrarySupportTest {
                 true))
         .isEqualTo(Map.class);
     assertThat(
-            support.getEmulatedCoreLibraryInvocationTarget(
+            support.getCoreInterfaceRewritingTarget(
                 Opcodes.INVOKEINTERFACE,
                 "java/util/concurrent/ConcurrentMap",
                 "getOrDefault",
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java b/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java
index 417248b8eb..e83ae41189 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java
@@ -51,7 +51,8 @@ public class CoreLibraryInvocationRewriter extends ClassVisitor {
     @Override
     public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
       Class<?> coreInterface =
-          support.getEmulatedCoreLibraryInvocationTarget(opcode, owner, name, desc, itf);
+          support.getCoreInterfaceRewritingTarget(opcode, owner, name, desc, itf);
+
       if (coreInterface != null) {
         String coreInterfaceName = coreInterface.getName().replace('.', '/');
         name =
@@ -60,18 +61,17 @@ public class CoreLibraryInvocationRewriter extends ClassVisitor {
         if (opcode == Opcodes.INVOKESTATIC) {
           checkState(owner.equals(coreInterfaceName));
         } else {
-          desc =
-              InterfaceDesugaring.companionDefaultMethodDescriptor(
-                  opcode == Opcodes.INVOKESPECIAL ? owner : coreInterfaceName, desc);
+          desc = InterfaceDesugaring.companionDefaultMethodDescriptor(coreInterfaceName, desc);
         }
 
         if (opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.INVOKESPECIAL) {
           checkArgument(itf, "Expected interface to rewrite %s.%s : %s", owner, name, desc);
-          owner = InterfaceDesugaring.getCompanionClassName(owner);
+          owner = InterfaceDesugaring.getCompanionClassName(coreInterfaceName);
         } else {
           // TODO(kmb): Simulate dynamic dispatch instead of calling most general default method
           owner = InterfaceDesugaring.getCompanionClassName(coreInterfaceName);
         }
+
         opcode = Opcodes.INVOKESTATIC;
         itf = false;
       }
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java b/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java
index 2437a19979..9f0163814e 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.android.desugar;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.collect.ImmutableList;
 import java.lang.reflect.Method;
@@ -37,8 +38,7 @@ class CoreLibrarySupport {
   private final ImmutableList<Class<?>> emulatedInterfaces;
 
   public CoreLibrarySupport(CoreLibraryRewriter rewriter, ClassLoader targetLoader,
-      ImmutableList<String> renamedPrefixes, ImmutableList<String> emulatedInterfaces)
-      throws ClassNotFoundException {
+      ImmutableList<String> renamedPrefixes, ImmutableList<String> emulatedInterfaces) {
     this.rewriter = rewriter;
     this.targetLoader = targetLoader;
     checkArgument(
@@ -47,7 +47,7 @@ class CoreLibrarySupport {
     ImmutableList.Builder<Class<?>> classBuilder = ImmutableList.builder();
     for (String itf : emulatedInterfaces) {
       checkArgument(itf.startsWith("java/util/"), itf);
-      Class<?> clazz = targetLoader.loadClass((rewriter.getPrefix() + itf).replace('/', '.'));
+      Class<?> clazz = loadFromInternal(rewriter.getPrefix() + itf);
       checkArgument(clazz.isInterface(), itf);
       classBuilder.add(clazz);
     }
@@ -56,7 +56,7 @@ class CoreLibrarySupport {
 
   public boolean isRenamedCoreLibrary(String internalName) {
     String unprefixedName = rewriter.unprefix(internalName);
-    if (!unprefixedName.startsWith("java/")) {
+    if (!unprefixedName.startsWith("java/") || renamedPrefixes.isEmpty()) {
       return false; // shortcut
     }
     // Rename any classes desugar might generate under java/ (for emulated interfaces) as well as
@@ -81,26 +81,60 @@ class CoreLibrarySupport {
     return getEmulatedCoreClassOrInterface(internalName) != null;
   }
 
-  public boolean isEmulatedCoreLibraryInvocation(
-      int opcode, String owner, String name, String desc, boolean itf) {
-    return getEmulatedCoreLibraryInvocationTarget(opcode, owner, name, desc, itf) != null;
-  }
-
+  /**
+   * If the given invocation needs to go through a companion class of an emulated or renamed
+   * core interface, this methods returns that interface.  This is a helper method for
+   * {@link CoreLibraryInvocationRewriter}.
+   *
+   * <p>Always returns an interface (or {@code null}), even if {@code owner} is a class. Can only
+   * return non-{@code null} if {@code owner} is a core library type.
+   */
   @Nullable
-  public Class<?> getEmulatedCoreLibraryInvocationTarget(
+  public Class<?> getCoreInterfaceRewritingTarget(
       int opcode, String owner, String name, String desc, boolean itf) {
-    Class<?> clazz = getEmulatedCoreClassOrInterface(owner);
-    if (clazz == null) {
+    if (owner.contains("$$Lambda$") || owner.endsWith("$$CC")) {
+      // Regular desugaring handles generated classes, no emulation is needed
+      return null;
+    }
+    if (!itf && (opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.INVOKESPECIAL)) {
+      // Ignore staticly dispatched invocations on classes--they never need rewriting
       return null;
     }
+    Class<?> clazz;
+    if (isRenamedCoreLibrary(owner)) {
+      // For renamed invocation targets we just need to do what InterfaceDesugaring does, that is,
+      // only worry about invokestatic and invokespecial interface invocations; nothing to do for
+      // invokevirtual and invokeinterface.  InterfaceDesugaring ignores bootclasspath interfaces,
+      // so we have to do its work here for renamed interfaces.
+      if (itf
+          && (opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.INVOKESPECIAL)) {
+        clazz = loadFromInternal(owner);
+      } else {
+        return null;
+      }
+    } else {
+      // If not renamed, see if the owner needs emulation.
+      clazz = getEmulatedCoreClassOrInterface(owner);
+      if (clazz == null) {
+        return null;
+      }
+    }
+    checkArgument(itf == clazz.isInterface(), "%s expected to be interface: %s", owner, itf);
 
-    if (itf && opcode == Opcodes.INVOKESTATIC) {
-      return clazz; // static interface method
+    if (opcode == Opcodes.INVOKESTATIC) {
+      // Static interface invocation always goes to the given owner
+      checkState(itf); // we should've bailed out above.
+      return clazz;
     }
 
+    // See if the invoked method is a default method, which will need rewriting.  For invokespecial
+    // we can only get here if its a default method, and invokestatic we handled above.
     Method callee = findInterfaceMethod(clazz, name, desc);
     if (callee != null && callee.isDefault()) {
       return callee.getDeclaringClass();
+    } else {
+      checkArgument(opcode != Opcodes.INVOKESPECIAL,
+          "Couldn't resolve interface super call %s.super.%s : %s", owner, name, desc);
     }
     return null;
   }
@@ -117,19 +151,21 @@ class CoreLibrarySupport {
       }
     }
 
-    Class<?> clazz;
-    try {
-      clazz = targetLoader.loadClass(internalName.replace('/', '.'));
-    } catch (ClassNotFoundException e) {
-      throw (NoClassDefFoundError) new NoClassDefFoundError().initCause(e);
-    }
-
+    Class<?> clazz = loadFromInternal(internalName);
     if (emulatedInterfaces.stream().anyMatch(itf -> itf.isAssignableFrom(clazz))) {
       return clazz;
     }
     return null;
   }
 
+  private Class<?> loadFromInternal(String internalName) {
+    try {
+      return targetLoader.loadClass(internalName.replace('/', '.'));
+    } catch (ClassNotFoundException e) {
+      throw (NoClassDefFoundError) new NoClassDefFoundError().initCause(e);
+    }
+  }
+
   private static Method findInterfaceMethod(Class<?> clazz, String name, String desc) {
     return collectImplementedInterfaces(clazz, new LinkedHashSet<>())
         .stream()
@@ -141,7 +177,6 @@ class CoreLibrarySupport {
         .orElse((Method) null);
   }
 
-
   private static Method findMethod(Class<?> clazz, String name, String desc) {
     for (Method m : clazz.getMethods()) {
       if (m.getName().equals(name) && Type.getMethodDescriptor(m).equals(desc)) {
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java b/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java
index 52964b7f70..6143940391 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java
@@ -494,7 +494,7 @@ public class DefaultMethodClassFixer extends ClassVisitor {
         // If we're visiting a bootclasspath interface then we most likely don't have the code.
         // That means we can't just copy the method bodies as we're trying to do below.
         checkState(!isBootclasspathInterface,
-            "TODO stub bridge methods for core interfaces if ever needed");
+            "TODO stub core interface %s bridge methods in %s", stubbedInterfaceName, internalName);
         // For bridges we just copy their bodies instead of going through the companion class.
         // Meanwhile, we also need to desugar the copied method bodies, so that any calls to
         // interface methods are correctly handled.