Implement -dontoptimize: disabling related flags.
Different optimizations have their own fine-grained flags with different
default values, and thus it is non-trivial to replace them with a single
flag. Rather, when converting proguard config to internal options,
-dontoptimize will be propagated by flipping optimization-related flags:
* Simple class merging
* Switch map collecting
* Inling
* Outlining
Bug: 36799686
Change-Id: Id634b5513a4d5297503ba3ba9e1b77a1c344bd68
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index 7bae1a1..4076a5a 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -440,6 +440,14 @@
assert !internal.debug;
internal.debug = getMode() == CompilationMode.DEBUG;
internal.minApiLevel = getMinApiLevel();
+ // -dontoptimize disables optimizations by flipping related flags.
+ if (!proguardConfiguration.isOptimizing()) {
+ internal.skipDebugLineNumberOpt = true;
+ internal.skipClassMerging = true;
+ internal.inlineAccessors = false;
+ internal.removeSwitchMaps = false;
+ internal.outline.enabled = false;
+ }
assert !internal.skipMinification;
internal.skipMinification = !useMinification() || !proguardConfiguration.isObfuscating();
assert internal.useTreeShaking;
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 7dca5bd..716db5e 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -133,9 +133,9 @@
}
/**
- * Create an IR converter for processing methods without full program optimization enabled.
+ * Create an IR converter for processing methods with full program optimization disabled.
*
- * The argument <code>enableDesugaring</code> if desugaing is enabled.
+ * The argument <code>enableDesugaring</code> if desugaring is enabled.
*/
public IRConverter(
DexApplication application,
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
index 87a36e6..87b425b 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
@@ -24,7 +24,7 @@
private String packagePrefix = "";
private boolean allowAccessModification = false;
private boolean ignoreWarnings = false;
- private boolean optimize = true;
+ private boolean optimizing = true;
private boolean obfuscating = true;
private boolean shrinking = true;
private boolean printUsage = false;
@@ -79,8 +79,8 @@
this.ignoreWarnings = ignoreWarnings;
}
- public void setOptimize(boolean optimize) {
- this.optimize = optimize;
+ public void setOptimizing(boolean optimizing) {
+ this.optimizing = optimizing;
}
public void setObfuscating(boolean obfuscate) {
@@ -172,6 +172,7 @@
packagePrefix,
allowAccessModification,
ignoreWarnings,
+ optimizing,
obfuscating,
shrinking,
printUsage,
@@ -200,6 +201,7 @@
private final String packagePrefix;
private final boolean allowAccessModification;
private final boolean ignoreWarnings;
+ private final boolean optimizing;
private final boolean obfuscating;
private final boolean shrinking;
private final boolean printUsage;
@@ -227,6 +229,7 @@
String packagePrefix,
boolean allowAccessModification,
boolean ignoreWarnings,
+ boolean optimizing,
boolean obfuscating,
boolean shrinking,
boolean printUsage,
@@ -252,6 +255,7 @@
this.packagePrefix = packagePrefix;
this.allowAccessModification = allowAccessModification;
this.ignoreWarnings = ignoreWarnings;
+ this.optimizing = optimizing;
this.obfuscating = obfuscating;
this.shrinking = shrinking;
this.printUsage = printUsage;
@@ -327,6 +331,10 @@
return ignoreWarnings;
}
+ public boolean isOptimizing() {
+ return optimizing;
+ }
+
public boolean isObfuscating() {
return obfuscating;
}
@@ -393,6 +401,7 @@
"" /* package prefix */,
false /* allowAccessModification */,
false /* ignoreWarnings */,
+ true /* optimizing */,
false /* obfuscating */,
false /* shrinking */,
false /* printUsage */,
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
index 9a90c68..bcbfc8b 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -168,8 +168,7 @@
ProguardWhyAreYouKeepingRule rule = parseWhyAreYouKeepingRule();
configurationBuilder.addRule(rule);
} else if (acceptString("dontoptimize")) {
- configurationBuilder.setOptimize(false);
- warnIgnoringOptions("dontoptimize");
+ configurationBuilder.setOptimizing(false);
} else if (acceptString("optimizationpasses")) {
skipWhitespace();
Integer expectedOptimizationPasses = acceptInteger();
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index a2a0446..36e19bf 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -45,9 +45,13 @@
public final int NOT_SPECIFIED = -1;
public boolean printTimes = false;
- // Skipping optimizations.
+
+ // Optimization-related flags. These should conform to -dontoptimize.
public boolean skipDebugLineNumberOpt = false;
public boolean skipClassMerging = true;
+ public boolean inlineAccessors = true;
+ public boolean removeSwitchMaps = true;
+ public final OutlineOptions outline = new OutlineOptions();
// Number of threads to use while processing the dex files.
public int numberOfThreads = NOT_SPECIFIED;
@@ -98,10 +102,7 @@
public Path printMainDexListFile;
public boolean ignoreMissingClasses = false;
public boolean skipMinification = false;
- public boolean inlineAccessors = true;
- public boolean removeSwitchMaps = true;
public boolean disableAssertions = true;
- public final OutlineOptions outline = new OutlineOptions();
public boolean debugKeepRules = false;
public final AttributeRemovalOptions attributeRemoval = new AttributeRemovalOptions();
public boolean allowParameterName = false;
diff --git a/src/test/examples/classmerging/keep-rules-dontoptimize.txt b/src/test/examples/classmerging/keep-rules-dontoptimize.txt
new file mode 100644
index 0000000..b2c1ff4
--- /dev/null
+++ b/src/test/examples/classmerging/keep-rules-dontoptimize.txt
@@ -0,0 +1,11 @@
+# Copyright (c) 2017, 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.
+
+# Keep the application entry point. Get rid of everything that is not
+# reachable from there.
+-keep public class classmerging.Test {
+ public static void main(...);
+}
+
+-dontoptimize
diff --git a/src/test/examples/classmerging/keep-rules.txt b/src/test/examples/classmerging/keep-rules.txt
index 58b262b..f83adbd 100644
--- a/src/test/examples/classmerging/keep-rules.txt
+++ b/src/test/examples/classmerging/keep-rules.txt
@@ -7,6 +7,3 @@
-keep public class classmerging.Test {
public static void main(...);
}
-
-# allow access modification to enable minification
--allowaccessmodification
diff --git a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
index 7bd18e0..a1692c8 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -1,16 +1,21 @@
package com.android.tools.r8.classmerging;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
import com.android.tools.r8.CompilationException;
import com.android.tools.r8.R8Command;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.shaking.ProguardRuleParserException;
import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.InternalOptions;
+import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.List;
import java.util.concurrent.ExecutionException;
-import org.junit.Assert;
-import org.junit.Before;
+import java.util.function.Consumer;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
@@ -21,54 +26,76 @@
.resolve("classmerging.jar");
private static final Path EXAMPLE_KEEP = Paths.get(ToolHelper.EXAMPLES_DIR)
.resolve("classmerging").resolve("keep-rules.txt");
+ private static final Path DONT_OPTIMIZE = Paths.get(ToolHelper.EXAMPLES_DIR)
+ .resolve("classmerging").resolve("keep-rules-dontoptimize.txt");
@Rule
public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
- @Before
- public void runR8()
+ private void configure(InternalOptions options) {
+ options.skipClassMerging = false;
+ }
+
+ private void runR8(Path proguardConfig, Consumer<InternalOptions> optionsConsumer)
throws IOException, ProguardRuleParserException, ExecutionException, CompilationException {
- // Disable access modification, as it otherwise is difficult to test visibility bridge methods.
ToolHelper.runR8(
R8Command.builder()
.setOutputPath(Paths.get(temp.getRoot().getCanonicalPath()))
.addProgramFiles(EXAMPLE_JAR)
- .addProguardConfigurationFiles(EXAMPLE_KEEP)
+ .addProguardConfigurationFiles(proguardConfig)
.setMinification(false)
- .build(), o -> {
- o.skipClassMerging = false;
- });
+ .build(),
+ optionsConsumer);
inspector = new DexInspector(
Paths.get(temp.getRoot().getCanonicalPath()).resolve("classes.dex"));
}
private DexInspector inspector;
+ private final List<String> CAN_BE_MERGED = ImmutableList.of(
+ "classmerging.GenericInterface",
+ "classmerging.GenericAbstractClass",
+ "classmerging.Outer$SuperClass",
+ "classmerging.SuperClass"
+ );
+
@Test
- public void testClassesHaveBeenMerged() throws IOException, ExecutionException {
+ public void testClassesHaveBeenMerged()
+ throws IOException, ExecutionException, CompilationException, ProguardRuleParserException {
+ runR8(EXAMPLE_KEEP, this::configure);
// GenericInterface should be merged into GenericInterfaceImpl.
- Assert.assertFalse(inspector.clazz("classmerging.GenericInterface").isPresent());
- Assert.assertTrue(inspector.clazz("classmerging.GenericInterfaceImpl").isPresent());
- Assert.assertFalse(inspector.clazz("classmerging.GenericAbstractClass").isPresent());
- Assert.assertTrue(inspector.clazz("classmerging.GenericInterfaceImpl").isPresent());
- Assert.assertFalse(inspector.clazz("classmerging.Outer$SuperClass").isPresent());
- Assert.assertTrue(inspector.clazz("classmerging.Outer$SubClass").isPresent());
- Assert.assertFalse(inspector.clazz("classmerging.SuperClass").isPresent());
- Assert.assertTrue(inspector.clazz("classmerging.SubClass").isPresent());
- }
-
-
- @Test
- public void testConflictWasDetected() throws IOException, ExecutionException {
- Assert.assertTrue(inspector.clazz("classmerging.ConflictingInterface").isPresent());
- Assert.assertTrue(inspector.clazz("classmerging.ConflictingInterfaceImpl").isPresent());
+ for (String candidate : CAN_BE_MERGED) {
+ assertFalse(inspector.clazz(candidate).isPresent());
+ }
+ assertTrue(inspector.clazz("classmerging.GenericInterfaceImpl").isPresent());
+ assertTrue(inspector.clazz("classmerging.GenericInterfaceImpl").isPresent());
+ assertTrue(inspector.clazz("classmerging.Outer$SubClass").isPresent());
+ assertTrue(inspector.clazz("classmerging.SubClass").isPresent());
}
@Test
- public void testSuperCallWasDetected() throws IOException, ExecutionException {
- Assert.assertTrue(inspector.clazz("classmerging.SuperClassWithReferencedMethod").isPresent());
- Assert
- .assertTrue(inspector.clazz("classmerging.SubClassThatReferencesSuperMethod").isPresent());
+ public void testClassesShouldNotMerged()
+ throws IOException, ExecutionException, CompilationException, ProguardRuleParserException {
+ runR8(DONT_OPTIMIZE, null);
+ for (String candidate : CAN_BE_MERGED) {
+ assertTrue(inspector.clazz(candidate).isPresent());
+ }
+ }
+
+ @Test
+ public void testConflictWasDetected()
+ throws IOException, ExecutionException, CompilationException, ProguardRuleParserException {
+ runR8(EXAMPLE_KEEP, this::configure);
+ assertTrue(inspector.clazz("classmerging.ConflictingInterface").isPresent());
+ assertTrue(inspector.clazz("classmerging.ConflictingInterfaceImpl").isPresent());
+ }
+
+ @Test
+ public void testSuperCallWasDetected()
+ throws IOException, ExecutionException, CompilationException, ProguardRuleParserException {
+ runR8(EXAMPLE_KEEP, this::configure);
+ assertTrue(inspector.clazz("classmerging.SuperClassWithReferencedMethod").isPresent());
+ assertTrue(inspector.clazz("classmerging.SubClassThatReferencesSuperMethod").isPresent());
}
}
