From 17d008dc0cc602c46135f4a4f6f55a6d93431d77 Mon Sep 17 00:00:00 2001 From: kmb Date: Thu, 8 Feb 2018 18:11:29 -0800 Subject: Stub default methods as needed for core library desugaring RELNOTES: None PiperOrigin-RevId: 185082719 GitOrigin-RevId: aa79fd483daff0db9be274c33de109257f8a6804 Change-Id: I90cad779653c93f9917f69fe06daad2bbf919f65 --- .../build/android/desugar/CoreLibrarySupport.java | 15 ++- .../android/desugar/DefaultMethodClassFixer.java | 130 ++++++++++++++------- .../devtools/build/android/desugar/Desugar.java | 14 ++- .../android/desugar/CoreLibrarySupportTest.java | 17 +++ .../desugar/DefaultMethodClassFixerTest.java | 1 + 5 files changed, 133 insertions(+), 44 deletions(-) diff --git a/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java b/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java index c6fd0b4..2437a19 100644 --- a/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java +++ b/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/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java b/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java index 1aaf0b6..2eda141 100644 --- a/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java +++ b/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 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> 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 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/java/com/google/devtools/build/android/desugar/Desugar.java b/java/com/google/devtools/build/android/desugar/Desugar.java index 4b35575..109093c 100644 --- a/java/com/google/devtools/build/android/desugar/Desugar.java +++ b/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, diff --git a/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java b/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java index 089e231..d52ef78 100644 --- a/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java +++ b/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java @@ -74,6 +74,23 @@ public class CoreLibrarySupportTest { assertThat(support.renameCoreLibrary("com/google/X")).isEqualTo("com/google/X"); } + @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 = diff --git a/test/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixerTest.java b/test/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixerTest.java index c74febb..27083db 100644 --- a/test/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixerTest.java +++ b/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); -- cgit v1.2.3