From fa858918563ba19d0dd3680884c59e92ba4e2a3e Mon Sep 17 00:00:00 2001 From: kmb Date: Tue, 17 Jul 2018 12:41:48 -0700 Subject: Make default method stubbing resilient to core library renaming, to preserve idempotency. RELNOTES: None. PiperOrigin-RevId: 204957400 --- .../build/android/desugar/CoreLibrarySupport.java | 13 +++++++++ .../build/android/desugar/CorePackageRenamer.java | 20 +------------- .../android/desugar/DefaultMethodClassFixer.java | 32 +++++++++++++++------- 3 files changed, 36 insertions(+), 29 deletions(-) (limited to 'src/tools') 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 f247074852..9dfd43af9d 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 @@ -43,6 +43,7 @@ import org.objectweb.asm.Label; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; import org.objectweb.asm.Type; +import org.objectweb.asm.commons.Remapper; /** * Helper that keeps track of which core library classes and methods we want to rewrite. @@ -62,6 +63,14 @@ class CoreLibrarySupport { /** Map from {@code owner#name} core library members to their new owners. */ private final ImmutableMap memberMoves; + /** ASM {@link Remapper} based on {@link #renamedPrefixes}. */ + private final Remapper corePackageRemapper = new Remapper() { + @Override + public String map(String typeName) { + return isRenamedCoreLibrary(typeName) ? renameCoreLibrary(typeName) : typeName; + } + }; + /** For the collection of definitions of emulated default methods (deterministic iteration). */ private final Multimap emulatedDefaultMethods = LinkedHashMultimap.create(); @@ -131,6 +140,10 @@ class CoreLibrarySupport { : internalName; } + public Remapper getRemapper() { + return corePackageRemapper; + } + @Nullable public String getMoveTarget(String owner, String name) { return memberMoves.get(rewriter.unprefix(owner) + '#' + name); diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/CorePackageRenamer.java b/src/tools/android/java/com/google/devtools/build/android/desugar/CorePackageRenamer.java index 5f1dc2e0f6..050e4bfcc0 100644 --- a/src/tools/android/java/com/google/devtools/build/android/desugar/CorePackageRenamer.java +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/CorePackageRenamer.java @@ -15,7 +15,6 @@ package com.google.devtools.build.android.desugar; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.commons.ClassRemapper; -import org.objectweb.asm.commons.Remapper; /** * A visitor that renames packages so configured using {@link CoreLibrarySupport}.. @@ -23,23 +22,6 @@ import org.objectweb.asm.commons.Remapper; class CorePackageRenamer extends ClassRemapper { public CorePackageRenamer(ClassVisitor cv, CoreLibrarySupport support) { - super(cv, new CorePackageRemapper(support)); - } - - private static final class CorePackageRemapper extends Remapper { - private final CoreLibrarySupport support; - - private CorePackageRemapper(CoreLibrarySupport support) { - this.support = support; - } - - public boolean isRenamed(String owner) { - return support.isRenamedCoreLibrary(owner); - } - - @Override - public String map(String typeName) { - return isRenamed(typeName) ? support.renameCoreLibrary(typeName) : typeName; - } + super(cv, support.getRemapper()); } } 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 1de48bf585..130c197f9a 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 @@ -302,14 +302,6 @@ public class DefaultMethodClassFixer extends ClassVisitor { } } - private void recordIfInstanceMethod(int access, String name, String desc) { - if (BitFlags.noneSet(access, Opcodes.ACC_STATIC)) { - // Record all declared instance methods, including abstract, bridge, and native methods, as - // they all take precedence over default methods. - instanceMethods.add(name + ":" + desc); - } - } - /** * Starting from the given interfaces, this method scans the interface hierarchy, finds the * interfaces that have default methods and , and returns the companion class names of @@ -410,7 +402,7 @@ public class DefaultMethodClassFixer extends ClassVisitor { // Note that an exception is that, if a bridge method is for a default interface method, javac // will NOT generate the bridge method in the implementing class. So we need extra logic to // handle these bridge methods. - return isNonBridgeDefaultMethod(access) && !instanceMethods.contains(name + ":" + desc); + return isNonBridgeDefaultMethod(access) && !recordedInstanceMethod(name, desc); } private static boolean isNonBridgeDefaultMethod(int access) { @@ -426,7 +418,27 @@ public class DefaultMethodClassFixer extends ClassVisitor { private boolean shouldStubAsBridgeDefaultMethod(int access, String name, String desc) { return BitFlags.isSet(access, Opcodes.ACC_BRIDGE | Opcodes.ACC_PUBLIC) && BitFlags.noneSet(access, Opcodes.ACC_ABSTRACT | Opcodes.ACC_STATIC) - && !instanceMethods.contains(name + ":" + desc); + && !recordedInstanceMethod(name, desc); + } + + private void recordIfInstanceMethod(int access, String name, String desc) { + if (BitFlags.noneSet(access, Opcodes.ACC_STATIC)) { + // Record all declared instance methods, including abstract, bridge, and native methods, as + // they all take precedence over default methods. + if (coreLibrarySupport != null) { + // Foreshadow any type renaming to avoid issues with double-desugaring (b/111447199) + desc = coreLibrarySupport.getRemapper().mapMethodDesc(desc); + } + instanceMethods.add(name + ":" + desc); + } + } + + private boolean recordedInstanceMethod(String name, String desc) { + if (coreLibrarySupport != null) { + // Foreshadow any type renaming to avoid issues with double-desugaring (b/111447199) + desc = coreLibrarySupport.getRemapper().mapMethodDesc(desc); + } + return instanceMethods.contains(name + ":" + desc); } /** -- cgit v1.2.3