diff options
author | kmb <kmb@google.com> | 2017-11-13 17:18:29 -0800 |
---|---|---|
committer | Colin Cross <ccross@android.com> | 2017-11-29 11:28:17 -0800 |
commit | 8daf1cc87a760edfda738e11be41d61776b4e630 (patch) | |
tree | e64f4b8ec08d0c3ebd14611dc238fcce16410265 | |
parent | 8095de2eba76d6b60bbdd23b025eed4671e7f812 (diff) | |
download | desugar-8daf1cc87a760edfda738e11be41d61776b4e630.tar.gz |
Fix EnclosingMethod attribute when moving interface methods to companion class
RELNOTES: None.
PiperOrigin-RevId: 175613518
GitOrigin-RevId: f581da7375d8548ffaac61ead74cdc3519eeb5b2
Change-Id: I2333812920923fa8050022b8f482e139c37f9027
4 files changed, 132 insertions, 16 deletions
diff --git a/java/com/google/devtools/build/android/desugar/BitFlags.java b/java/com/google/devtools/build/android/desugar/BitFlags.java index 8542719..bb32c45 100644 --- a/java/com/google/devtools/build/android/desugar/BitFlags.java +++ b/java/com/google/devtools/build/android/desugar/BitFlags.java @@ -13,6 +13,8 @@ // limitations under the License. package com.google.devtools.build.android.desugar; +import org.objectweb.asm.Opcodes; + /** * Convenience method for working with {@code int} bitwise flags. */ @@ -34,6 +36,18 @@ class BitFlags { return (flags & bitmask) == 0; } + public static boolean isInterface(int access) { + return isSet(access, Opcodes.ACC_INTERFACE); + } + + public static boolean isStatic(int access) { + return isSet(access, Opcodes.ACC_STATIC); + } + + public static boolean isSynthetic(int access) { + return isSet(access, Opcodes.ACC_SYNTHETIC); + } + // Static methods only private BitFlags() {} } diff --git a/java/com/google/devtools/build/android/desugar/ClassVsInterface.java b/java/com/google/devtools/build/android/desugar/ClassVsInterface.java new file mode 100644 index 0000000..cb62deb --- /dev/null +++ b/java/com/google/devtools/build/android/desugar/ClassVsInterface.java @@ -0,0 +1,63 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.android.desugar; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; + +import java.util.HashMap; +import javax.annotation.Nullable; +import org.objectweb.asm.ClassReader; + +/** + * Simple memoizer for whether types are classes or interfaces. + */ +class ClassVsInterface { + /** Map from internal names to whether they are an interface ({@code false} thus means class). */ + private final HashMap<String, Boolean> known = new HashMap<>(); + private final ClassReaderFactory classpath; + + public ClassVsInterface(ClassReaderFactory classpath) { + this.classpath = classpath; + } + + public ClassVsInterface addKnownClass(@Nullable String internalName) { + if (internalName != null) { + Boolean previous = known.put(internalName, false); + checkState(previous == null || !previous, "Already recorded as interface: %s", internalName); + } + return this; + } + + public ClassVsInterface addKnownInterfaces(String... internalNames) { + for (String internalName : internalNames) { + Boolean previous = known.put(internalName, true); + checkState(previous == null || previous, "Already recorded as class: %s", internalName); + } + return this; + } + + public boolean isOuterInterface(String outerName, String innerName) { + Boolean result = known.get(outerName); + if (result == null) { + // We could just load the outer class here, but this tolerates incomplete classpaths better. + // Note the outer class should be in the Jar we're desugaring, so it should always be there. + ClassReader outerClass = checkNotNull(classpath.readIfKnown(outerName), + "Couldn't find outer class %s of %s", outerName, innerName); + result = BitFlags.isInterface(outerClass.getAccess()); + known.put(outerName, result); + } + return result; + } +} diff --git a/java/com/google/devtools/build/android/desugar/Desugar.java b/java/com/google/devtools/build/android/desugar/Desugar.java index 87aa0d7..2b02386 100644 --- a/java/com/google/devtools/build/android/desugar/Desugar.java +++ b/java/com/google/devtools/build/android/desugar/Desugar.java @@ -24,7 +24,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSet.Builder; import com.google.common.io.ByteStreams; import com.google.common.io.Closer; import com.google.devtools.build.android.Converters.ExistingPathConverter; @@ -358,7 +357,7 @@ class Desugar { } ImmutableSet.Builder<String> interfaceLambdaMethodCollector = ImmutableSet.builder(); - + ClassVsInterface interfaceCache = new ClassVsInterface(classpathReader); desugarClassesInInput( inputFiles, outputFileProvider, @@ -366,6 +365,7 @@ class Desugar { classpathReader, depsCollector, bootclasspathReader, + interfaceCache, interfaceLambdaMethodCollector); desugarAndWriteDumpedLambdaClassesToOutput( @@ -374,6 +374,7 @@ class Desugar { classpathReader, depsCollector, bootclasspathReader, + interfaceCache, interfaceLambdaMethodCollector.build(), bridgeMethodReader); @@ -445,7 +446,8 @@ class Desugar { @Nullable ClassReaderFactory classpathReader, DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, - Builder<String> interfaceLambdaMethodCollector) + ClassVsInterface interfaceCache, + ImmutableSet.Builder<String> interfaceLambdaMethodCollector) throws IOException { for (String filename : inputFiles) { if (OutputFileProvider.DESUGAR_DEPS_FILENAME.equals(filename)) { @@ -465,6 +467,7 @@ class Desugar { classpathReader, depsCollector, bootclasspathReader, + interfaceCache, interfaceLambdaMethodCollector, writer, reader); @@ -492,6 +495,7 @@ class Desugar { @Nullable ClassReaderFactory classpathReader, DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, + ClassVsInterface interfaceCache, ImmutableSet<String> interfaceLambdaMethods, @Nullable ClassReaderFactory bridgeMethodReader) throws IOException { @@ -522,6 +526,7 @@ class Desugar { classpathReader, depsCollector, bootclasspathReader, + interfaceCache, interfaceLambdaMethods, bridgeMethodReader, lambdaClass.getValue(), @@ -564,6 +569,7 @@ class Desugar { @Nullable ClassReaderFactory classpathReader, DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, + ClassVsInterface interfaceCache, ImmutableSet<String> interfaceLambdaMethods, @Nullable ClassReaderFactory bridgeMethodReader, LambdaInfo lambdaClass, @@ -591,7 +597,12 @@ class Desugar { visitor, classpathReader, depsCollector, bootclasspathReader, loader); visitor = new InterfaceDesugaring( - visitor, depsCollector, bootclasspathReader, store, options.legacyJacocoFix); + visitor, + interfaceCache, + depsCollector, + bootclasspathReader, + store, + options.legacyJacocoFix); } } visitor = @@ -621,7 +632,8 @@ class Desugar { @Nullable ClassReaderFactory classpathReader, DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, - Builder<String> interfaceLambdaMethodCollector, + ClassVsInterface interfaceCache, + ImmutableSet.Builder<String> interfaceLambdaMethodCollector, UnprefixingClassWriter writer, ClassReader input) { ClassVisitor visitor = checkNotNull(writer); @@ -645,7 +657,12 @@ class Desugar { visitor, classpathReader, depsCollector, bootclasspathReader, loader); visitor = new InterfaceDesugaring( - visitor, depsCollector, bootclasspathReader, store, options.legacyJacocoFix); + visitor, + interfaceCache, + depsCollector, + bootclasspathReader, + store, + options.legacyJacocoFix); } } // LambdaDesugaring is relatively expensive, so check first whether we need it. Additionally, diff --git a/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java b/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java index d7e3886..9f24646 100644 --- a/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java +++ b/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java @@ -44,6 +44,7 @@ class InterfaceDesugaring extends ClassVisitor { static final String INTERFACE_STATIC_COMPANION_METHOD_SUFFIX = "$$STATIC$$"; + private final ClassVsInterface interfaceCache; private final DependencyCollector depsCollector; private final ClassReaderFactory bootclasspath; private final GeneratedClassStore store; @@ -58,11 +59,13 @@ class InterfaceDesugaring extends ClassVisitor { public InterfaceDesugaring( ClassVisitor dest, + ClassVsInterface interfaceCache, DependencyCollector depsCollector, ClassReaderFactory bootclasspath, GeneratedClassStore store, boolean legacyJaCoCo) { super(Opcodes.ASM5, dest); + this.interfaceCache = interfaceCache; this.depsCollector = depsCollector; this.bootclasspath = bootclasspath; this.store = store; @@ -83,10 +86,14 @@ class InterfaceDesugaring extends ClassVisitor { bytecodeVersion = version; accessFlags = access; if (isInterface()) { + interfaceCache.addKnownInterfaces(name); // Record interface hierarchy. This helps avoid parsing .class files when double-checking // desugaring results later using collected dependency information. depsCollector.recordExtendedInterfaces(name, interfaces); + } else { + interfaceCache.addKnownClass(name); } + interfaceCache.addKnownClass(superName).addKnownInterfaces(interfaces); super.visit(version, access, name, signature, superName, interfaces); } @@ -174,16 +181,14 @@ class InterfaceDesugaring extends ClassVisitor { checkArgument(BitFlags.noneSet(access, Opcodes.ACC_NATIVE), "Forbidden per JLS ch 9.4"); boolean isLambdaBody = - name.startsWith("lambda$") && BitFlags.isSet(access, Opcodes.ACC_SYNTHETIC); + name.startsWith("lambda$") && BitFlags.isSynthetic(access); if (isLambdaBody) { access &= ~Opcodes.ACC_PUBLIC; // undo visibility change from LambdaDesugaring } - name = - normalizeInterfaceMethodName( - name, isLambdaBody, BitFlags.isSet(access, Opcodes.ACC_STATIC)); + name = normalizeInterfaceMethodName(name, isLambdaBody, BitFlags.isStatic(access)); codeOwner = getCompanionClassName(internalName); - if (BitFlags.isSet(access, Opcodes.ACC_STATIC)) { + if (BitFlags.isStatic(access)) { // Completely move static interface methods, which requires rewriting call sites result = companion() @@ -233,8 +238,27 @@ class InterfaceDesugaring extends ClassVisitor { : null; } + @Override + public void visitOuterClass(String owner, String name, String desc) { + // Proguard gets grumpy if an outer method doesn't exist, which can be the result of moving + // interface methods to companion classes (b/68260836). In that case (for which we need to + // figure out if "owner" is an interface) need to adjust the outer method information. + if (name != null && interfaceCache.isOuterInterface(owner, internalName)) { + // Just drop outer method info. That's unfortunate, but the only alternative would be to + // change the outer method to point to the companion class, which would mean the + // reflection methods that use this information would return a companion ($$CC) class name + // as well as a possibly-modified method name and signature, so it seems better to return + // the correct original interface name and no method information. Doing this also saves + // us from doing even more work to figure out whether the method is static and a lambda + // method, which we'd need to known to adjust name and descriptor correctly. + name = null; + desc = null; + } // otherwise there's no enclosing method that could've been moved, or owner is a class + super.visitOuterClass(owner, name, desc); + } + private boolean isInterface() { - return BitFlags.isSet(accessFlags, Opcodes.ACC_INTERFACE); + return BitFlags.isInterface(accessFlags); } private static boolean isStaticInitializer(String methodName) { @@ -243,18 +267,16 @@ class InterfaceDesugaring extends ClassVisitor { private static String normalizeInterfaceMethodName( String name, boolean isLambda, boolean isStatic) { - String suffix; if (isLambda) { // Rename lambda method to reflect the new owner. Not doing so confuses LambdaDesugaring // if it's run over this class again. LambdaDesugaring has already renamed the method from // its original name to include the interface name at this point. - suffix = DependencyCollector.INTERFACE_COMPANION_SUFFIX; + return name + DependencyCollector.INTERFACE_COMPANION_SUFFIX; } else if (isStatic) { - suffix = INTERFACE_STATIC_COMPANION_METHOD_SUFFIX; + return name + INTERFACE_STATIC_COMPANION_METHOD_SUFFIX; } else { return name; } - return name + suffix; } static String getCompanionClassName(String interfaceName) { |