Merge remote-tracking branch 'aosp/upstream-master' into master am: 9657e58fb2 am: dc0fa56b6e

am: 06d3b0dc01

Change-Id: Ief1cdf409dfc49756c6d82b5c466b0457064960f

14 files changed, 1046 insertions, 458 deletions

diff --git a/java/com/google/devtools/build/android/desugar/CoreLibraryRewriter.java b/java/com/google/devtools/build/android/desugar/CoreLibraryRewriter.java
index 4a25109..85c39ac 100644
--- a/java/com/google/devtools/build/android/desugar/CoreLibraryRewriter.java
+++ b/java/com/google/devtools/build/android/desugar/CoreLibraryRewriter.java
@@ -19,8 +19,10 @@ import org.objectweb.asm.Attribute;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.commons.ClassRemapper;
+import org.objectweb.asm.commons.MethodRemapper;
 import org.objectweb.asm.commons.Remapper;
 
 /** Utility class to prefix or unprefix class names of core library classes */
@@ -87,7 +89,7 @@ class CoreLibraryRewriter {
 
   /** Prefixes core library class names with prefix */
   public String prefix(String typeName) {
-    if (prefix.length() >  0 && shouldPrefix(typeName)) {
+    if (prefix.length() > 0 && shouldPrefix(typeName)) {
       return prefix + typeName;
     }
     return typeName;
@@ -101,9 +103,7 @@ class CoreLibraryRewriter {
     return typeName.substring(prefix.length());
   }
 
-  /**
-   * ClassReader that prefixes core library class names as they are read
-   */
+  /** ClassReader that prefixes core library class names as they are read */
   private class PrefixingClassReader extends ClassReader {
     PrefixingClassReader(InputStream content) throws IOException {
       super(content);
@@ -112,7 +112,7 @@ class CoreLibraryRewriter {
     @Override
     public void accept(ClassVisitor cv, Attribute[] attrs, int flags) {
       cv =
-          new ClassRemapper(
+          new ClassRemapperWithBugFix(
               cv,
               new Remapper() {
                 @Override
@@ -137,7 +137,7 @@ class CoreLibraryRewriter {
       this.cv = this.writer;
       if (prefix.length() != 0) {
         this.cv =
-            new ClassRemapper(
+            new ClassRemapperWithBugFix(
                 this.cv,
                 new Remapper() {
                   @Override
@@ -152,4 +152,56 @@ class CoreLibraryRewriter {
       return writer.toByteArray();
     }
   }
+
+  /** ClassRemapper subclass to work around b/36654936 (caused by ASM bug 317785) */
+  private static class ClassRemapperWithBugFix extends ClassRemapper {
+
+    public ClassRemapperWithBugFix(ClassVisitor cv, Remapper remapper) {
+      super(cv, remapper);
+    }
+
+    @Override
+    protected MethodVisitor createMethodRemapper(MethodVisitor mv) {
+      return new MethodRemapper(mv, this.remapper) {
+
+        @Override
+        public void visitFrame(int type, int nLocal, Object[] local, int nStack, Object[] stack) {
+          if (this.mv != null) {
+            mv.visitFrame(
+                type,
+                nLocal,
+                remapEntriesWithBugfix(nLocal, local),
+                nStack,
+                remapEntriesWithBugfix(nStack, stack));
+          }
+        }
+
+        /**
+         * In {@code FrameNode.accept(MethodVisitor)}, when the frame is Opcodes.F_CHOP, it is
+         * possible that nLocal is greater than 0, and local is null, which causes MethodRemapper to
+         * throw a NPE. So the patch is to make sure that the {@code nLocal<=local.length} and
+         * {@code nStack<=stack.length}
+         */
+        private Object[] remapEntriesWithBugfix(int n, Object[] entries) {
+          if (entries == null || entries.length == 0) {
+            return entries;
+          }
+          for (int i = 0; i < n; i++) {
+            if (entries[i] instanceof String) {
+              Object[] newEntries = new Object[n];
+              if (i > 0) {
+                System.arraycopy(entries, 0, newEntries, 0, i);
+              }
+              do {
+                Object t = entries[i];
+                newEntries[i++] = t instanceof String ? remapper.mapType((String) t) : t;
+              } while (i < n);
+              return newEntries;
+            }
+          }
+          return entries;
+        }
+      };
+    }
+  }
 }
diff --git a/java/com/google/devtools/build/android/desugar/Desugar.java b/java/com/google/devtools/build/android/desugar/Desugar.java
index 22780ea..797cbc4 100644
--- a/java/com/google/devtools/build/android/desugar/Desugar.java
+++ b/java/com/google/devtools/build/android/desugar/Desugar.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.android.desugar;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 
@@ -20,12 +21,16 @@ import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSet.Builder;
 import com.google.common.io.Closer;
 import com.google.devtools.build.android.Converters.ExistingPathConverter;
 import com.google.devtools.build.android.Converters.PathConverter;
+import com.google.devtools.build.android.desugar.CoreLibraryRewriter.UnprefixingClassWriter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParser;
+import com.google.devtools.common.options.OptionsParser.OptionUsageRestrictions;
+import com.google.errorprone.annotations.MustBeClosed;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.FileVisitResult;
@@ -57,8 +62,8 @@ class Desugar {
       converter = ExistingPathConverter.class,
       abbrev = 'i',
       help =
-        "Input Jar or directory with classes to desugar (required, the n-th input is paired with"
-        + "the n-th output)."
+          "Input Jar or directory with classes to desugar (required, the n-th input is paired with"
+              + "the n-th output)."
     )
     public List<Path> inputJars;
 
@@ -68,7 +73,9 @@ class Desugar {
       defaultValue = "",
       category = "input",
       converter = ExistingPathConverter.class,
-      help = "Ordered classpath to resolve symbols in the --input Jar, like javac's -cp flag."
+      help =
+          "Ordered classpath (Jar or directory) to resolve symbols in the --input Jar, like "
+              + "javac's -cp flag."
     )
     public List<Path> classpath;
 
@@ -78,15 +85,16 @@ class Desugar {
       defaultValue = "",
       category = "input",
       converter = ExistingPathConverter.class,
-      help = "Bootclasspath that was used to compile the --input Jar with, like javac's "
-          + "-bootclasspath flag (required)."
+      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 = "undocumented"
+      optionUsageRestrictions = OptionUsageRestrictions.UNDOCUMENTED
     )
     public boolean allowEmptyBootclasspath;
 
@@ -95,11 +103,19 @@ class Desugar {
       defaultValue = "false",
       help =
           "A temporary flag specifically for android lint, subject to removal anytime (DO NOT USE)",
-      category = "undocumented"
+      optionUsageRestrictions = OptionUsageRestrictions.UNDOCUMENTED
     )
     public boolean onlyDesugarJavac9ForLint;
 
     @Option(
+      name = "rewrite_calls_to_long_compare",
+      defaultValue = "true",
+      help = "rewrite calls to Long.compare(long, long) to the JVM instruction lcmp",
+      category = "misc"
+    )
+    public boolean enableRewritingOfLongCompare;
+
+    @Option(
       name = "output",
       allowMultiple = true,
       defaultValue = "",
@@ -140,26 +156,261 @@ class Desugar {
     @Option(
       name = "core_library",
       defaultValue = "false",
-      category = "undocumented",
+      optionUsageRestrictions = OptionUsageRestrictions.UNDOCUMENTED,
       implicitRequirements = "--allow_empty_bootclasspath",
       help = "Enables rewriting to desugar java.* classes."
     )
     public boolean coreLibrary;
   }
 
+  private final Options options;
+  private final Path dumpDirectory;
+  private final CoreLibraryRewriter rewriter;
+  private final LambdaClassMaker lambdas;
+  private final boolean allowDefaultMethods;
+  private final boolean allowCallsToObjectsNonNull;
+  /** An instance of Desugar is expected to be used ONLY ONCE */
+  private boolean used;
+
+  private Desugar(Options options, Path dumpDirectory) {
+    this.options = options;
+    this.dumpDirectory = dumpDirectory;
+    this.rewriter = new CoreLibraryRewriter(options.coreLibrary ? "__desugar__/" : "");
+    this.lambdas = new LambdaClassMaker(dumpDirectory);
+    this.allowDefaultMethods = options.minSdkVersion >= 24;
+    this.allowCallsToObjectsNonNull = options.minSdkVersion >= 19;
+    this.used = false;
+  }
+
+  private void desugar() throws Exception {
+    checkState(!this.used, "This Desugar instance has been used. Please create another one.");
+    this.used = true;
+
+    try (Closer closer = Closer.create()) {
+      IndexedInputs indexedClasspath =
+          new IndexedInputs(toRegisteredInputFileProvider(closer, options.classpath));
+      // 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 bootclassloader =
+          options.bootclasspath.isEmpty()
+              ? new ThrowingClassLoader()
+              : new HeaderClassLoader(
+                  new IndexedInputs(toRegisteredInputFileProvider(closer, options.bootclasspath)),
+                  rewriter,
+                  new ThrowingClassLoader());
+
+      // Process each input separately
+      for (InputOutputPair inputOutputPair : toInputOutputPairs(options)) {
+        desugarOneInput(inputOutputPair, indexedClasspath, bootclassloader);
+      }
+    }
+  }
+
+  private void desugarOneInput(
+      InputOutputPair inputOutputPair, IndexedInputs indexedClasspath, ClassLoader bootclassloader)
+      throws Exception {
+    Path inputPath = inputOutputPair.getInput();
+    Path outputPath = inputOutputPair.getOutput();
+    checkArgument(
+        Files.isDirectory(inputPath) || !Files.isDirectory(outputPath),
+        "Input jar file requires an output jar file");
+
+    try (OutputFileProvider outputFileProvider = toOutputFileProvider(outputPath);
+        InputFileProvider inputFiles = toInputFileProvider(inputPath)) {
+      IndexedInputs indexedInputFiles = new IndexedInputs(ImmutableList.of(inputFiles));
+      // Prepend classpath with input file itself so LambdaDesugaring can load classes with
+      // lambdas.
+      IndexedInputs indexedClasspathAndInputFiles = indexedClasspath.withParent(indexedInputFiles);
+      // Note that input file 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.
+      ClassLoader loader =
+          new HeaderClassLoader(indexedClasspathAndInputFiles, rewriter, bootclassloader);
+
+      ClassReaderFactory readerFactory =
+          new ClassReaderFactory(
+              (options.copyBridgesFromClasspath && !allowDefaultMethods)
+                  ? indexedClasspathAndInputFiles
+                  : indexedInputFiles,
+              rewriter);
+
+      ImmutableSet.Builder<String> interfaceLambdaMethodCollector = ImmutableSet.builder();
+
+      desugarClassesInInput(
+          inputFiles, outputFileProvider, loader, readerFactory, interfaceLambdaMethodCollector);
+
+      desugarAndWriteDumpedLambdaClassesToOutput(
+          outputFileProvider, loader, readerFactory, interfaceLambdaMethodCollector);
+    }
+
+    ImmutableMap<Path, LambdaInfo> leftBehind = lambdas.drain();
+    checkState(leftBehind.isEmpty(), "Didn't process %s", leftBehind);
+  }
+
+  /** Desugar the classes that are in the inputs specified in the command line arguments. */
+  private void desugarClassesInInput(
+      InputFileProvider inputFiles,
+      OutputFileProvider outputFileProvider,
+      ClassLoader loader,
+      ClassReaderFactory readerFactory,
+      Builder<String> interfaceLambdaMethodCollector)
+      throws IOException {
+    for (String filename : inputFiles) {
+      try (InputStream content = inputFiles.getInputStream(filename)) {
+        // We can write classes uncompressed since they need to be converted to .dex format
+        // for Android anyways. Resources are written as they were in the input jar to avoid
+        // any danger of accidentally uncompressed resources ending up in an .apk.
+        if (filename.endsWith(".class")) {
+          ClassReader reader = rewriter.reader(content);
+          UnprefixingClassWriter writer =
+              rewriter.writer(ClassWriter.COMPUTE_MAXS /*for bridge methods*/);
+          ClassVisitor visitor =
+              createClassVisitorsForClassesInInputs(
+                  loader, readerFactory, interfaceLambdaMethodCollector, writer);
+          reader.accept(visitor, 0);
+
+          outputFileProvider.write(filename, writer.toByteArray());
+        } else {
+          outputFileProvider.copyFrom(filename, inputFiles);
+        }
+      }
+    }
+  }
+
+  /**
+   * Desugar the classes that are generated on the fly when we are desugaring the classes in the
+   * specified inputs.
+   */
+  private void desugarAndWriteDumpedLambdaClassesToOutput(
+      OutputFileProvider outputFileProvider,
+      ClassLoader loader,
+      ClassReaderFactory readerFactory,
+      Builder<String> interfaceLambdaMethodCollector)
+      throws IOException {
+    ImmutableSet<String> interfaceLambdaMethods = interfaceLambdaMethodCollector.build();
+    checkState(
+        !allowDefaultMethods || interfaceLambdaMethods.isEmpty(),
+        "Desugaring with default methods enabled moved interface lambdas");
+
+    // Write out the lambda classes we generated along the way
+    ImmutableMap<Path, LambdaInfo> lambdaClasses = lambdas.drain();
+    checkState(
+        !options.onlyDesugarJavac9ForLint || lambdaClasses.isEmpty(),
+        "There should be no lambda classes generated: %s",
+        lambdaClasses.keySet());
+
+    for (Map.Entry<Path, LambdaInfo> lambdaClass : lambdaClasses.entrySet()) {
+      try (InputStream bytecode =
+          Files.newInputStream(dumpDirectory.resolve(lambdaClass.getKey()))) {
+        ClassReader reader = rewriter.reader(bytecode);
+        UnprefixingClassWriter writer =
+            rewriter.writer(ClassWriter.COMPUTE_MAXS /*for invoking bridges*/);
+        ClassVisitor visitor =
+            createClassVisitorsForDumpedLambdaClasses(
+                loader, readerFactory, interfaceLambdaMethods, lambdaClass.getValue(), writer);
+        reader.accept(visitor, 0);
+        String filename =
+            rewriter.unprefix(lambdaClass.getValue().desiredInternalName()) + ".class";
+        outputFileProvider.write(filename, writer.toByteArray());
+      }
+    }
+  }
+
+  /**
+   * Create the class visitors for the lambda classes that are generated on the fly. If no new class
+   * visitors are not generated, then the passed-in {@code writer} will be returned.
+   */
+  private ClassVisitor createClassVisitorsForDumpedLambdaClasses(
+      ClassLoader loader,
+      ClassReaderFactory readerFactory,
+      ImmutableSet<String> interfaceLambdaMethods,
+      LambdaInfo lambdaClass,
+      UnprefixingClassWriter writer) {
+    ClassVisitor visitor = writer;
+
+    if (!allowDefaultMethods) {
+      // null ClassReaderFactory b/c we don't expect to need it for lambda classes
+      visitor = new Java7Compatibility(visitor, (ClassReaderFactory) null);
+    }
+
+    visitor =
+        new LambdaClassFixer(
+            visitor, lambdaClass, readerFactory, interfaceLambdaMethods, allowDefaultMethods);
+    // Send lambda classes through desugaring to make sure there's no invokedynamic
+    // instructions in generated lambda classes (checkState below will fail)
+    visitor = new LambdaDesugaring(visitor, loader, lambdas, null, allowDefaultMethods);
+    if (!allowCallsToObjectsNonNull) {
+      // Not sure whether there will be implicit null check emitted by javac, so we rerun
+      // the inliner again
+      visitor = new ObjectsRequireNonNullMethodRewriter(visitor);
+    }
+    if (options.enableRewritingOfLongCompare) {
+      visitor = new LongCompareMethodRewriter(visitor);
+    }
+    return visitor;
+  }
+
+  /**
+   * Create the class visitors for the classes which are in the inputs. If new visitors are created,
+   * then all these visitors and the passed-in writer will be chained together. If no new visitor is
+   * created, then the passed-in {@code writer} will be returned.
+   */
+  private ClassVisitor createClassVisitorsForClassesInInputs(
+      ClassLoader loader,
+      ClassReaderFactory readerFactory,
+      Builder<String> interfaceLambdaMethodCollector,
+      UnprefixingClassWriter writer) {
+    checkArgument(writer != null, "The class writer cannot be null");
+    ClassVisitor visitor = writer;
+
+    if (!options.onlyDesugarJavac9ForLint) {
+      if (!allowDefaultMethods) {
+        visitor = new Java7Compatibility(visitor, readerFactory);
+      }
+
+      visitor =
+          new LambdaDesugaring(
+              visitor, loader, lambdas, interfaceLambdaMethodCollector, allowDefaultMethods);
+    }
+
+    if (!allowCallsToObjectsNonNull) {
+      visitor = new ObjectsRequireNonNullMethodRewriter(visitor);
+    }
+    if (options.enableRewritingOfLongCompare) {
+      visitor = new LongCompareMethodRewriter(visitor);
+    }
+    return visitor;
+  }
+
+
   public static void main(String[] args) throws Exception {
-    // LambdaClassMaker generates lambda classes for us, but it does so by essentially simulating
-    // the call to LambdaMetafactory that the JVM would make when encountering an invokedynamic.
-    // LambdaMetafactory is in the JDK and its implementation has a property to write out ("dump")
-    // generated classes, which we take advantage of here.  Set property before doing anything else
-    // since the property is read in the static initializer; if this breaks we can investigate
-    // setting the property when calling the tool.
+    // It is important that this method is called first. See its javadoc.
+    Path dumpDirectory = createAndRegisterLambdaDumpDirectory();
+    Options options = parseCommandLineOptions(args);
+    if (options.verbose) {
+      System.out.printf("Lambda classes will be written under %s%n", dumpDirectory);
+    }
+    new Desugar(options, dumpDirectory).desugar();
+  }
+
+  /**
+   * LambdaClassMaker generates lambda classes for us, but it does so by essentially simulating the
+   * call to LambdaMetafactory that the JVM would make when encountering an invokedynamic.
+   * LambdaMetafactory is in the JDK and its implementation has a property to write out ("dump")
+   * generated classes, which we take advantage of here. Set property before doing anything else
+   * since the property is read in the static initializer; if this breaks we can investigate setting
+   * the property when calling the tool.
+   */
+  private static Path createAndRegisterLambdaDumpDirectory() throws IOException {
     Path dumpDirectory = Files.createTempDirectory("lambdas");
     System.setProperty(
         LambdaClassMaker.LAMBDA_METAFACTORY_DUMPER_PROPERTY, dumpDirectory.toString());
 
     deleteTreeOnExit(dumpDirectory);
+    return dumpDirectory;
+  }
 
+  private static Options parseCommandLineOptions(String[] args) throws IOException {
     if (args.length == 1 && args[0].startsWith("@")) {
       args = Files.readAllLines(Paths.get(args[0].substring(1)), ISO_8859_1).toArray(new String[0]);
     }
@@ -167,155 +418,25 @@ class Desugar {
     OptionsParser optionsParser = OptionsParser.newOptionsParser(Options.class);
     optionsParser.setAllowResidue(false);
     optionsParser.parseAndExitUponError(args);
+
     Options options = optionsParser.getOptions(Options.class);
 
-    checkState(!options.inputJars.isEmpty(), "--input is required");
-    checkState(
+    checkArgument(!options.inputJars.isEmpty(), "--input is required");
+    checkArgument(
         options.inputJars.size() == options.outputJars.size(),
-        "Desugar requires the same number of inputs and outputs to pair them");
-    checkState(!options.bootclasspath.isEmpty() || options.allowEmptyBootclasspath,
+        "Desugar requires the same number of inputs and outputs to pair them. #input=%s,#output=%s",
+        options.inputJars.size(),
+        options.outputJars.size());
+    checkArgument(
+        !options.bootclasspath.isEmpty() || options.allowEmptyBootclasspath,
         "At least one --bootclasspath_entry is required");
-    for (Path path : options.classpath) {
-      checkState(!Files.isDirectory(path), "Classpath entry must be a jar file: %s", path);
-    }
     for (Path path : options.bootclasspath) {
-      checkState(!Files.isDirectory(path), "Bootclasspath entry must be a jar file: %s", path);
-    }
-    if (options.verbose) {
-      System.out.printf("Lambda classes will be written under %s%n", dumpDirectory);
-    }
-
-    CoreLibraryRewriter rewriter =
-        new CoreLibraryRewriter(options.coreLibrary ? "__desugar__/" : "");
-
-    boolean allowDefaultMethods = options.minSdkVersion >= 24;
-    boolean allowCallsToObjectsNonNull = options.minSdkVersion >= 19;
-
-    LambdaClassMaker lambdas = new LambdaClassMaker(dumpDirectory);
-
-    // Process each input separately
-    for (InputOutputPair inputOutputPair : toInputOutputPairs(options)) {
-      Path inputPath = inputOutputPair.getInput();
-      Path outputPath = inputOutputPair.getOutput();
-      checkState(
-          Files.isDirectory(inputPath) || !Files.isDirectory(outputPath),
-          "Input jar file requires an output jar file");
-      
-      try (Closer closer = Closer.create();
-          OutputFileProvider outputFileProvider = toOutputFileProvider(outputPath)) {
-        InputFileProvider appInputFiles = toInputFileProvider(closer, inputPath);
-        List<InputFileProvider> classpathInputFiles =
-            toInputFileProvider(closer, options.classpath);
-        IndexedInputs appIndexedInputs = new IndexedInputs(ImmutableList.of(appInputFiles));
-        IndexedInputs appAndClasspathIndexedInputs =
-            new IndexedInputs(classpathInputFiles, appIndexedInputs);
-        ClassLoader loader =
-            createClassLoader(
-                rewriter,
-                toInputFileProvider(closer, options.bootclasspath),
-                appAndClasspathIndexedInputs);
-
-        ClassReaderFactory readerFactory =
-            new ClassReaderFactory(
-                (options.copyBridgesFromClasspath && !allowDefaultMethods)
-                    ? appAndClasspathIndexedInputs
-                    : appIndexedInputs,
-                rewriter);
-
-        ImmutableSet.Builder<String> interfaceLambdaMethodCollector = ImmutableSet.builder();
-
-        // Process inputs, desugaring as we go
-        for (String filename : appInputFiles) {
-          try (InputStream content = appInputFiles.getInputStream(filename)) {
-            // We can write classes uncompressed since they need to be converted to .dex format
-            // for Android anyways. Resources are written as they were in the input jar to avoid
-            // any danger of accidentally uncompressed resources ending up in an .apk.
-            if (filename.endsWith(".class")) {
-              ClassReader reader = rewriter.reader(content);
-              CoreLibraryRewriter.UnprefixingClassWriter writer =
-                  rewriter.writer(ClassWriter.COMPUTE_MAXS /*for bridge methods*/);
-              ClassVisitor visitor = writer;
-
-              if (!options.onlyDesugarJavac9ForLint) {
-                if (!allowDefaultMethods) {
-                  visitor = new Java7Compatibility(visitor, readerFactory);
-                }
-
-                visitor =
-                    new LambdaDesugaring(
-                        visitor,
-                        loader,
-                        lambdas,
-                        interfaceLambdaMethodCollector,
-                        allowDefaultMethods);
-              }
-
-              if (!allowCallsToObjectsNonNull) {
-                visitor = new ObjectsRequireNonNullMethodInliner(visitor);
-              }
-              reader.accept(visitor, 0);
-
-              outputFileProvider.write(filename, writer.toByteArray());
-            } else {
-              outputFileProvider.copyFrom(filename, appInputFiles);
-            }
-          }
-        }
-
-        ImmutableSet<String> interfaceLambdaMethods = interfaceLambdaMethodCollector.build();
-        checkState(
-            !allowDefaultMethods || interfaceLambdaMethods.isEmpty(),
-            "Desugaring with default methods enabled moved interface lambdas");
-
-        // Write out the lambda classes we generated along the way
-        ImmutableMap<Path, LambdaInfo> lambdaClasses = lambdas.drain();
-        checkState(
-            !options.onlyDesugarJavac9ForLint || lambdaClasses.isEmpty(),
-            "There should be no lambda classes generated: %s",
-            lambdaClasses.keySet());
-
-        for (Map.Entry<Path, LambdaInfo> lambdaClass : lambdaClasses.entrySet()) {
-          try (InputStream bytecode =
-              Files.newInputStream(dumpDirectory.resolve(lambdaClass.getKey()))) {
-            ClassReader reader = rewriter.reader(bytecode);
-            CoreLibraryRewriter.UnprefixingClassWriter writer =
-                rewriter.writer(ClassWriter.COMPUTE_MAXS /*for invoking bridges*/);
-            ClassVisitor visitor = writer;
-
-            if (!allowDefaultMethods) {
-              // null ClassReaderFactory b/c we don't expect to need it for lambda classes
-              visitor = new Java7Compatibility(visitor, (ClassReaderFactory) null);
-            }
-
-            visitor =
-                new LambdaClassFixer(
-                    visitor,
-                    lambdaClass.getValue(),
-                    readerFactory,
-                    interfaceLambdaMethods,
-                    allowDefaultMethods);
-            // Send lambda classes through desugaring to make sure there's no invokedynamic
-            // instructions in generated lambda classes (checkState below will fail)
-            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";
-            outputFileProvider.write(filename, writer.toByteArray());
-          }
-        }
-
-        Map<Path, LambdaInfo> leftBehind = lambdas.drain();
-        checkState(leftBehind.isEmpty(), "Didn't process %s", leftBehind);
-      }
+      checkArgument(!Files.isDirectory(path), "Bootclasspath entry must be a jar file: %s", path);
     }
+    return options;
   }
 
-  private static List<InputOutputPair> toInputOutputPairs(Options options) {
+  private static ImmutableList<InputOutputPair> toInputOutputPairs(Options options) {
     final ImmutableList.Builder<InputOutputPair> ioPairListbuilder = ImmutableList.builder();
     for (Iterator<Path> inputIt = options.inputJars.iterator(),
                 outputIt = options.outputJars.iterator();
@@ -325,24 +446,6 @@ class Desugar {
     return ioPairListbuilder.build();
   }
 
-  private static ClassLoader createClassLoader(
-      CoreLibraryRewriter rewriter,
-      List<InputFileProvider> bootclasspath,
-      IndexedInputs appAndClasspathIndexedInputs)
-      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 IndexedInputs(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.
-    return new HeaderClassLoader(appAndClasspathIndexedInputs, rewriter, parent);
-  }
-
   private static class ThrowingClassLoader extends ClassLoader {
     @Override
     protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
@@ -394,6 +497,7 @@ class Desugar {
   }
 
   /** Transform a Path to an {@link OutputFileProvider} */
+  @MustBeClosed
   private static OutputFileProvider toOutputFileProvider(Path path)
       throws IOException {
     if (Files.isDirectory(path)) {
@@ -403,26 +507,31 @@ class Desugar {
     }
   }
 
-  /** Transform a Path to an InputFileProvider and register it to close it at the end of desugar */
-  private static InputFileProvider toInputFileProvider(Closer closer, Path path)
+  /** Transform a Path to an InputFileProvider that needs to be closed by the caller. */
+  @MustBeClosed
+  private static InputFileProvider toInputFileProvider(Path path)
       throws IOException {
     if (Files.isDirectory(path)) {
-      return closer.register(new DirectoryInputFileProvider(path));
+      return new DirectoryInputFileProvider(path);
     } else {
-      return closer.register(new ZipInputFileProvider(path));
+      return new ZipInputFileProvider(path);
     }
   }
 
-  private static ImmutableList<InputFileProvider> toInputFileProvider(
+  /**
+   * Transform a list of Path to a list of InputFileProvider and register them with the given
+   * closer.
+   */
+  @SuppressWarnings("MustBeClosedChecker")
+  private static ImmutableList<InputFileProvider> toRegisteredInputFileProvider(
       Closer closer, List<Path> paths) throws IOException {
     ImmutableList.Builder<InputFileProvider> builder = new ImmutableList.Builder<>();
     for (Path path : paths) {
-      checkState(!Files.isDirectory(path), "Directory is not supported: %s", path);
-      builder.add(closer.register(new ZipInputFileProvider(path)));
+      builder.add(closer.register(toInputFileProvider(path)));
     }
     return builder.build();
   }
-  
+
   /**
    * Pair input and output.
    */
diff --git a/java/com/google/devtools/build/android/desugar/IndexedInputs.java b/java/com/google/devtools/build/android/desugar/IndexedInputs.java
index 58459cc..33c6132 100644
--- a/java/com/google/devtools/build/android/desugar/IndexedInputs.java
+++ b/java/com/google/devtools/build/android/desugar/IndexedInputs.java
@@ -13,11 +13,14 @@
 // limitations under the License.
 package com.google.devtools.build.android.desugar;
 
-import com.google.common.base.Preconditions;
-import java.io.IOException;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.collect.ImmutableMap;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import javax.annotation.CheckReturnValue;
 import javax.annotation.Nullable;
 
 /**
@@ -27,36 +30,47 @@ import javax.annotation.Nullable;
  */
 class IndexedInputs {
 
-  private final Map<String, InputFileProvider> inputFiles = new HashMap<>();
+  private final ImmutableMap<String, InputFileProvider> inputFiles;
 
-  /** Parent indexed inputs to use before to search a file name into this indexed inputs. */
+  /**
+   * Parent {@link IndexedInputs} to use before to search a file name into this {@link
+   * IndexedInputs}.
+   */
   @Nullable
-  private final IndexedInputs parentIndexedInputs;
+  private final IndexedInputs parent;
 
-  /** Index a list of input files without a parent indexed inputs. */
-  public IndexedInputs(List<InputFileProvider> inputProviders) throws IOException {
-    this(inputProviders, null);
+  /** Index a list of input files without a parent {@link IndexedInputs}. */
+  public IndexedInputs(List<InputFileProvider> inputProviders) {
+    this.parent = null;
+    this.inputFiles = indexInputs(inputProviders);
   }
 
   /**
-   * Index a list of input files and set a parent indexed inputs that is firstly used during the
-   * search of a file name.
+   * Create a new {@link IndexedInputs} with input files previously indexed and with a parent {@link
+   * IndexedInputs}.
    */
-  public IndexedInputs(
-      List<InputFileProvider> inputProviders, @Nullable IndexedInputs parentIndexedInputs)
-      throws IOException {
-    this.parentIndexedInputs = parentIndexedInputs;
-    for (InputFileProvider inputProvider : inputProviders) {
-      indexInput(inputProvider);
-    }
+  private IndexedInputs(
+      ImmutableMap<String, InputFileProvider> inputFiles, IndexedInputs parentIndexedInputs) {
+    this.parent = parentIndexedInputs;
+    this.inputFiles = inputFiles;
+  }
+
+  /**
+   * Create a new {@link IndexedInputs} with input files already indexed and with a parent {@link
+   * IndexedInputs}.
+   */
+  @CheckReturnValue
+  public IndexedInputs withParent(IndexedInputs parent) {
+    checkState(this.parent == null);
+    return new IndexedInputs(this.inputFiles, parent);
   }
 
   @Nullable
   public InputFileProvider getInputFileProvider(String filename) {
-    Preconditions.checkArgument(filename.endsWith(".class"));
+    checkArgument(filename.endsWith(".class"));
 
-    if (parentIndexedInputs != null) {
-      InputFileProvider inputFileProvider = parentIndexedInputs.getInputFileProvider(filename);
+    if (parent != null) {
+      InputFileProvider inputFileProvider = parent.getInputFileProvider(filename);
       if (inputFileProvider != null) {
         return inputFileProvider;
       }
@@ -65,11 +79,16 @@ class IndexedInputs {
     return inputFiles.get(filename);
   }
 
-  private void indexInput(final InputFileProvider inputFileProvider) throws IOException {
-    for (String relativePath : inputFileProvider) {
-      if (relativePath.endsWith(".class") && !inputFiles.containsKey(relativePath)) {
-        inputFiles.put(relativePath, inputFileProvider);
+  private ImmutableMap<String, InputFileProvider> indexInputs(
+      List<InputFileProvider> inputProviders) {
+    Map<String, InputFileProvider> indexedInputs = new HashMap<>();
+    for (InputFileProvider inputProvider : inputProviders) {
+      for (String relativePath : inputProvider) {
+        if (relativePath.endsWith(".class") && !indexedInputs.containsKey(relativePath)) {
+          indexedInputs.put(relativePath, inputProvider);
+        }
       }
     }
+    return ImmutableMap.copyOf(indexedInputs);
   }
 }
diff --git a/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java b/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java
index e942c95..dea6339 100644
--- a/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java
+++ b/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java
@@ -117,10 +117,16 @@ class LambdaClassFixer extends ClassVisitor {
     }
     if (FACTORY_METHOD_NAME.equals(name)) {
       hasFactory = true;
+      if (!lambdaInfo.needFactory()) {
+        return null; // drop generated factory method if we won't call it
+      }
       access &= ~Opcodes.ACC_PRIVATE; // make factory method accessible
     } else if ("<init>".equals(name)) {
       this.desc = desc;
       this.signature = signature;
+      if (!lambdaInfo.needFactory() && !desc.startsWith("()")) {
+        access &= ~Opcodes.ACC_PRIVATE; // make constructor accessible if we'll call it directly
+      }
     }
     MethodVisitor methodVisitor =
         new LambdaClassMethodRewriter(super.visitMethod(access, name, desc, signature, exceptions));
diff --git a/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java b/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java
index c808831..be06e37 100644
--- a/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java
+++ b/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java
@@ -39,6 +39,11 @@ import org.objectweb.asm.Handle;
 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.FieldInsnNode;
+import org.objectweb.asm.tree.InsnNode;
+import org.objectweb.asm.tree.MethodNode;
+import org.objectweb.asm.tree.TypeInsnNode;
 
 /**
  * Visitor that desugars classes with uses of lambdas into Java 7-looking code.  This includes
@@ -167,7 +172,9 @@ class LambdaDesugaring extends ClassVisitor {
       name = uniqueInPackage(internalName, name);
     }
     MethodVisitor dest = super.visitMethod(access, name, desc, signature, exceptions);
-    return new InvokedynamicRewriter(dest);
+    return dest != null
+        ? new InvokedynamicRewriter(dest, access, name, desc, signature, exceptions)
+        : null;
   }
 
   @Override
@@ -328,10 +335,19 @@ class LambdaDesugaring extends ClassVisitor {
    * Desugaring that replaces invokedynamics for {@link java.lang.invoke.LambdaMetafactory} with
    * static factory method invocations and triggers a class to be generated for each invokedynamic.
    */
-  private class InvokedynamicRewriter extends MethodVisitor {
+  private class InvokedynamicRewriter extends MethodNode {
 
-    public InvokedynamicRewriter(MethodVisitor dest) {
-      super(ASM5, dest);
+    private final MethodVisitor dest;
+
+    public InvokedynamicRewriter(MethodVisitor dest,
+        int access, String name, String desc, String signature, String[] exceptions) {
+      super(ASM5, access, name, desc, signature, exceptions);
+      this.dest = checkNotNull(dest, "Null destination for %s.%s : %s", internalName, name, desc);
+    }
+
+    @Override
+    public void visitEnd() {
+      accept(dest);
     }
 
     @Override
@@ -365,24 +381,42 @@ class LambdaDesugaring extends ClassVisitor {
         // 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++);
+        Type[] capturedTypes = Type.getArgumentTypes(desc);
+        boolean needFactory = capturedTypes.length != 0
+            && !attemptAllocationBeforeArgumentLoads(lambdaClassName, capturedTypes);
         lambdas.generateLambdaClass(
             internalName,
             LambdaInfo.create(
-                lambdaClassName, desc, bridgeInfo.methodReference(), bridgeInfo.bridgeMethod()),
+                lambdaClassName,
+                desc,
+                needFactory,
+                bridgeInfo.methodReference(),
+                bridgeInfo.bridgeMethod()),
             bsmMethod,
             args);
         if (desc.startsWith("()")) {
           // For stateless lambda classes we'll generate a singleton instance that we can just load
+          checkState(capturedTypes.length == 0);
           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
+        } else if (needFactory) {
+          // If we were unable to inline the allocation of the generated lambda class then
+          // invoke factory method of generated lambda class with the arguments on the stack
           super.visitMethodInsn(
               Opcodes.INVOKESTATIC,
               lambdaClassName,
               LambdaClassFixer.FACTORY_METHOD_NAME,
               desc,
               /*itf*/ false);
+        } else {
+          // Otherwise we inserted a new/dup pair of instructions above and now just need to invoke
+          // the constructor of generated lambda class with the arguments on the stack
+          super.visitMethodInsn(
+              Opcodes.INVOKESPECIAL,
+              lambdaClassName,
+              "<init>",
+              Type.getMethodDescriptor(Type.VOID_TYPE, capturedTypes),
+              /*itf*/ false);
         }
       } catch (IOException | ReflectiveOperationException e) {
         throw new IllegalStateException("Couldn't desugar invokedynamic for " + internalName + "."
@@ -390,6 +424,76 @@ class LambdaDesugaring extends ClassVisitor {
       }
     }
 
+    /**
+     * Tries to insert a new/dup for the given class name before expected existing instructions that
+     * set up arguments for an invokedynamic factory method with the given types.
+     *
+     * <p>For lambda expressions and simple method references we can assume that arguments are set
+     * up with loads of the captured (effectively) final variables.  But method references, can in
+     * general capture an expression, such as in {@code myObject.toString()::charAt} (a {@code
+     * Function&lt;Integer, Character&gt;}), which can also cause null checks to be inserted.  In
+     * such more complicated cases this method may fail to insert a new/dup pair and returns {@code
+     * false}.
+     *
+     * @param internalName internal name of the class to instantiate
+     * @param paramTypes expected invokedynamic argument types, which also must be the parameters of
+     *     {@code internalName}'s constructor.
+     * @return {@code true} if we were able to insert a new/dup, {@code false} otherwise
+     */
+    private boolean attemptAllocationBeforeArgumentLoads(String internalName, Type[] paramTypes) {
+      checkArgument(paramTypes.length > 0, "Expected at least one param for %s", internalName);
+      // Walk backwards past loads corresponding to constructor arguments to find the instruction
+      // after which we need to insert our NEW/DUP pair
+      AbstractInsnNode insn = instructions.getLast();
+      for (int i = paramTypes.length - 1; 0 <= i; --i) {
+        if (insn.getOpcode() == Opcodes.GETFIELD) {
+          // Lambdas in anonymous inner classes have to load outer scope variables from fields,
+          // which manifest as an ALOAD followed by one or more GETFIELDs
+          FieldInsnNode getfield = (FieldInsnNode) insn;
+          checkState(
+              getfield.desc.length() == 1
+                  ? getfield.desc.equals(paramTypes[i].getDescriptor())
+                  : paramTypes[i].getDescriptor().length() > 1,
+              "Expected getfield for %s to set up parameter %s for %s but got %s : %s",
+              paramTypes[i], i, internalName, getfield.name, getfield.desc);
+          insn = insn.getPrevious();
+
+          while (insn.getOpcode() == Opcodes.GETFIELD) {
+            // Nested inner classes can cause a cascade of getfields from the outermost one inwards
+            checkState(((FieldInsnNode) insn).desc.startsWith("L"),
+                "expect object type getfields to get to %s to set up parameter %s for %s, not: %s",
+                paramTypes[i], i, internalName, ((FieldInsnNode) insn).desc);
+            insn = insn.getPrevious();
+          }
+
+          checkState(insn.getOpcode() == Opcodes.ALOAD, // should be a this pointer to be precise
+              "Expected aload before getfield for %s to set up parameter %s for %s but got %s",
+              getfield.name, i, internalName, insn.getOpcode());
+        } else if (insn.getOpcode() != paramTypes[i].getOpcode(Opcodes.ILOAD)) {
+          // Otherwise expect load of a (effectively) final local variable.  Not seeing that means
+          // we're dealing with a method reference on some arbitrary expression, <expression>::m.
+          // In that case we give up and keep using the factory method for now, since inserting
+          // the NEW/DUP so the new object ends up in the right stack slot is hard in that case.
+          // Note this still covers simple cases such as this::m or x::m, where x is a local.
+          checkState(paramTypes.length == 1,
+              "Expected a load for %s to set up parameter %s for %s but got %s",
+              paramTypes[i], i, internalName, insn.getOpcode());
+          return false;
+        }
+        insn = insn.getPrevious();
+      }
+
+      TypeInsnNode newInsn = new TypeInsnNode(Opcodes.NEW, internalName);
+      if (insn == null) {
+        // Ran off the front of the instruction list
+        instructions.insert(newInsn);
+      } else {
+        instructions.insert(insn, newInsn);
+      }
+      instructions.insert(newInsn, new InsnNode(Opcodes.DUP));
+      return true;
+    }
+
     private Lookup createLookup(String lookupClass) throws ReflectiveOperationException {
       Class<?> clazz = loadFromInternal(lookupClass);
       Constructor<Lookup> constructor = Lookup.class.getDeclaredConstructor(Class.class);
diff --git a/java/com/google/devtools/build/android/desugar/LambdaInfo.java b/java/com/google/devtools/build/android/desugar/LambdaInfo.java
index dad340c..b7e7a3c 100644
--- a/java/com/google/devtools/build/android/desugar/LambdaInfo.java
+++ b/java/com/google/devtools/build/android/desugar/LambdaInfo.java
@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.android.desugar;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import com.google.auto.value.AutoValue;
 import org.objectweb.asm.Handle;
 
@@ -21,14 +23,19 @@ abstract class LambdaInfo {
   public static LambdaInfo create(
       String desiredInternalName,
       String factoryMethodDesc,
+      boolean needFactory,
       Handle methodReference,
       Handle bridgeMethod) {
+    checkArgument(!needFactory || !factoryMethodDesc.startsWith("()"),
+        "Shouldn't need a factory method for %s : %s", desiredInternalName, factoryMethodDesc);
     return new AutoValue_LambdaInfo(
-        desiredInternalName, factoryMethodDesc, methodReference, bridgeMethod);
+        desiredInternalName, factoryMethodDesc, needFactory, methodReference, bridgeMethod);
   }
 
   public abstract String desiredInternalName();
   public abstract String factoryMethodDesc();
+  /** Returns {@code true} if we need the generated class to have a factory method. */
+  public abstract boolean needFactory();
   public abstract Handle methodReference();
   public abstract Handle bridgeMethod();
 }
diff --git a/java/com/google/devtools/build/android/desugar/LongCompareMethodRewriter.java b/java/com/google/devtools/build/android/desugar/LongCompareMethodRewriter.java
new file mode 100644
index 0000000..23d7a0d
--- /dev/null
+++ b/java/com/google/devtools/build/android/desugar/LongCompareMethodRewriter.java
@@ -0,0 +1,58 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.android.desugar;
+
+import static org.objectweb.asm.Opcodes.ASM5;
+import static org.objectweb.asm.Opcodes.INVOKESTATIC;
+import static org.objectweb.asm.Opcodes.LCMP;
+
+import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.MethodVisitor;
+
+/**
+ * This class rewrites any call to Long.compare with the JVM instruction lcmp that is semantically
+ * equivalent to Long.compare.
+ */
+public class LongCompareMethodRewriter extends ClassVisitor {
+
+  public LongCompareMethodRewriter(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 LongCompareMethodVisitor(visitor);
+  }
+
+  private static class LongCompareMethodVisitor extends MethodVisitor {
+
+    public LongCompareMethodVisitor(MethodVisitor visitor) {
+      super(ASM5, visitor);
+    }
+
+    @Override
+    public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
+      if (opcode != INVOKESTATIC
+          || !owner.equals("java/lang/Long")
+          || !name.equals("compare")
+          || !desc.equals("(JJ)I")) {
+        super.visitMethodInsn(opcode, owner, name, desc, itf);
+        return;
+      }
+      super.visitInsn(LCMP);
+    }
+  }
+}
diff --git a/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java b/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodRewriter.java
index e56cfe1..5ce0bee 100644
--- a/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java
+++ b/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodRewriter.java
@@ -25,12 +25,11 @@ 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.
+ * o.getClass().
  */
-public class ObjectsRequireNonNullMethodInliner extends ClassVisitor {
+public class ObjectsRequireNonNullMethodRewriter extends ClassVisitor {
 
-  public ObjectsRequireNonNullMethodInliner(ClassVisitor cv) {
+  public ObjectsRequireNonNullMethodRewriter(ClassVisitor cv) {
     super(ASM5, cv);
   }
 
diff --git a/java/com/google/devtools/common/options/DuplicateOptionDeclarationException.java b/java/com/google/devtools/common/options/DuplicateOptionDeclarationException.java
index 5c636b8..ad4c975 100644
--- a/java/com/google/devtools/common/options/DuplicateOptionDeclarationException.java
+++ b/java/com/google/devtools/common/options/DuplicateOptionDeclarationException.java
@@ -14,9 +14,7 @@
 
 package com.google.devtools.common.options;
 
-/**
- * Indicates that an option is declared in more than one class.
- */
+/** Indicates that a flag is declared more than once. */
 public class DuplicateOptionDeclarationException extends RuntimeException {
 
   DuplicateOptionDeclarationException(String message) {
diff --git a/java/com/google/devtools/common/options/IsolatedOptionsData.java b/java/com/google/devtools/common/options/IsolatedOptionsData.java
index 27f42f4..0dc787c 100644
--- a/java/com/google/devtools/common/options/IsolatedOptionsData.java
+++ b/java/com/google/devtools/common/options/IsolatedOptionsData.java
@@ -16,73 +16,89 @@ package com.google.devtools.common.options;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
+import com.google.common.collect.Ordering;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import javax.annotation.concurrent.Immutable;
 
 /**
- * An immutable selection of options data corresponding to a set of options classes. The data is
- * collected using reflection, which can be expensive. Therefore this class can be used internally
- * to cache the results.
+ * A selection of options data corresponding to a set of {@link OptionsBase} subclasses (options
+ * classes). The data is collected using reflection, which can be expensive. Therefore this class
+ * can be used internally to cache the results.
  *
- * <p>The data is isolated in the sense that it has not yet been processed to add inter-option-
- * dependent information -- namely, the results of evaluating expansion functions. The {@link
- * OptionsData} subclass stores this added information. The reason for the split is so that we can
- * avoid exposing to expansion functions the effects of evaluating other expansion functions, to
- * ensure that the order in which they run is not significant.
+ * <p>The data is isolated in the sense that it has not yet been processed to add
+ * inter-option-dependent information -- namely, the results of evaluating expansion functions. The
+ * {@link OptionsData} subclass stores this added information. The reason for the split is so that
+ * we can avoid exposing to expansion functions the effects of evaluating other expansion functions,
+ * to ensure that the order in which they run is not significant.
+ *
+ * <p>This class is immutable so long as the converters and default values associated with the
+ * options are immutable.
  */
-// TODO(brandjon): This class is technically not necessarily immutable due to optionsDefault
-// accepting Object values, and the List in allOptionsField should be ImmutableList. Either fix
-// this or remove @Immutable.
 @Immutable
-class IsolatedOptionsData extends OpaqueOptionsData {
+public class IsolatedOptionsData extends OpaqueOptionsData {
 
   /**
-   * These are the options-declaring classes which are annotated with {@link Option} annotations.
+   * Mapping from each options class to its no-arg constructor. Entries appear in the same order
+   * that they were passed to {@link #from(Collection)}.
    */
   private final ImmutableMap<Class<? extends OptionsBase>, Constructor<?>> optionsClasses;
 
-  /** Maps option name to Option-annotated Field. */
+  /**
+   * Mapping from option name to {@code @Option}-annotated field. Entries appear ordered first by
+   * their options class (the order in which they were passed to {@link #from(Collection)}, and then
+   * in alphabetic order within each options class.
+   */
   private final ImmutableMap<String, Field> nameToField;
 
-  /** Maps option abbreviation to Option-annotated Field. */
+  /** Mapping from option abbreviation to {@code Option}-annotated field (unordered). */
   private final ImmutableMap<Character, Field> abbrevToField;
 
-  /** For each options class, contains a list of all Option-annotated fields in that class. */
-  private final ImmutableMap<Class<? extends OptionsBase>, List<Field>> allOptionsFields;
+  /**
+   * Mapping from options class to a list of all {@code Option}-annotated fields in that class. The
+   * map entries are unordered, but the fields in the lists are ordered alphabetically.
+   */
+  private final ImmutableMap<Class<? extends OptionsBase>, ImmutableList<Field>> allOptionsFields;
 
-  /** Mapping from each Option-annotated field to the default value for that field. */
-  // Immutable like the others, but uses Collections.unmodifiableMap because of null values.
+  /**
+   * Mapping from each {@code Option}-annotated field to the default value for that field
+   * (unordered).
+   *
+   * <p>(This is immutable like the others, but uses {@code Collections.unmodifiableMap} to support
+   * null values.)
+   */
   private final Map<Field, Object> optionDefaults;
 
   /**
-   * Mapping from each Option-annotated field to the proper converter.
+   * Mapping from each {@code Option}-annotated field to the proper converter (unordered).
    *
    * @see #findConverter
    */
   private final ImmutableMap<Field, Converter<?>> converters;
 
   /**
-   * Mapping from each Option-annotated field to a boolean for whether that field allows multiple
-   * values.
+   * Mapping from each {@code Option}-annotated field to a boolean for whether that field allows
+   * multiple values (unordered).
    */
   private final ImmutableMap<Field, Boolean> allowMultiple;
 
   private IsolatedOptionsData(
-      Map<Class<? extends OptionsBase>, Constructor<?>> optionsClasses,
+      Map<Class<? extends OptionsBase>,
+      Constructor<?>> optionsClasses,
       Map<String, Field> nameToField,
       Map<Character, Field> abbrevToField,
-      Map<Class<? extends OptionsBase>, List<Field>> allOptionsFields,
+      Map<Class<? extends OptionsBase>, ImmutableList<Field>> allOptionsFields,
       Map<Field, Object> optionDefaults,
       Map<Field, Converter<?>> converters,
       Map<Field, Boolean> allowMultiple) {
@@ -107,6 +123,10 @@ class IsolatedOptionsData extends OpaqueOptionsData {
         other.allowMultiple);
   }
 
+  /**
+   * Returns all options classes indexed by this options data object, in the order they were passed
+   * to {@link #from(Collection)}.
+   */
   public Collection<Class<? extends OptionsBase>> getOptionsClasses() {
     return optionsClasses.keySet();
   }
@@ -120,6 +140,11 @@ class IsolatedOptionsData extends OpaqueOptionsData {
     return nameToField.get(name);
   }
 
+  /**
+   * Returns all pairs of option names (not field names) and their corresponding {@link Field}
+   * objects. Entries appear ordered first by their options class (the order in which they were
+   * passed to {@link #from(Collection)}, and then in alphabetic order within each options class.
+   */
   public Iterable<Map.Entry<String, Field>> getAllNamedFields() {
     return nameToField.entrySet();
   }
@@ -128,7 +153,11 @@ class IsolatedOptionsData extends OpaqueOptionsData {
     return abbrevToField.get(abbrev);
   }
 
-  public List<Field> getFieldsForClass(Class<? extends OptionsBase> optionsClass) {
+  /**
+   * Returns a list of all {@link Field} objects for options in the given options class, ordered
+   * alphabetically by option name.
+   */
+  public ImmutableList<Field> getFieldsForClass(Class<? extends OptionsBase> optionsClass) {
     return allOptionsFields.get(optionsClass);
   }
 
@@ -181,6 +210,11 @@ class IsolatedOptionsData extends OpaqueOptionsData {
     return field.getType().equals(Void.class);
   }
 
+  /** Returns whether the arg is an expansion option. */
+  public static boolean isExpansionOption(Option annotation) {
+    return (annotation.expansion().length > 0 || OptionsData.usesExpansionFunction(annotation));
+  }
+
   /**
    * Returns whether the arg is an expansion option defined by an expansion function (and not a
    * constant expansion value).
@@ -221,17 +255,29 @@ class IsolatedOptionsData extends OpaqueOptionsData {
     }
   }
 
-  private static List<Field> getAllAnnotatedFields(Class<? extends OptionsBase> optionsClass) {
-    List<Field> allFields = Lists.newArrayList();
+  private static final Ordering<Field> fieldOrdering =
+      new Ordering<Field>() {
+    @Override
+    public int compare(Field f1, Field f2) {
+      String n1 = f1.getAnnotation(Option.class).name();
+      String n2 = f2.getAnnotation(Option.class).name();
+      return n1.compareTo(n2);
+    }
+  };
+
+  /**
+   * Return all {@code @Option}-annotated fields, alphabetically ordered by their option name (not
+   * their field name).
+   */
+  private static ImmutableList<Field> getAllAnnotatedFieldsSorted(
+      Class<? extends OptionsBase> optionsClass) {
+    List<Field> unsortedFields = new ArrayList<>();
     for (Field field : optionsClass.getFields()) {
       if (field.isAnnotationPresent(Option.class)) {
-        allFields.add(field);
+        unsortedFields.add(field);
       }
     }
-    if (allFields.isEmpty()) {
-      throw new IllegalStateException(optionsClass + " has no public @Option-annotated fields");
-    }
-    return ImmutableList.copyOf(allFields);
+    return fieldOrdering.immutableSortedCopy(unsortedFields);
   }
 
   private static Object retrieveDefaultFromAnnotation(Field optionField) {
@@ -260,19 +306,60 @@ class IsolatedOptionsData extends OpaqueOptionsData {
     return convertedValue;
   }
 
+  private static <A> void checkForCollisions(
+      Map<A, Field> aFieldMap,
+      A optionName,
+      String description) {
+    if (aFieldMap.containsKey(optionName)) {
+      throw new DuplicateOptionDeclarationException(
+          "Duplicate option name, due to " + description + ": --" + optionName);
+    }
+  }
+
+  private static void checkForBooleanAliasCollisions(
+      Map<String, String> booleanAliasMap,
+      String optionName,
+      String description) {
+    if (booleanAliasMap.containsKey(optionName)) {
+      throw new DuplicateOptionDeclarationException(
+          "Duplicate option name, due to "
+              + description
+              + " --"
+              + optionName
+              + ", it conflicts with a negating alias for boolean flag --"
+              + booleanAliasMap.get(optionName));
+    }
+  }
+
+  private static void checkAndUpdateBooleanAliases(
+      Map<String, Field> nameToFieldMap,
+      Map<String, String> booleanAliasMap,
+      String optionName) {
+    // Check that the negating alias does not conflict with existing flags.
+    checkForCollisions(nameToFieldMap, "no" + optionName, "boolean option alias");
+
+    // Record that the boolean option takes up additional namespace for its negating alias.
+    booleanAliasMap.put("no" + optionName, optionName);
+  }
+
   /**
    * Constructs an {@link IsolatedOptionsData} object for a parser that knows about the given
    * {@link OptionsBase} classes. No inter-option analysis is done. Performs basic sanity checking
    * on each option in isolation.
    */
   static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes) {
-    Map<Class<? extends OptionsBase>, Constructor<?>> constructorBuilder = Maps.newHashMap();
-    Map<Class<? extends OptionsBase>, List<Field>> allOptionsFieldsBuilder = Maps.newHashMap();
-    Map<String, Field> nameToFieldBuilder = Maps.newHashMap();
-    Map<Character, Field> abbrevToFieldBuilder = Maps.newHashMap();
-    Map<Field, Object> optionDefaultsBuilder = Maps.newHashMap();
-    Map<Field, Converter<?>> convertersBuilder = Maps.newHashMap();
-    Map<Field, Boolean> allowMultipleBuilder = Maps.newHashMap();
+    // Mind which fields have to preserve order.
+    Map<Class<? extends OptionsBase>, Constructor<?>> constructorBuilder = new LinkedHashMap<>();
+    Map<Class<? extends OptionsBase>, ImmutableList<Field>> allOptionsFieldsBuilder =
+        new HashMap<>();
+    Map<String, Field> nameToFieldBuilder = new LinkedHashMap<>();
+    Map<Character, Field> abbrevToFieldBuilder = new HashMap<>();
+    Map<Field, Object> optionDefaultsBuilder = new HashMap<>();
+    Map<Field, Converter<?>> convertersBuilder = new HashMap<>();
+    Map<Field, Boolean> allowMultipleBuilder = new HashMap<>();
+
+    // Maps the negated boolean flag aliases to the original option name.
+    Map<String, String> booleanAliasMap = new HashMap<>();
 
     // Read all Option annotations:
     for (Class<? extends OptionsBase> parsedOptionsClass : classes) {
@@ -284,13 +371,13 @@ class IsolatedOptionsData extends OpaqueOptionsData {
         throw new IllegalArgumentException(parsedOptionsClass
             + " lacks an accessible default constructor");
       }
-      List<Field> fields = getAllAnnotatedFields(parsedOptionsClass);
+      ImmutableList<Field> fields = getAllAnnotatedFieldsSorted(parsedOptionsClass);
       allOptionsFieldsBuilder.put(parsedOptionsClass, fields);
 
       for (Field field : fields) {
         Option annotation = field.getAnnotation(Option.class);
-
-        if (annotation.name() == null) {
+        String optionName = annotation.name();
+        if (optionName == null) {
           throw new AssertionError("Option cannot have a null name");
         }
 
@@ -326,7 +413,7 @@ class IsolatedOptionsData extends OpaqueOptionsData {
             Type elementType =
                 ((ParameterizedType) converterResultType).getActualTypeArguments()[0];
             if (!GenericTypeHelper.isAssignableFrom(fieldType, elementType)) {
-              throw new AssertionError("If the converter return type of a multiple occurance " +
+              throw new AssertionError("If the converter return type of a multiple occurrence " +
                   "option is a list, then the type of list elements (" + fieldType + ") must be " +
                   "assignable from the converter list element type (" + elementType + ")");
             }
@@ -345,21 +432,28 @@ class IsolatedOptionsData extends OpaqueOptionsData {
           }
         }
 
-        if (nameToFieldBuilder.put(annotation.name(), field) != null) {
-          throw new DuplicateOptionDeclarationException(
-              "Duplicate option name: --" + annotation.name());
+        if (isBooleanField(field)) {
+          checkAndUpdateBooleanAliases(nameToFieldBuilder, booleanAliasMap, optionName);
         }
+
+        checkForCollisions(nameToFieldBuilder, optionName, "option");
+        checkForBooleanAliasCollisions(booleanAliasMap, optionName, "option");
+        nameToFieldBuilder.put(optionName, field);
+
         if (!annotation.oldName().isEmpty()) {
-          if (nameToFieldBuilder.put(annotation.oldName(), field) != null) {
-            throw new DuplicateOptionDeclarationException(
-                "Old option name duplicates option name: --" + annotation.oldName());
+          String oldName = annotation.oldName();
+          checkForCollisions(nameToFieldBuilder, oldName, "old option name");
+          checkForBooleanAliasCollisions(booleanAliasMap, oldName, "old option name");
+          nameToFieldBuilder.put(annotation.oldName(), field);
+
+          // If boolean, repeat the alias dance for the old name.
+          if (isBooleanField(field)) {
+            checkAndUpdateBooleanAliases(nameToFieldBuilder, booleanAliasMap, oldName);
           }
         }
         if (annotation.abbrev() != '\0') {
-          if (abbrevToFieldBuilder.put(annotation.abbrev(), field) != null) {
-            throw new DuplicateOptionDeclarationException(
-                  "Duplicate option abbrev: -" + annotation.abbrev());
-          }
+          checkForCollisions(abbrevToFieldBuilder, annotation.abbrev(), "option abbreviation");
+          abbrevToFieldBuilder.put(annotation.abbrev(), field);
         }
 
         optionDefaultsBuilder.put(field, retrieveDefaultFromAnnotation(field));
@@ -379,4 +473,5 @@ class IsolatedOptionsData extends OpaqueOptionsData {
         convertersBuilder,
         allowMultipleBuilder);
   }
+
 }
diff --git a/java/com/google/devtools/common/options/Option.java b/java/com/google/devtools/common/options/Option.java
index 249ee70..c43966c 100644
--- a/java/com/google/devtools/common/options/Option.java
+++ b/java/com/google/devtools/common/options/Option.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.common.options;
 
+import com.google.devtools.common.options.OptionsParser.OptionUsageRestrictions;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -71,21 +72,39 @@ public @interface Option {
   String defaultValue();
 
   /**
-   * A string describing the category of options that this belongs to. {@link
-   * OptionsParser#describeOptions} prints options of the same category grouped together.
+   * A string describing the role of the option. Some existing categories are "input," "output,"
+   * "config," "semantics," and "strategy," among others.
    *
-   * <p>There are three special category values:
+   * <p>The category of options that this belongs to dictates how options are grouped by {@link
+   * OptionsParser#describeOptions}, for the usage documentation.
+   *
+   * <p>For undocumented flags that aren't listed anywhere, this is currently a no-op. Feel free to
+   * set the value that it would have if it were documented, which might be helpful if a flag is
+   * part of an experimental feature that might become documented in the future, or just leave it
+   * unset as the default.
+   *
+   * <p>For hidden or internal options, use the category field only if it is helpful for yourself or
+   * other Bazel developers.
+   */
+  String category() default "misc";
+
+  // TODO(b/37353610) the old convention was to include documentation level in the category(),
+  // which is still permitted for backwards compatibility. This field should be used for any new
+  // options, as the old category use will be removed.
+  /**
+   * Options have multiple uses, some flags, some not. For user-visible flags, they are
+   * "documented," but otherwise, there are 3 types of undocumented options.
    *
    * <ul>
-   *   <li>{@code "undocumented"}: options which are useful for (some subset of) users, but not
-   *       meant to be publicly advertised. For example, experimental options which are only meant
-   *       to be used by specific testers or team members, but which should otherwise be treated
-   *       normally. These options will not be listed in the usage info displayed for the {@code
-   *       --help} option. They are otherwise normal - {@link
+   *   <li>{@code UNDOCUMENTED}: undocumented but user-usable flags. These options are useful for
+   *       (some subset of) users, but not meant to be publicly advertised. For example,
+   *       experimental options which are only meant to be used by specific testers or team members.
+   *       These options will not be listed in the usage info displayed for the {@code --help}
+   *       option. They are otherwise normal - {@link
    *       OptionsParser.UnparsedOptionValueDescription#isHidden()} returns {@code false} for them,
    *       and they can be parsed normally from the command line or RC files.
-   *   <li>{@code "hidden"}: options which users should not pass or know about, but which are used
-   *       by the program (e.g., communication between a command-line client and a backend server).
+   *   <li>{@code HIDDEN}: flags which users should not pass or know about, but which are used by
+   *       the program (e.g., communication between a command-line client and a backend server).
    *       Like {@code "undocumented"} options, these options will not be listed in the usage info
    *       displayed for the {@code --help} option. However, in addition to this, calling {@link
    *       OptionsParser.UnparsedOptionValueDescription#isHidden()} on these options will return
@@ -93,16 +112,16 @@ public @interface Option {
    *       logging or otherwise reporting the command line to the user. This category does not
    *       affect the option in any other way; it can still be parsed normally from the command line
    *       or an RC file.
-   *   <li>{@code "internal"}: options which are purely for internal use within the JVM, and should
-   *       never be shown to the user, nor ever need to be parsed by the options parser. Like {@code
-   *       "hidden"} options, these options will not be listed in the usage info displayed for the
-   *       --help option, and are considered hidden by {@link
+   *   <li>{@code INTERNAL}: these are not flags, but options which are purely for internal use
+   *       within the JVM, and should never be shown to the user, nor be parsed by the options
+   *       parser. Like {@code "hidden"} options, these options will not be listed in the usage info
+   *       displayed for the --help option, and are considered hidden by {@link
    *       OptionsParser.UnparsedOptionValueDescription#isHidden()}. Unlike those, this type of
    *       option cannot be parsed by any call to {@link OptionsParser#parse} - it will be treated
    *       as if it was not defined.
    * </ul>
    */
-  String category() default "misc";
+  OptionUsageRestrictions optionUsageRestrictions() default OptionUsageRestrictions.DOCUMENTED;
 
   /**
    * The converter that we'll use to convert the string representation of this option's value into
diff --git a/java/com/google/devtools/common/options/OptionsParser.java b/java/com/google/devtools/common/options/OptionsParser.java
index 1c4b278..9574a90 100644
--- a/java/com/google/devtools/common/options/OptionsParser.java
+++ b/java/com/google/devtools/common/options/OptionsParser.java
@@ -20,18 +20,17 @@ 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;
 import java.lang.reflect.Field;
 import java.nio.file.FileSystem;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import javax.annotation.Nullable;
 
 /**
  * A parser for options. Typical use case in a main method:
@@ -62,6 +61,29 @@ import java.util.Map;
 public class OptionsParser implements OptionsProvider {
 
   /**
+   * An unchecked exception thrown when there is a problem constructing a parser, e.g. an error
+   * while validating an {@link Option} field in one of its {@link OptionsBase} subclasses.
+   *
+   * <p>This exception is unchecked because it generally indicates an internal error affecting all
+   * invocations of the program. I.e., any such error should be immediately obvious to the
+   * developer. Although unchecked, we explicitly mark some methods as throwing it as a reminder in
+   * the API.
+   */
+  public static class ConstructionException extends RuntimeException {
+    public ConstructionException(String message) {
+      super(message);
+    }
+
+    public ConstructionException(Throwable cause) {
+      super(cause);
+    }
+
+    public ConstructionException(String message, Throwable cause) {
+      super(message, cause);
+    }
+  }
+
+  /**
    * A cache for the parsed options data. Both keys and values are immutable, so
    * this is always safe. Only access this field through the {@link
    * #getOptionsData} method for thread-safety! The cache is very unlikely to
@@ -69,45 +91,53 @@ public class OptionsParser implements OptionsProvider {
    * options classes on the classpath.
    */
   private static final Map<ImmutableList<Class<? extends OptionsBase>>, OptionsData> optionsData =
-      Maps.newHashMap();
+      new HashMap<>();
 
   /**
-   * Returns {@link OpaqueOptionsData} suitable for passing along to
-   * {@link #newOptionsParser(OpaqueOptionsData optionsData)}.
+   * Returns {@link OpaqueOptionsData} suitable for passing along to {@link
+   * #newOptionsParser(OpaqueOptionsData optionsData)}.
    *
-   * This is useful when you want to do the work of analyzing the given {@code optionsClasses}
+   * <p>This is useful when you want to do the work of analyzing the given {@code optionsClasses}
    * exactly once, but you want to parse lots of different lists of strings (and thus need to
-   * construct lots of different {@link OptionsParser} instances). 
+   * construct lots of different {@link OptionsParser} instances).
    */
   public static OpaqueOptionsData getOptionsData(
-      ImmutableList<Class<? extends OptionsBase>> optionsClasses) {
+      List<Class<? extends OptionsBase>> optionsClasses) throws ConstructionException {
     return getOptionsDataInternal(optionsClasses);
   }
 
-  private static synchronized OptionsData getOptionsDataInternal(
-      ImmutableList<Class<? extends OptionsBase>> optionsClasses) {
-    OptionsData result = optionsData.get(optionsClasses);
+  /**
+   * Returns the {@link OptionsData} associated with the given list of options classes.
+   */
+  static synchronized OptionsData getOptionsDataInternal(
+      List<Class<? extends OptionsBase>> optionsClasses) throws ConstructionException {
+    ImmutableList<Class<? extends OptionsBase>> immutableOptionsClasses =
+        ImmutableList.copyOf(optionsClasses);
+    OptionsData result = optionsData.get(immutableOptionsClasses);
     if (result == null) {
-      result = OptionsData.from(optionsClasses);
-      optionsData.put(optionsClasses, result);
+      try {
+        result = OptionsData.from(immutableOptionsClasses);
+      } catch (Exception e) {
+        throw new ConstructionException(e.getMessage(), e);
+      }
+      optionsData.put(immutableOptionsClasses, result);
     }
     return result;
   }
 
   /**
-   * Returns all the annotated fields for the given class, including inherited
-   * ones.
+   * Returns the {@link OptionsData} associated with the given options class.
    */
-  static Collection<Field> getAllAnnotatedFields(Class<? extends OptionsBase> optionsClass) {
-    OptionsData data = getOptionsDataInternal(
-        ImmutableList.<Class<? extends OptionsBase>>of(optionsClass));
-    return data.getFieldsForClass(optionsClass);
+  static OptionsData getOptionsDataInternal(Class<? extends OptionsBase> optionsClass)
+      throws ConstructionException {
+    return getOptionsDataInternal(ImmutableList.<Class<? extends OptionsBase>>of(optionsClass));
   }
 
   /**
    * @see #newOptionsParser(Iterable)
    */
-  public static OptionsParser newOptionsParser(Class<? extends OptionsBase> class1) {
+  public static OptionsParser newOptionsParser(Class<? extends OptionsBase> class1)
+      throws ConstructionException {
     return newOptionsParser(ImmutableList.<Class<? extends OptionsBase>>of(class1));
   }
 
@@ -115,15 +145,15 @@ public class OptionsParser implements OptionsProvider {
    * @see #newOptionsParser(Iterable)
    */
   public static OptionsParser newOptionsParser(Class<? extends OptionsBase> class1,
-                                               Class<? extends OptionsBase> class2) {
+                                               Class<? extends OptionsBase> class2)
+      throws ConstructionException {
     return newOptionsParser(ImmutableList.of(class1, class2));
   }
 
-  /**
-   * Create a new {@link OptionsParser}.
-   */
+  /** Create a new {@link OptionsParser}. */
   public static OptionsParser newOptionsParser(
-      Iterable<? extends Class<? extends OptionsBase>> optionsClasses) {
+      Iterable<? extends Class<? extends OptionsBase>> optionsClasses)
+      throws ConstructionException {
     return newOptionsParser(
         getOptionsDataInternal(ImmutableList.<Class<? extends OptionsBase>>copyOf(optionsClasses)));
   }
@@ -161,7 +191,7 @@ public class OptionsParser implements OptionsProvider {
   public void setAllowSingleDashLongOptions(boolean allowSingleDashLongOptions) {
     this.impl.setAllowSingleDashLongOptions(allowSingleDashLongOptions);
   }
-  
+
   /** Enables the Parser to handle params files loacted insinde the provided {@link FileSystem}. */
   public void enableParamsFileSupport(FileSystem fs) {
     this.impl.setArgsPreProcessor(new ParamsFilePreProcessor(fs));
@@ -199,16 +229,28 @@ public class OptionsParser implements OptionsProvider {
   public static final class OptionDescription {
 
     private final String name;
+
+    // For valued flags
     private final Object defaultValue;
     private final Converter<?> converter;
     private final boolean allowMultiple;
 
-    public OptionDescription(String name, Object defaultValue, Converter<?> converter,
-        boolean allowMultiple) {
+    private final ImmutableList<OptionValueDescription> expansions;
+    private final ImmutableList<OptionValueDescription> implicitRequirements;
+
+    OptionDescription(
+        String name,
+        Object defaultValue,
+        Converter<?> converter,
+        boolean allowMultiple,
+        ImmutableList<OptionValueDescription> expansions,
+        ImmutableList<OptionValueDescription> implicitRequirements) {
       this.name = name;
       this.defaultValue = defaultValue;
       this.converter = converter;
       this.allowMultiple = allowMultiple;
+      this.expansions = expansions;
+      this.implicitRequirements = implicitRequirements;
     }
 
     public String getName() {
@@ -226,8 +268,16 @@ public class OptionsParser implements OptionsProvider {
     public boolean getAllowMultiple() {
       return allowMultiple;
     }
+
+    public ImmutableList<OptionValueDescription> getImplicitRequirements() {
+      return implicitRequirements;
+    }
+
+    public ImmutableList<OptionValueDescription> getExpansions() {
+      return expansions;
+    }
   }
-  
+
   /**
    * The name and value of an option with additional metadata describing its
    * priority, source, whether it was set via an implicit dependency, and if so,
@@ -235,22 +285,25 @@ public class OptionsParser implements OptionsProvider {
    */
   public static class OptionValueDescription {
     private final String name;
-    private final Object value;
-    private final OptionPriority priority;
-    private final String source;
-    private final String implicitDependant;
-    private final String expandedFrom;
+    @Nullable private final String originalValueString;
+    @Nullable private final Object value;
+    @Nullable private final OptionPriority priority;
+    @Nullable private final String source;
+    @Nullable private final String implicitDependant;
+    @Nullable private final String expandedFrom;
     private final boolean allowMultiple;
 
     public OptionValueDescription(
         String name,
-        Object value,
-        OptionPriority priority,
-        String source,
-        String implicitDependant,
-        String expandedFrom,
+        @Nullable String originalValueString,
+        @Nullable Object value,
+        @Nullable OptionPriority priority,
+        @Nullable String source,
+        @Nullable String implicitDependant,
+        @Nullable String expandedFrom,
         boolean allowMultiple) {
       this.name = name;
+      this.originalValueString = originalValueString;
       this.value = value;
       this.priority = priority;
       this.source = source;
@@ -263,6 +316,10 @@ public class OptionsParser implements OptionsProvider {
       return name;
     }
 
+    public String getOriginalValueString() {
+      return originalValueString;
+    }
+
     // Need to suppress unchecked warnings, because the "multiple occurrence"
     // options use unchecked ListMultimaps due to limitations of Java generics.
     @SuppressWarnings({"unchecked", "rawtypes"})
@@ -272,7 +329,7 @@ public class OptionsParser implements OptionsProvider {
         // 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();
+        List result = new ArrayList<>();
         ListMultimap realValue = (ListMultimap) value;
         for (OptionPriority priority : OptionPriority.values()) {
           // If there is no mapping for this key, this check avoids object creation (because
@@ -285,6 +342,7 @@ public class OptionsParser implements OptionsProvider {
       }
       return value;
     }
+
     /**
      * @return the priority of the thing that set this value for this flag
      */
@@ -315,6 +373,10 @@ public class OptionsParser implements OptionsProvider {
       return expandedFrom != null;
     }
 
+    public boolean getAllowMultiple() {
+      return allowMultiple;
+    }
+
     @Override
     public String toString() {
       StringBuilder result = new StringBuilder();
@@ -381,24 +443,22 @@ public class OptionsParser implements OptionsProvider {
       return field.getType().equals(boolean.class);
     }
 
-    private DocumentationLevel documentationLevel() {
+    private OptionUsageRestrictions optionUsageRestrictions() {
       Option option = field.getAnnotation(Option.class);
-      return OptionsParser.documentationLevel(option.category());
+      return OptionsParser.documentationLevel(option);
     }
 
     public boolean isDocumented() {
-      return documentationLevel() == DocumentationLevel.DOCUMENTED;
+      return optionUsageRestrictions() == OptionUsageRestrictions.DOCUMENTED;
     }
 
     public boolean isHidden() {
-      return documentationLevel() == DocumentationLevel.HIDDEN
-          || documentationLevel() == DocumentationLevel.INTERNAL;
+      return optionUsageRestrictions() == OptionUsageRestrictions.HIDDEN
+          || optionUsageRestrictions() == OptionUsageRestrictions.INTERNAL;
     }
 
     boolean isExpansion() {
-      Option option = field.getAnnotation(Option.class);
-      return (option.expansion().length > 0
-          || OptionsData.usesExpansionFunction(option));
+      return OptionsData.isExpansionOption(field.getAnnotation(Option.class));
     }
 
     boolean isImplicitRequirement() {
@@ -448,15 +508,18 @@ public class OptionsParser implements OptionsProvider {
   public enum HelpVerbosity { LONG, MEDIUM, SHORT }
 
   /**
-   * The level of documentation. Only documented options are output as part of
-   * the help.
+   * The restrictions on an option. Only documented options are output as part of the help and are
+   * intended for general user use. Undocumented options can be used by any user but aren't
+   * advertised and in practice should be used by bazel developers or early adopters helping to test
+   * a feature.
    *
-   * <p>We use 'hidden' so that options that form the protocol between the
-   * client and the server are not logged.
+   * <p>We use HIDDEN so that options that form the protocol between the client and the server are
+   * not logged. These are flags, but should never be set by a user.
    *
-   * <p>Options which are 'internal' are not recognized by the parser at all.
+   * <p>Options which are INTERNAL are not recognized by the parser at all, and so cannot be used as
+   * flags.
    */
-  enum DocumentationLevel {
+  public enum OptionUsageRestrictions {
     DOCUMENTED, UNDOCUMENTED, HIDDEN, INTERNAL
   }
 
@@ -474,29 +537,31 @@ public class OptionsParser implements OptionsProvider {
    */
   public String describeOptions(
       Map<String, String> categoryDescriptions, HelpVerbosity helpVerbosity) {
+    OptionsData data = impl.getOptionsData();
     StringBuilder desc = new StringBuilder();
-    if (!impl.getOptionsClasses().isEmpty()) {
-      List<Field> allFields = Lists.newArrayList();
-      for (Class<? extends OptionsBase> optionsClass : impl.getOptionsClasses()) {
-        allFields.addAll(impl.getAnnotatedFieldsFor(optionsClass));
+    if (!data.getOptionsClasses().isEmpty()) {
+      List<Field> allFields = new ArrayList<>();
+      for (Class<? extends OptionsBase> optionsClass : data.getOptionsClasses()) {
+        allFields.addAll(data.getFieldsForClass(optionsClass));
       }
       Collections.sort(allFields, OptionsUsage.BY_CATEGORY);
       String prevCategory = null;
 
       for (Field optionField : allFields) {
-        String category = optionField.getAnnotation(Option.class).category();
+        Option option = optionField.getAnnotation(Option.class);
+        String category = option.category();
         if (!category.equals(prevCategory)) {
           prevCategory = category;
           String description = categoryDescriptions.get(category);
           if (description == null) {
             description = "Options category '" + category + "'";
           }
-          if (documentationLevel(category) == DocumentationLevel.DOCUMENTED) {
+          if (documentationLevel(option) == OptionUsageRestrictions.DOCUMENTED) {
             desc.append("\n").append(description).append(":\n");
           }
         }
 
-        if (documentationLevel(prevCategory) == DocumentationLevel.DOCUMENTED) {
+        if (documentationLevel(option) == OptionUsageRestrictions.DOCUMENTED) {
           OptionsUsage.getUsage(optionField, desc, helpVerbosity, impl.getOptionsData());
         }
       }
@@ -516,19 +581,21 @@ public class OptionsParser implements OptionsProvider {
    *   of the category.
    */
   public String describeOptionsHtml(Map<String, String> categoryDescriptions, Escaper escaper) {
+    OptionsData data = impl.getOptionsData();
     StringBuilder desc = new StringBuilder();
-    if (!impl.getOptionsClasses().isEmpty()) {
-      List<Field> allFields = Lists.newArrayList();
-      for (Class<? extends OptionsBase> optionsClass : impl.getOptionsClasses()) {
-        allFields.addAll(impl.getAnnotatedFieldsFor(optionsClass));
+    if (!data.getOptionsClasses().isEmpty()) {
+      List<Field> allFields = new ArrayList<>();
+      for (Class<? extends OptionsBase> optionsClass : data.getOptionsClasses()) {
+        allFields.addAll(data.getFieldsForClass(optionsClass));
       }
       Collections.sort(allFields, OptionsUsage.BY_CATEGORY);
       String prevCategory = null;
 
       for (Field optionField : allFields) {
-        String category = optionField.getAnnotation(Option.class).category();
-        DocumentationLevel level = documentationLevel(category);
-        if (!category.equals(prevCategory) && level == DocumentationLevel.DOCUMENTED) {
+        Option option = optionField.getAnnotation(Option.class);
+        String category = option.category();
+        OptionUsageRestrictions level = documentationLevel(option);
+        if (!category.equals(prevCategory) && level == OptionUsageRestrictions.DOCUMENTED) {
           String description = categoryDescriptions.get(category);
           if (description == null) {
             description = "Options category '" + category + "'";
@@ -541,7 +608,7 @@ public class OptionsParser implements OptionsProvider {
           prevCategory = category;
         }
 
-        if (level == DocumentationLevel.DOCUMENTED) {
+        if (level == OptionUsageRestrictions.DOCUMENTED) {
           OptionsUsage.getUsageHtml(optionField, desc, escaper, impl.getOptionsData());
         }
       }
@@ -556,12 +623,13 @@ public class OptionsParser implements OptionsProvider {
    * details on the format for the flag completion.
    */
   public String getOptionsCompletion() {
+    OptionsData data = impl.getOptionsData();
     StringBuilder desc = new StringBuilder();
 
     // List all options
-    List<Field> allFields = Lists.newArrayList();
-    for (Class<? extends OptionsBase> optionsClass : impl.getOptionsClasses()) {
-      allFields.addAll(impl.getAnnotatedFieldsFor(optionsClass));
+    List<Field> allFields = new ArrayList<>();
+    for (Class<? extends OptionsBase> optionsClass : data.getOptionsClasses()) {
+      allFields.addAll(data.getFieldsForClass(optionsClass));
     }
     // Sort field for deterministic ordering
     Collections.sort(allFields, new Comparator<Field>() {
@@ -573,8 +641,8 @@ public class OptionsParser implements OptionsProvider {
       }
     });
     for (Field optionField : allFields) {
-      String category = optionField.getAnnotation(Option.class).category();
-      if (documentationLevel(category) == DocumentationLevel.DOCUMENTED) {
+      Option option = optionField.getAnnotation(Option.class);
+      if (documentationLevel(option) == OptionUsageRestrictions.DOCUMENTED) {
         OptionsUsage.getCompletion(optionField, desc);
       }
     }
@@ -585,10 +653,10 @@ public class OptionsParser implements OptionsProvider {
   /**
    * Returns a description of the option.
    *
-   * @return The {@link OptionValueDescription} for the option, or null if there is no option by
-   *        the given name.
+   * @return The {@link OptionDescription} for the option, or null if there is no option by the
+   *     given name.
    */
-  public OptionDescription getOptionDescription(String name) {
+  public OptionDescription getOptionDescription(String name) throws OptionsParsingException {
     return impl.getOptionDescription(name);
   }
 
@@ -606,15 +674,27 @@ public class OptionsParser implements OptionsProvider {
     return impl.getOptionValueDescription(name);
   }
 
-  static DocumentationLevel documentationLevel(String category) {
+  @Deprecated
+  // TODO(b/37353610) the old convention was to include documentation level in the category(),
+  // which is still permitted for backwards compatibility. The enum field should be used for any new
+  // options, as the old category, and this function, will be removed.
+  public static OptionUsageRestrictions documentationLevel(Option option) {
+    // Until all options use the new documentationLabel attribute of an option, only rely on it if
+    // it is not set to the default value.
+    if (option.optionUsageRestrictions() != OptionUsageRestrictions.DOCUMENTED) {
+      return option.optionUsageRestrictions();
+    }
+
+    // Otherwise, continue reading from the category.
+    String category = option.category();
     if ("undocumented".equals(category)) {
-      return DocumentationLevel.UNDOCUMENTED;
+      return OptionUsageRestrictions.UNDOCUMENTED;
     } else if ("hidden".equals(category)) {
-      return DocumentationLevel.HIDDEN;
+      return OptionUsageRestrictions.HIDDEN;
     } else if ("internal".equals(category)) {
-      return DocumentationLevel.INTERNAL;
+      return OptionUsageRestrictions.INTERNAL;
     } else {
-      return DocumentationLevel.DOCUMENTED;
+      return OptionUsageRestrictions.DOCUMENTED;
     }
   }
 
@@ -623,7 +703,7 @@ public class OptionsParser implements OptionsProvider {
    * {@code parse(OptionPriority.COMMAND_LINE, null, Arrays.asList(args))}.
    */
   public void parse(String... args) throws OptionsParsingException {
-    parse(OptionPriority.COMMAND_LINE, (String) null, Arrays.asList(args));
+    parse(OptionPriority.COMMAND_LINE, null, Arrays.asList(args));
   }
 
   /**
@@ -631,7 +711,7 @@ public class OptionsParser implements OptionsProvider {
    * {@code parse(OptionPriority.COMMAND_LINE, null, args)}.
    */
   public void parse(List<String> args) throws OptionsParsingException {
-    parse(OptionPriority.COMMAND_LINE, (String) null, args);
+    parse(OptionPriority.COMMAND_LINE, null, args);
   }
 
   /**
@@ -670,8 +750,7 @@ public class OptionsParser implements OptionsProvider {
   }
 
   /**
-   * Clears the given option. Also clears expansion arguments and implicit requirements for that
-   * option.
+   * Clears the given option.
    *
    * <p>This will not affect options objects that have already been retrieved from this parser
    * through {@link #getOptions(Class)}.
@@ -680,11 +759,10 @@ public class OptionsParser implements OptionsProvider {
    * @return A map of an option name to the old value of the options that were cleared.
    * @throws IllegalArgumentException If the flag does not exist.
    */
-  public Map<String, OptionValueDescription> clearValue(String optionName)
+  public OptionValueDescription clearValue(String optionName)
       throws OptionsParsingException {
-    Map<String, OptionValueDescription> clearedValues = Maps.newHashMap();
-    impl.clearValue(optionName, clearedValues);
-    return clearedValues;
+    OptionValueDescription clearedValue = impl.clearValue(optionName);
+    return clearedValue;
   }
 
   @Override
diff --git a/java/com/google/devtools/common/options/OptionsParserImpl.java b/java/com/google/devtools/common/options/OptionsParserImpl.java
index 5c6498a..5c635fb 100644
--- a/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -25,17 +25,18 @@ import com.google.common.collect.Iterables;
 import com.google.common.collect.Iterators;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
 import com.google.devtools.common.options.OptionsParser.OptionDescription;
+import com.google.devtools.common.options.OptionsParser.OptionUsageRestrictions;
 import com.google.devtools.common.options.OptionsParser.OptionValueDescription;
 import com.google.devtools.common.options.OptionsParser.UnparsedOptionValueDescription;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
+import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -60,14 +61,14 @@ class OptionsParserImpl {
    *
    * This map is modified by repeated calls to {@link #parse(OptionPriority,Function,List)}.
    */
-  private final Map<Field, OptionValueDescription> parsedValues = Maps.newHashMap();
+  private final Map<Field, OptionValueDescription> parsedValues = new HashMap<>();
 
   /**
    * We store the pre-parsed, explicit options for each priority in here.
    * We use partially preparsed options, which can be different from the original
    * representation, e.g. "--nofoo" becomes "--foo=0".
    */
-  private final List<UnparsedOptionValueDescription> unparsedValues = Lists.newArrayList();
+  private final List<UnparsedOptionValueDescription> unparsedValues = new ArrayList<>();
 
   /**
    * Unparsed values for use with the canonicalize command are stored separately from
@@ -81,7 +82,7 @@ class OptionsParserImpl {
   private final Multimap<Field, UnparsedOptionValueDescription> canonicalizeValues
       = LinkedHashMultimap.create();
 
-  private final List<String> warnings = Lists.newArrayList();
+  private final List<String> warnings = new ArrayList<>();
 
   private boolean allowSingleDashLongOptions = false;
   
@@ -121,8 +122,10 @@ class OptionsParserImpl {
    * The implementation of {@link OptionsBase#asMap}.
    */
   static Map<String, Object> optionsAsMap(OptionsBase optionsInstance) {
-    Map<String, Object> map = Maps.newHashMap();
-    for (Field field : OptionsParser.getAllAnnotatedFields(optionsInstance.getClass())) {
+    Class<? extends OptionsBase> optionsClass = optionsInstance.getClass();
+    OptionsData data = OptionsParser.getOptionsDataInternal(optionsClass);
+    Map<String, Object> map = new HashMap<>();
+    for (Field field : data.getFieldsForClass(optionsClass)) {
       try {
         String name = field.getAnnotation(Option.class).name();
         Object value = field.get(optionsInstance);
@@ -134,10 +137,6 @@ class OptionsParserImpl {
     return map;
   }
 
-  List<Field> getAnnotatedFieldsFor(Class<? extends OptionsBase> clazz) {
-    return optionsData.getFieldsForClass(clazz);
-  }
-
   /**
    * Implements {@link OptionsParser#asListOfUnparsedOptions()}.
    */
@@ -199,7 +198,7 @@ class OptionsParserImpl {
       }
     });
 
-    List<String> result = Lists.newArrayList();
+    List<String> result = new ArrayList<>();
     for (UnparsedOptionValueDescription value : processed) {
 
       // Ignore expansion options.
@@ -216,7 +215,7 @@ class OptionsParserImpl {
    * Implements {@link OptionsParser#asListOfEffectiveOptions()}.
    */
   List<OptionValueDescription> asListOfEffectiveOptions() {
-    List<OptionValueDescription> result = Lists.newArrayList();
+    List<OptionValueDescription> result = new ArrayList<>();
     for (Map.Entry<String, Field> mapEntry : optionsData.getAllNamedFields()) {
       String fieldName = mapEntry.getKey();
       Field field = mapEntry.getValue();
@@ -225,7 +224,14 @@ class OptionsParserImpl {
         Object value = optionsData.getDefaultValue(field);
         result.add(
             new OptionValueDescription(
-                fieldName, value, OptionPriority.DEFAULT, null, null, null, false));
+                fieldName,
+                /* originalValueString */null,
+                value,
+                OptionPriority.DEFAULT,
+                /* source */ null,
+                /* implicitDependant */ null,
+                /* expandedFrom */ null,
+                false));
       } else {
         result.add(entry);
       }
@@ -233,10 +239,6 @@ class OptionsParserImpl {
     return result;
   }
 
-  Collection<Class<?  extends OptionsBase>> getOptionsClasses() {
-    return optionsData.getOptionsClasses();
-  }
-
   private void maybeAddDeprecationWarning(Field field) {
     Option option = field.getAnnotation(Option.class);
     // Continue to support the old behavior for @Deprecated options.
@@ -289,30 +291,40 @@ class OptionsParserImpl {
           // 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 + "'");
+        } else if ((entry.getExpansionParent() != null) && (expandedFrom != null)) {
+          warnings.add(
+              "The option '"
+                  + name
+                  + "' was expanded to from both options '"
+                  + entry.getExpansionParent()
+                  + "' and '"
+                  + expandedFrom
+                  + "'");
         }
 
         // Record the new value:
         parsedValues.put(
             field,
             new OptionValueDescription(
-                name, value, priority, source, implicitDependant, expandedFrom, false));
+                name, null, value, priority, source, implicitDependant, expandedFrom, false));
       }
     } else {
       parsedValues.put(
           field,
           new OptionValueDescription(
-              name, value, priority, source, implicitDependant, expandedFrom, false));
+              name, null, value, priority, source, implicitDependant, expandedFrom, false));
       maybeAddDeprecationWarning(field);
     }
   }
 
-  private void addListValue(Field field, Object value, OptionPriority priority, String source,
-      String implicitDependant, String expandedFrom) {
+  private void addListValue(Field field, String originalName, Object value, OptionPriority priority,
+      String source, String implicitDependant, String expandedFrom) {
     OptionValueDescription entry = parsedValues.get(field);
     if (entry == null) {
       entry =
           new OptionValueDescription(
-              field.getName(),
+              originalName,
+              /* originalValueString */ null,
               ArrayListMultimap.create(),
               priority,
               source,
@@ -325,38 +337,16 @@ class OptionsParserImpl {
     entry.addValue(priority, value);
   }
 
-  void clearValue(String optionName, Map<String, OptionValueDescription> clearedValues)
+  OptionValueDescription clearValue(String optionName)
       throws OptionsParsingException {
     Field field = optionsData.getFieldFromName(optionName);
     if (field == null) {
       throw new IllegalArgumentException("No such option '" + optionName + "'");
     }
-    Option option = field.getAnnotation(Option.class);
-    clearValue(field, option, clearedValues);
-  }
-
-  private void clearValue(
-      Field field, Option option, Map<String, OptionValueDescription> clearedValues)
-      throws OptionsParsingException {
-
-    OptionValueDescription removed = parsedValues.remove(field);
-    if (removed != null) {
-      clearedValues.put(option.name(), removed);
-    }
 
+    // Actually remove the value from various lists tracking effective options.
     canonicalizeValues.removeAll(field);
-
-    // Recurse to remove any implicit or expansion flags that this flag may have added when
-    // originally parsed.
-    String[] expansion = optionsData.getEvaluatedExpansion(field);
-    for (String[] args : new String[][] {option.implicitRequirements(), expansion}) {
-      Iterator<String> argsIterator = Iterators.forArray(args);
-      while (argsIterator.hasNext()) {
-        String arg = argsIterator.next();
-        ParseOptionResult parseOptionResult = parseOption(arg, argsIterator);
-        clearValue(parseOptionResult.field, parseOptionResult.option, clearedValues);
-      }
-    }
+    return parsedValues.remove(field);
   }
 
   OptionValueDescription getOptionValueDescription(String name) {
@@ -367,7 +357,7 @@ class OptionsParserImpl {
     return parsedValues.get(field);
   }
 
-  OptionDescription getOptionDescription(String name) {
+  OptionDescription getOptionDescription(String name) throws OptionsParsingException {
     Field field = optionsData.getFieldFromName(name);
     if (field == null) {
       return null;
@@ -378,7 +368,44 @@ class OptionsParserImpl {
         name,
         optionsData.getDefaultValue(field),
         optionsData.getConverter(field),
-        optionAnnotation.allowMultiple());
+        optionsData.getAllowMultiple(field),
+        getExpansionDescriptions(
+            optionsData.getEvaluatedExpansion(field),
+            /* expandedFrom */ name,
+            /* implicitDependant */ null),
+        getExpansionDescriptions(
+            optionAnnotation.implicitRequirements(),
+            /* expandedFrom */ null,
+            /* implicitDependant */ name));
+  }
+
+  /**
+   * @return A list of the descriptions corresponding to the list of unparsed flags passed in.
+   * These descriptions are are divorced from the command line - there is no correct priority or
+   * source for these, as they are not actually set values. The value itself is also a string, no
+   * conversion has taken place.
+   */
+  private ImmutableList<OptionValueDescription> getExpansionDescriptions(
+      String[] optionStrings, String expandedFrom, String implicitDependant)
+      throws OptionsParsingException {
+    ImmutableList.Builder<OptionValueDescription> builder = ImmutableList.builder();
+    ImmutableList<String> options = ImmutableList.copyOf(optionStrings);
+    Iterator<String> optionsIterator = options.iterator();
+
+    while (optionsIterator.hasNext()) {
+      String unparsedFlagExpression = optionsIterator.next();
+      ParseOptionResult parseResult = parseOption(unparsedFlagExpression, optionsIterator);
+      builder.add(new OptionValueDescription(
+          parseResult.option.name(),
+          parseResult.value,
+          /* value */ null,
+          /* priority */ null,
+          /* source */null,
+          implicitDependant,
+          expandedFrom,
+          optionsData.getAllowMultiple(parseResult.field)));
+    }
+    return builder.build();
   }
 
   boolean containsExplicitOption(String name) {
@@ -416,8 +443,8 @@ class OptionsParserImpl {
       String expandedFrom,
       List<String> args) throws OptionsParsingException {
 
-    List<String> unparsedArgs = Lists.newArrayList();
-    LinkedHashMap<String,List<String>> implicitRequirements = Maps.newLinkedHashMap();
+    List<String> unparsedArgs = new ArrayList<>();
+    LinkedHashMap<String, List<String>> implicitRequirements = new LinkedHashMap<>();
 
     Iterator<String> argsIterator = argsPreProcessor.preProcess(args).iterator();
     while (argsIterator.hasNext()) {
@@ -433,10 +460,10 @@ class OptionsParserImpl {
         break;
       }
 
-      ParseOptionResult optionParseResult = parseOption(arg, argsIterator);
-      Field field = optionParseResult.field;
-      Option option = optionParseResult.option;
-      String value = optionParseResult.value;
+      ParseOptionResult parseOptionResult = parseOption(arg, argsIterator);
+      Field field = parseOptionResult.field;
+      Option option = parseOptionResult.option;
+      String value = parseOptionResult.value;
 
       final String originalName = option.name();
 
@@ -451,8 +478,11 @@ class OptionsParserImpl {
               ImmutableList.of(value));
 
           if (!unparsed.isEmpty()) {
-            throw new OptionsParsingException("Unparsed options remain after unwrapping " +
-              arg + ": " + Joiner.on(' ').join(unparsed));
+            throw new OptionsParsingException(
+                "Unparsed options remain after unwrapping "
+                    + arg
+                    + ": "
+                    + Joiner.on(' ').join(unparsed));
           }
 
           // Don't process implicitRequirements or expansions for wrapper options. In particular,
@@ -501,8 +531,11 @@ class OptionsParserImpl {
         if (!unparsed.isEmpty()) {
           // Throw an assertion, because this indicates an error in the code that specified the
           // expansion for the current option.
-          throw new AssertionError("Unparsed options remain after parsing expansion of " +
-            arg + ": " + Joiner.on(' ').join(unparsed));
+          throw new AssertionError(
+              "Unparsed options remain after parsing expansion of "
+                  + arg
+                  + ": "
+                  + Joiner.on(' ').join(unparsed));
         }
       } else {
         Converter<?> converter = optionsData.getConverter(field);
@@ -527,8 +560,8 @@ class OptionsParserImpl {
           // Note: The type of the list member is not known; Java introspection
           // only makes it available in String form via the signature string
           // for the field declaration.
-          addListValue(field, convertedValue, priority, sourceFunction.apply(originalName),
-              implicitDependent, expandedFrom);
+          addListValue(field, originalName, convertedValue, priority,
+              sourceFunction.apply(originalName), implicitDependent, expandedFrom);
         }
       }
 
@@ -603,10 +636,21 @@ class OptionsParserImpl {
       value = equalsAt == -1 ? null : arg.substring(equalsAt + 1);
       field = optionsData.getFieldFromName(name);
 
-      // look for a "no"-prefixed option name: "no<optionname>";
-      // (Undocumented: we also allow --no_foo.  We're generous like that.)
+      // Look for a "no"-prefixed option name: "no<optionName>".
       if (field == null && name.startsWith("no")) {
-        name = name.substring(name.startsWith("no_") ? 3 : 2);
+        // Give a nice error if someone is using the deprecated --no_ prefix.
+        // Note: With this check in place, is impossible to specify "--no_foo" for a flag named
+        // "--_foo", if a --foo flag also exists, since that'll be interpreted as the "no_"
+        // negating prefix for "--foo". Let that be a warning to anyone wanting to make flags that
+        // start with underscores.
+        // TODO(Bazel-team): Remove the --no_ check when sufficient time has passed for users of
+        // that feature to have stopped using it.
+        if (name.startsWith("no_") && optionsData.getFieldFromName(name.substring(3)) != null) {
+          throw new OptionsParsingException(
+              "'no_' prefixes are no longer accepted, --no<flag> is an accepted alternative.",
+              name.substring(3));
+        }
+        name = name.substring(2);
         field = optionsData.getFieldFromName(name);
         booleanValue = false;
         if (field != null) {
@@ -630,8 +674,7 @@ class OptionsParserImpl {
     Option option = field == null ? null : field.getAnnotation(Option.class);
 
     if (option == null
-        || OptionsParser.documentationLevel(option.category())
-            == OptionsParser.DocumentationLevel.INTERNAL) {
+        || OptionsParser.documentationLevel(option) == OptionUsageRestrictions.INTERNAL) {
       // This also covers internal options, which are treated as if they did not exist.
       throw new OptionsParsingException("Unrecognized option: " + arg, arg);
     }
@@ -663,7 +706,7 @@ class OptionsParserImpl {
       if (constructor == null) {
         return null;
       }
-      optionsInstance = constructor.newInstance(new Object[0]);
+      optionsInstance = constructor.newInstance();
     } catch (Exception e) {
       throw new IllegalStateException(e);
     }
diff --git a/java/com/google/devtools/common/options/OptionsUsage.java b/java/com/google/devtools/common/options/OptionsUsage.java
index aa48cb7..1c9f519 100644
--- a/java/com/google/devtools/common/options/OptionsUsage.java
+++ b/java/com/google/devtools/common/options/OptionsUsage.java
@@ -16,10 +16,10 @@ package com.google.devtools.common.options;
 import com.google.common.base.Joiner;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
-import com.google.common.collect.Lists;
 import com.google.common.escape.Escaper;
 import java.lang.reflect.Field;
 import java.text.BreakIterator;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
@@ -40,8 +40,8 @@ class OptionsUsage {
    * OptionsBase} subclasses they depend on until a complete parser is constructed).
    */
   static void getUsage(Class<? extends OptionsBase> optionsClass, StringBuilder usage) {
-    List<Field> optionFields =
-        Lists.newArrayList(OptionsParser.getAllAnnotatedFields(optionsClass));
+    OptionsData data = OptionsParser.getOptionsDataInternal(optionsClass);
+    List<Field> optionFields = new ArrayList<>(data.getFieldsForClass(optionsClass));
     Collections.sort(optionFields, BY_NAME);
     for (Field optionField : optionFields) {
       getUsage(optionField, usage, OptionsParser.HelpVerbosity.LONG, null);
@@ -270,6 +270,7 @@ class OptionsUsage {
     }
   }
 
+  // TODO(brandjon): Should this use sorting by option name instead of field name?
   private static final Comparator<Field> BY_NAME = new Comparator<Field>() {
     @Override
     public int compare(Field left, Field right) {