Version 1.5.28
Cherry pick: Don't inline methods that are kept.
CL: https://r8-review.googlesource.com/c/r8/+/37976
Cherry pick: Do not compute optimization info summary for kept methods
CL: https://r8-review.googlesource.com/c/r8/+/38168
Bug: 132056258, 119076934, 131619590
Change-Id: I48d531427e2c5a3f3db52f90fc2f33cb5271d371
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 3ca15f2..6847ccb 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.5.27";
+ public static final String LABEL = "1.5.28";
private Version() {
}
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 c6c1ba8..8b3a6c1 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
@@ -1114,22 +1114,6 @@
previous =
printMethod(code, "IR after idempotent function call canonicalization (SSA)", previous);
- codeRewriter.identifyReturnsArgument(method, code, feedback);
- if (options.enableInlining && inliner != null) {
- codeRewriter.identifyInvokeSemanticsForInlining(method, code, appView, feedback);
- }
-
- if (appView.enableWholeProgramOptimizations()) {
- // Track usage of parameters and compute their nullability and possibility of NPE.
- if (method.getOptimizationInfo().getNonNullParamOrThrow() == null) {
- computeNonNullParamHints(feedback, method, code);
- }
-
- computeDynamicReturnType(feedback, method, code);
- computeInitializedClassesOnNormalExit(feedback, method, code);
- computeMayHaveSideEffects(feedback, method, code);
- }
-
// Insert code to log arguments if requested.
if (options.methodMatchesLogArgumentsFilter(method)) {
codeRewriter.logArgumentTypes(method, code);
@@ -1138,19 +1122,32 @@
previous = printMethod(code, "IR after argument type logging (SSA)", previous);
- // Analysis must be done after method is rewritten by logArgumentTypes()
- codeRewriter.identifyClassInlinerEligibility(method, code, feedback);
-
- previous = printMethod(code, "IR after class inliner eligibility (SSA)", previous);
-
- if (method.isInstanceInitializer() || method.isClassInitializer()) {
- codeRewriter.identifyTrivialInitializer(method, code, feedback);
- }
- codeRewriter.identifyParameterUsages(method, code, feedback);
if (classStaticizer != null) {
classStaticizer.examineMethodCode(method, code);
}
+ // Compute optimization info summary for the current method unless it is pinned (in that case,
+ // we should not be making any assumptions about the behavior of the method).
+ if (appView.enableWholeProgramOptimizations()
+ && !appView.appInfo().withLiveness().isPinned(method.method)) {
+ codeRewriter.identifyClassInlinerEligibility(method, code, feedback);
+ codeRewriter.identifyParameterUsages(method, code, feedback);
+ codeRewriter.identifyReturnsArgument(method, code, feedback);
+ codeRewriter.identifyTrivialInitializer(method, code, feedback);
+
+ if (options.enableInlining && inliner != null) {
+ codeRewriter.identifyInvokeSemanticsForInlining(method, code, appView, feedback);
+ }
+
+ computeDynamicReturnType(feedback, method, code);
+ computeInitializedClassesOnNormalExit(feedback, method, code);
+ computeMayHaveSideEffects(feedback, method, code);
+ computeNonNullParamOrThrow(feedback, method, code);
+ }
+
+ previous =
+ printMethod(code, "IR after computation of optimization info summary (SSA)", previous);
+
if (options.canHaveNumberConversionRegisterAllocationBug()) {
codeRewriter.workaroundNumberConversionRegisterAllocationBug(code);
}
@@ -1170,8 +1167,13 @@
finalizeIR(method, code, feedback);
}
- private void computeNonNullParamHints(
- OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) {
+ // Track usage of parameters and compute their nullability and possibility of NPE.
+ private void computeNonNullParamOrThrow(
+ OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) {
+ if (method.getOptimizationInfo().getNonNullParamOrThrow() != null) {
+ return;
+ }
+
List<Value> arguments = code.collectArguments();
BitSet paramsCheckedForNull = new BitSet();
for (int index = 0; index < arguments.size(); index++) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 87c28bf..f214373 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -11,7 +11,6 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unreachable;
-import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexClass;
@@ -1169,22 +1168,23 @@
public void identifyTrivialInitializer(
DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
- boolean isInstanceInitializer = method.isInstanceInitializer();
- boolean isClassInitializer = method.isClassInitializer();
- assert isInstanceInitializer || isClassInitializer;
+ if (!method.isInstanceInitializer() && !method.isClassInitializer()) {
+ return;
+ }
+
if (method.accessFlags.isNative()) {
return;
}
- AppInfo appInfo = appView.appInfo();
- DexClass clazz = appInfo.definitionFor(method.method.holder);
+ DexClass clazz = appView.appInfo().definitionFor(method.method.holder);
if (clazz == null) {
return;
}
- feedback.setTrivialInitializer(method,
- isInstanceInitializer
- ? computeInstanceInitializerInfo(code, clazz, appInfo::definitionFor)
+ feedback.setTrivialInitializer(
+ method,
+ method.isInstanceInitializer()
+ ? computeInstanceInitializerInfo(code, clazz, appView::definitionFor)
: computeClassInitializerInfo(code, clazz));
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index bb002cf..ad7dfb8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -81,6 +81,7 @@
public boolean isBlackListed(DexMethod method) {
return blackList.contains(appView.graphLense().getOriginalMethodSignature(method))
+ || appView.appInfo().isPinned(method)
|| appView.appInfo().neverInline.contains(method)
|| TwrCloseResourceRewriter.isSynthesizedCloseResourceMethod(method, appView);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index 7d05a19..ab30104 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -690,7 +690,7 @@
public boolean mayPropagateValueFor(DexReference reference) {
assert checkIfObsolete();
- return !neverPropagateValue.contains(reference);
+ return !isPinned(reference) && !neverPropagateValue.contains(reference);
}
private boolean isLibraryOrClasspathField(DexField field) {
diff --git a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
index afb32f4..3dd711d 100644
--- a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
@@ -145,7 +145,11 @@
private static String getMethodLine(MethodReference method) {
// Should we encode modifiers in method references?
StringBuilder builder = new StringBuilder();
- builder.append(method.getMethodName()).append("(");
+ builder
+ .append(method.getReturnType() == null ? "void" : method.getReturnType().getTypeName())
+ .append(' ')
+ .append(method.getMethodName())
+ .append("(");
boolean first = true;
for (TypeReference parameterType : method.getFormalTypes()) {
if (!first) {
diff --git a/src/test/java/com/android/tools/r8/compatproguard/CompatKeepClassMemberNamesTestRunner.java b/src/test/java/com/android/tools/r8/compatproguard/CompatKeepClassMemberNamesTestRunner.java
index c0ca540..efd4467 100644
--- a/src/test/java/com/android/tools/r8/compatproguard/CompatKeepClassMemberNamesTestRunner.java
+++ b/src/test/java/com/android/tools/r8/compatproguard/CompatKeepClassMemberNamesTestRunner.java
@@ -130,7 +130,7 @@
RR extends TestRunResult<RR>,
T extends TestShrinkerBuilder<C, B, CR, RR, T>>
T buildWithMembersRule(TestShrinkerBuilder<C, B, CR, RR, T> builder) {
- return buildWithMembersRuleAllowRenaming(builder).noMinification();
+ return buildWithMembersRuleEnableMinification(builder).noMinification();
}
private <CR extends TestCompileResult<CR, RR>, RR extends TestRunResult<RR>>
@@ -159,6 +159,8 @@
}
@Test
+ @Ignore("b/119076934")
+ // TODO(b/119076934): Fails because the compat rule is applied regardless of mode, keeping Bar.
public void testWithMembersRuleFullR8() throws Exception {
// In full mode for R8 we do *not* expect a -keepclassmembers to cause retention of the class.
assertBarIsAbsent(buildWithMembersRule(testForR8(parameters.getBackend())).compile());
@@ -172,7 +174,7 @@
CR extends TestCompileResult<CR, RR>,
RR extends TestRunResult<RR>,
T extends TestShrinkerBuilder<C, B, CR, RR, T>>
- T buildWithMembersRuleAllowRenaming(TestShrinkerBuilder<C, B, CR, RR, T> builder) {
+ T buildWithMembersRuleEnableMinification(TestShrinkerBuilder<C, B, CR, RR, T> builder) {
return builder
.addProgramClasses(CLASSES)
.addKeepMainRule(MAIN_CLASS)
@@ -180,7 +182,7 @@
}
private <CR extends TestCompileResult<CR, RR>, RR extends TestRunResult<RR>>
- void assertMembersRuleAllowRenamingCompatResult(CR compileResult) throws Exception {
+ void assertMembersRuleEnableMinificationCompatResult(CR compileResult) throws Exception {
compileResult
.inspect(
inspector -> {
@@ -207,23 +209,23 @@
}
@Test
- public void testWithMembersRuleAllowRenamingPG() throws Exception {
- assertMembersRuleAllowRenamingCompatResult(
- buildWithMembersRuleAllowRenaming(testForProguard()).compile());
+ public void testWithMembersRuleEnableMinificationPG() throws Exception {
+ assertMembersRuleEnableMinificationCompatResult(
+ buildWithMembersRuleEnableMinification(testForProguard()).compile());
+ }
+
+ @Test
+ public void testWithMembersRuleEnableMinificationCompatR8() throws Exception {
+ assertMembersRuleEnableMinificationCompatResult(
+ buildWithMembersRuleEnableMinification(testForR8Compat(parameters.getBackend())).compile());
}
@Test
@Ignore("b/119076934")
- // TODO(b/119076934): This fails the Bar.instance() is not inlined check.
- public void testWithMembersRuleAllowRenamingCompatR8() throws Exception {
- assertMembersRuleAllowRenamingCompatResult(
- buildWithMembersRuleAllowRenaming(testForR8Compat(parameters.getBackend())).compile());
- }
-
- @Test
- public void testWithMembersRuleAllowRenamingFullR8() throws Exception {
+ // TODO(b/119076934): Fails because the compat rule is applied regardless of mode, keeping Bar.
+ public void testWithMembersRuleEnableMinificationFullR8() throws Exception {
assertBarIsAbsent(
- buildWithMembersRuleAllowRenaming(testForR8(parameters.getBackend())).compile());
+ buildWithMembersRuleEnableMinification(testForR8(parameters.getBackend())).compile());
}
// Tests for "-keepclassmembers class Bar", i.e, with no members specified.
@@ -268,7 +270,7 @@
RR extends TestRunResult<RR>,
T extends TestShrinkerBuilder<C, B, CR, RR, T>>
T buildWithMemberNamesRule(TestShrinkerBuilder<C, B, CR, RR, T> builder) {
- return buildWithMemberNamesRuleAllowRenaming(builder).noMinification();
+ return buildWithMemberNamesRuleEnableMinification(builder).noMinification();
}
private <CR extends TestCompileResult<CR, RR>, RR extends TestRunResult<RR>>
@@ -319,7 +321,7 @@
CR extends TestCompileResult<CR, RR>,
RR extends TestRunResult<RR>,
T extends TestShrinkerBuilder<C, B, CR, RR, T>>
- T buildWithMemberNamesRuleAllowRenaming(TestShrinkerBuilder<C, B, CR, RR, T> builder) {
+ T buildWithMemberNamesRuleEnableMinification(TestShrinkerBuilder<C, B, CR, RR, T> builder) {
return builder
.addProgramClasses(CLASSES)
.addKeepMainRule(MAIN_CLASS)
@@ -327,7 +329,7 @@
}
private <CR extends TestCompileResult<CR, RR>, RR extends TestRunResult<RR>>
- void assertMemberNamesRuleAllowRenamingCompatResult(CR compileResult) throws Exception {
+ void assertMemberNamesRuleEnableMinificationCompatResult(CR compileResult) throws Exception {
compileResult
.inspect(
inspector -> {
@@ -350,22 +352,23 @@
}
@Test
- public void testWithMemberNamesRuleAllowRenamingPG() throws Exception {
- assertMemberNamesRuleAllowRenamingCompatResult(
- buildWithMemberNamesRuleAllowRenaming(testForProguard()).compile());
+ public void testWithMemberNamesRuleEnableMinificationPG() throws Exception {
+ assertMemberNamesRuleEnableMinificationCompatResult(
+ buildWithMemberNamesRuleEnableMinification(testForProguard()).compile());
}
@Test
@Ignore("b/119076934")
// TODO(b/119076934): This fails the Bar.instance() is not inlined check.
- public void testWithMemberNamesRuleAllowRenamingCompatR8() throws Exception {
- assertMemberNamesRuleAllowRenamingCompatResult(
- buildWithMemberNamesRuleAllowRenaming(testForR8Compat(parameters.getBackend())).compile());
+ public void testWithMemberNamesRuleEnableMinificationCompatR8() throws Exception {
+ assertMemberNamesRuleEnableMinificationCompatResult(
+ buildWithMemberNamesRuleEnableMinification(testForR8Compat(parameters.getBackend()))
+ .compile());
}
@Test
- public void testWithMemberNamesRuleAllowRenamingFullR8() throws Exception {
+ public void testWithMemberNamesRuleEnableMinificationFullR8() throws Exception {
assertBarIsAbsent(
- buildWithMemberNamesRuleAllowRenaming(testForR8(parameters.getBackend())).compile());
+ buildWithMemberNamesRuleEnableMinification(testForR8(parameters.getBackend())).compile());
}
}
diff --git a/src/test/java/com/android/tools/r8/debug/DebugInfoWhenInliningTest.java b/src/test/java/com/android/tools/r8/debug/DebugInfoWhenInliningTest.java
index 635f77e..319327d 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugInfoWhenInliningTest.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugInfoWhenInliningTest.java
@@ -36,6 +36,7 @@
DEX_FORCE_JUMBO
};
+ private static final String CLASS_NAME = "Inlining1";
private static final String SOURCE_FILE = "Inlining1.java";
private DebugTestConfig makeConfig(
@@ -50,7 +51,12 @@
R8Command.builder()
.addProgramFiles(DEBUGGEE_JAR)
.setMode(CompilationMode.RELEASE)
- .setDisableTreeShaking(true)
+ .addProguardConfiguration(
+ ImmutableList.of(
+ "-keep class "
+ + CLASS_NAME
+ + " { public static void main(java.lang.String[]); }"),
+ Origin.unknown())
.setDisableMinification(true)
.addProguardConfiguration(
ImmutableList.of("-keepattributes SourceFile,LineNumberTable"), Origin.unknown());
@@ -175,11 +181,10 @@
private void testEachLine(
DebugTestConfig config, int[] lineNumbers, List<List<SignatureAndLine>> inlineFramesList)
throws Throwable {
- final String className = "Inlining1";
final String mainSignature = "([Ljava/lang/String;)V";
List<Command> commands = new ArrayList<Command>();
- commands.add(breakpoint(className, "main", mainSignature));
+ commands.add(breakpoint(CLASS_NAME, "main", mainSignature));
commands.add(run());
boolean first = true;
assert inlineFramesList == null || inlineFramesList.size() == lineNumbers.length;
@@ -189,13 +194,13 @@
} else {
commands.add(stepOver());
}
- commands.add(checkMethod(className, "main", mainSignature));
+ commands.add(checkMethod(CLASS_NAME, "main", mainSignature));
commands.add(checkLine(SOURCE_FILE, lineNumbers[idx]));
if (inlineFramesList != null) {
commands.add(checkInlineFrames(inlineFramesList.get(idx)));
}
}
commands.add(run());
- runDebugTest(config, className, commands);
+ runDebugTest(config, CLASS_NAME, commands);
}
}
diff --git a/src/test/java/com/android/tools/r8/debuginfo/InliningWithoutPositionsTestRunner.java b/src/test/java/com/android/tools/r8/debuginfo/InliningWithoutPositionsTestRunner.java
index 0588b33..3e60ef1 100644
--- a/src/test/java/com/android/tools/r8/debuginfo/InliningWithoutPositionsTestRunner.java
+++ b/src/test/java/com/android/tools/r8/debuginfo/InliningWithoutPositionsTestRunner.java
@@ -48,6 +48,7 @@
private static final String TEST_CLASS = "InliningWithoutPositionsTestSource";
private static final String TEST_PACKAGE = "com.android.tools.r8.debuginfo";
+ private static final String MAIN_CLASS = TEST_PACKAGE + "." + TEST_CLASS;
@ClassRule public static TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
@@ -123,7 +124,10 @@
.setOutput(outputPath, OutputMode.ClassFile);
}
builder
- .setDisableTreeShaking(true)
+ .addProguardConfiguration(
+ ImmutableList.of(
+ "-keep class " + MAIN_CLASS + " { public static void main(java.lang.String[]); }"),
+ Origin.unknown())
.setDisableMinification(true)
.addProguardConfiguration(
ImmutableList.of("-keepattributes SourceFile,LineNumberTable"), Origin.unknown());
@@ -134,11 +138,11 @@
if (backend == Backend.DEX) {
ArtCommandBuilder artCommandBuilder = new ArtCommandBuilder();
artCommandBuilder.appendClasspath(outputPath.resolve("classes.dex").toString());
- artCommandBuilder.setMainClass(TEST_PACKAGE + "." + TEST_CLASS);
+ artCommandBuilder.setMainClass(MAIN_CLASS);
result = ToolHelper.runArtRaw(artCommandBuilder);
} else {
- result = ToolHelper.runJava(outputPath, TEST_PACKAGE + "." + TEST_CLASS);
+ result = ToolHelper.runJava(outputPath, MAIN_CLASS);
}
assertNotEquals(result.exitCode, 0);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/OptimizationSummaryForKeptMethodTest.java b/src/test/java/com/android/tools/r8/ir/optimize/OptimizationSummaryForKeptMethodTest.java
new file mode 100644
index 0000000..7dc327c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/OptimizationSummaryForKeptMethodTest.java
@@ -0,0 +1,94 @@
+// Copyright (c) 2019, 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.ir.optimize;
+
+import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethod;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertNotEquals;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class OptimizationSummaryForKeptMethodTest extends TestBase {
+
+ private final boolean enableExtraKeepRule;
+ private final TestParameters parameters;
+
+ @Parameters(name = "{1}, enable extra keep rule: {0}")
+ public static List<Object[]> params() {
+ return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build());
+ }
+
+ public OptimizationSummaryForKeptMethodTest(
+ boolean enableExtraKeepRule, TestParameters parameters) {
+ this.enableExtraKeepRule = enableExtraKeepRule;
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(OptimizationSummaryForKeptMethodTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addKeepClassAndMembersRules(
+ (Class<?>[]) (enableExtraKeepRule ? new Class<?>[] {KeptClass.class} : new Class<?>[0]))
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(this::verifyOutput);
+ }
+
+ private void verifyOutput(CodeInspector inspector) {
+ ClassSubject testClassSubject = inspector.clazz(TestClass.class);
+ assertThat(testClassSubject, isPresent());
+
+ MethodSubject mainMethodSubject = testClassSubject.mainMethod();
+ assertThat(mainMethodSubject, isPresent());
+
+ // Main method invokes noNormalExits() since it is not allowed to be inlined.
+ assertThat(
+ mainMethodSubject,
+ invokesMethod(inspector.clazz(KeptClass.class).uniqueMethodWithName("noNormalExits")));
+
+ // The fact that noNormalExits() never returns normally has only been exploited if it is not
+ // kept.
+ assertNotEquals(
+ enableExtraKeepRule,
+ mainMethodSubject.streamInstructions().anyMatch(InstructionSubject::isThrow));
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ try {
+ KeptClass.noNormalExits();
+ System.out.println(" world!");
+ } catch (Exception e) {
+ System.out.println("Caught " + e.getClass().getName());
+ }
+ }
+ }
+
+ static class KeptClass {
+
+ @NeverInline
+ public static void noNormalExits() {
+ throw new RuntimeException();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/instanceofremoval/InstanceOfRemovalTest.java b/src/test/java/com/android/tools/r8/ir/optimize/instanceofremoval/InstanceOfRemovalTest.java
index 1e33d3c..efd4766 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/instanceofremoval/InstanceOfRemovalTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/instanceofremoval/InstanceOfRemovalTest.java
@@ -152,7 +152,6 @@
testForR8(backend)
.addProgramClasses(A.class, B.class, TestClass.class)
.addKeepMainRule(TestClass.class)
- .addKeepAllClassesRule()
.enableInliningAnnotations()
.run(TestClass.class)
.assertSuccessWithOutput(expected)
diff --git a/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java b/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
index 3a5ffdd..9df1d87 100644
--- a/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
+++ b/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.neverreturnsnormally;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -11,20 +12,24 @@
import com.android.tools.r8.ForceInline;
import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.cf.code.CfStackInstruction.Opcode;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.codeinspector.CfInstructionSubject;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.DexInstructionSubject;
+import com.android.tools.r8.utils.codeinspector.FoundMethodSubject;
import com.android.tools.r8.utils.codeinspector.InstructionSubject;
import com.android.tools.r8.utils.codeinspector.InstructionSubject.JumboStringMode;
import com.android.tools.r8.utils.codeinspector.InvokeInstructionSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
import com.google.common.collect.ImmutableList;
import java.util.Iterator;
+import java.util.List;
import java.util.function.BiConsumer;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -34,15 +39,17 @@
@RunWith(Parameterized.class)
public class NeverReturnsNormallyTest extends TestBase {
- private Backend backend;
+ private boolean keepOuterTrivial;
+ private TestParameters parameters;
- @Parameters(name = "Backend: {0}")
- public static Backend[] data() {
- return ToolHelper.getBackends();
+ @Parameters(name = "{1}, keep outerTrivial: {0}")
+ public static List<Object[]> data() {
+ return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build());
}
- public NeverReturnsNormallyTest(Backend backend) {
- this.backend = backend;
+ public NeverReturnsNormallyTest(boolean keepOuterTrivial, TestParameters parameters) {
+ this.keepOuterTrivial = keepOuterTrivial;
+ this.parameters = parameters;
}
private void runTest(
@@ -51,75 +58,90 @@
R8Command.Builder builder = R8Command.builder();
builder.addProgramFiles(ToolHelper.getClassFileForTestClass(ForceInline.class));
builder.addProgramFiles(ToolHelper.getClassFileForTestClass(TestClass.class));
- builder.setProgramConsumer(emptyConsumer(backend));
- builder.addLibraryFiles(runtimeJar(backend));
+ builder.setProgramConsumer(emptyConsumer(parameters.getBackend()));
+ builder.addLibraryFiles(runtimeJar(parameters.getBackend()));
builder.setMode(mode);
builder.addProguardConfiguration(
ImmutableList.of(
"-forceinline class * { @com.android.tools.r8.ForceInline *; }",
- "-keep class " + TestClass.class.getCanonicalName() + "{",
+ "-keep class " + TestClass.class.getTypeName() + " {",
" public static void main(java.lang.String[]);",
" *** test*(...);",
- " *** throwToBeInlined(...);",
- " *** outerTrivial(...);",
- "}",
- "",
- "-checkdiscard class " + TestClass.class.getCanonicalName() + "{",
- " *** assertRemoved(...);",
"}",
"",
"-dontobfuscate",
"-allowaccessmodification"),
Origin.unknown());
+ if (keepOuterTrivial) {
+ builder.addProguardConfiguration(
+ ImmutableList.of(
+ "-keep class " + TestClass.class.getTypeName() + " {",
+ " *** outerTrivial(...);",
+ "}"),
+ Origin.unknown());
+ } else {
+ builder.addProguardConfiguration(
+ ImmutableList.of(
+ "-checkdiscard class " + TestClass.class.getTypeName() + " {",
+ " *** assertRemoved(...);",
+ "}"),
+ Origin.unknown());
+ }
ToolHelper.allowTestProguardOptions(builder);
AndroidApp app =
ToolHelper.runR8(builder.build(), opts -> opts.enableClassInlining = enableClassInliner);
inspection.accept(new CodeInspector(app), mode);
- if (backend == Backend.DEX) {
+ if (parameters.isDexRuntime()) {
// Run on Art to check generated code against verifier.
runOnArt(app, TestClass.class);
} else {
- assert backend == Backend.CF;
+ assert parameters.isCfRuntime();
runOnJava(app, TestClass.class);
}
}
private void validate(CodeInspector inspector, CompilationMode mode) {
- assert (backend == Backend.DEX || backend == Backend.CF);
+ assert parameters.isCfRuntime() || parameters.isDexRuntime();
ClassSubject clazz = inspector.clazz(TestClass.class);
assertTrue(clazz.isPresent());
// All calls to 'assertRemoved' are to be removed.
- clazz.forAllMethods(method -> {
- Iterator<InstructionSubject> instructions =
- method.iterateInstructions(InstructionSubject::isInvoke);
- while (instructions.hasNext()) {
- InvokeInstructionSubject invoke = (InvokeInstructionSubject) instructions.next();
- assertFalse(invoke.invokedMethod().name.toString().equals("assertRemoved"));
- }
- });
+ int numberOfCallsToAssertRemoved = 0;
+ for (FoundMethodSubject method : clazz.allMethods()) {
+ numberOfCallsToAssertRemoved +=
+ method
+ .streamInstructions()
+ .filter(InstructionSubject::isInvoke)
+ .filter(invoke -> invoke.getMethod().name.toString().equals("assertRemoved"))
+ .count();
+ }
+ assertEquals(keepOuterTrivial ? 2 : 0, numberOfCallsToAssertRemoved);
// Check the instruction used for testInlinedIntoVoidMethod
MethodSubject methodThrowToBeInlined =
clazz.method("int", "throwToBeInlined", ImmutableList.of());
- assertTrue(methodThrowToBeInlined.isPresent());
- Iterator<InstructionSubject> instructions = methodThrowToBeInlined.iterateInstructions();
- // Call, followed by throw null.
- InstructionSubject insn = nextInstructionSkippingCfPositionAndLabel(instructions);
- assertTrue(insn != null && insn.isConstString(JumboStringMode.ALLOW));
- insn = nextInstruction(instructions);
- assertTrue(insn.isInvoke());
- assertTrue(((InvokeInstructionSubject) insn)
- .invokedMethod().name.toString().equals("throwNpe"));
- verifyTrailingPattern(instructions);
+ boolean expectedToBePresent = mode == CompilationMode.DEBUG;
+ assertEquals(expectedToBePresent, methodThrowToBeInlined.isPresent());
+ if (expectedToBePresent) {
+ Iterator<InstructionSubject> instructions = methodThrowToBeInlined.iterateInstructions();
+ // Call, followed by throw null.
+ InstructionSubject insn = nextInstructionSkippingCfPositionAndLabel(instructions);
+ assertTrue(insn != null && insn.isConstString(JumboStringMode.ALLOW));
+ insn = nextInstruction(instructions);
+ assertTrue(insn.isInvoke());
+ assertTrue(((InvokeInstructionSubject) insn)
+ .invokedMethod().name.toString().equals("throwNpe"));
+ verifyTrailingPattern(instructions);
+ }
// Check the instruction used for testInlinedIntoVoidMethod
MethodSubject methodTestInlinedIntoVoidMethod =
clazz.method("void", "testInlinedIntoVoidMethod", ImmutableList.of());
assertTrue(methodTestInlinedIntoVoidMethod.isPresent());
- instructions = methodTestInlinedIntoVoidMethod.iterateInstructions();
- insn = nextInstructionSkippingCfPositionAndLabel(instructions);
+ Iterator<InstructionSubject> instructions =
+ methodTestInlinedIntoVoidMethod.iterateInstructions();
+ InstructionSubject insn = nextInstructionSkippingCfPositionAndLabel(instructions);
if (mode == CompilationMode.DEBUG) {
// Not inlined call to throwToBeInlined.
assertTrue(insn.isInvoke());
@@ -136,16 +158,17 @@
verifyTrailingPattern(instructions);
// Check the instruction used for outerTrivial
- MethodSubject methodOuterTrivial =
- clazz.method("int", "outerTrivial", ImmutableList.of());
- assertTrue(methodOuterTrivial.isPresent());
- instructions = methodOuterTrivial.iterateInstructions();
- // Call, followed by [nop, goto]
- insn = nextInstructionSkippingCfPositionAndLabel(instructions);
- assertTrue(insn.isInvoke());
- assertTrue(((InvokeInstructionSubject) insn)
- .invokedMethod().name.toString().equals("innerNotReachable"));
- verifyTrailingPattern(instructions);
+ MethodSubject methodOuterTrivial = clazz.method("int", "outerTrivial", ImmutableList.of());
+ assertEquals(keepOuterTrivial || mode == CompilationMode.DEBUG, methodOuterTrivial.isPresent());
+
+ if (methodOuterTrivial.isPresent()) {
+ instructions = methodOuterTrivial.iterateInstructions();
+ // Call, followed by [nop, goto]
+ insn = nextInstructionSkippingCfPositionAndLabel(instructions);
+ assertTrue(insn.isInvoke());
+ assertEquals("innerNotReachable", insn.getMethod().name.toString());
+ verifyTrailingPattern(instructions);
+ }
}
private InstructionSubject nextInstruction(Iterator<InstructionSubject> instructions) {
@@ -171,7 +194,7 @@
private void verifyTrailingPattern(Iterator<InstructionSubject> instructions) {
InstructionSubject insn = nextInstruction(instructions);
- if (backend == Backend.DEX) {
+ if (parameters.isDexRuntime()) {
assertTrue(
insn instanceof DexInstructionSubject && ((DexInstructionSubject) insn).isConst4());
} else {
diff --git a/src/test/java/com/android/tools/r8/resolution/SingleStaticTargetLookupTest.java b/src/test/java/com/android/tools/r8/resolution/SingleStaticTargetLookupTest.java
new file mode 100644
index 0000000..3686b19
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/resolution/SingleStaticTargetLookupTest.java
@@ -0,0 +1,15 @@
+// Copyright (c) 2019, 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.resolution;
+
+class SingleStaticTargetLookupTest {
+
+ public static String staticFoo() {
+ return "foo";
+ }
+
+ public static void main(String[] args) {
+ System.out.println(staticFoo());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/resolution/SingleStaticTargetLookupTestRunner.java b/src/test/java/com/android/tools/r8/resolution/SingleStaticTargetLookupTestRunner.java
new file mode 100644
index 0000000..fef40af
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/resolution/SingleStaticTargetLookupTestRunner.java
@@ -0,0 +1,91 @@
+// Copyright (c) 2019, 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.resolution;
+
+import static com.android.tools.r8.references.Reference.methodFromMethod;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.BaseCompilerCommand;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestCompileResult;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.TestRunResult;
+import com.android.tools.r8.TestShrinkerBuilder;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class SingleStaticTargetLookupTestRunner extends TestBase {
+
+ static final String EXPECTED = StringUtils.lines("foo");
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withCfRuntimes().build();
+ }
+
+ public SingleStaticTargetLookupTestRunner(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ public <
+ C extends BaseCompilerCommand,
+ B extends BaseCompilerCommand.Builder<C, B>,
+ CR extends TestCompileResult<CR, RR>,
+ RR extends TestRunResult<RR>,
+ T extends TestShrinkerBuilder<C, B, CR, RR, T>>
+ void test(boolean keepFoo, TestShrinkerBuilder<C, B, CR, RR, T> builder) throws Exception {
+ builder
+ .addProgramClassesAndInnerClasses(SingleStaticTargetLookupTest.class)
+ .addKeepMainRule(SingleStaticTargetLookupTest.class)
+ .apply(
+ b -> {
+ if (keepFoo) {
+ b.addKeepMethodRules(
+ methodFromMethod(SingleStaticTargetLookupTest.class.getMethod("staticFoo")));
+ }
+ })
+ .run(parameters.getRuntime(), SingleStaticTargetLookupTest.class)
+ .assertSuccessWithOutput(EXPECTED)
+ .inspect(
+ inspector -> {
+ ClassSubject clazz = inspector.clazz(SingleStaticTargetLookupTest.class);
+ MethodSubject main = clazz.uniqueMethodWithName("main");
+ MethodSubject staticFoo = clazz.uniqueMethodWithName("staticFoo");
+ assertThat(clazz, isPresent());
+ assertThat(main, isPresent());
+ assertEquals(keepFoo, staticFoo.isPresent());
+ assertEquals(keepFoo, main.streamInstructions().anyMatch(i -> i.isInvokeStatic()));
+ });
+ }
+
+ @Test
+ public void testInlinedPG() throws Exception {
+ test(false, testForProguard());
+ }
+
+ @Test
+ public void testKeptPG() throws Exception {
+ test(true, testForProguard());
+ }
+
+ @Test
+ public void testInlinedR8() throws Exception {
+ test(false, testForR8(parameters.getBackend()));
+ }
+
+ @Test
+ public void testKeptR8() throws Exception {
+ test(true, testForR8(parameters.getBackend()));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/whyareyoukeeping/WhyAreYouKeepingTest.java b/src/test/java/com/android/tools/r8/shaking/whyareyoukeeping/WhyAreYouKeepingTest.java
index 87f73bd..d78542e 100644
--- a/src/test/java/com/android/tools/r8/shaking/whyareyoukeeping/WhyAreYouKeepingTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/whyareyoukeeping/WhyAreYouKeepingTest.java
@@ -46,7 +46,7 @@
StringUtils.lines(
"com.android.tools.r8.shaking.whyareyoukeeping.A",
"|- is referenced in keep rule:",
- "| -keep class com.android.tools.r8.shaking.whyareyoukeeping.A { foo(); }");
+ "| -keep class com.android.tools.r8.shaking.whyareyoukeeping.A { void foo(); }");
// TODO(b/120959039): This should be "- is invoked from:\n com.android.....A.bar()" etc.
public static final String expectedPathToBaz =
@@ -55,7 +55,7 @@
"|- is reachable from:",
"| com.android.tools.r8.shaking.whyareyoukeeping.A",
"|- is referenced in keep rule:",
- "| -keep class com.android.tools.r8.shaking.whyareyoukeeping.A { foo(); }");
+ "| -keep class com.android.tools.r8.shaking.whyareyoukeeping.A { void foo(); }");
@Parameters(name = "{0}")
public static Backend[] parameters() {
