diff options
author | Googler <noreply@google.com> | 2017-02-24 22:49:46 +0000 |
---|---|---|
committer | Yue Gan <yueg@google.com> | 2017-02-27 15:06:15 +0000 |
commit | 0dcdb06fb5dce3425209de9bb66fc76e34279fb7 (patch) | |
tree | e1afca4b97bd25fa05b5544e4867e36a615a59f2 /src/tools/android/java/com/google/devtools/build | |
parent | d00e44684f1513fff532c0008560ade17ea390f3 (diff) |
More stable naming scheme for lambda classes in desugared android code
RELNOTES: More stable naming scheme for lambda classes in desugared android code
--
PiperOrigin-RevId: 148506830
MOS_MIGRATED_REVID=148506830
Diffstat (limited to 'src/tools/android/java/com/google/devtools/build')
5 files changed, 71 insertions, 40 deletions
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 2d973de959..9dc07d7b29 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 @@ -184,12 +184,9 @@ class Desugar { visitor = new Java7Compatibility(visitor, readerFactory); } - visitor = new LambdaDesugaring( - visitor, - loader, - lambdas, - interfaceLambdaMethodCollector, - allowDefaultMethods); + visitor = + new LambdaDesugaring( + visitor, loader, lambdas, interfaceLambdaMethodCollector, allowDefaultMethods); reader.accept(visitor, 0); @@ -225,7 +222,7 @@ class Desugar { visitor = new Java7Compatibility(visitor, (ClassReaderFactory) null); } - LambdaClassFixer lambdaFixer = + visitor = new LambdaClassFixer( visitor, lambdaClass.getValue(), @@ -235,9 +232,11 @@ class Desugar { // Send lambda classes through desugaring to make sure there's no invokedynamic // instructions in generated lambda classes (checkState below will fail) reader.accept( - new LambdaDesugaring(lambdaFixer, loader, lambdas, null, allowDefaultMethods), 0); - String name = rewriter.unprefix(lambdaFixer.getInternalName() + ".class"); - writeStoredEntry(out, name, writer.toByteArray()); + new LambdaDesugaring(visitor, loader, lambdas, null, allowDefaultMethods), + 0); + String filename = + rewriter.unprefix(lambdaClass.getValue().desiredInternalName()) + ".class"; + writeStoredEntry(out, filename, writer.toByteArray()); } } diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java index 97c0249c5b..12f735d896 100644 --- a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java @@ -51,7 +51,7 @@ class LambdaClassFixer extends ClassVisitor { private final HashSet<String> implementedMethods = new HashSet<>(); private final LinkedHashSet<String> methodsToMoveIn = new LinkedHashSet<>(); - private String internalName; + private String originalInternalName; private ImmutableList<String> interfaces; private boolean hasState; @@ -71,10 +71,6 @@ class LambdaClassFixer extends ClassVisitor { this.allowDefaultMethods = allowDefaultMethods; } - public String getInternalName() { - return internalName; - } - @Override public void visit( int version, @@ -84,15 +80,16 @@ class LambdaClassFixer extends ClassVisitor { String superName, String[] interfaces) { checkArgument(BitFlags.noneSet(access, Opcodes.ACC_INTERFACE), "Not a class: %s", name); - checkState(internalName == null, "Already visited %s, can't reuse for %s", internalName, name); - internalName = name; + checkState(this.originalInternalName == null, "not intended for reuse but reused for %s", name); + originalInternalName = name; hasState = false; hasFactory = false; desc = null; this.signature = null; exceptions = null; this.interfaces = ImmutableList.copyOf(interfaces); - super.visit(version, access, name, signature, superName, interfaces); + // Rename to desired name + super.visit(version, access, getInternalName(), signature, superName, interfaces); } @Override @@ -143,14 +140,14 @@ class LambdaClassFixer extends ClassVisitor { @Override public void visitEnd() { checkState(!hasState || hasFactory, - "Expected factory method for capturing lambda %s", internalName); + "Expected factory method for capturing lambda %s", getInternalName()); if (!hasFactory) { // Fake factory method if LambdaMetafactory didn't generate it checkState(signature == null, - "Didn't expect generic constructor signature %s %s", internalName, signature); + "Didn't expect generic constructor signature %s %s", getInternalName(), signature); // Since this is a stateless class we populate and use a static singleton field "$instance" - String singletonFieldDesc = Type.getObjectType(internalName).getDescriptor(); + String singletonFieldDesc = Type.getObjectType(getInternalName()).getDescriptor(); super.visitField( Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL, "$instance", @@ -166,11 +163,12 @@ class LambdaClassFixer extends ClassVisitor { "()V", (String) null, new String[0]); - codeBuilder.visitTypeInsn(Opcodes.NEW, internalName); + codeBuilder.visitTypeInsn(Opcodes.NEW, getInternalName()); codeBuilder.visitInsn(Opcodes.DUP); - codeBuilder.visitMethodInsn(Opcodes.INVOKESPECIAL, internalName, "<init>", - checkNotNull(desc, "didn't see a constructor for %s", internalName), /*itf*/ false); - codeBuilder.visitFieldInsn(Opcodes.PUTSTATIC, internalName, "$instance", singletonFieldDesc); + codeBuilder.visitMethodInsn(Opcodes.INVOKESPECIAL, getInternalName(), "<init>", + checkNotNull(desc, "didn't see a constructor for %s", getInternalName()), /*itf*/ false); + codeBuilder.visitFieldInsn( + Opcodes.PUTSTATIC, getInternalName(), "$instance", singletonFieldDesc); codeBuilder.visitInsn(Opcodes.RETURN); codeBuilder.visitMaxs(2, 0); // two values are pushed onto the stack codeBuilder.visitEnd(); @@ -182,7 +180,8 @@ class LambdaClassFixer extends ClassVisitor { lambdaInfo.factoryMethodDesc(), (String) null, exceptions); - codeBuilder.visitFieldInsn(Opcodes.GETSTATIC, internalName, "$instance", singletonFieldDesc); + codeBuilder.visitFieldInsn( + Opcodes.GETSTATIC, getInternalName(), "$instance", singletonFieldDesc); codeBuilder.visitInsn(Opcodes.ARETURN); codeBuilder.visitMaxs(1, 0); // one value on the stack } @@ -195,6 +194,10 @@ class LambdaClassFixer extends ClassVisitor { super.visitEnd(); } + private String getInternalName() { + return lambdaInfo.desiredInternalName(); + } + private void copyRewrittenLambdaMethods() { for (String rewritten : methodsToMoveIn) { String interfaceInternalName = rewritten.substring(0, rewritten.indexOf('#')); @@ -233,17 +236,41 @@ class LambdaClassFixer extends ClassVisitor { // Rewrite invocations of lambda methods in interfaces to anticipate the lambda method being // moved into the lambda class (i.e., the class being visited here). checkArgument(opcode == Opcodes.INVOKESTATIC, "Cannot move instance method %s", method); - owner = internalName; + owner = getInternalName(); itf = false; // owner was interface but is now a class methodsToMoveIn.add(method); - } else if (name.startsWith("lambda$")) { - // Reflect renaming of lambda$ instance methods to avoid accidental overrides - name = LambdaDesugaring.uniqueInPackage(owner, name); + } else { + if (originalInternalName.equals(owner)) { + // Reflect renaming of lambda classes + owner = getInternalName(); + } + if (name.startsWith("lambda$")) { + // Reflect renaming of lambda$ instance methods to avoid accidental overrides + name = LambdaDesugaring.uniqueInPackage(owner, name); + } } super.visitMethodInsn(opcode, owner, name, desc, itf); } @Override + public void visitTypeInsn(int opcode, String type) { + if (originalInternalName.equals(type)) { + // Reflect renaming of lambda classes + type = getInternalName(); + } + super.visitTypeInsn(opcode, type); + } + + @Override + public void visitFieldInsn(int opcode, String owner, String name, String desc) { + if (originalInternalName.equals(owner)) { + // Reflect renaming of lambda classes + owner = getInternalName(); + } + super.visitFieldInsn(opcode, owner, name, desc); + } + + @Override public AnnotationVisitor visitAnnotation(String desc, boolean visible) { // Drop annotation that's part of the generated lambda class that's not available on Android. // Proguard complains about this otherwise. diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java index 8a2ebea51b..ef8740770b 100644 --- a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java @@ -14,7 +14,6 @@ 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.ImmutableMap; import com.google.common.collect.Iterators; @@ -41,7 +40,7 @@ class LambdaClassMaker { this.rootDirectory = rootDirectory; } - public String generateLambdaClass(String invokerInternalName, LambdaInfo lambdaInfo, + public void generateLambdaClass(String invokerInternalName, LambdaInfo lambdaInfo, MethodHandle bootstrapMethod, ArrayList<Object> bsmArgs) throws IOException { // Invoking the bootstrap method will dump the generated class try { @@ -52,11 +51,7 @@ class LambdaClassMaker { } Path generatedClassFile = findOnlyUnprocessed(invokerInternalName + "$$Lambda$"); - String lambdaClassName = generatedClassFile.toString(); - checkState(lambdaClassName.endsWith(".class"), "Unexpected filename %s", lambdaClassName); - lambdaClassName = lambdaClassName.substring(0, lambdaClassName.length() - ".class".length()); generatedClasses.put(generatedClassFile, lambdaInfo); - return lambdaClassName; } /** diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java index 79be2a3270..a3cf3bf532 100644 --- a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java @@ -55,6 +55,7 @@ class LambdaDesugaring extends ClassVisitor { private String internalName; private boolean isInterface; + private int lambdaCount; public LambdaDesugaring( ClassVisitor dest, @@ -77,6 +78,7 @@ class LambdaDesugaring extends ClassVisitor { String signature, String superName, String[] interfaces) { + checkState(internalName == null, "not intended for reuse but reused for %s", name); internalName = name; isInterface = BitFlags.isSet(access, Opcodes.ACC_INTERFACE); super.visit(version, access, name, signature, superName, interfaces); @@ -360,9 +362,13 @@ class LambdaDesugaring extends ClassVisitor { // and ultimately we don't care if the bootstrap method was even on the bootclasspath // when this class was compiled (although it must've been since javac is unhappy otherwise). MethodHandle bsmMethod = toMethodHandle(publicLookup(), bsm, /*target*/ false); - String lambdaClassName = lambdas.generateLambdaClass( + // Give generated classes to have more stable names (b/35643761). Use BSM's naming scheme + // but with separate counter for each surrounding class. + String lambdaClassName = internalName + "$$Lambda$" + (lambdaCount++); + lambdas.generateLambdaClass( internalName, - LambdaInfo.create(desc, bridgeInfo.methodReference(), bridgeInfo.bridgeMethod()), + LambdaInfo.create( + lambdaClassName, desc, bridgeInfo.methodReference(), bridgeInfo.bridgeMethod()), bsmMethod, args); // Emit invokestatic that calls the factory method generated in the lambda class diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaInfo.java b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaInfo.java index e3bb84f891..dad340ceef 100644 --- a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaInfo.java +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaInfo.java @@ -19,11 +19,15 @@ import org.objectweb.asm.Handle; @AutoValue abstract class LambdaInfo { public static LambdaInfo create( - String factoryMethodDesc, Handle methodReference, Handle bridgeMethod) { + String desiredInternalName, + String factoryMethodDesc, + Handle methodReference, + Handle bridgeMethod) { return new AutoValue_LambdaInfo( - factoryMethodDesc, methodReference, bridgeMethod); + desiredInternalName, factoryMethodDesc, methodReference, bridgeMethod); } + public abstract String desiredInternalName(); public abstract String factoryMethodDesc(); public abstract Handle methodReference(); public abstract Handle bridgeMethod(); |