Stub default methods as needed for core library desugaring

RELNOTES: None

PiperOrigin-RevId: 185082719

5 files changed, 133 insertions, 44 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 089e23174c..d52ef7821e 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
@@ -75,6 +75,23 @@ public class CoreLibrarySupportTest {
   }
 
   @Test
+  public void testIsEmulatedCoreClassOrInterface() throws Exception {
+    CoreLibrarySupport support =
+        new CoreLibrarySupport(
+            new CoreLibraryRewriter(""),
+            Thread.currentThread().getContextClassLoader(),
+            ImmutableList.of("java/util/concurrent/"),
+            ImmutableList.of("java/util/Map"));
+    assertThat(support.isEmulatedCoreClassOrInterface("java/util/Map")).isTrue();
+    assertThat(support.isEmulatedCoreClassOrInterface("java/util/Map$$Lambda$17")).isFalse();
+    assertThat(support.isEmulatedCoreClassOrInterface("java/util/Map$$CC")).isFalse();
+    assertThat(support.isEmulatedCoreClassOrInterface("java/util/HashMap")).isTrue();
+    assertThat(support.isEmulatedCoreClassOrInterface("java/util/concurrent/ConcurrentMap"))
+        .isFalse(); // false for renamed prefixes
+    assertThat(support.isEmulatedCoreClassOrInterface("com/google/Map")).isFalse();
+  }
+
+  @Test
   public void testIsEmulatedCoreLibraryInvocation() throws Exception {
     CoreLibrarySupport support =
         new CoreLibrarySupport(
diff --git a/src/test/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixerTest.java b/src/test/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixerTest.java
index c74febb184..27083dbfeb 100644
--- a/src/test/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixerTest.java
+++ b/src/test/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixerTest.java
@@ -102,6 +102,7 @@ public class DefaultMethodClassFixerTest {
             writer,
             classpathReader,
             DependencyCollector.NoWriteCollectors.FAIL_ON_MISSING,
+            /*coreLibrarySupport=*/ null,
             bootclassPath,
             classLoader);
     reader.accept(fixer, 0);
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 c6fd0b4817..2437a19979 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
@@ -73,6 +73,14 @@ class CoreLibrarySupport {
         : internalName;
   }
 
+  /**
+   * Returns {@code true} for java.* classes or interfaces that are subtypes of emulated interfaces.
+   * Note that implies that this method always returns {@code false} for user-written classes.
+   */
+  public boolean isEmulatedCoreClassOrInterface(String internalName) {
+    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;
@@ -81,9 +89,6 @@ class CoreLibrarySupport {
   @Nullable
   public Class<?> getEmulatedCoreLibraryInvocationTarget(
       int opcode, String owner, String name, String desc, boolean itf) {
-    if (owner.contains("$$Lambda$") || owner.endsWith("$$CC")) {
-      return null;  // regular desugaring handles invocations on generated classes, no emulation
-    }
     Class<?> clazz = getEmulatedCoreClassOrInterface(owner);
     if (clazz == null) {
       return null;
@@ -101,6 +106,10 @@ class CoreLibrarySupport {
   }
 
   private Class<?> getEmulatedCoreClassOrInterface(String internalName) {
+    if (internalName.contains("$$Lambda$") || internalName.endsWith("$$CC")) {
+      // Regular desugaring handles generated classes, no emulation is needed
+      return null;
+    }
     {
       String unprefixedOwner = rewriter.unprefix(internalName);
       if (!unprefixedOwner.startsWith("java/util/") || isRenamedCoreLibrary(unprefixedOwner)) {
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 1aaf0b69de..2eda141fb9 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
@@ -23,6 +23,7 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
 import java.util.TreeSet;
+import javax.annotation.Nullable;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.MethodVisitor;
@@ -44,6 +45,7 @@ public class DefaultMethodClassFixer extends ClassVisitor {
   private final ClassReaderFactory bootclasspath;
   private final ClassLoader targetLoader;
   private final DependencyCollector depsCollector;
+  @Nullable private final CoreLibrarySupport coreLibrarySupport;
   private final HashSet<String> instanceMethods = new HashSet<>();
 
   private boolean isInterface;
@@ -57,10 +59,12 @@ public class DefaultMethodClassFixer extends ClassVisitor {
       ClassVisitor dest,
       ClassReaderFactory classpath,
       DependencyCollector depsCollector,
+      @Nullable CoreLibrarySupport coreLibrarySupport,
       ClassReaderFactory bootclasspath,
       ClassLoader targetLoader) {
     super(Opcodes.ASM6, dest);
     this.classpath = classpath;
+    this.coreLibrarySupport = coreLibrarySupport;
     this.bootclasspath = bootclasspath;
     this.targetLoader = targetLoader;
     this.depsCollector = depsCollector;
@@ -88,7 +92,9 @@ public class DefaultMethodClassFixer extends ClassVisitor {
 
   @Override
   public void visitEnd() {
-    if (!isInterface && defaultMethodsDefined(directInterfaces)) {
+    if (!isInterface
+        && (mayNeedInterfaceStubsForEmulatedSuperclass()
+            || defaultMethodsDefined(directInterfaces))) {
       // Inherited methods take precedence over default methods, so visit all superclasses and
       // figure out what methods they declare before stubbing in any missing default methods.
       recordInheritedMethods();
@@ -190,6 +196,12 @@ public class DefaultMethodClassFixer extends ClassVisitor {
     return super.visitMethod(access, name, desc, signature, exceptions);
   }
 
+  private boolean mayNeedInterfaceStubsForEmulatedSuperclass() {
+    return coreLibrarySupport != null
+        && !coreLibrarySupport.isEmulatedCoreClassOrInterface(internalName)
+        && coreLibrarySupport.isEmulatedCoreClassOrInterface(superName);
+  }
+
   private void stubMissingDefaultAndBridgeMethods() {
     TreeSet<Class<?>> allInterfaces = new TreeSet<>(InterfaceComparator.INSTANCE);
     for (String direct : directInterfaces) {
@@ -203,19 +215,51 @@ public class DefaultMethodClassFixer extends ClassVisitor {
     }
 
     Class<?> superclass = loadFromInternal(superName);
+    boolean mayNeedStubsForSuperclass = mayNeedInterfaceStubsForEmulatedSuperclass();
+    if (mayNeedStubsForSuperclass) {
+      // Collect interfaces inherited from emulated superclasses as well, to handle things like
+      // extending AbstractList without explicitly implementing List.
+      for (Class<?> clazz = superclass; clazz != null; clazz = clazz.getSuperclass()) {
+        for (Class<?> itf : superclass.getInterfaces()) {
+          collectInterfaces(itf, allInterfaces);
+        }
+      }
+    }
     for (Class<?> interfaceToVisit : allInterfaces) {
       // if J extends I, J is allowed to redefine I's default methods.  The comparator we used
       // above makes sure we visit J before I in that case so we can use J's definition.
-      if (superclass != null && interfaceToVisit.isAssignableFrom(superclass)) {
-        // superclass already implements this interface, so we must skip it.  The superclass will
-        // be similarly rewritten or comes from the bootclasspath; either way we don't need to and
-        // shouldn't stub default methods for this interface.
+      if (!mayNeedStubsForSuperclass && interfaceToVisit.isAssignableFrom(superclass)) {
+        // superclass is also rewritten and already implements this interface, so we _must_ skip it.
         continue;
       }
       stubMissingDefaultAndBridgeMethods(interfaceToVisit.getName().replace('.', '/'));
     }
   }
 
+  private void stubMissingDefaultAndBridgeMethods(String implemented) {
+    ClassReader bytecode;
+    boolean isBootclasspath;
+    if (bootclasspath.isKnown(implemented)) {
+      if (coreLibrarySupport != null
+          && (coreLibrarySupport.isRenamedCoreLibrary(implemented)
+              || coreLibrarySupport.isEmulatedCoreClassOrInterface(implemented))) {
+        bytecode = checkNotNull(bootclasspath.readIfKnown(implemented), implemented);
+        isBootclasspath = true;
+      } else {
+        // Default methods from interfaces on the bootclasspath that we're not renaming or emulating
+        // are assumed available at runtime, so just ignore them.
+        return;
+      }
+    } else {
+      bytecode =
+          checkNotNull(
+              classpath.readIfKnown(implemented),
+              "Couldn't find interface %s implemented by %s", implemented, internalName);
+      isBootclasspath = false;
+    }
+    bytecode.accept(new DefaultMethodStubber(isBootclasspath), ClassReader.SKIP_DEBUG);
+  }
+
   private Class<?> loadFromInternal(String internalName) {
     try {
       return targetLoader.loadClass(internalName.replace('/', '.'));
@@ -313,26 +357,38 @@ public class DefaultMethodClassFixer extends ClassVisitor {
    */
   private boolean defaultMethodsDefined(ImmutableList<String> interfaces) {
     for (String implemented : interfaces) {
+      ClassReader bytecode;
       if (bootclasspath.isKnown(implemented)) {
-        continue;
-      }
-      ClassReader bytecode = classpath.readIfKnown(implemented);
-      if (bytecode == null) {
-        // Interface isn't on the classpath, which indicates incomplete classpaths. Record missing
-        // dependency so we can check it later.  If we don't check then we may get runtime failures
-        // or wrong behavior from default methods that should've been stubbed in.
-        // TODO(kmb): Print a warning so people can start fixing their deps?
-        depsCollector.missingImplementedInterface(internalName, implemented);
+        if (coreLibrarySupport != null
+            && coreLibrarySupport.isEmulatedCoreClassOrInterface(implemented)) {
+          return true; // need to stub in emulated interface methods such as Collection.stream()
+        } else if (coreLibrarySupport != null
+            && coreLibrarySupport.isRenamedCoreLibrary(implemented)) {
+          // Check default methods of renamed interfaces
+          bytecode = checkNotNull(bootclasspath.readIfKnown(implemented), implemented);
+        } else {
+          continue;
+        }
       } else {
-        // Class in classpath and bootclasspath is a bad idea but in any event, assume the
-        // bootclasspath will take precedence like in a classloader.
-        // We can skip code attributes as we just need to find default methods to stub.
-        DefaultMethodFinder finder = new DefaultMethodFinder();
-        bytecode.accept(finder, ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG);
-        if (finder.foundDefaultMethods()) {
-          return true;
+        bytecode = classpath.readIfKnown(implemented);
+        if (bytecode == null) {
+          // Interface isn't on the classpath, which indicates incomplete classpaths. Record missing
+          // dependency so we can check it later.  If we don't check then we may get runtime
+          // failures or wrong behavior from default methods that should've been stubbed in.
+          // TODO(kmb): Print a warning so people can start fixing their deps?
+          depsCollector.missingImplementedInterface(internalName, implemented);
+          continue;
         }
       }
+
+      // Class in classpath and bootclasspath is a bad idea but in any event, assume the
+      // bootclasspath will take precedence like in a classloader.
+      // We can skip code attributes as we just need to find default methods to stub.
+      DefaultMethodFinder finder = new DefaultMethodFinder();
+      bytecode.accept(finder, ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG);
+      if (finder.foundDefaultMethods()) {
+        return true;
+      }
     }
     return false;
   }
@@ -365,30 +421,18 @@ public class DefaultMethodClassFixer extends ClassVisitor {
         && !instanceMethods.contains(name + ":" + desc);
   }
 
-  private void stubMissingDefaultAndBridgeMethods(String implemented) {
-    if (bootclasspath.isKnown(implemented)) {
-      // Default methods on the bootclasspath will be available at runtime, so just ignore them.
-      return;
-    }
-    ClassReader bytecode =
-        checkNotNull(
-            classpath.readIfKnown(implemented),
-            "Couldn't find interface %s implemented by %s",
-            implemented,
-            internalName);
-    bytecode.accept(new DefaultMethodStubber(), ClassReader.SKIP_DEBUG);
-  }
-
   /**
    * Visitor for interfaces that produces delegates in the class visited by the outer {@link
    * DefaultMethodClassFixer} for every default method encountered.
    */
   private class DefaultMethodStubber extends ClassVisitor {
 
+    private final boolean isBootclasspathInterface;
     private String stubbedInterfaceName;
 
-    public DefaultMethodStubber() {
+    public DefaultMethodStubber(boolean isBootclasspathInterface) {
       super(Opcodes.ASM6);
+      this.isBootclasspathInterface = isBootclasspathInterface;
     }
 
     @Override
@@ -413,8 +457,11 @@ public class DefaultMethodClassFixer extends ClassVisitor {
         // definitions conflict, but see stubMissingDefaultMethods() for how we deal with default
         // methods redefined in interfaces extending another.
         recordIfInstanceMethod(access, name, desc);
-        depsCollector.assumeCompanionClass(
-            internalName, InterfaceDesugaring.getCompanionClassName(stubbedInterfaceName));
+        if (!isBootclasspathInterface) {
+          // Don't record these dependencies, as we can't check them
+          depsCollector.assumeCompanionClass(
+              internalName, InterfaceDesugaring.getCompanionClassName(stubbedInterfaceName));
+        }
 
         // Add this method to the class we're desugaring and stub in a body to call the default
         // implementation in the interface's companion class. ijar omits these methods when setting
@@ -444,6 +491,10 @@ public class DefaultMethodClassFixer extends ClassVisitor {
         return null;
       } else if (shouldStubAsBridgeDefaultMethod(access, name, desc)) {
         recordIfInstanceMethod(access, name, desc);
+        // 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");
         // 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.
@@ -454,7 +505,7 @@ public class DefaultMethodClassFixer extends ClassVisitor {
             depsCollector,
             internalName);
       } else {
-        return null; // we don't care about the actual code in these methods
+        return null; // not a default or bridge method or the class already defines this method.
       }
     }
   }
@@ -529,6 +580,7 @@ public class DefaultMethodClassFixer extends ClassVisitor {
     @Override
     public MethodVisitor visitMethod(
         int access, String name, String desc, String signature, String[] exceptions) {
+      // TODO(kmb): what if this method only exists on some devices, e.g., ArrayList.spliterator?
       recordIfInstanceMethod(access, name, desc);
       return null;
     }
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java b/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java
index 4b3557563e..109093c978 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java
@@ -662,7 +662,12 @@ class Desugar {
       if (options.desugarInterfaceMethodBodiesIfNeeded) {
         visitor =
             new DefaultMethodClassFixer(
-                visitor, classpathReader, depsCollector, bootclasspathReader, loader);
+                visitor,
+                classpathReader,
+                depsCollector,
+                coreLibrarySupport,
+                bootclasspathReader,
+                loader);
         visitor =
             new InterfaceDesugaring(
                 visitor,
@@ -736,7 +741,12 @@ class Desugar {
         if (options.desugarInterfaceMethodBodiesIfNeeded) {
           visitor =
               new DefaultMethodClassFixer(
-                  visitor, classpathReader, depsCollector, bootclasspathReader, loader);
+                  visitor,
+                  classpathReader,
+                  depsCollector,
+                  coreLibrarySupport,
+                  bootclasspathReader,
+                  loader);
           visitor =
               new InterfaceDesugaring(
                   visitor,