diff options
author | kmb <kmb@google.com> | 2017-05-06 16:15:34 -0400 |
---|---|---|
committer | Kristina Chodorow <kchodorow@google.com> | 2017-05-08 09:49:07 -0400 |
commit | 909619aa9d14f8b92c1700c9f6ec4cae881245f0 (patch) | |
tree | 66af0bea957ebb787378affef3aa27cdec8cfe6e /src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java | |
parent | 40d9cb30f9f1a9b94367036b42fabe9fcdfece2e (diff) |
fix issue with interfaces redefining (overriding) inherited default methods
RELNOTES: none
PiperOrigin-RevId: 155293316
Diffstat (limited to 'src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java')
-rw-r--r-- | src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java | 134 |
1 files changed, 97 insertions, 37 deletions
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 732f064bf5..c42062a3f7 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 @@ -18,7 +18,10 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import com.google.common.collect.ImmutableList; +import java.util.Comparator; import java.util.HashSet; +import java.util.Set; +import java.util.TreeSet; import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.MethodVisitor; @@ -33,18 +36,20 @@ public class DefaultMethodClassFixer extends ClassVisitor { private final ClassReaderFactory classpath; private final ClassReaderFactory bootclasspath; + private final ClassLoader targetLoader; private final HashSet<String> instanceMethods = new HashSet<>(); - private final HashSet<String> seenInterfaces = new HashSet<>(); private boolean isInterface; - private ImmutableList<String> interfaces; + private String internalName; + private ImmutableList<String> directInterfaces; private String superName; public DefaultMethodClassFixer(ClassVisitor dest, ClassReaderFactory classpath, - ClassReaderFactory bootclasspath) { + ClassReaderFactory bootclasspath, ClassLoader targetLoader) { super(Opcodes.ASM5, dest); this.classpath = classpath; this.bootclasspath = bootclasspath; + this.targetLoader = targetLoader; } @Override @@ -55,22 +60,23 @@ public class DefaultMethodClassFixer extends ClassVisitor { String signature, String superName, String[] interfaces) { - checkState(this.interfaces == null); + checkState(this.directInterfaces == null); isInterface = BitFlags.isSet(access, Opcodes.ACC_INTERFACE); + internalName = name; checkArgument(superName != null || "java/lang/Object".equals(name), // ASM promises this "Type without superclass: %s", name); - this.interfaces = ImmutableList.copyOf(interfaces); + this.directInterfaces = ImmutableList.copyOf(interfaces); this.superName = superName; super.visit(version, access, name, signature, superName, interfaces); } @Override public void visitEnd() { - if (!isInterface && defaultMethodsDefined(interfaces)) { + if (!isInterface && 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(); - stubMissingDefaultMethods(interfaces); + stubMissingDefaultMethods(); } super.visitEnd(); } @@ -85,6 +91,51 @@ public class DefaultMethodClassFixer extends ClassVisitor { return super.visitMethod(access, name, desc, signature, exceptions); } + private void stubMissingDefaultMethods() { + TreeSet<Class<?>> allInterfaces = new TreeSet<>(InterfaceComparator.INSTANCE); + for (String direct : directInterfaces) { + // Loading ensures all transitively implemented interfaces can be loaded, which is necessary + // to produce correct default method stubs in all cases. We could do without classloading but + // it's convenient to rely on Class.isAssignableFrom to compute subtype relationships, and + // we'd still have to insist that all transitively implemented interfaces can be loaded. + // We don't load the visited class, however, in case it's a generated lambda class. + Class<?> itf = loadFromInternal(direct); + collectInterfaces(itf, allInterfaces); + } + + Class<?> superclass = loadFromInternal(superName); + 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. + continue; + } + stubMissingDefaultMethods(interfaceToVisit.getName().replace('.', '/')); + } + } + + private Class<?> loadFromInternal(String internalName) { + try { + return targetLoader.loadClass(internalName.replace('/', '.')); + } catch (ClassNotFoundException e) { + throw new IllegalStateException( + "Couldn't load " + internalName + ", is the classpath complete?", e); + } + } + + private void collectInterfaces(Class<?> itf, Set<Class<?>> dest) { + checkArgument(itf.isInterface()); + if (!dest.add(itf)) { + return; + } + for (Class<?> implemented : itf.getInterfaces()) { + collectInterfaces(implemented, dest); + } + } + private void recordInheritedMethods() { InstanceMethodRecorder recorder = new InstanceMethodRecorder(); String internalName = superName; @@ -145,29 +196,23 @@ public class DefaultMethodClassFixer extends ClassVisitor { && !instanceMethods.contains(name + ":" + desc); } - private void stubMissingDefaultMethods(ImmutableList<String> interfaces) { - for (String implemented : interfaces) { - if (!seenInterfaces.add(implemented)) { - // Skip: a superclass already implements this interface, or we've seen it here - continue; - } - ClassReader bytecode = classpath.readIfKnown(implemented); - if (bytecode != null && !bootclasspath.isKnown(implemented)) { - // 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. - bytecode.accept(new DefaultMethodStubber(), ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG); - } + private void stubMissingDefaultMethods(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); + // We can skip code attributes as we just need to find default methods to stub. + bytecode.accept(new DefaultMethodStubber(), ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG); } /** * Visitor for interfaces that produces delegates in the class visited by the outer * {@link DefaultMethodClassFixer} for every default method encountered. */ - public class DefaultMethodStubber extends ClassVisitor { + private class DefaultMethodStubber extends ClassVisitor { - @SuppressWarnings("hiding") private ImmutableList<String> interfaces; private String interfaceName; public DefaultMethodStubber() { @@ -183,20 +228,20 @@ public class DefaultMethodClassFixer extends ClassVisitor { String superName, String[] interfaces) { checkArgument(BitFlags.isSet(access, Opcodes.ACC_INTERFACE)); - checkState(this.interfaces == null); - this.interfaces = ImmutableList.copyOf(interfaces); + checkState(interfaceName == null); interfaceName = name; } @Override - public void visitEnd() { - stubMissingDefaultMethods(this.interfaces); - } - - @Override public MethodVisitor visitMethod( int access, String name, String desc, String signature, String[] exceptions) { if (shouldStub(access, name, desc)) { + // Remember we stubbed this method in case it's also defined by subsequently visited + // interfaces. javac would force the method to be defined explicitly if there any two + // definitions conflict, but see stubMissingDefaultMethods() for how we deal with default + // methods redefined in interfaces extending another. + recordIfInstanceMethod(access, name, desc); + // 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 // ACC_SYNTHETIC modifier, so don't. @@ -230,7 +275,7 @@ public class DefaultMethodClassFixer extends ClassVisitor { /** * Visitor for interfaces that recursively searches interfaces for default method declarations. */ - public class DefaultMethodFinder extends ClassVisitor { + private class DefaultMethodFinder extends ClassVisitor { @SuppressWarnings("hiding") private ImmutableList<String> interfaces; private boolean found; @@ -290,13 +335,6 @@ public class DefaultMethodClassFixer extends ClassVisitor { String superName, String[] interfaces) { checkArgument(BitFlags.noneSet(access, Opcodes.ACC_INTERFACE)); - for (String inheritedInterface : interfaces) { - // No point copying default methods that we'll also copy for a superclass. Note we may - // be processing a class in the bootclasspath, in which case the interfaces must also - // be in the bootclasspath and we can skip those as well. Also note this is best-effort, - // since these interfaces may extend other interfaces that we're not recording here. - seenInterfaces.add(inheritedInterface); - } super.visit(version, access, name, signature, superName, interfaces); } @@ -307,4 +345,26 @@ public class DefaultMethodClassFixer extends ClassVisitor { return null; } } + + /** Comparator for interfaces that compares by whether interfaces extend one another. */ + enum InterfaceComparator implements Comparator<Class<?>> { + INSTANCE; + + @Override + public int compare(Class<?> o1, Class<?> o2) { + checkArgument(o1.isInterface()); + checkArgument(o2.isInterface()); + if (o1 == o2) { + return 0; + } + if (o1.isAssignableFrom(o2)) { // o1 is supertype of o2 + return 1; // we want o1 to come after o2 + } + if (o2.isAssignableFrom(o1)) { // o2 is supertype of o1 + return -1; // we want o2 to come after o1 + } + // o1 and o2 aren't comparable so arbitrarily impose lexicographical ordering + return o1.getName().compareTo(o2.getName()); + } + } } |