Bottom-up propagation of specific assume* rules.
Bug: 133208961
Change-Id: I11126424628552358db523a144c285c80625a0ef
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
index 66e9d9a..bef1b15 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.BottomUpClassHierarchyTraversal;
import com.android.tools.r8.graph.DexAnnotation;
import com.android.tools.r8.graph.DexAnnotationSet;
import com.android.tools.r8.graph.DexApplication;
@@ -276,6 +277,10 @@
} finally {
application.timing.end();
}
+ if (!noSideEffects.isEmpty() || !assumedValues.isEmpty()) {
+ BottomUpClassHierarchyTraversal.forAllClasses(appView)
+ .visit(appView.appInfo().classes(), this::propagateAssumeRules);
+ }
return new RootSet(
noShrinking,
noOptimization,
@@ -299,6 +304,47 @@
ifRules);
}
+ private void propagateAssumeRules(DexClass clazz) {
+ Set<DexType> subTypes = appView.appInfo().allImmediateSubtypes(clazz.type);
+ if (subTypes.isEmpty()) {
+ return;
+ }
+ for (DexEncodedMethod encodedMethod : clazz.virtualMethods()) {
+ propagateAssumeRules(encodedMethod.method, subTypes, noSideEffects);
+ propagateAssumeRules(encodedMethod.method, subTypes, assumedValues);
+ }
+ }
+
+ private void propagateAssumeRules(
+ DexMethod reference,
+ Set<DexType> subTypes,
+ Map<DexReference, ProguardMemberRule> assumeRulePool) {
+ ProguardMemberRule ruleToBePropagated = null;
+ for (DexType subType : subTypes) {
+ DexMethod referenceInSubType =
+ appView.dexItemFactory().createMethod(subType, reference.proto, reference.name);
+ ProguardMemberRule ruleInSubType = assumeRulePool.get(referenceInSubType);
+ // We are looking for the greatest lower bound of assume rules from all sub types.
+ // If any subtype doesn't have a matching assume rule, the lower bound is literally nothing.
+ if (ruleInSubType == null) {
+ ruleToBePropagated = null;
+ break;
+ }
+ if (ruleToBePropagated == null) {
+ ruleToBePropagated = ruleInSubType;
+ } else {
+ // TODO(b/133208961): Introduce comparison/meet of assume rules.
+ if (!ruleToBePropagated.equals(ruleInSubType)) {
+ ruleToBePropagated = null;
+ break;
+ }
+ }
+ }
+ if (ruleToBePropagated != null) {
+ assumeRulePool.put(reference, ruleToBePropagated);
+ }
+ }
+
IfRuleEvaluator getIfRuleEvaluator(
Set<DexEncodedField> liveFields,
Set<DexEncodedMethod> liveMethods,
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationTest.java
new file mode 100644
index 0000000..6900672
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationTest.java
@@ -0,0 +1,296 @@
+// 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.shaking.assumenosideeffects;
+
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.utils.StringUtils;
+import com.google.common.collect.ImmutableList;
+import java.util.Collection;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+interface Itf {
+ void info(String message);
+ void debug(String message);
+ void verbose(String message);
+}
+
+class Base1 implements Itf {
+ @Override
+ public void info(String message) {
+ System.out.println("[Base1, info]: " + message);
+ }
+
+ @Override
+ public void debug(String message) {
+ System.out.println("[Base1, debug]: " + message);
+ }
+
+ @Override
+ public void verbose(String message) {
+ System.out.println("[Base1, verbose]: " + message);
+ }
+}
+
+class Sub1 extends Base1 {
+ @Override
+ public void info(String message) {
+ System.out.println("[Sub1, info]: " + message);
+ }
+
+ @Override
+ public void verbose(String message) {
+ System.out.println("[Sub1, verbose]: " + message);
+ }
+}
+
+class Sub2 extends Base1 {
+ @Override
+ public void info(String message) {
+ System.out.println("[Sub2, info]: " + message);
+ }
+
+ @Override
+ public void verbose(String message) {
+ System.out.println("[Sub2, verbose]: " + message);
+ }
+}
+
+class Base2 implements Itf {
+ @Override
+ public void info(String message) {
+ System.out.println("[Base2, info]: " + message);
+ }
+
+ @Override
+ public void debug(String message) {
+ System.out.println("[Base2, debug]: " + message);
+ }
+
+ @Override
+ public void verbose(String message) {
+ System.out.println("[Base2, verbose]: " + message);
+ }
+}
+
+class AnotherSub1 extends Base2 {
+ @Override
+ public void debug(String message) {
+ System.out.println("[AnotherSub1, debug]: " + message);
+ }
+
+ @Override
+ public void verbose(String message) {
+ System.out.println("[AnotherSub1, verbose]: " + message);
+ }
+}
+
+class AnotherSub2 extends Base2 {
+ @Override
+ public void debug(String message) {
+ System.out.println("[AnotherSub2, debug]: " + message);
+ }
+
+ @Override
+ public void verbose(String message) {
+ System.out.println("[AnotherSub2, verbose]: " + message);
+ }
+}
+
+class SubsUser {
+ static Base1 createBase1() {
+ return System.currentTimeMillis() > 0 ? new Sub1() : new Sub2();
+ }
+
+ static Base2 createBase2() {
+ return System.currentTimeMillis() < 0 ? new AnotherSub1() : new AnotherSub2();
+ }
+
+ @NeverInline
+ private static void testInvokeInterface(Itf itf, String message) {
+ itf.info(message);
+ itf.debug(message);
+ itf.verbose(message);
+ }
+
+ public static void main(String... args) {
+ Base1 instance1 = createBase1();
+ testInvokeInterface(instance1, "message00");
+ instance1.info("message1");
+ instance1.debug("message2");
+ instance1.verbose("message3");
+ Base2 instance2 = createBase2();
+ testInvokeInterface(instance2, "message08");
+ instance2.info("message4");
+ instance2.debug("message5");
+ instance2.verbose("message6");
+ System.out.println("The end");
+ }
+}
+
+@RunWith(Parameterized.class)
+public class AssumenosideeffectsPropagationTest extends TestBase {
+ private static final Class<?> MAIN = SubsUser.class;
+ private static final List<Class<?>> CLASSES = ImmutableList.of(
+ MAIN, Itf.class,
+ Base1.class, Sub1.class, Sub2.class,
+ Base2.class, AnotherSub1.class, AnotherSub2.class
+ );
+
+ private static final String JVM_OUTPUT = StringUtils.lines(
+ "[Sub1, info]: message00",
+ "[Base1, debug]: message00",
+ "[Sub1, verbose]: message00",
+ "[Sub1, info]: message1",
+ "[Base1, debug]: message2",
+ "[Sub1, verbose]: message3",
+ "[Base2, info]: message08",
+ "[AnotherSub2, debug]: message08",
+ "[AnotherSub2, verbose]: message08",
+ "[Base2, info]: message4",
+ "[AnotherSub2, debug]: message5",
+ "[AnotherSub2, verbose]: message6",
+ "The end"
+ );
+ private static final String OUTPUT_WITHOUT_MESSAGES = StringUtils.lines(
+ "The end"
+ );
+
+ enum TestConfig {
+ SPECIFIC_RULES,
+ NON_SPECIFIC_RULES_PARTIAL,
+ NON_SPECIFIC_RULES_ALL;
+
+ public String getKeepRules() {
+ switch (this) {
+ case SPECIFIC_RULES:
+ return StringUtils.lines(
+ // Intentionally miss sub types of Base2 for info().
+ "-assumenosideeffects class **.Sub* {",
+ " void info(...); ",
+ "}",
+ // All debug() and verbose() should be removed.
+ "-assumenosideeffects class **.*Sub* {",
+ " void debug(...);",
+ " void verbose(...);",
+ "}"
+ );
+ case NON_SPECIFIC_RULES_PARTIAL:
+ return StringUtils.lines(
+ // Intentionally miss sub types of Base2 for debug().
+ "-assumenosideeffects class **.Sub* {",
+ " void debug(...);"
+ ,"}",
+ // Targeting info() and verbose() in Base1, without wildcards.
+ "-assumenosideeffects class * extends **.Base1 {",
+ " void info(...);",
+ " void verbose(...);",
+ "}",
+ // Targeting info() and verbose() in Base2, with wildcards.
+ "-assumenosideeffects class * extends **.Base2 {",
+ " void *o*(java.lang.String);",
+ "}"
+ );
+ case NON_SPECIFIC_RULES_ALL:
+ return StringUtils.lines(
+ "-assumenosideeffects class **.*Sub* {",
+ " void *(java.lang.String);",
+ "}"
+ );
+ default:
+ throw new Unreachable();
+ }
+ }
+
+ public String expectedOutput(boolean isR8) {
+ if (!isR8) {
+ return OUTPUT_WITHOUT_MESSAGES;
+ }
+ switch (this) {
+ case SPECIFIC_RULES:
+ return StringUtils.lines(
+ // Itf#info has side effects due to the missing Base2.
+ "[Sub1, info]: message00",
+ "[Base2, info]: message08",
+ // Base2#info also has side effects.
+ "[Base2, info]: message4",
+ "The end"
+ );
+ case NON_SPECIFIC_RULES_PARTIAL:
+ return StringUtils.lines(
+ // TODO(b/133208961): Introduce comparison/meet of assume rules.
+ // Itf has side effects for all methods, since we don't compute the meet yet.
+ "[Sub1, info]: message00",
+ "[Base1, debug]: message00",
+ "[Sub1, verbose]: message00",
+ "[Base2, info]: message08",
+ "[AnotherSub2, debug]: message08",
+ "[AnotherSub2, verbose]: message08",
+ // Base2#debug also has side effects.
+ "[AnotherSub2, debug]: message5",
+ "The end"
+ );
+ case NON_SPECIFIC_RULES_ALL:
+ return OUTPUT_WITHOUT_MESSAGES;
+ default:
+ throw new Unreachable();
+ }
+ }
+ }
+
+ private final TestParameters parameters;
+ private final TestConfig config;
+
+ @Parameterized.Parameters(name = "{0} {1}")
+ public static Collection<Object[]> data() {
+ return buildParameters(getTestParameters().withAllRuntimes().build(), TestConfig.values());
+ }
+
+ public AssumenosideeffectsPropagationTest(TestParameters parameters, TestConfig config) {
+ this.parameters = parameters;
+ this.config = config;
+ }
+
+ @Test
+ public void testJVMOutput() throws Exception {
+ assumeTrue(parameters.isCfRuntime() && config == TestConfig.SPECIFIC_RULES);
+ testForJvm()
+ .addTestClasspath()
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(JVM_OUTPUT);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(CLASSES)
+ .addKeepMainRule(MAIN)
+ .addKeepRules(config.getKeepRules())
+ .enableInliningAnnotations()
+ .noMinification()
+ .setMinApi(parameters.getRuntime())
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(config.expectedOutput(true));
+ }
+
+ @Test
+ public void testProguard() throws Exception {
+ assumeTrue(parameters.isCfRuntime());
+ testForProguard()
+ .addProgramClasses(CLASSES)
+ .addProgramClasses(NeverInline.class)
+ .addKeepMainRule(MAIN)
+ .addKeepRules(config.getKeepRules())
+ .noMinification()
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(config.expectedOutput(false));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java
index 7afd064..58f08ea 100644
--- a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java
@@ -100,28 +100,9 @@
throw new Unreachable();
}
- private static final String OUTPUT_WITH_PARTIAL_INFO = StringUtils.lines(
- "TestClass: message2",
+ static final String OUTPUT_WITHOUT_INFO = StringUtils.lines(
"The end"
);
- private static final String OUTPUT_WITHOUT_INFO = StringUtils.lines(
- "The end"
- );
-
- public String expectedOutput(boolean isR8) {
- if (!isR8) {
- return OUTPUT_WITHOUT_INFO;
- }
- switch (this) {
- case RULE_THAT_DIRECTLY_REFERS_CLASS:
- case RULE_THAT_DIRECTLY_REFERS_INTERFACE:
- return OUTPUT_WITHOUT_INFO;
- case RULE_WITH_IMPLEMENTS:
- return OUTPUT_WITH_PARTIAL_INFO;
- default:
- throw new Unreachable();
- }
- }
public void inspect(CodeInspector inspector, boolean isR8) {
ClassSubject main = inspector.clazz(MAIN);
@@ -135,26 +116,7 @@
i -> i.isInvoke() && i.getMethod().name.toString().equals("info"))).count());
MethodSubject testInvokeInterface = main.uniqueMethodWithName("testInvokeInterface");
- if (isR8) {
- switch (this) {
- case RULE_THAT_DIRECTLY_REFERS_CLASS:
- case RULE_THAT_DIRECTLY_REFERS_INTERFACE:
- assertThat(testInvokeInterface, not(isPresent()));
- break;
- case RULE_WITH_IMPLEMENTS:
- assertThat(testInvokeInterface, isPresent());
- // TODO(b/132216744): upwards propagation of member rules.
- assertEquals(
- 1,
- Streams.stream(testInvokeInterface.iterateInstructions(
- i -> i.isInvoke() && i.getMethod().name.toString().equals("info"))).count());
- break;
- default:
- throw new Unreachable();
- }
- } else {
- assertThat(testInvokeInterface, not(isPresent()));
- }
+ assertThat(testInvokeInterface, not(isPresent()));
FieldSubject tag = main.uniqueFieldWithName("TAG");
if (isR8) {
@@ -190,7 +152,7 @@
.noMinification()
.setMinApi(parameters.getRuntime())
.run(parameters.getRuntime(), MAIN)
- .assertSuccessWithOutput(config.expectedOutput(true))
+ .assertSuccessWithOutput(TestConfig.OUTPUT_WITHOUT_INFO)
.inspect(inspector -> config.inspect(inspector, true));
}
@@ -205,7 +167,7 @@
.addKeepRules(config.getKeepRule())
.noMinification()
.run(parameters.getRuntime(), MAIN)
- .assertSuccessWithOutput(config.expectedOutput(false))
+ .assertSuccessWithOutput(TestConfig.OUTPUT_WITHOUT_INFO)
.inspect(inspector -> config.inspect(inspector, false));
}
diff --git a/src/test/java/com/android/tools/r8/shaking/assumevalues/AssumevaluesWithMultipleTargetsTest.java b/src/test/java/com/android/tools/r8/shaking/assumevalues/AssumevaluesWithMultipleTargetsTest.java
index e41e1c8..555fc7c 100644
--- a/src/test/java/com/android/tools/r8/shaking/assumevalues/AssumevaluesWithMultipleTargetsTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/assumevalues/AssumevaluesWithMultipleTargetsTest.java
@@ -112,13 +112,6 @@
throw new Unreachable();
}
- private static final String OUTPUT_WITH_PARTIAL_REPLACEMENT = StringUtils.lines(
- "8",
- // TODO(b/132216744): upwards propagation of member rules.
- "42",
- "8",
- "The end"
- );
private static final String OUTPUT_WITH_FULL_REPLACEMENT = StringUtils.lines(
"8",
"8",
@@ -126,18 +119,6 @@
"The end"
);
- public String expectedOutput() {
- switch (this) {
- case RULE_THAT_DIRECTLY_REFERS_CLASS:
- case RULE_THAT_DIRECTLY_REFERS_INTERFACE:
- return OUTPUT_WITH_FULL_REPLACEMENT;
- case RULE_WITH_IMPLEMENTS:
- return OUTPUT_WITH_PARTIAL_REPLACEMENT;
- default:
- throw new Unreachable();
- }
- }
-
public void inspect(CodeInspector inspector) {
ClassSubject main = inspector.clazz(MAIN);
assertThat(main, isPresent());
@@ -151,7 +132,6 @@
MethodSubject testInvokeInterface = main.uniqueMethodWithName("testInvokeInterface");
assertThat(testInvokeInterface, isPresent());
- // TODO(b/132216744): upwards propagation of member rules.
assertEquals(
1,
Streams.stream(testInvokeInterface.iterateInstructions(
@@ -183,7 +163,7 @@
.noMinification()
.setMinApi(parameters.getRuntime())
.run(parameters.getRuntime(), MAIN)
- .assertSuccessWithOutput(config.expectedOutput())
+ .assertSuccessWithOutput(TestConfig.OUTPUT_WITH_FULL_REPLACEMENT)
.inspect(config::inspect);
}
}
