Disallow multiple --min-api and add --no-desugaring / setDisableDesugaring.
R=sgjesse
Change-Id: Ieff5b55d8e2b32722737778bd35c293dd16f732f
diff --git a/src/main/java/com/android/tools/r8/BaseCompilerCommand.java b/src/main/java/com/android/tools/r8/BaseCompilerCommand.java
index ea4ee2f..42982d5 100644
--- a/src/main/java/com/android/tools/r8/BaseCompilerCommand.java
+++ b/src/main/java/com/android/tools/r8/BaseCompilerCommand.java
@@ -4,11 +4,13 @@
package com.android.tools.r8;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DefaultDiagnosticsHandler;
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.Reporter;
+import com.android.tools.r8.utils.StringDiagnostic;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
@@ -102,7 +104,7 @@
private CompilationMode mode;
private int minApiLevel = AndroidApiLevel.getDefault().getLevel();
- private boolean enableDesugaring = true;
+ private boolean disableDesugaring = false;
Builder() {}
@@ -236,23 +238,31 @@
return self();
}
- /**
- * Force enable or disable desugaring.
- *
- * <p>There are a few use cases where it makes sense to force disable desugaring, such as:
- * <li>if all inputs are known to be at most Java 7; or
- * <li>if a separate desugar tool has been used prior to compiling with D8.
- *
- * <p>Note that even for API 27, desugaring is still required for closures support on ART.
- */
+ @Deprecated
public B setEnableDesugaring(boolean enableDesugaring) {
- this.enableDesugaring = enableDesugaring;
+ this.disableDesugaring = !enableDesugaring;
return self();
}
- /** Get the enable/disable state of desugaring. */
- public boolean getEnableDesugaring() {
- return enableDesugaring;
+ /**
+ * Force disable desugaring.
+ *
+ * <p>There are a few use cases where it makes sense to force disable desugaring, such as:
+ * <ul>
+ * <li>if all inputs are known to be at most Java 7; or
+ * <li>if a separate desugar tool has been used prior to compiling with D8.
+ * </ul>
+ *
+ * <p>Note that even for API 27, desugaring is still required for closures support on ART.
+ */
+ public B setDisableDesugaring(boolean disableDesugaring) {
+ this.disableDesugaring = disableDesugaring;
+ return self();
+ }
+
+ /** Is desugaring forcefully disabled. */
+ public boolean getDisableDesugaring() {
+ return disableDesugaring;
}
@Override
@@ -290,4 +300,32 @@
super.validate();
}
}
+
+ static <C extends BaseCompilerCommand, B extends BaseCompilerCommand.Builder<C, B>>
+ boolean parseMinApi(
+ BaseCompilerCommand.Builder<C, B> builder,
+ String minApiString,
+ boolean hasDefinedApiLevel,
+ Origin origin) {
+ if (hasDefinedApiLevel) {
+ builder.getReporter().error(new StringDiagnostic(
+ "Cannot set multiple --min-api options", origin));
+ return false;
+ }
+ int minApi;
+ try {
+ minApi = Integer.valueOf(minApiString);
+ } catch (NumberFormatException e) {
+ builder.getReporter().error(new StringDiagnostic(
+ "Invalid argument to --min-api: " + minApiString, origin));
+ return false;
+ }
+ if (minApi < 1) {
+ builder.getReporter().error(new StringDiagnostic(
+ "Invalid argument to --min-api: " + minApiString, origin));
+ return false;
+ }
+ builder.setMinApiLevel(minApi);
+ return true;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/CompatProguardCommandBuilder.java b/src/main/java/com/android/tools/r8/CompatProguardCommandBuilder.java
index e950d87..ddb0244 100644
--- a/src/main/java/com/android/tools/r8/CompatProguardCommandBuilder.java
+++ b/src/main/java/com/android/tools/r8/CompatProguardCommandBuilder.java
@@ -36,7 +36,7 @@
public CompatProguardCommandBuilder(boolean forceProguardCompatibility) {
super(forceProguardCompatibility);
setIgnoreDexInArchive(true);
- setEnableDesugaring(false);
+ setDisableDesugaring(true);
addProguardConfiguration(REFLECTIONS, EmbeddedOrigin.INSTANCE);
}
diff --git a/src/main/java/com/android/tools/r8/D8Command.java b/src/main/java/com/android/tools/r8/D8Command.java
index 82d02db..d01d8ca 100644
--- a/src/main/java/com/android/tools/r8/D8Command.java
+++ b/src/main/java/com/android/tools/r8/D8Command.java
@@ -136,7 +136,7 @@
getProgramConsumer(),
getMinApiLevel(),
getReporter(),
- getEnableDesugaring(),
+ !getDisableDesugaring(),
intermediate);
}
@@ -173,6 +173,7 @@
" --intermediate # Compile an intermediate result intended for later",
" # merging.",
" --file-per-class # Produce a separate dex file per input class",
+ " --no-desugaring # Force disable desugaring.",
" --main-dex-list <file> # List of classes to place in the primary dex file.",
" --version # Print the version of d8.",
" --help # Print this message."));
@@ -202,10 +203,28 @@
* @return D8 command builder with state set up according to parsed command line.
*/
public static Builder parse(String[] args, Origin origin) {
+ return parse(builder(), args, origin);
+ }
+
+ /**
+ * Parse the D8 command-line.
+ *
+ * <p>Parsing will set the supplied options or their default value if they have any.
+ *
+ * @param args Command-line arguments array.
+ * @param origin Origin description of the command-line arguments.
+ * @param handler Custom defined diagnostics handler.
+ * @return D8 command builder with state set up according to parsed command line.
+ */
+ public static Builder parse(String[] args, Origin origin, DiagnosticsHandler handler) {
+ return parse(builder(handler), args, origin);
+ }
+
+ private static Builder parse(Builder builder, String[] args, Origin origin) {
CompilationMode compilationMode = null;
Path outputPath = null;
OutputMode outputMode = null;
- Builder builder = builder();
+ boolean hasDefinedApiLevel = false;
try {
for (int i = 0; i < args.length; i++) {
String arg = args[i].trim();
@@ -249,9 +268,11 @@
} else if (arg.equals("--main-dex-list")) {
builder.addMainDexListFiles(Paths.get(args[++i]));
} else if (arg.equals("--min-api")) {
- builder.setMinApiLevel(Integer.valueOf(args[++i]));
+ hasDefinedApiLevel = parseMinApi(builder, args[++i], hasDefinedApiLevel, origin);
} else if (arg.equals("--intermediate")) {
builder.setIntermediate(true);
+ } else if (arg.equals("--no-desugaring")) {
+ builder.setDisableDesugaring(true);
} else {
if (arg.startsWith("--")) {
builder.getReporter().error(new StringDiagnostic("Unknown option: " + arg,
@@ -291,7 +312,6 @@
minApiLevel,
diagnosticsHandler,
enableDesugaring);
- assert programConsumer != null;
this.intermediate = intermediate;
}
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index 4d6ae7b..cb014c7 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -317,7 +317,7 @@
getMode(),
getMinApiLevel(),
reporter,
- getEnableDesugaring(),
+ !getDisableDesugaring(),
useTreeShaking,
useMinification,
forceProguardCompatibility,
@@ -367,6 +367,7 @@
CompilationMode mode = null;
OutputMode outputMode = null;
Path outputPath = null;
+ boolean hasDefinedApiLevel = false;
}
static final String USAGE_MESSAGE = String.join("\n", ImmutableList.of(
@@ -385,6 +386,7 @@
" --pg-map-output <file> # Output the resulting name and line mapping to <file>.",
" --no-tree-shaking # Force disable tree shaking of unreachable classes.",
" --no-minification # Force disable minification of names.",
+ " --no-desugaring # Force disable desugaring.",
" --main-dex-rules <file> # Proguard keep rules for classes to place in the",
" # primary dex file.",
" --main-dex-list <file> # List of classes to place in the primary dex file.",
@@ -431,7 +433,24 @@
* @return R8 command builder with state set up according to parsed command line.
*/
public static Builder parse(String[] args, Origin origin) {
- Builder builder = builder();
+ return parse(builder(), args, origin);
+ }
+
+ /**
+ * Parse the R8 command-line.
+ *
+ * Parsing will set the supplied options or their default value if they have any.
+ *
+ * @param args Command-line arguments array.
+ * @param origin Origin description of the command-line arguments.
+ * @param handler Custom defined diagnostics handler.
+ * @return R8 command builder with state set up according to parsed command line.
+ */
+ public static Builder parse(String[] args, Origin origin, DiagnosticsHandler handler) {
+ return parse(builder(handler), args, origin);
+ }
+
+ private static Builder parse(Builder builder, String[] args, Origin origin) {
ParseState state = new ParseState();
parse(args, origin, builder, state);
if (state.mode != null) {
@@ -495,11 +514,14 @@
} else if (arg.equals("--lib")) {
builder.addLibraryFiles(Paths.get(args[++i]));
} else if (arg.equals("--min-api")) {
- builder.setMinApiLevel(Integer.valueOf(args[++i]));
+ state.hasDefinedApiLevel =
+ parseMinApi(builder, args[++i], state.hasDefinedApiLevel, argsOrigin);
} else if (arg.equals("--no-tree-shaking")) {
builder.setDisableTreeShaking(true);
} else if (arg.equals("--no-minification")) {
builder.setDisableMinification(true);
+ } else if (arg.equals("--no-desugaring")) {
+ builder.setDisableDesugaring(true);
} else if (arg.equals("--main-dex-rules")) {
builder.addMainDexRulesFiles(Paths.get(args[++i]));
} else if (arg.equals("--main-dex-list")) {
@@ -543,8 +565,7 @@
return state;
}
- private
- R8Command(
+ private R8Command(
AndroidApp inputApp,
ProgramConsumer programConsumer,
ImmutableList<ProguardConfigurationRule> mainDexKeepRules,
diff --git a/src/main/java/com/android/tools/r8/benchmarks/FrameworkIncrementalDexingBenchmark.java b/src/main/java/com/android/tools/r8/benchmarks/FrameworkIncrementalDexingBenchmark.java
index bf01553..250ca3b 100644
--- a/src/main/java/com/android/tools/r8/benchmarks/FrameworkIncrementalDexingBenchmark.java
+++ b/src/main/java/com/android/tools/r8/benchmarks/FrameworkIncrementalDexingBenchmark.java
@@ -141,7 +141,7 @@
.setMode(CompilationMode.DEBUG)
.addProgramFiles(input)
.addLibraryFiles(LIB)
- .setEnableDesugaring(desugar)
+ .setDisableDesugaring(!desugar)
.setProgramConsumer(consumer)
.build(),
executor);
@@ -187,7 +187,7 @@
.addClasspathResourceProvider(provider)
.addLibraryFiles(LIB)
.setProgramConsumer(consumer)
- .setEnableDesugaring(desugar);
+ .setDisableDesugaring(!desugar);
for (int j = 0; j < count; j++) {
builder.addClassProgramData(provider.resources.get(descriptors.get(index + j)),
Origin.unknown());
@@ -206,7 +206,7 @@
.setIntermediate(false)
.setMode(CompilationMode.DEBUG)
.setProgramConsumer(DexIndexedConsumer.emptyConsumer())
- .setEnableDesugaring(false);
+ .setDisableDesugaring(true);
for (ProgramResource input : outputs.values()) {
try (InputStream inputStream = input.getByteStream()) {
builder.addDexProgramData(ByteStreams.toByteArray(inputStream), input.getOrigin());
diff --git a/src/main/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilder.java b/src/main/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilder.java
index df79ae4..899b187 100644
--- a/src/main/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilder.java
+++ b/src/main/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilder.java
@@ -161,7 +161,7 @@
.setProgramConsumer(consumer)
.setMode(noLocals ? CompilationMode.RELEASE : CompilationMode.DEBUG)
.setMinApiLevel(AndroidApiLevel.H_MR2.getLevel())
- .setEnableDesugaring(false);
+ .setDisableDesugaring(true);
try (InputStream stream = zipFile.getInputStream(classEntry)) {
builder.addClassProgramData(
ByteStreams.toByteArray(stream),
diff --git a/src/test/apiUsageSample/com/android/tools/apiusagesample/D8ApiUsageSample.java b/src/test/apiUsageSample/com/android/tools/apiusagesample/D8ApiUsageSample.java
index 471c6c2..ad31005 100644
--- a/src/test/apiUsageSample/com/android/tools/apiusagesample/D8ApiUsageSample.java
+++ b/src/test/apiUsageSample/com/android/tools/apiusagesample/D8ApiUsageSample.java
@@ -333,7 +333,7 @@
.addClasspathFiles(classpath)
.addLibraryFiles(libraries)
.addProgramFiles(inputs)
- .setEnableDesugaring(true)
+ .setDisableDesugaring(false)
.build());
} catch (CompilationFailedException e) {
throw new RuntimeException("Unexpected compilation exceptions", e);
@@ -367,7 +367,7 @@
D8Command.builder(handler)
.setMinApiLevel(minApiLevel)
.setProgramConsumer(new EnsureOutputConsumer())
- .setEnableDesugaring(false);
+ .setDisableDesugaring(true);
for (byte[] intermediate : intermediates) {
builder.addDexProgramData(intermediate, Origin.unknown());
}
diff --git a/src/test/java/com/android/tools/r8/D8CommandTest.java b/src/test/java/com/android/tools/r8/D8CommandTest.java
index 7583e97..76856a8 100644
--- a/src/test/java/com/android/tools/r8/D8CommandTest.java
+++ b/src/test/java/com/android/tools/r8/D8CommandTest.java
@@ -23,7 +23,6 @@
import java.util.List;
import java.util.Set;
import java.util.zip.ZipFile;
-import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
@@ -33,7 +32,6 @@
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
- @Ignore("Enable when deprecated API is removed")
@Test(expected = CompilationFailedException.class)
public void emptyBuilder() throws Throwable {
verifyEmptyCommand(D8Command.builder().build());
@@ -325,7 +323,41 @@
D8Command.builder().setProgramConsumer(new MultiTypeConsumer()).build();
}
+ @Test(expected = CompilationFailedException.class)
+ public void duplicateApiLevel() throws CompilationFailedException {
+ DiagnosticsChecker.checkErrorsContains(
+ "multiple --min-api", handler -> parse(handler, "--min-api", "19", "--min-api", "21"));
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void invalidApiLevel() throws CompilationFailedException {
+ DiagnosticsChecker.checkErrorsContains(
+ "Invalid argument to --min-api", handler -> parse(handler, "--min-api", "foobar"));
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void negativeApiLevel() throws CompilationFailedException {
+ DiagnosticsChecker.checkErrorsContains(
+ "Invalid argument to --min-api", handler -> parse(handler, "--min-api", "-21"));
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void zeroApiLevel() throws CompilationFailedException {
+ DiagnosticsChecker.checkErrorsContains(
+ "Invalid argument to --min-api", handler -> parse(handler, "--min-api", "0"));
+ }
+
+ @Test
+ public void disableDesugaring() throws CompilationFailedException {
+ assertFalse(parse("--no-desugaring").getEnableDesugaring());
+ }
+
private D8Command parse(String... args) throws CompilationFailedException {
return D8Command.parse(args, EmbeddedOrigin.INSTANCE).build();
}
+
+ private D8Command parse(DiagnosticsHandler handler, String... args)
+ throws CompilationFailedException {
+ return D8Command.parse(args, EmbeddedOrigin.INSTANCE, handler).build();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/DiagnosticsChecker.java b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
new file mode 100644
index 0000000..7cd32c7
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
@@ -0,0 +1,52 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8;
+
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.utils.ListUtils;
+import java.util.ArrayList;
+import java.util.List;
+
+// Helper to check that a particular error occurred.
+class DiagnosticsChecker implements DiagnosticsHandler {
+ public List<Diagnostic> errors = new ArrayList<>();
+ public List<Diagnostic> warnings = new ArrayList<>();
+ public List<Diagnostic> infos = new ArrayList<>();
+
+ @Override
+ public void error(Diagnostic error) {
+ errors.add(error);
+ }
+
+ @Override
+ public void warning(Diagnostic warning) {
+ warnings.add(warning);
+ }
+
+ @Override
+ public void info(Diagnostic info) {
+ infos.add(info);
+ }
+
+ public interface FailingRunner {
+ void run(DiagnosticsHandler handler) throws CompilationFailedException;
+ }
+
+ public static void checkErrorsContains(String snippet, FailingRunner runner)
+ throws CompilationFailedException {
+ DiagnosticsChecker handler = new DiagnosticsChecker();
+ try {
+ runner.run(handler);
+ } catch (CompilationFailedException e) {
+ assertTrue(
+ "Expected to find snippet '"
+ + snippet
+ + "' in error messages:\n"
+ + String.join("\n", ListUtils.map(handler.errors, Diagnostic::getDiagnosticMessage)),
+ handler.errors.stream().anyMatch(d -> d.getDiagnosticMessage().contains(snippet)));
+ throw e;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/R8CommandTest.java b/src/test/java/com/android/tools/r8/R8CommandTest.java
index f782a94..6f18657 100644
--- a/src/test/java/com/android/tools/r8/R8CommandTest.java
+++ b/src/test/java/com/android/tools/r8/R8CommandTest.java
@@ -18,7 +18,6 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
@@ -28,44 +27,6 @@
public class R8CommandTest {
- // Helper to check that a particular error occurred.
- static class DiagnosticsChecker implements DiagnosticsHandler {
- public List<Diagnostic> errors = new ArrayList<>();
- public List<Diagnostic> warnings = new ArrayList<>();
- public List<Diagnostic> infos = new ArrayList<>();
-
- @Override
- public void error(Diagnostic error) {
- errors.add(error);
- }
-
- @Override
- public void warning(Diagnostic warning) {
- warnings.add(warning);
- }
-
- @Override
- public void info(Diagnostic info) {
- infos.add(info);
- }
-
- public interface FailingRunner {
- void run(DiagnosticsHandler handler) throws CompilationFailedException;
- }
-
- public static void checkErrorsContains(String snippet, FailingRunner runner)
- throws CompilationFailedException {
- DiagnosticsChecker handler = new DiagnosticsChecker();
- try {
- runner.run(handler);
- } catch (CompilationFailedException e) {
- assertTrue(handler.errors.stream()
- .anyMatch(d -> d.getDiagnosticMessage().contains(snippet)));
- throw e;
- }
- }
- }
-
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
@@ -363,7 +324,41 @@
.build();
}
+ @Test(expected = CompilationFailedException.class)
+ public void duplicateApiLevel() throws CompilationFailedException {
+ DiagnosticsChecker.checkErrorsContains(
+ "multiple --min-api", handler -> parse(handler, "--min-api", "19", "--min-api", "21"));
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void invalidApiLevel() throws CompilationFailedException {
+ DiagnosticsChecker.checkErrorsContains(
+ "Invalid argument to --min-api", handler -> parse(handler, "--min-api", "foobar"));
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void negativeApiLevel() throws CompilationFailedException {
+ DiagnosticsChecker.checkErrorsContains(
+ "Invalid argument to --min-api", handler -> parse(handler, "--min-api", "-21"));
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void zeroApiLevel() throws CompilationFailedException {
+ DiagnosticsChecker.checkErrorsContains(
+ "Invalid argument to --min-api", handler -> parse(handler, "--min-api", "0"));
+ }
+
+ @Test
+ public void disableDesugaring() throws CompilationFailedException {
+ assertFalse(parse("--no-desugaring").getEnableDesugaring());
+ }
+
private R8Command parse(String... args) throws CompilationFailedException {
return R8Command.parse(args, EmbeddedOrigin.INSTANCE).build();
}
+
+ private R8Command parse(DiagnosticsHandler handler, String... args)
+ throws CompilationFailedException {
+ return R8Command.parse(args, EmbeddedOrigin.INSTANCE, handler).build();
+ }
}
