Don't inline methods that are kept.
Bug: 132056258
Bug: 119076934
Change-Id: I238c708aa3e95ba623ed47b604465c13c9a25db4
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/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..d6ca6e1 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;
@@ -60,7 +61,6 @@
"-keep class " + TestClass.class.getCanonicalName() + "{",
" public static void main(java.lang.String[]);",
" *** test*(...);",
- " *** throwToBeInlined(...);",
" *** outerTrivial(...);",
"}",
"",
@@ -103,23 +103,27 @@
// 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());
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() {
