Merge remote-tracking branch 'aosp/upstream-master' into master
* aosp/upstream-master:
Clean up android desugar tool's flags a bit
Change how desugar finds desugared classes to have it working on Windows
Remove bootclasspath fallback in Android desugaring tool
Add an --copy_bridges_from_classpath argument
Desugar calls to Objects.requireNonNull(Object o) to o.getClass()
Avoid factory methods when desugaring stateless lambdas for Android RELNOTES: Avoid factory methods when desugaring stateless lambdas for Android
Remove duplicate class.
More stable naming scheme for lambda classes in desugared android code RELNOTES: More stable naming scheme for lambda classes in desugared android code
Test: m -j ANDROID_COMPILE_WITH_JACK=false
Change-Id: I0ff5b9eb3cac6a020f347c51b79633af13b0576f
diff --git a/java/com/google/devtools/build/android/desugar/ClassReaderFactory.java b/java/com/google/devtools/build/android/desugar/ClassReaderFactory.java
index 6ec4e0d..56f99a3 100644
--- a/java/com/google/devtools/build/android/desugar/ClassReaderFactory.java
+++ b/java/com/google/devtools/build/android/desugar/ClassReaderFactory.java
@@ -15,31 +15,39 @@
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 ZipFile jar;
+ private final IndexedJars indexedJars;
private final CoreLibraryRewriter rewriter;
- public ClassReaderFactory(ZipFile jar, CoreLibraryRewriter rewriter) {
- this.jar = jar;
+ public ClassReaderFactory(IndexedJars indexedJars, CoreLibraryRewriter rewriter) {
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 excpetions
+ * and {@code null} otherwise. For simplicity this method turns checked into runtime exceptions
* under the assumption that all classes have already been read once when this method is called.
*/
@Nullable
public ClassReader readIfKnown(String internalClassName) {
- ZipEntry entry = jar.getEntry(rewriter.unprefix(internalClassName) + ".class");
- if (entry == null) {
- return null;
+ String filename = rewriter.unprefix(internalClassName) + ".class";
+ JarFile jarFile = indexedJars.getJarFile(filename);
+
+ if (jarFile != null) {
+ return getClassReader(internalClassName, jarFile, jarFile.getEntry(filename));
}
+
+ 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 2d973de..877d230 100644
--- a/java/com/google/devtools/build/android/desugar/Desugar.java
+++ b/java/com/google/devtools/build/android/desugar/Desugar.java
@@ -50,55 +50,63 @@
*/
class Desugar {
- /**
- * Commandline options for {@link Desugar}.
- */
+ /** Commandline options for {@link Desugar}. */
public static class Options extends OptionsBase {
- @Option(name = "input",
- defaultValue = "null",
- category = "input",
- converter = ExistingPathConverter.class,
- abbrev = 'i',
- help = "Input Jar with classes to desugar.")
+ @Option(
+ name = "input",
+ defaultValue = "null",
+ category = "input",
+ converter = ExistingPathConverter.class,
+ abbrev = 'i',
+ help = "Input Jar with classes to desugar (required)."
+ )
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.")
+ @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. If no bootclasspath is explicitly given then the tool's own "
- + "bootclasspath is used.")
+ @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)."
+ )
public List<Path> bootclasspath;
- @Option(name = "allow_empty_bootclasspath",
- defaultValue = "false",
- category = "misc",
- help = "Don't use the tool's bootclasspath if no bootclasspath is given.")
+ @Option(
+ name = "allow_empty_bootclasspath",
+ defaultValue = "false",
+ category = "undocumented"
+ )
public boolean allowEmptyBootclasspath;
- @Option(name = "output",
- defaultValue = "null",
- category = "output",
- converter = PathConverter.class,
- abbrev = 'o',
- help = "Output Jar to write desugared classes into.")
+ @Option(
+ name = "output",
+ defaultValue = "null",
+ category = "output",
+ converter = PathConverter.class,
+ abbrev = 'o',
+ help = "Output Jar to write desugared classes into (required)."
+ )
public Path outputJar;
- @Option(name = "verbose",
- defaultValue = "false",
- category = "misc",
- abbrev = 'v',
- help = "Enables verbose debugging output.")
+ @Option(
+ name = "verbose",
+ defaultValue = "false",
+ category = "misc",
+ abbrev = 'v',
+ help = "Enables verbose debugging output."
+ )
public boolean verbose;
@Option(
@@ -109,10 +117,21 @@
)
public int minSdkVersion;
- @Option(name = "core_library",
- defaultValue = "false",
- category = "undocumented",
- help = "Enables rewriting to desugar java.* classes.")
+ @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."
+ )
public boolean coreLibrary;
}
@@ -133,38 +152,40 @@
args = Files.readAllLines(Paths.get(args[0].substring(1)), ISO_8859_1).toArray(new String[0]);
}
- OptionsParser optionsParser =
- OptionsParser.newOptionsParser(Options.class);
+ OptionsParser optionsParser = OptionsParser.newOptionsParser(Options.class);
+ optionsParser.setAllowResidue(false);
optionsParser.parseAndExitUponError(args);
Options options = optionsParser.getOptions(Options.class);
+ checkState(options.inputJar != null, "--input is required");
+ checkState(options.outputJar != null, "--output is required");
+ checkState(!options.bootclasspath.isEmpty() || options.allowEmptyBootclasspath,
+ "Bootclasspath required to desugar %s", options.inputJar);
+
if (options.verbose) {
System.out.printf("Lambda classes will be written under %s%n", dumpDirectory);
}
- boolean allowDefaultMethods = options.minSdkVersion >= 24;
-
- 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();
- }
-
CoreLibraryRewriter rewriter =
new CoreLibraryRewriter(options.coreLibrary ? "__desugar__/" : "");
+ IndexedJars appIndexedJar = new IndexedJars(ImmutableList.of(options.inputJar));
+ IndexedJars appAndClasspathIndexedJars = new IndexedJars(options.classpath, appIndexedJar);
ClassLoader loader =
- createClassLoader(
- rewriter, options.bootclasspath, options.inputJar, options.classpath, parent);
-
+ createClassLoader(rewriter, options.bootclasspath, appAndClasspathIndexedJars);
+ boolean allowDefaultMethods = options.minSdkVersion >= 24;
+ boolean allowCallsToObjectsNonNull = options.minSdkVersion >= 19;
try (ZipFile in = new ZipFile(options.inputJar.toFile());
- ZipOutputStream out = new ZipOutputStream(new BufferedOutputStream(
- Files.newOutputStream(options.outputJar)))) {
+ ZipOutputStream out =
+ new ZipOutputStream(
+ new BufferedOutputStream(Files.newOutputStream(options.outputJar)))) {
LambdaClassMaker lambdas = new LambdaClassMaker(dumpDirectory);
- ClassReaderFactory readerFactory = new ClassReaderFactory(in, rewriter);
+ ClassReaderFactory readerFactory =
+ new ClassReaderFactory(
+ (options.copyBridgesFromClasspath && !allowDefaultMethods)
+ ? appAndClasspathIndexedJars
+ : appIndexedJar,
+ rewriter);
ImmutableSet.Builder<String> interfaceLambdaMethodCollector = ImmutableSet.builder();
@@ -184,13 +205,13 @@
visitor = new Java7Compatibility(visitor, readerFactory);
}
- visitor = new LambdaDesugaring(
- visitor,
- loader,
- lambdas,
- interfaceLambdaMethodCollector,
- allowDefaultMethods);
+ visitor =
+ new LambdaDesugaring(
+ visitor, loader, lambdas, interfaceLambdaMethodCollector, allowDefaultMethods);
+ if (!allowCallsToObjectsNonNull) {
+ visitor = new ObjectsRequireNonNullMethodInliner(visitor);
+ }
reader.accept(visitor, 0);
writeStoredEntry(out, entry.getName(), writer.toByteArray());
@@ -207,7 +228,8 @@
ImmutableSet<String> interfaceLambdaMethods = interfaceLambdaMethodCollector.build();
if (allowDefaultMethods) {
- checkState(interfaceLambdaMethods.isEmpty(),
+ checkState(
+ interfaceLambdaMethods.isEmpty(),
"Desugaring with default methods enabled moved interface lambdas");
}
@@ -225,7 +247,7 @@
visitor = new Java7Compatibility(visitor, (ClassReaderFactory) null);
}
- LambdaClassFixer lambdaFixer =
+ visitor =
new LambdaClassFixer(
visitor,
lambdaClass.getValue(),
@@ -234,10 +256,16 @@
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());
+ 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());
}
}
@@ -266,25 +294,23 @@
}
private static ClassLoader createClassLoader(CoreLibraryRewriter rewriter,
- List<Path> bootclasspath, Path inputJar, List<Path> classpath,
- ClassLoader parent) throws IOException {
+ 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);
+ }
// 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.
- 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);
+ return new HeaderClassLoader(appAndClasspathIndexedJars, 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.
diff --git a/java/com/google/devtools/build/android/desugar/HeaderClassLoader.java b/java/com/google/devtools/build/android/desugar/HeaderClassLoader.java
index 053d52d..44c3932 100644
--- a/java/com/google/devtools/build/android/desugar/HeaderClassLoader.java
+++ b/java/com/google/devtools/build/android/desugar/HeaderClassLoader.java
@@ -16,12 +16,6 @@
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;
@@ -41,44 +35,20 @@
*/
class HeaderClassLoader extends ClassLoader {
- private final Map<String, JarFile> jarfiles;
+ private final IndexedJars indexedJars;
private final CoreLibraryRewriter rewriter;
- /** 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) {
+ public HeaderClassLoader(
+ IndexedJars indexedJars, CoreLibraryRewriter rewriter, ClassLoader parent) {
super(parent);
this.rewriter = rewriter;
- this.jarfiles = jarfiles;
+ this.indexedJars = indexedJars;
}
@Override
protected Class<?> findClass(String name) throws ClassNotFoundException {
String filename = rewriter.unprefix(name.replace('.', '/') + ".class");
- JarFile jarfile = jarfiles.get(filename);
+ JarFile jarfile = indexedJars.getJarFile(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
new file mode 100644
index 0000000..bdb5b47
--- /dev/null
+++ b/java/com/google/devtools/build/android/desugar/IndexedJars.java
@@ -0,0 +1,84 @@
+// 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 97c0249..e942c95 100644
--- a/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java
+++ b/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java
@@ -27,7 +27,6 @@
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;
@@ -41,8 +40,10 @@
*/
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;
@@ -51,7 +52,7 @@
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;
@@ -59,7 +60,6 @@
private String desc;
private String signature;
- private String[] exceptions;
public LambdaClassFixer(ClassVisitor dest, LambdaInfo lambdaInfo, ClassReaderFactory factory,
@@ -71,10 +71,6 @@
this.allowDefaultMethods = allowDefaultMethods;
}
- public String getInternalName() {
- return internalName;
- }
-
@Override
public void visit(
int version,
@@ -84,15 +80,15 @@
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
@@ -125,7 +121,6 @@
} 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));
@@ -143,17 +138,19 @@
@Override
public void visitEnd() {
checkState(!hasState || hasFactory,
- "Expected factory method for capturing lambda %s", internalName);
- if (!hasFactory) {
- // Fake factory method if LambdaMetafactory didn't generate it
+ "Expected factory method for capturing lambda %s", getInternalName());
+ if (!hasState) {
checkState(signature == null,
- "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();
+ "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());
super.visitField(
- Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL,
- "$instance",
+ Opcodes.ACC_STATIC | Opcodes.ACC_FINAL,
+ SINGLETON_FIELD_NAME,
singletonFieldDesc,
(String) null,
(Object) null)
@@ -166,35 +163,28 @@
"()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(), SINGLETON_FIELD_NAME, 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('#'));
@@ -233,17 +223,41 @@
// 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/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java b/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java
index 8a2ebea..d8f2e28 100644
--- a/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java
+++ b/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;
@@ -25,7 +24,6 @@
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;
@@ -41,7 +39,7 @@
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 +50,7 @@
}
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;
}
/**
@@ -69,23 +63,22 @@
return result;
}
- private Path findOnlyUnprocessed(final String pathPrefix) throws IOException {
+ 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
+ final String rootPathPrefixStr = rootDirectory.resolve(pathPrefix).toString();
+
// 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> 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(pathPrefix)
+ return path.toString().startsWith(rootPathPrefixStr)
&& !generatedClasses.containsKey(path);
}
})) {
diff --git a/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java b/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java
index 79be2a3..c52c4a4 100644
--- a/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java
+++ b/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java
@@ -55,6 +55,7 @@
private String internalName;
private boolean isInterface;
+ private int lambdaCount;
public LambdaDesugaring(
ClassVisitor dest,
@@ -77,6 +78,7 @@
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,18 +362,28 @@
// 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
- super.visitMethodInsn(
- Opcodes.INVOKESTATIC,
- lambdaClassName,
- LambdaClassFixer.FACTORY_METHOD_NAME,
- desc,
- /*itf*/ false);
+ 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);
+ }
} 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 e3bb84f..dad340c 100644
--- a/java/com/google/devtools/build/android/desugar/LambdaInfo.java
+++ b/java/com/google/devtools/build/android/desugar/LambdaInfo.java
@@ -19,11 +19,15 @@
@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();
diff --git a/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java b/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java
new file mode 100644
index 0000000..e56cfe1
--- /dev/null
+++ b/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java
@@ -0,0 +1,68 @@
+// 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 166379b..946d73b 100644
--- a/java/com/google/devtools/common/options/OptionsParser.java
+++ b/java/com/google/devtools/common/options/OptionsParser.java
@@ -19,6 +19,7 @@
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;
@@ -243,25 +244,51 @@
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) {
+ public OptionValueDescription(
+ String name,
+ Object value,
+ OptionPriority priority,
+ String source,
+ String implicitDependant,
+ String expandedFrom,
+ boolean allowMultiple) {
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
*/
@@ -306,6 +333,19 @@
}
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 fb7292c..c15f927 100644
--- a/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -24,7 +24,6 @@
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;
@@ -145,85 +144,19 @@
}});
}
- /**
- * 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, ParsedOptionEntry> parsedValues = Maps.newHashMap();
+ private final Map<Field, OptionValueDescription> parsedValues = Maps.newHashMap();
/**
* We store the pre-parsed, explicit options for each priority in here.
@@ -376,16 +309,17 @@
*/
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();
- ParsedOptionEntry entry = parsedValues.get(field);
+ OptionValueDescription entry = parsedValues.get(field);
if (entry == null) {
Object value = optionsData.getDefaultValue(field);
- result.add(new OptionValueDescription(fieldName, value, OptionPriority.DEFAULT,
- null, null, null));
+ result.add(
+ new OptionValueDescription(
+ fieldName, value, OptionPriority.DEFAULT, null, null, null, false));
} else {
- result.add(entry.asOptionValueDescription(fieldName));
+ result.add(entry);
}
}
return result;
@@ -412,46 +346,71 @@
// 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) {
- ParsedOptionEntry entry = parsedValues.get(field);
+ OptionValueDescription entry = parsedValues.get(field);
if (entry != null) {
// Override existing option if the new value has higher or equal priority.
- if (priority.compareTo(entry.priority) >= 0) {
+ if (priority.compareTo(entry.getPriority()) >= 0) {
// Output warnings:
- 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 + "'");
+ 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
+ + "'");
}
- } 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))) {
+ } 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))) {
// 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 ParsedOptionEntry(value, priority, source, implicitDependant, expandedFrom, false));
+ parsedValues.put(
+ field,
+ new OptionValueDescription(
+ name, value, priority, source, implicitDependant, expandedFrom, false));
}
} else {
- parsedValues.put(field,
- new ParsedOptionEntry(value, priority, source, implicitDependant, expandedFrom, false));
+ parsedValues.put(
+ field,
+ new OptionValueDescription(
+ name, value, priority, source, implicitDependant, expandedFrom, false));
maybeAddDeprecationWarning(field);
}
}
private void addListValue(Field field, Object value, OptionPriority priority, String source,
String implicitDependant, String expandedFrom) {
- ParsedOptionEntry entry = parsedValues.get(field);
+ OptionValueDescription entry = parsedValues.get(field);
if (entry == null) {
- entry = new ParsedOptionEntry(ArrayListMultimap.create(), priority, source,
- implicitDependant, expandedFrom, true);
+ entry =
+ new OptionValueDescription(
+ field.getName(),
+ ArrayListMultimap.create(),
+ priority,
+ source,
+ implicitDependant,
+ expandedFrom,
+ true);
parsedValues.put(field, entry);
maybeAddDeprecationWarning(field);
}
@@ -472,9 +431,9 @@
Field field, Option option, Map<String, OptionValueDescription> clearedValues)
throws OptionsParsingException {
- ParsedOptionEntry removed = parsedValues.remove(field);
+ OptionValueDescription removed = parsedValues.remove(field);
if (removed != null) {
- clearedValues.put(option.name(), removed.asOptionValueDescription(option.name()));
+ clearedValues.put(option.name(), removed);
}
canonicalizeValues.removeAll(field);
@@ -496,11 +455,7 @@
if (field == null) {
throw new IllegalArgumentException("No such option '" + name + "'");
}
- ParsedOptionEntry entry = parsedValues.get(field);
- if (entry == null) {
- return null;
- }
- return entry.asOptionValueDescription(name);
+ return parsedValues.get(field);
}
OptionDescription getOptionDescription(String name) {
@@ -624,9 +579,12 @@
// Handle expansion options.
if (option.expansion().length > 0) {
- Function<Object, String> expansionSourceFunction = Functions.<String>constant(
- "expanded from option --" + originalName + " from " +
- sourceFunction.apply(originalName));
+ Function<Object, String> expansionSourceFunction =
+ Functions.constant(
+ "expanded from option --"
+ + originalName
+ + " from "
+ + sourceFunction.apply(originalName));
maybeAddDeprecationWarning(field);
List<String> unparsed = parse(priority, expansionSourceFunction, null, originalName,
ImmutableList.copyOf(option.expansion()));
@@ -673,10 +631,13 @@
// 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.<String>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.constant(
+ "implicit requirement of option --"
+ + entry.getKey()
+ + " from "
+ + sourceFunction.apply(entry.getKey()));
List<String> unparsed = parse(priority, requirementSourceFunction, entry.getKey(), null,
entry.getValue());
@@ -797,7 +758,7 @@
// Set the fields
for (Field field : optionsData.getFieldsForClass(optionsClass)) {
Object value;
- ParsedOptionEntry entry = parsedValues.get(field);
+ OptionValueDescription entry = parsedValues.get(field);
if (entry == null) {
value = optionsData.getDefaultValue(field);
} else {
