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

11 files changed, 490 insertions, 310 deletions

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 @@ 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 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 @@ 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",
-        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 @@ class Desugar {
     )
     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 @@ 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 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 @@ class Desugar {
               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 @@ class Desugar {
 
       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 @@ class Desugar {
             visitor = new Java7Compatibility(visitor, (ClassReaderFactory) null);
           }
 
-          LambdaClassFixer lambdaFixer =
+          visitor =
               new LambdaClassFixer(
                   visitor,
                   lambdaClass.getValue(),
@@ -234,10 +256,16 @@ class Desugar {
                   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 @@ class Desugar {
   }
 
   private static ClassLoader createClassLoader(CoreLibraryRewriter rewriter,
-      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.
-    classpath = ImmutableList.<Path>builder().add(inputJar).addAll(classpath).build();
+      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 = HeaderClassLoader.fromClassPath(bootclasspath, rewriter, parent);
+      parent = new HeaderClassLoader(new IndexedJars(bootclasspath), rewriter, parent);
     }
-    return HeaderClassLoader.fromClassPath(classpath, 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.
+    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 @@ 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;
@@ -41,44 +35,20 @@ import org.objectweb.asm.Opcodes;
  */
 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.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;
@@ -41,8 +40,10 @@ 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;
@@ -51,7 +52,7 @@ class LambdaClassFixer extends ClassVisitor {
   private final HashSet<String> implementedMethods = new HashSet<>();
   private final LinkedHashSet<String> methodsToMoveIn = new LinkedHashSet<>();
 
-  private String internalName;
+  private String originalInternalName;
   private ImmutableList<String> interfaces;
 
   private boolean hasState;
@@ -59,7 +60,6 @@ class LambdaClassFixer extends ClassVisitor {
 
   private String desc;
   private String signature;
-  private String[] exceptions;
 
 
   public LambdaClassFixer(ClassVisitor dest, LambdaInfo lambdaInfo, ClassReaderFactory factory,
@@ -71,10 +71,6 @@ class LambdaClassFixer extends ClassVisitor {
     this.allowDefaultMethods = allowDefaultMethods;
   }
 
-  public String getInternalName() {
-    return internalName;
-  }
-
   @Override
   public void visit(
       int version,
@@ -84,15 +80,15 @@ class LambdaClassFixer extends ClassVisitor {
       String superName,
       String[] interfaces) {
     checkArgument(BitFlags.noneSet(access, Opcodes.ACC_INTERFACE), "Not a class: %s", name);
-    checkState(internalName == null, "Already visited %s, can't reuse for %s", internalName, name);
-    internalName = name;
+    checkState(this.originalInternalName == null, "not intended for reuse but reused for %s", name);
+    originalInternalName = name;
     hasState = false;
     hasFactory = false;
     desc = null;
     this.signature = null;
-    exceptions = null;
     this.interfaces = ImmutableList.copyOf(interfaces);
-    super.visit(version, access, name, signature, superName, interfaces);
+    // Rename to desired name
+    super.visit(version, access, getInternalName(), signature, superName, interfaces);
   }
 
   @Override
@@ -125,7 +121,6 @@ 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));
@@ -143,17 +138,19 @@ class LambdaClassFixer extends ClassVisitor {
   @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 @@ class LambdaClassFixer extends ClassVisitor {
               "()V",
               (String) null,
               new String[0]);
-      codeBuilder.visitTypeInsn(Opcodes.NEW, internalName);
+      codeBuilder.visitTypeInsn(Opcodes.NEW, getInternalName());
       codeBuilder.visitInsn(Opcodes.DUP);
-      codeBuilder.visitMethodInsn(Opcodes.INVOKESPECIAL, internalName, "<init>",
-          checkNotNull(desc, "didn't see a constructor for %s", internalName), /*itf*/ false);
-      codeBuilder.visitFieldInsn(Opcodes.PUTSTATIC, internalName, "$instance", singletonFieldDesc);
+      codeBuilder.visitMethodInsn(Opcodes.INVOKESPECIAL, getInternalName(), "<init>",
+          checkNotNull(desc, "didn't see a constructor for %s", getInternalName()), /*itf*/ false);
+      codeBuilder.visitFieldInsn(
+          Opcodes.PUTSTATIC, getInternalName(), 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 @@ class LambdaClassFixer extends ClassVisitor {
         // Rewrite invocations of lambda methods in interfaces to anticipate the lambda method being
         // moved into the lambda class (i.e., the class being visited here).
         checkArgument(opcode == Opcodes.INVOKESTATIC, "Cannot move instance method %s", method);
-        owner = internalName;
+        owner = getInternalName();
         itf = false; // owner was interface but is now a class
         methodsToMoveIn.add(method);
-      } else if (name.startsWith("lambda$")) {
-        // Reflect renaming of lambda$ instance methods to avoid accidental overrides
-        name = LambdaDesugaring.uniqueInPackage(owner, name);
+      } else {
+        if (originalInternalName.equals(owner)) {
+          // Reflect renaming of lambda classes
+          owner = getInternalName();
+        }
+        if (name.startsWith("lambda$")) {
+          // Reflect renaming of lambda$ instance methods to avoid accidental overrides
+          name = LambdaDesugaring.uniqueInPackage(owner, name);
+        }
       }
       super.visitMethodInsn(opcode, owner, name, desc, itf);
     }
 
     @Override
+    public void visitTypeInsn(int opcode, String type) {
+      if (originalInternalName.equals(type)) {
+        // Reflect renaming of lambda classes
+        type = getInternalName();
+      }
+      super.visitTypeInsn(opcode, type);
+    }
+
+    @Override
+    public void visitFieldInsn(int opcode, String owner, String name, String desc) {
+      if (originalInternalName.equals(owner)) {
+        // Reflect renaming of lambda classes
+        owner = getInternalName();
+      }
+      super.visitFieldInsn(opcode, owner, name, desc);
+    }
+
+    @Override
     public AnnotationVisitor visitAnnotation(String desc, boolean visible) {
       // Drop annotation that's part of the generated lambda class that's not available on Android.
       // Proguard complains about this otherwise.
diff --git a/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.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;
 
@@ -41,7 +39,7 @@ class LambdaClassMaker {
     this.rootDirectory = rootDirectory;
   }
 
-  public String generateLambdaClass(String invokerInternalName, LambdaInfo lambdaInfo,
+  public void generateLambdaClass(String invokerInternalName, LambdaInfo lambdaInfo,
       MethodHandle bootstrapMethod, ArrayList<Object> bsmArgs) throws IOException {
     // Invoking the bootstrap method will dump the generated class
     try {
@@ -52,11 +50,7 @@ class LambdaClassMaker {
     }
 
     Path generatedClassFile = findOnlyUnprocessed(invokerInternalName + "$$Lambda$");
-    String lambdaClassName = generatedClassFile.toString();
-    checkState(lambdaClassName.endsWith(".class"), "Unexpected filename %s", lambdaClassName);
-    lambdaClassName = lambdaClassName.substring(0, lambdaClassName.length() - ".class".length());
     generatedClasses.put(generatedClassFile, lambdaInfo);
-    return lambdaClassName;
   }
 
   /**
@@ -69,23 +63,22 @@ class LambdaClassMaker {
     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 @@ class LambdaDesugaring extends ClassVisitor {
 
   private String internalName;
   private boolean isInterface;
+  private int lambdaCount;
 
   public LambdaDesugaring(
       ClassVisitor dest,
@@ -77,6 +78,7 @@ class LambdaDesugaring extends ClassVisitor {
       String signature,
       String superName,
       String[] interfaces) {
+    checkState(internalName == null, "not intended for reuse but reused for %s", name);
     internalName = name;
     isInterface = BitFlags.isSet(access, Opcodes.ACC_INTERFACE);
     super.visit(version, access, name, signature, superName, interfaces);
@@ -360,18 +362,28 @@ class LambdaDesugaring extends ClassVisitor {
         // and ultimately we don't care if the bootstrap method was even on the bootclasspath
         // when this class was compiled (although it must've been since javac is unhappy otherwise).
         MethodHandle bsmMethod = toMethodHandle(publicLookup(), bsm, /*target*/ false);
-        String lambdaClassName = lambdas.generateLambdaClass(
+        // Give generated classes to have more stable names (b/35643761).  Use BSM's naming scheme
+        // but with separate counter for each surrounding class.
+        String lambdaClassName = internalName + "$$Lambda$" + (lambdaCount++);
+        lambdas.generateLambdaClass(
             internalName,
-            LambdaInfo.create(desc, bridgeInfo.methodReference(), bridgeInfo.bridgeMethod()),
+            LambdaInfo.create(
+                lambdaClassName, desc, bridgeInfo.methodReference(), bridgeInfo.bridgeMethod()),
             bsmMethod,
             args);
-        // Emit invokestatic that calls the factory method generated in the lambda class
-        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 @@ import org.objectweb.asm.Handle;
 @AutoValue
 abstract class LambdaInfo {
   public static LambdaInfo create(
-      String factoryMethodDesc, Handle methodReference, Handle bridgeMethod) {
+      String desiredInternalName,
+      String factoryMethodDesc,
+      Handle methodReference,
+      Handle bridgeMethod) {
     return new AutoValue_LambdaInfo(
-        factoryMethodDesc, methodReference, bridgeMethod);
+        desiredInternalName, factoryMethodDesc, methodReference, bridgeMethod);
   }
 
+  public abstract String desiredInternalName();
   public abstract String factoryMethodDesc();
   public abstract Handle methodReference();
   public abstract Handle bridgeMethod();
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.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;
@@ -243,25 +244,51 @@ 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) {
+    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 @@ 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 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.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;
@@ -145,85 +144,19 @@ 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, 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 @@ 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();
-      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 @@ 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) {
-    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 @@ class OptionsParserImpl {
       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 @@ class OptionsParserImpl {
     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 @@ class OptionsParserImpl {
 
       // 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 @@ 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.<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 @@ class OptionsParserImpl {
     // 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 {