Use a resolved target during bottom-up propagation.
Bug: 137038659, 133208961
Change-Id: I8ad319466462e73fe684ab42b0900cd442f9e80f
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 f59def9..f9efa44 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -319,7 +319,16 @@
for (DexType subType : subTypes) {
DexMethod referenceInSubType =
appView.dexItemFactory().createMethod(subType, reference.proto, reference.name);
- ProguardMemberRule ruleInSubType = assumeRulePool.get(referenceInSubType);
+ // Those rules are bound to definitions, not references. If the current subtype does not
+ // override the method, and when the retrieval of bound rule fails, it is unclear whether it
+ // is due to the lack of the definition or it indeed means no matching rules. Similar to how
+ // we apply those assume rules, here we use a resolved target.
+ DexEncodedMethod target =
+ appView.appInfo().resolveMethod(subType, referenceInSubType).asResultOfResolve();
+ if (target == null) {
+ continue;
+ }
+ ProguardMemberRule ruleInSubType = assumeRulePool.get(target.method);
// 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) {
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationWithoutMatchingDefinitionTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationWithoutMatchingDefinitionTest.java
index 871b1d8..8095379 100644
--- a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationWithoutMatchingDefinitionTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationWithoutMatchingDefinitionTest.java
@@ -4,8 +4,9 @@
package com.android.tools.r8.shaking.assumenosideeffects;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertEquals;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NeverMerge;
@@ -45,6 +46,7 @@
// We propagated assume* rules only if all the subtypes' corresponding methods have the same rule.
// The lack of matching definitions in this sub type blocks us from marking methods in the super
// type, in this example, LoggerInterface#debug(...).
+ // By using a resolved target, we will look up a rule for BaseImplementer#debug(...) instead.
}
// To bother single target resolution. In fact, not used at all.
@@ -125,8 +127,7 @@
.noMinification()
.setMinApi(parameters.getRuntime())
.run(parameters.getRuntime(), MAIN)
- // TODO(b/137038659): should be able to remove logging.
- .assertSuccessWithOutput(JVM_OUTPUT)
+ .assertSuccessWithOutput(OUTPUT_WITHOUT_LOGGING)
.inspect(this::inspect);
}
@@ -136,14 +137,12 @@
MethodSubject mainMethod = main.mainMethod();
assertThat(mainMethod, isPresent());
- // TODO(b/137038659): can be zero.
- assertNotEquals(
+ assertEquals(
0,
Streams.stream(mainMethod.iterateInstructions(
i -> i.isInvoke() && i.getMethod().name.toString().equals("debug"))).count());
MethodSubject testInvokeInterface = main.uniqueMethodWithName("testInvokeInterface");
- // TODO(b/137038659): can be removed entirely.
- assertThat(testInvokeInterface, isPresent());
+ assertThat(testInvokeInterface, not(isPresent()));
}
}
