diff options
11 files changed, 376 insertions, 599 deletions
diff --git a/java/com/google/devtools/build/android/desugar/ClassReaderFactory.java b/java/com/google/devtools/build/android/desugar/ClassReaderFactory.java index 56f99a3..6ec4e0d 100644 --- a/java/com/google/devtools/build/android/desugar/ClassReaderFactory.java +++ b/java/com/google/devtools/build/android/desugar/ClassReaderFactory.java @@ -15,39 +15,31 @@ package com.google.devtools.build.android.desugar; import java.io.IOException; import java.io.InputStream; -import java.util.jar.JarFile; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import javax.annotation.Nullable; import org.objectweb.asm.ClassReader; class ClassReaderFactory { - private final IndexedJars indexedJars; + private final ZipFile jar; private final CoreLibraryRewriter rewriter; - public ClassReaderFactory(IndexedJars indexedJars, CoreLibraryRewriter rewriter) { + public ClassReaderFactory(ZipFile jar, CoreLibraryRewriter rewriter) { + this.jar = jar; this.rewriter = rewriter; - this.indexedJars = indexedJars; } /** * Returns a reader for the given/internal/Class$Name if the class is defined in the wrapped Jar - * and {@code null} otherwise. For simplicity this method turns checked into runtime exceptions + * and {@code null} otherwise. For simplicity this method turns checked into runtime excpetions * under the assumption that all classes have already been read once when this method is called. */ @Nullable public ClassReader readIfKnown(String internalClassName) { - String filename = rewriter.unprefix(internalClassName) + ".class"; - JarFile jarFile = indexedJars.getJarFile(filename); - - if (jarFile != null) { - return getClassReader(internalClassName, jarFile, jarFile.getEntry(filename)); + ZipEntry entry = jar.getEntry(rewriter.unprefix(internalClassName) + ".class"); + if (entry == null) { + return null; } - - return null; - } - - private ClassReader getClassReader(String internalClassName, ZipFile jar, ZipEntry entry) { try (InputStream bytecode = jar.getInputStream(entry)) { // ClassReader doesn't take ownership and instead eagerly reads the stream's contents return rewriter.reader(bytecode); diff --git a/java/com/google/devtools/build/android/desugar/Desugar.java b/java/com/google/devtools/build/android/desugar/Desugar.java index 6f9e0f0..2d973de 100644 --- a/java/com/google/devtools/build/android/desugar/Desugar.java +++ b/java/com/google/devtools/build/android/desugar/Desugar.java @@ -16,7 +16,6 @@ package com.google.devtools.build.android.desugar; import static com.google.common.base.Preconditions.checkState; import static java.nio.charset.StandardCharsets.ISO_8859_1; -import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.io.ByteStreams; @@ -35,7 +34,6 @@ import java.nio.file.Paths; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; import java.util.Enumeration; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.zip.CRC32; @@ -52,69 +50,55 @@ import org.objectweb.asm.ClassWriter; */ class Desugar { - /** Commandline options for {@link Desugar}. */ + /** + * Commandline options for {@link Desugar}. + */ public static class Options extends OptionsBase { - @Option( - name = "input", - allowMultiple = true, - defaultValue = "", - category = "input", - converter = ExistingPathConverter.class, - abbrev = 'i', - help = - "Input Jar with classes to desugar (required, the n-th input is paired with the n-th " - + "output)." - ) - public List<Path> inputJars; - - @Option( - name = "classpath_entry", - allowMultiple = true, - defaultValue = "", - category = "input", - converter = ExistingPathConverter.class, - help = "Ordered classpath to resolve symbols in the --input Jar, like javac's -cp flag." - ) + @Option(name = "input", + defaultValue = "null", + category = "input", + converter = ExistingPathConverter.class, + abbrev = 'i', + help = "Input Jar with classes to desugar.") + public Path inputJar; + + @Option(name = "classpath_entry", + allowMultiple = true, + defaultValue = "", + category = "input", + converter = ExistingPathConverter.class, + help = "Ordered classpath to resolve symbols in the --input Jar, like javac's -cp flag.") public List<Path> classpath; - @Option( - name = "bootclasspath_entry", - allowMultiple = true, - defaultValue = "", - category = "input", - converter = ExistingPathConverter.class, - help = "Bootclasspath that was used to compile the --input Jar with, like javac's " - + "-bootclasspath flag (required)." - ) + @Option(name = "bootclasspath_entry", + allowMultiple = true, + defaultValue = "", + category = "input", + converter = ExistingPathConverter.class, + help = "Bootclasspath that was used to compile the --input Jar with, like javac's " + + "-bootclasspath flag. If no bootclasspath is explicitly given then the tool's own " + + "bootclasspath is used.") public List<Path> bootclasspath; - @Option( - name = "allow_empty_bootclasspath", - defaultValue = "false", - category = "undocumented" - ) + @Option(name = "allow_empty_bootclasspath", + defaultValue = "false", + category = "misc", + help = "Don't use the tool's bootclasspath if no bootclasspath is given.") public boolean allowEmptyBootclasspath; - @Option( - name = "output", - allowMultiple = true, - defaultValue = "", - category = "output", - converter = PathConverter.class, - abbrev = 'o', - help = - "Output Jar to write desugared classes into (required, the n-th output is paired with " - + "the n-th input)." - ) - public List<Path> outputJars; - - @Option( - name = "verbose", - defaultValue = "false", - category = "misc", - abbrev = 'v', - help = "Enables verbose debugging output." - ) + @Option(name = "output", + defaultValue = "null", + category = "output", + converter = PathConverter.class, + abbrev = 'o', + help = "Output Jar to write desugared classes into.") + public Path outputJar; + + @Option(name = "verbose", + defaultValue = "false", + category = "misc", + abbrev = 'v', + help = "Enables verbose debugging output.") public boolean verbose; @Option( @@ -125,21 +109,10 @@ class Desugar { ) public int minSdkVersion; - @Option( - name = "copy_bridges_from_classpath", - defaultValue = "false", - category = "misc", - help = "Copy bridges from classpath to desugared classes." - ) - public boolean copyBridgesFromClasspath; - - @Option( - name = "core_library", - defaultValue = "false", - category = "undocumented", - implicitRequirements = "--allow_empty_bootclasspath", - help = "Enables rewriting to desugar java.* classes." - ) + @Option(name = "core_library", + defaultValue = "false", + category = "undocumented", + help = "Enables rewriting to desugar java.* classes.") public boolean coreLibrary; } @@ -160,146 +133,117 @@ class Desugar { args = Files.readAllLines(Paths.get(args[0].substring(1)), ISO_8859_1).toArray(new String[0]); } - OptionsParser optionsParser = OptionsParser.newOptionsParser(Options.class); - optionsParser.setAllowResidue(false); + OptionsParser optionsParser = + OptionsParser.newOptionsParser(Options.class); optionsParser.parseAndExitUponError(args); Options options = optionsParser.getOptions(Options.class); - checkState(!options.inputJars.isEmpty(), "--input is required"); - checkState( - options.inputJars.size() == options.outputJars.size(), - "Desugar requires the same number of inputs and outputs to pair them"); - checkState(!options.bootclasspath.isEmpty() || options.allowEmptyBootclasspath, - "At least one --bootclasspath_entry is required"); - if (options.verbose) { System.out.printf("Lambda classes will be written under %s%n", dumpDirectory); } - CoreLibraryRewriter rewriter = - new CoreLibraryRewriter(options.coreLibrary ? "__desugar__/" : ""); - boolean allowDefaultMethods = options.minSdkVersion >= 24; - boolean allowCallsToObjectsNonNull = options.minSdkVersion >= 19; - - LambdaClassMaker lambdas = new LambdaClassMaker(dumpDirectory); - - // Process each input separately - for (InputOutputPair inputOutputPair : toInputOutputPairs(options)) { - Path inputJar = inputOutputPair.getInput(); - IndexedJars appIndexedJar = new IndexedJars(ImmutableList.of(inputJar)); - IndexedJars appAndClasspathIndexedJars = new IndexedJars(options.classpath, appIndexedJar); - ClassLoader loader = - createClassLoader(rewriter, options.bootclasspath, appAndClasspathIndexedJars); - - try (ZipFile in = new ZipFile(inputJar.toFile()); - ZipOutputStream out = - new ZipOutputStream( - new BufferedOutputStream(Files.newOutputStream(inputOutputPair.getOutput())))) { - ClassReaderFactory readerFactory = - new ClassReaderFactory( - (options.copyBridgesFromClasspath && !allowDefaultMethods) - ? appAndClasspathIndexedJars - : appIndexedJar, - rewriter); - - ImmutableSet.Builder<String> interfaceLambdaMethodCollector = ImmutableSet.builder(); - - // Process input Jar, desugaring as we go - for (Enumeration<? extends ZipEntry> entries = in.entries(); entries.hasMoreElements(); ) { - ZipEntry entry = entries.nextElement(); - try (InputStream content = in.getInputStream(entry)) { - // We can write classes uncompressed since they need to be converted to .dex format for - // Android anyways. Resources are written as they were in the input jar to avoid any - // danger of accidentally uncompressed resources ending up in an .apk. - if (entry.getName().endsWith(".class")) { - ClassReader reader = rewriter.reader(content); - CoreLibraryRewriter.UnprefixingClassWriter writer = - rewriter.writer(ClassWriter.COMPUTE_MAXS /*for bridge methods*/); - ClassVisitor visitor = writer; - if (!allowDefaultMethods) { - visitor = new Java7Compatibility(visitor, readerFactory); - } - - visitor = - new LambdaDesugaring( - visitor, loader, lambdas, interfaceLambdaMethodCollector, - allowDefaultMethods); - - if (!allowCallsToObjectsNonNull) { - visitor = new ObjectsRequireNonNullMethodInliner(visitor); - } - reader.accept(visitor, 0); - - writeStoredEntry(out, entry.getName(), writer.toByteArray()); - } else { - // TODO(bazel-team): Avoid de- and re-compressing resource files - ZipEntry destEntry = new ZipEntry(entry); - destEntry.setCompressedSize(-1); - out.putNextEntry(destEntry); - ByteStreams.copy(content, out); - out.closeEntry(); - } - } - } - ImmutableSet<String> interfaceLambdaMethods = interfaceLambdaMethodCollector.build(); - if (allowDefaultMethods) { - checkState( - interfaceLambdaMethods.isEmpty(), - "Desugaring with default methods enabled moved interface lambdas"); - } + ClassLoader parent; + if (options.bootclasspath.isEmpty() && !options.allowEmptyBootclasspath) { + // TODO(b/31547323): Require bootclasspath once Bazel always provides it. Using the tool's + // bootclasspath as a fallback is iffy at best and produces wrong results at worst. + parent = ClassLoader.getSystemClassLoader(); + } else { + parent = new ThrowingClassLoader(); + } - // Write out the lambda classes we generated along the way - for (Map.Entry<Path, LambdaInfo> lambdaClass : lambdas.drain().entrySet()) { - try (InputStream bytecode = - Files.newInputStream(dumpDirectory.resolve(lambdaClass.getKey()))) { - ClassReader reader = rewriter.reader(bytecode); + CoreLibraryRewriter rewriter = + new CoreLibraryRewriter(options.coreLibrary ? "__desugar__/" : ""); + + ClassLoader loader = + createClassLoader( + rewriter, options.bootclasspath, options.inputJar, options.classpath, parent); + + try (ZipFile in = new ZipFile(options.inputJar.toFile()); + ZipOutputStream out = new ZipOutputStream(new BufferedOutputStream( + Files.newOutputStream(options.outputJar)))) { + LambdaClassMaker lambdas = new LambdaClassMaker(dumpDirectory); + ClassReaderFactory readerFactory = new ClassReaderFactory(in, rewriter); + + ImmutableSet.Builder<String> interfaceLambdaMethodCollector = ImmutableSet.builder(); + + // Process input Jar, desugaring as we go + for (Enumeration<? extends ZipEntry> entries = in.entries(); entries.hasMoreElements(); ) { + ZipEntry entry = entries.nextElement(); + try (InputStream content = in.getInputStream(entry)) { + // We can write classes uncompressed since they need to be converted to .dex format for + // Android anyways. Resources are written as they were in the input jar to avoid any + // danger of accidentally uncompressed resources ending up in an .apk. + if (entry.getName().endsWith(".class")) { + ClassReader reader = rewriter.reader(content); CoreLibraryRewriter.UnprefixingClassWriter writer = - rewriter.writer(ClassWriter.COMPUTE_MAXS /*for invoking bridges*/); + rewriter.writer(ClassWriter.COMPUTE_MAXS /*for bridge methods*/); ClassVisitor visitor = writer; - if (!allowDefaultMethods) { - // null ClassReaderFactory b/c we don't expect to need it for lambda classes - visitor = new Java7Compatibility(visitor, (ClassReaderFactory) null); + visitor = new Java7Compatibility(visitor, readerFactory); } - visitor = - new LambdaClassFixer( + visitor = new LambdaDesugaring( visitor, - lambdaClass.getValue(), - readerFactory, - interfaceLambdaMethods, + loader, + lambdas, + interfaceLambdaMethodCollector, allowDefaultMethods); - // Send lambda classes through desugaring to make sure there's no invokedynamic - // instructions in generated lambda classes (checkState below will fail) - visitor = new LambdaDesugaring(visitor, loader, lambdas, null, allowDefaultMethods); - if (!allowCallsToObjectsNonNull) { - // Not sure whether there will be implicit null check emitted by javac, so we rerun - // the inliner again - visitor = new ObjectsRequireNonNullMethodInliner(visitor); - } + reader.accept(visitor, 0); - String filename = - rewriter.unprefix(lambdaClass.getValue().desiredInternalName()) + ".class"; - writeStoredEntry(out, filename, writer.toByteArray()); + + writeStoredEntry(out, entry.getName(), writer.toByteArray()); + } else { + // TODO(bazel-team): Avoid de- and re-compressing resource files + ZipEntry destEntry = new ZipEntry(entry); + destEntry.setCompressedSize(-1); + out.putNextEntry(destEntry); + ByteStreams.copy(content, out); + out.closeEntry(); } } + } - Map<Path, LambdaInfo> leftBehind = lambdas.drain(); - checkState(leftBehind.isEmpty(), "Didn't process %s", leftBehind); + ImmutableSet<String> interfaceLambdaMethods = interfaceLambdaMethodCollector.build(); + if (allowDefaultMethods) { + checkState(interfaceLambdaMethods.isEmpty(), + "Desugaring with default methods enabled moved interface lambdas"); } - } - } - private static List<InputOutputPair> toInputOutputPairs(Options options) { - final ImmutableList.Builder<InputOutputPair> ioPairListbuilder = ImmutableList.builder(); - for (Iterator<Path> inputIt = options.inputJars.iterator(), - outputIt = options.outputJars.iterator(); - inputIt.hasNext();) { - ioPairListbuilder.add(InputOutputPair.create(inputIt.next(), outputIt.next())); + // Write out the lambda classes we generated along the way + for (Map.Entry<Path, LambdaInfo> lambdaClass : lambdas.drain().entrySet()) { + try (InputStream bytecode = + Files.newInputStream(dumpDirectory.resolve(lambdaClass.getKey()))) { + ClassReader reader = rewriter.reader(bytecode); + CoreLibraryRewriter.UnprefixingClassWriter writer = + rewriter.writer(ClassWriter.COMPUTE_MAXS /*for invoking bridges*/); + ClassVisitor visitor = writer; + + if (!allowDefaultMethods) { + // null ClassReaderFactory b/c we don't expect to need it for lambda classes + visitor = new Java7Compatibility(visitor, (ClassReaderFactory) null); + } + + LambdaClassFixer lambdaFixer = + new LambdaClassFixer( + visitor, + lambdaClass.getValue(), + readerFactory, + interfaceLambdaMethods, + allowDefaultMethods); + // 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()); + } + } + + Map<Path, LambdaInfo> leftBehind = lambdas.drain(); + checkState(leftBehind.isEmpty(), "Didn't process %s", leftBehind); } - return ioPairListbuilder.build(); } private static void writeStoredEntry(ZipOutputStream out, String filename, byte[] content) @@ -322,23 +266,25 @@ class Desugar { } private static ClassLoader createClassLoader(CoreLibraryRewriter rewriter, - List<Path> bootclasspath, IndexedJars appAndClasspathIndexedJars) throws IOException { - // Use a classloader that as much as possible uses the provided bootclasspath instead of - // the tool's system classloader. Unfortunately we can't do that for java. classes. - ClassLoader parent = new ThrowingClassLoader(); - if (!bootclasspath.isEmpty()) { - parent = new HeaderClassLoader(new IndexedJars(bootclasspath), rewriter, parent); - } + List<Path> bootclasspath, Path inputJar, List<Path> classpath, + ClassLoader parent) throws IOException { // Prepend classpath with input jar itself so LambdaDesugaring can load classes with lambdas. // Note that inputJar and classpath need to be in the same classloader because we typically get // the header Jar for inputJar on the classpath and having the header Jar in a parent loader // means the header version is preferred over the real thing. - return new HeaderClassLoader(appAndClasspathIndexedJars, rewriter, parent); + classpath = ImmutableList.<Path>builder().add(inputJar).addAll(classpath).build(); + // Use a classloader that as much as possible uses the provided bootclasspath instead of + // the tool's system classloader. Unfortunately we can't do that for java. classes. + if (!bootclasspath.isEmpty()) { + parent = HeaderClassLoader.fromClassPath(bootclasspath, rewriter, parent); + } + return HeaderClassLoader.fromClassPath(classpath, rewriter, parent); } private static class ThrowingClassLoader extends ClassLoader { @Override - protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { + protected Class<?> loadClass(String name, boolean resolve) + throws ClassNotFoundException { if (name.startsWith("java.")) { // Use system class loader for java. classes, since ClassLoader.defineClass gets // grumpy when those don't come from the standard place. @@ -385,19 +331,4 @@ class Desugar { }); } } - - /** - * Pair input and output. - */ - @AutoValue - abstract static class InputOutputPair { - - static InputOutputPair create(Path input, Path output) { - return new AutoValue_Desugar_InputOutputPair(input, output); - } - - abstract Path getInput(); - - abstract Path getOutput(); - } } diff --git a/java/com/google/devtools/build/android/desugar/HeaderClassLoader.java b/java/com/google/devtools/build/android/desugar/HeaderClassLoader.java index 44c3932..053d52d 100644 --- a/java/com/google/devtools/build/android/desugar/HeaderClassLoader.java +++ b/java/com/google/devtools/build/android/desugar/HeaderClassLoader.java @@ -16,6 +16,12 @@ package com.google.devtools.build.android.desugar; import java.io.IOError; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Path; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.zip.ZipEntry; import org.objectweb.asm.ClassReader; @@ -35,20 +41,44 @@ import org.objectweb.asm.Opcodes; */ class HeaderClassLoader extends ClassLoader { - private final IndexedJars indexedJars; + private final Map<String, JarFile> jarfiles; private final CoreLibraryRewriter rewriter; - public HeaderClassLoader( - IndexedJars indexedJars, CoreLibraryRewriter rewriter, ClassLoader parent) { + /** Creates a classloader from the given classpath with the given parent. */ + public static HeaderClassLoader fromClassPath( + List<Path> classpath, CoreLibraryRewriter rewriter, ClassLoader parent) throws IOException { + return new HeaderClassLoader(indexJars(classpath), rewriter, parent); + } + + /** + * Opens the given list of Jar files and returns an index of all classes in them, to avoid + * scanning all Jars over and over for each class in {@link #findClass}. + */ + private static Map<String, JarFile> indexJars(List<Path> classpath) throws IOException { + HashMap<String, JarFile> result = new HashMap<>(); + for (Path jarfile : classpath) { + JarFile jar = new JarFile(jarfile.toFile()); + for (Enumeration<JarEntry> cur = jar.entries(); cur.hasMoreElements(); ) { + JarEntry entry = cur.nextElement(); + if (entry.getName().endsWith(".class") && !result.containsKey(entry.getName())) { + result.put(entry.getName(), jar); + } + } + } + return result; + } + + private HeaderClassLoader( + Map<String, JarFile> jarfiles, CoreLibraryRewriter rewriter, ClassLoader parent) { super(parent); this.rewriter = rewriter; - this.indexedJars = indexedJars; + this.jarfiles = jarfiles; } @Override protected Class<?> findClass(String name) throws ClassNotFoundException { String filename = rewriter.unprefix(name.replace('.', '/') + ".class"); - JarFile jarfile = indexedJars.getJarFile(filename); + JarFile jarfile = jarfiles.get(filename); if (jarfile == null) { throw new ClassNotFoundException(); } diff --git a/java/com/google/devtools/build/android/desugar/IndexedJars.java b/java/com/google/devtools/build/android/desugar/IndexedJars.java deleted file mode 100644 index bdb5b47..0000000 --- a/java/com/google/devtools/build/android/desugar/IndexedJars.java +++ /dev/null @@ -1,84 +0,0 @@ -// 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 com.google.common.base.Preconditions; -import java.io.IOException; -import java.nio.file.Path; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; -import javax.annotation.Nullable; - -/** - * Opens the given list of Jar files and compute an index of all classes in them, to avoid - * scanning all Jars over and over for each class to load. An indexed jars can have a parent - * that is firstly used when a file name is searched. - */ -class IndexedJars { - - private final Map<String, JarFile> jarfiles = new HashMap<>(); - - /** - * Parent indexed jars to use before to search a file name into this indexed jars. - */ - @Nullable - private final IndexedJars parentIndexedJar; - - /** - * Index a list of Jar files without a parent indexed jars. - */ - public IndexedJars(List<Path> jarFiles) throws IOException { - this(jarFiles, null); - } - - /** - * Index a list of Jar files and set a parent indexed jars that is firstly used during the search - * of a file name. - */ - public IndexedJars(List<Path> jarFiles, @Nullable IndexedJars parentIndexedJar) - throws IOException { - this.parentIndexedJar = parentIndexedJar; - for (Path jarfile : jarFiles) { - indexJar(jarfile); - } - } - - @Nullable - public JarFile getJarFile(String filename) { - Preconditions.checkArgument(filename.endsWith(".class")); - - if (parentIndexedJar != null) { - JarFile jarFile = parentIndexedJar.getJarFile(filename); - if (jarFile != null) { - return jarFile; - } - } - - return jarfiles.get(filename); - } - - private void indexJar(Path jarfile) throws IOException { - JarFile jar = new JarFile(jarfile.toFile()); - for (Enumeration<JarEntry> cur = jar.entries(); cur.hasMoreElements(); ) { - JarEntry entry = cur.nextElement(); - if (entry.getName().endsWith(".class") && !jarfiles.containsKey(entry.getName())) { - jarfiles.put(entry.getName(), jar); - } - } - } -} diff --git a/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java b/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java index e942c95..97c0249 100644 --- a/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java +++ b/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java @@ -27,6 +27,7 @@ import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.FieldVisitor; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; import org.objectweb.asm.tree.AbstractInsnNode; import org.objectweb.asm.tree.MethodNode; import org.objectweb.asm.tree.TypeInsnNode; @@ -40,10 +41,8 @@ import org.objectweb.asm.tree.TypeInsnNode; */ class LambdaClassFixer extends ClassVisitor { - /** Magic method name used by {@link java.lang.invoke.LambdaMetafactory}. */ + /** Magic method name used by {@link java.lang.invoke.LambdaMetafactory} */ public static final String FACTORY_METHOD_NAME = "get$Lambda"; - /** Field name we'll use to hold singleton instances where possible. */ - public static final String SINGLETON_FIELD_NAME = "$instance"; private final LambdaInfo lambdaInfo; private final ClassReaderFactory factory; @@ -52,7 +51,7 @@ class LambdaClassFixer extends ClassVisitor { private final HashSet<String> implementedMethods = new HashSet<>(); private final LinkedHashSet<String> methodsToMoveIn = new LinkedHashSet<>(); - private String originalInternalName; + private String internalName; private ImmutableList<String> interfaces; private boolean hasState; @@ -60,6 +59,7 @@ class LambdaClassFixer extends ClassVisitor { private String desc; private String signature; + private String[] exceptions; public LambdaClassFixer(ClassVisitor dest, LambdaInfo lambdaInfo, ClassReaderFactory factory, @@ -71,6 +71,10 @@ class LambdaClassFixer extends ClassVisitor { this.allowDefaultMethods = allowDefaultMethods; } + public String getInternalName() { + return internalName; + } + @Override public void visit( int version, @@ -80,15 +84,15 @@ class LambdaClassFixer extends ClassVisitor { String superName, String[] interfaces) { checkArgument(BitFlags.noneSet(access, Opcodes.ACC_INTERFACE), "Not a class: %s", name); - checkState(this.originalInternalName == null, "not intended for reuse but reused for %s", name); - originalInternalName = name; + checkState(internalName == null, "Already visited %s, can't reuse for %s", internalName, name); + internalName = name; hasState = false; hasFactory = false; desc = null; this.signature = null; + exceptions = null; this.interfaces = ImmutableList.copyOf(interfaces); - // Rename to desired name - super.visit(version, access, getInternalName(), signature, superName, interfaces); + super.visit(version, access, name, signature, superName, interfaces); } @Override @@ -121,6 +125,7 @@ class LambdaClassFixer extends ClassVisitor { } else if ("<init>".equals(name)) { this.desc = desc; this.signature = signature; + this.exceptions = exceptions; } MethodVisitor methodVisitor = new LambdaClassMethodRewriter(super.visitMethod(access, name, desc, signature, exceptions)); @@ -138,19 +143,17 @@ class LambdaClassFixer extends ClassVisitor { @Override public void visitEnd() { checkState(!hasState || hasFactory, - "Expected factory method for capturing lambda %s", getInternalName()); - if (!hasState) { + "Expected factory method for capturing lambda %s", internalName); + if (!hasFactory) { + // Fake factory method if LambdaMetafactory didn't generate it checkState(signature == null, - "Didn't expect generic constructor signature %s %s", getInternalName(), signature); - checkState(lambdaInfo.factoryMethodDesc().startsWith("()"), - "Expected 0-arg factory method for %s but found %s", getInternalName(), - lambdaInfo.factoryMethodDesc()); - // Since this is a stateless class we populate and use a static singleton field "$instance". - // Field is package-private so we can read it from the class that had the invokedynamic. - String singletonFieldDesc = lambdaInfo.factoryMethodDesc().substring("()".length()); + "Didn't expect generic constructor signature %s %s", internalName, signature); + + // Since this is a stateless class we populate and use a static singleton field "$instance" + String singletonFieldDesc = Type.getObjectType(internalName).getDescriptor(); super.visitField( - Opcodes.ACC_STATIC | Opcodes.ACC_FINAL, - SINGLETON_FIELD_NAME, + Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL, + "$instance", singletonFieldDesc, (String) null, (Object) null) @@ -163,28 +166,35 @@ class LambdaClassFixer extends ClassVisitor { "()V", (String) null, new String[0]); - codeBuilder.visitTypeInsn(Opcodes.NEW, getInternalName()); + codeBuilder.visitTypeInsn(Opcodes.NEW, internalName); codeBuilder.visitInsn(Opcodes.DUP); - codeBuilder.visitMethodInsn(Opcodes.INVOKESPECIAL, getInternalName(), "<init>", - checkNotNull(desc, "didn't see a constructor for %s", getInternalName()), /*itf*/ false); - codeBuilder.visitFieldInsn( - Opcodes.PUTSTATIC, getInternalName(), SINGLETON_FIELD_NAME, singletonFieldDesc); + 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.visitInsn(Opcodes.RETURN); codeBuilder.visitMaxs(2, 0); // two values are pushed onto the stack codeBuilder.visitEnd(); + + codeBuilder = // reuse codeBuilder variable to avoid accidental additions to previous method + super.visitMethod( + Opcodes.ACC_STATIC, + FACTORY_METHOD_NAME, + lambdaInfo.factoryMethodDesc(), + (String) null, + exceptions); + codeBuilder.visitFieldInsn(Opcodes.GETSTATIC, internalName, "$instance", singletonFieldDesc); + codeBuilder.visitInsn(Opcodes.ARETURN); + codeBuilder.visitMaxs(1, 0); // one value on the stack } copyRewrittenLambdaMethods(); + if (!allowDefaultMethods) { copyBridgeMethods(interfaces); } super.visitEnd(); } - private String getInternalName() { - return lambdaInfo.desiredInternalName(); - } - private void copyRewrittenLambdaMethods() { for (String rewritten : methodsToMoveIn) { String interfaceInternalName = rewritten.substring(0, rewritten.indexOf('#')); @@ -223,41 +233,17 @@ 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 = getInternalName(); + owner = internalName; itf = false; // owner was interface but is now a class methodsToMoveIn.add(method); - } 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); - } + } else 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/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java b/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java index 2a9f9be..8a2ebea 100644 --- a/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java +++ b/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java @@ -14,6 +14,7 @@ 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; @@ -24,6 +25,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.Map; +import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; @@ -39,7 +41,7 @@ class LambdaClassMaker { this.rootDirectory = rootDirectory; } - public void generateLambdaClass(String invokerInternalName, LambdaInfo lambdaInfo, + public String generateLambdaClass(String invokerInternalName, LambdaInfo lambdaInfo, MethodHandle bootstrapMethod, ArrayList<Object> bsmArgs) throws IOException { // Invoking the bootstrap method will dump the generated class try { @@ -50,11 +52,15 @@ 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; } /** - * Returns absolute paths to .class files generated since the last call to this method together + * Returns relative paths to .class files generated since the last call to this method together * with a string descriptor of the factory method. */ public Map<Path, LambdaInfo> drain() { @@ -63,26 +69,27 @@ class LambdaClassMaker { return result; } - private Path findOnlyUnprocessed(String pathPrefix) throws IOException { - // pathPrefix is an internal class name prefix containing '/', but paths obtained on Windows - // will not contain '/' and searches will fail. So, construct an absolute path from the given - // string and use its string representation to find the file we need regardless of host - // system's file system - Path rootPathPrefix = rootDirectory.resolve(pathPrefix); - final String rootPathPrefixStr = rootPathPrefix.toString(); - + private Path findOnlyUnprocessed(final String pathPrefix) throws IOException { + // TODO(kmb): Investigate making this faster in the case of many lambdas // TODO(bazel-team): This could be much nicer with lambdas - try (Stream<Path> paths = - Files.list(rootPathPrefix.getParent()) + try (Stream<Path> results = + Files.walk(rootDirectory) + .map( + new Function<Path, Path>() { + @Override + public Path apply(Path path) { + return rootDirectory.relativize(path); + } + }) .filter( new Predicate<Path>() { @Override public boolean test(Path path) { - return path.toString().startsWith(rootPathPrefixStr) + return path.toString().startsWith(pathPrefix) && !generatedClasses.containsKey(path); } })) { - return Iterators.getOnlyElement(paths.iterator()); + return Iterators.getOnlyElement(results.iterator()); } } } diff --git a/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java b/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java index c808831..79be2a3 100644 --- a/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java +++ b/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java @@ -55,7 +55,6 @@ class LambdaDesugaring extends ClassVisitor { private String internalName; private boolean isInterface; - private int lambdaCount; public LambdaDesugaring( ClassVisitor dest, @@ -78,7 +77,6 @@ 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); @@ -220,7 +218,7 @@ class LambdaDesugaring extends ClassVisitor { return result; // we're already queued up a bridge method for this method reference } - String name = uniqueInPackage(internalName, "bridge$lambda$" + bridgeMethods.size()); + String name = "bridge$lambda$" + bridgeMethods.size(); Handle bridgeMethod; switch (invokedMethod.getTag()) { case Opcodes.H_INVOKESTATIC: @@ -362,28 +360,18 @@ 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); - // 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( + String lambdaClassName = lambdas.generateLambdaClass( internalName, - LambdaInfo.create( - lambdaClassName, desc, bridgeInfo.methodReference(), bridgeInfo.bridgeMethod()), + LambdaInfo.create(desc, bridgeInfo.methodReference(), bridgeInfo.bridgeMethod()), bsmMethod, args); - if (desc.startsWith("()")) { - // For stateless lambda classes we'll generate a singleton instance that we can just load - super.visitFieldInsn(Opcodes.GETSTATIC, lambdaClassName, - LambdaClassFixer.SINGLETON_FIELD_NAME, desc.substring("()".length())); - } else { - // Emit invokestatic that calls the factory method generated in the lambda class - super.visitMethodInsn( - Opcodes.INVOKESTATIC, - lambdaClassName, - LambdaClassFixer.FACTORY_METHOD_NAME, - desc, - /*itf*/ false); - } + // Emit invokestatic that calls the factory method generated in the lambda class + super.visitMethodInsn( + Opcodes.INVOKESTATIC, + lambdaClassName, + LambdaClassFixer.FACTORY_METHOD_NAME, + desc, + /*itf*/ false); } catch (IOException | ReflectiveOperationException e) { throw new IllegalStateException("Couldn't desugar invokedynamic for " + internalName + "." + name + " using " + bsm + " with arguments " + Arrays.toString(bsmArgs), e); diff --git a/java/com/google/devtools/build/android/desugar/LambdaInfo.java b/java/com/google/devtools/build/android/desugar/LambdaInfo.java index dad340c..e3bb84f 100644 --- a/java/com/google/devtools/build/android/desugar/LambdaInfo.java +++ b/java/com/google/devtools/build/android/desugar/LambdaInfo.java @@ -19,15 +19,11 @@ import org.objectweb.asm.Handle; @AutoValue abstract class LambdaInfo { public static LambdaInfo create( - String desiredInternalName, - String factoryMethodDesc, - Handle methodReference, - Handle bridgeMethod) { + String factoryMethodDesc, Handle methodReference, Handle bridgeMethod) { return new AutoValue_LambdaInfo( - desiredInternalName, factoryMethodDesc, methodReference, bridgeMethod); + factoryMethodDesc, methodReference, bridgeMethod); } - public abstract String desiredInternalName(); public abstract String factoryMethodDesc(); public abstract Handle methodReference(); public abstract Handle bridgeMethod(); diff --git a/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java b/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java deleted file mode 100644 index e56cfe1..0000000 --- a/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright 2016 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 org.objectweb.asm.Opcodes.ASM5; -import static org.objectweb.asm.Opcodes.DUP; -import static org.objectweb.asm.Opcodes.INVOKESTATIC; -import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL; -import static org.objectweb.asm.Opcodes.POP; - -import org.objectweb.asm.ClassVisitor; -import org.objectweb.asm.MethodVisitor; - -/** - * This class desugars any call to Objects.requireNonNull(Object o), Objects.requireNonNull(Object - * o, String msg), and Objects.requireNonNull(Object o, Supplier msg), by replacing the call with - * o.getClass(). Note that currently we discard the message, as inlining the message involves - * changes to the control flow graph, which further requires re-computation of the stack map frames. - */ -public class ObjectsRequireNonNullMethodInliner extends ClassVisitor { - - public ObjectsRequireNonNullMethodInliner(ClassVisitor cv) { - super(ASM5, cv); - } - - @Override - public MethodVisitor visitMethod( - int access, String name, String desc, String signature, String[] exceptions) { - MethodVisitor visitor = super.cv.visitMethod(access, name, desc, signature, exceptions); - return visitor == null ? visitor : new ObjectsMethodInlinerMethodVisitor(visitor); - } - - private static class ObjectsMethodInlinerMethodVisitor extends MethodVisitor { - - public ObjectsMethodInlinerMethodVisitor(MethodVisitor mv) { - super(ASM5, mv); - } - - @Override - public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) { - if (opcode != INVOKESTATIC - || !owner.equals("java/util/Objects") - || !name.equals("requireNonNull") - || !desc.equals("(Ljava/lang/Object;)Ljava/lang/Object;")) { - super.visitMethodInsn(opcode, owner, name, desc, itf); - return; - } - - // a call to Objects.requireNonNull(Object o) - // duplicate the first argument 'o', as this method returns 'o'. - super.visitInsn(DUP); - super.visitMethodInsn( - INVOKEVIRTUAL, "java/lang/Object", "getClass", "()Ljava/lang/Class;", false); - super.visitInsn(POP); - } - } -} diff --git a/java/com/google/devtools/common/options/OptionsParser.java b/java/com/google/devtools/common/options/OptionsParser.java index 946d73b..166379b 100644 --- a/java/com/google/devtools/common/options/OptionsParser.java +++ b/java/com/google/devtools/common/options/OptionsParser.java @@ -19,7 +19,6 @@ import com.google.common.base.Functions; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.escape.Escaper; @@ -244,51 +243,25 @@ public class OptionsParser implements OptionsProvider { private final String source; private final String implicitDependant; private final String expandedFrom; - private final boolean allowMultiple; - public OptionValueDescription( - String name, - Object value, - OptionPriority priority, - String source, - String implicitDependant, - String expandedFrom, - boolean allowMultiple) { + public OptionValueDescription(String name, Object value, + OptionPriority priority, String source, String implicitDependant, String expandedFrom) { this.name = name; this.value = value; this.priority = priority; this.source = source; this.implicitDependant = implicitDependant; this.expandedFrom = expandedFrom; - this.allowMultiple = allowMultiple; } public String getName() { return name; } - // Need to suppress unchecked warnings, because the "multiple occurrence" - // options use unchecked ListMultimaps due to limitations of Java generics. - @SuppressWarnings({"unchecked", "rawtypes"}) public Object getValue() { - if (allowMultiple) { - // Sort the results by option priority and return them in a new list. - // The generic type of the list is not known at runtime, so we can't - // use it here. It was already checked in the constructor, so this is - // type-safe. - List result = Lists.newArrayList(); - ListMultimap realValue = (ListMultimap) value; - for (OptionPriority priority : OptionPriority.values()) { - // If there is no mapping for this key, this check avoids object creation (because - // ListMultimap has to return a new object on get) and also an unnecessary addAll call. - if (realValue.containsKey(priority)) { - result.addAll(realValue.get(priority)); - } - } - return result; - } return value; } + /** * @return the priority of the thing that set this value for this flag */ @@ -333,19 +306,6 @@ public class OptionsParser implements OptionsProvider { } return result.toString(); } - - // Need to suppress unchecked warnings, because the "multiple occurrence" - // options use unchecked ListMultimaps due to limitations of Java generics. - @SuppressWarnings({"unchecked", "rawtypes"}) - void addValue(OptionPriority addedPriority, Object addedValue) { - Preconditions.checkState(allowMultiple); - ListMultimap optionValueList = (ListMultimap) value; - if (addedValue instanceof List<?>) { - optionValueList.putAll(addedPriority, (List<?>) addedValue); - } else { - optionValueList.put(addedPriority, addedValue); - } - } } /** diff --git a/java/com/google/devtools/common/options/OptionsParserImpl.java b/java/com/google/devtools/common/options/OptionsParserImpl.java index c15f927..fb7292c 100644 --- a/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Iterators; import com.google.common.collect.LinkedHashMultimap; +import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; @@ -144,19 +145,85 @@ class OptionsParserImpl { }}); } + /** + * For every value, this class keeps track of its priority, its free-form source + * description, whether it was set as an implicit dependency, and the value. + */ + private static final class ParsedOptionEntry { + private final Object value; + private final OptionPriority priority; + private final String source; + private final String implicitDependant; + private final String expandedFrom; + private final boolean allowMultiple; + + ParsedOptionEntry(Object value, + OptionPriority priority, String source, String implicitDependant, String expandedFrom, + boolean allowMultiple) { + this.value = value; + this.priority = priority; + this.source = source; + this.implicitDependant = implicitDependant; + this.expandedFrom = expandedFrom; + this.allowMultiple = allowMultiple; + } + + // Need to suppress unchecked warnings, because the "multiple occurrence" + // options use unchecked ListMultimaps due to limitations of Java generics. + @SuppressWarnings({"unchecked", "rawtypes"}) + Object getValue() { + if (allowMultiple) { + // Sort the results by option priority and return them in a new list. + // The generic type of the list is not known at runtime, so we can't + // use it here. It was already checked in the constructor, so this is + // type-safe. + List result = Lists.newArrayList(); + ListMultimap realValue = (ListMultimap) value; + for (OptionPriority priority : OptionPriority.values()) { + // If there is no mapping for this key, this check avoids object creation (because + // ListMultimap has to return a new object on get) and also an unnecessary addAll call. + if (realValue.containsKey(priority)) { + result.addAll(realValue.get(priority)); + } + } + return result; + } + return value; + } + + // Need to suppress unchecked warnings, because the "multiple occurrence" + // options use unchecked ListMultimaps due to limitations of Java generics. + @SuppressWarnings({"unchecked", "rawtypes"}) + void addValue(OptionPriority addedPriority, Object addedValue) { + Preconditions.checkState(allowMultiple); + ListMultimap optionValueList = (ListMultimap) value; + if (addedValue instanceof List<?>) { + for (Object element : (List<?>) addedValue) { + optionValueList.put(addedPriority, element); + } + } else { + optionValueList.put(addedPriority, addedValue); + } + } + + OptionValueDescription asOptionValueDescription(String fieldName) { + return new OptionValueDescription(fieldName, getValue(), priority, + source, implicitDependant, expandedFrom); + } + } + private final OptionsData optionsData; /** * We store the results of parsing the arguments in here. It'll look like - * * <pre> * Field("--host") -> "www.google.com" * Field("--port") -> 80 * </pre> - * - * This map is modified by repeated calls to {@link #parse(OptionPriority,Function,List)}. + * This map is modified by repeated calls to + * {@link #parse(OptionPriority,Function,List)}. */ - private final Map<Field, OptionValueDescription> parsedValues = Maps.newHashMap(); + private final Map<Field, ParsedOptionEntry> parsedValues = Maps.newHashMap(); /** * We store the pre-parsed, explicit options for each priority in here. @@ -309,17 +376,16 @@ class OptionsParserImpl { */ List<OptionValueDescription> asListOfEffectiveOptions() { List<OptionValueDescription> result = Lists.newArrayList(); - for (Map.Entry<String, Field> mapEntry : optionsData.getAllNamedFields()) { + for (Map.Entry<String,Field> mapEntry : optionsData.getAllNamedFields()) { String fieldName = mapEntry.getKey(); Field field = mapEntry.getValue(); - OptionValueDescription entry = parsedValues.get(field); + ParsedOptionEntry entry = parsedValues.get(field); if (entry == null) { Object value = optionsData.getDefaultValue(field); - result.add( - new OptionValueDescription( - fieldName, value, OptionPriority.DEFAULT, null, null, null, false)); + result.add(new OptionValueDescription(fieldName, value, OptionPriority.DEFAULT, + null, null, null)); } else { - result.add(entry); + result.add(entry.asOptionValueDescription(fieldName)); } } return result; @@ -346,71 +412,46 @@ class OptionsParserImpl { // Warnings should not end with a '.' because the internal reporter adds one automatically. private void setValue(Field field, String name, Object value, OptionPriority priority, String source, String implicitDependant, String expandedFrom) { - OptionValueDescription entry = parsedValues.get(field); + ParsedOptionEntry entry = parsedValues.get(field); if (entry != null) { // Override existing option if the new value has higher or equal priority. - if (priority.compareTo(entry.getPriority()) >= 0) { + if (priority.compareTo(entry.priority) >= 0) { // Output warnings: - if ((implicitDependant != null) && (entry.getImplicitDependant() != null)) { - if (!implicitDependant.equals(entry.getImplicitDependant())) { - warnings.add( - "Option '" - + name - + "' is implicitly defined by both option '" - + entry.getImplicitDependant() - + "' and option '" - + implicitDependant - + "'"); + if ((implicitDependant != null) && (entry.implicitDependant != null)) { + if (!implicitDependant.equals(entry.implicitDependant)) { + warnings.add("Option '" + name + "' is implicitly defined by both option '" + + entry.implicitDependant + "' and option '" + implicitDependant + "'"); } - } else if ((implicitDependant != null) && priority.equals(entry.getPriority())) { - warnings.add( - "Option '" - + name - + "' is implicitly defined by option '" - + implicitDependant - + "'; the implicitly set value overrides the previous one"); - } else if (entry.getImplicitDependant() != null) { - warnings.add( - "A new value for option '" - + name - + "' overrides a previous implicit setting of that option by option '" - + entry.getImplicitDependant() - + "'"); - } else if ((priority == entry.getPriority()) - && ((entry.getExpansionParent() == null) && (expandedFrom != null))) { + } else if ((implicitDependant != null) && priority.equals(entry.priority)) { + warnings.add("Option '" + name + "' is implicitly defined by option '" + + implicitDependant + "'; the implicitly set value overrides the previous one"); + } else if (entry.implicitDependant != null) { + warnings.add("A new value for option '" + name + "' overrides a previous " + + "implicit setting of that option by option '" + entry.implicitDependant + "'"); + } else if ((priority == entry.priority) && + ((entry.expandedFrom == null) && (expandedFrom != null))) { // Create a warning if an expansion option overrides an explicit option: warnings.add("The option '" + expandedFrom + "' was expanded and now overrides a " + "previous explicitly specified option '" + name + "'"); } // Record the new value: - parsedValues.put( - field, - new OptionValueDescription( - name, value, priority, source, implicitDependant, expandedFrom, false)); + parsedValues.put(field, + new ParsedOptionEntry(value, priority, source, implicitDependant, expandedFrom, false)); } } else { - parsedValues.put( - field, - new OptionValueDescription( - name, value, priority, source, implicitDependant, expandedFrom, false)); + parsedValues.put(field, + new ParsedOptionEntry(value, priority, source, implicitDependant, expandedFrom, false)); maybeAddDeprecationWarning(field); } } private void addListValue(Field field, Object value, OptionPriority priority, String source, String implicitDependant, String expandedFrom) { - OptionValueDescription entry = parsedValues.get(field); + ParsedOptionEntry entry = parsedValues.get(field); if (entry == null) { - entry = - new OptionValueDescription( - field.getName(), - ArrayListMultimap.create(), - priority, - source, - implicitDependant, - expandedFrom, - true); + entry = new ParsedOptionEntry(ArrayListMultimap.create(), priority, source, + implicitDependant, expandedFrom, true); parsedValues.put(field, entry); maybeAddDeprecationWarning(field); } @@ -431,9 +472,9 @@ class OptionsParserImpl { Field field, Option option, Map<String, OptionValueDescription> clearedValues) throws OptionsParsingException { - OptionValueDescription removed = parsedValues.remove(field); + ParsedOptionEntry removed = parsedValues.remove(field); if (removed != null) { - clearedValues.put(option.name(), removed); + clearedValues.put(option.name(), removed.asOptionValueDescription(option.name())); } canonicalizeValues.removeAll(field); @@ -455,7 +496,11 @@ class OptionsParserImpl { if (field == null) { throw new IllegalArgumentException("No such option '" + name + "'"); } - return parsedValues.get(field); + ParsedOptionEntry entry = parsedValues.get(field); + if (entry == null) { + return null; + } + return entry.asOptionValueDescription(name); } OptionDescription getOptionDescription(String name) { @@ -579,12 +624,9 @@ class OptionsParserImpl { // Handle expansion options. if (option.expansion().length > 0) { - Function<Object, String> expansionSourceFunction = - Functions.constant( - "expanded from option --" - + originalName - + " from " - + sourceFunction.apply(originalName)); + Function<Object, String> expansionSourceFunction = Functions.<String>constant( + "expanded from option --" + originalName + " from " + + sourceFunction.apply(originalName)); maybeAddDeprecationWarning(field); List<String> unparsed = parse(priority, expansionSourceFunction, null, originalName, ImmutableList.copyOf(option.expansion())); @@ -631,13 +673,10 @@ class OptionsParserImpl { // Now parse any implicit requirements that were collected. // TODO(bazel-team): this should happen when the option is encountered. if (!implicitRequirements.isEmpty()) { - for (Map.Entry<String, List<String>> entry : implicitRequirements.entrySet()) { - Function<Object, String> requirementSourceFunction = - Functions.constant( - "implicit requirement of option --" - + entry.getKey() - + " from " - + sourceFunction.apply(entry.getKey())); + for (Map.Entry<String,List<String>> entry : implicitRequirements.entrySet()) { + Function<Object, String> requirementSourceFunction = Functions.<String>constant( + "implicit requirement of option --" + entry.getKey() + " from " + + sourceFunction.apply(entry.getKey())); List<String> unparsed = parse(priority, requirementSourceFunction, entry.getKey(), null, entry.getValue()); @@ -758,7 +797,7 @@ class OptionsParserImpl { // Set the fields for (Field field : optionsData.getFieldsForClass(optionsClass)) { Object value; - OptionValueDescription entry = parsedValues.get(field); + ParsedOptionEntry entry = parsedValues.get(field); if (entry == null) { value = optionsData.getDefaultValue(field); } else { |