Collect all root rules that keep a particular program item alive.
Bug: 122297131
Change-Id: Ide92a9af4e1acff47c6fb84985130a96e2073ff3
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 279d381..f10d000 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -297,11 +297,11 @@
this.options = options;
}
- private void enqueueRootItems(Map<DexDefinition, ProguardKeepRule> items) {
+ private void enqueueRootItems(Map<DexDefinition, Set<ProguardKeepRule>> items) {
items.entrySet().forEach(this::enqueueRootItem);
}
- private void enqueueRootItem(Entry<DexDefinition, ProguardKeepRule> root) {
+ private void enqueueRootItem(Entry<DexDefinition, Set<ProguardKeepRule>> root) {
enqueueRootItem(root.getKey(), root.getValue());
}
@@ -309,7 +309,26 @@
enqueueRootItem(item, KeepReason.dueToKeepRule(rule));
}
+ private void enqueueRootItem(DexDefinition item, Set<ProguardKeepRule> rules) {
+ assert !rules.isEmpty();
+ if (keptGraphConsumer != null) {
+ GraphNode node = getGraphNode(item);
+ for (ProguardKeepRule rule : rules) {
+ registerEdge(node, KeepReason.dueToKeepRule(rule));
+ }
+ }
+ internalEnqueueRootItem(item, KeepReason.dueToKeepRule(rules.iterator().next()));
+ }
+
private void enqueueRootItem(DexDefinition item, KeepReason reason) {
+ if (keptGraphConsumer != null) {
+ registerEdge(getGraphNode(item), reason);
+ }
+ internalEnqueueRootItem(item, reason);
+ }
+
+ private void internalEnqueueRootItem(DexDefinition item, KeepReason reason) {
+ // TODO(b/120959039): do we need to propagate the reason to the action now?
if (item.isDexClass()) {
DexClass clazz = item.asDexClass();
workList.add(Action.markInstantiated(clazz, reason));
@@ -349,11 +368,11 @@
}
private void enqueueHolderIfDependentNonStaticMember(
- DexClass holder, Map<DexDefinition, ProguardKeepRule> dependentItems) {
+ DexClass holder, Map<DexDefinition, Set<ProguardKeepRule>> dependentItems) {
// Check if any dependent members are not static, and in that case enqueue the class as well.
// Having a dependent rule like -keepclassmembers with non static items indicates that class
// instances will be present even if tracing do not find any instantiation. See b/115867670.
- for (Entry<DexDefinition, ProguardKeepRule> entry : dependentItems.entrySet()) {
+ for (Entry<DexDefinition, Set<ProguardKeepRule>> entry : dependentItems.entrySet()) {
DexDefinition dependentItem = entry.getKey();
if (dependentItem.isDexClass()) {
continue;
@@ -791,7 +810,7 @@
annotations.forEach(this::handleAnnotationOfLiveType);
}
- Map<DexDefinition, ProguardKeepRule> dependentItems = rootSet.getDependentItems(holder);
+ Map<DexDefinition, Set<ProguardKeepRule>> dependentItems = rootSet.getDependentItems(holder);
enqueueHolderIfDependentNonStaticMember(holder, dependentItems);
// Add all dependent members to the workqueue.
enqueueRootItems(dependentItems);
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 2f7f54a..e86966f 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -61,7 +61,7 @@
private final AppView<? extends AppInfo> appView;
private final DirectMappedDexApplication application;
private final Collection<ProguardConfigurationRule> rules;
- private final Map<DexDefinition, ProguardKeepRule> noShrinking = new IdentityHashMap<>();
+ private final Map<DexDefinition, Set<ProguardKeepRule>> noShrinking = new IdentityHashMap<>();
private final Set<DexDefinition> noOptimization = Sets.newIdentityHashSet();
private final Set<DexDefinition> noObfuscation = Sets.newIdentityHashSet();
private final LinkedHashMap<DexDefinition, DexDefinition> reasonAsked = new LinkedHashMap<>();
@@ -76,7 +76,7 @@
private final Set<DexMethod> keepUnusedArguments = Sets.newIdentityHashSet();
private final Set<DexType> neverClassInline = Sets.newIdentityHashSet();
private final Set<DexType> neverMerge = Sets.newIdentityHashSet();
- private final Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking =
+ private final Map<DexDefinition, Map<DexDefinition, Set<ProguardKeepRule>>> dependentNoShrinking =
new IdentityHashMap<>();
private final Map<DexDefinition, ProguardMemberRule> noSideEffects = new IdentityHashMap<>();
private final Map<DexDefinition, ProguardMemberRule> assumedValues = new IdentityHashMap<>();
@@ -854,8 +854,10 @@
return;
}
// Keep the type if the item is also kept.
- dependentNoShrinking.computeIfAbsent(item, x -> new IdentityHashMap<>())
- .put(definition, context);
+ dependentNoShrinking
+ .computeIfAbsent(item, x -> new IdentityHashMap<>())
+ .computeIfAbsent(definition, k -> new HashSet<>())
+ .add(context);
// Unconditionally add to no-obfuscation, as that is only checked for surviving items.
noObfuscation.add(definition);
}
@@ -890,10 +892,12 @@
ProguardKeepRuleModifiers modifiers = keepRule.getModifiers();
if (!modifiers.allowsShrinking) {
if (precondition != null) {
- dependentNoShrinking.computeIfAbsent(precondition, x -> new IdentityHashMap<>())
- .put(item, keepRule);
+ dependentNoShrinking
+ .computeIfAbsent(precondition, x -> new IdentityHashMap<>())
+ .computeIfAbsent(item, i -> new HashSet<>())
+ .add(keepRule);
} else {
- noShrinking.put(item, keepRule);
+ noShrinking.computeIfAbsent(item, i -> new HashSet<>()).add(keepRule);
}
}
if (!modifiers.allowsOptimization) {
@@ -970,7 +974,7 @@
public static class RootSet {
- public final Map<DexDefinition, ProguardKeepRule> noShrinking;
+ public final Map<DexDefinition, Set<ProguardKeepRule>> noShrinking;
public final Set<DexDefinition> noOptimization;
public final Set<DexDefinition> noObfuscation;
public final ImmutableList<DexDefinition> reasonAsked;
@@ -985,12 +989,13 @@
public final Set<DexType> neverMerge;
public final Map<DexDefinition, ProguardMemberRule> noSideEffects;
public final Map<DexDefinition, ProguardMemberRule> assumedValues;
- private final Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking;
+ private final Map<DexDefinition, Map<DexDefinition, Set<ProguardKeepRule>>>
+ dependentNoShrinking;
public final Set<DexReference> identifierNameStrings;
public final Set<ProguardIfRule> ifRules;
private RootSet(
- Map<DexDefinition, ProguardKeepRule> noShrinking,
+ Map<DexDefinition, Set<ProguardKeepRule>> noShrinking,
Set<DexDefinition> noOptimization,
Set<DexDefinition> noObfuscation,
ImmutableList<DexDefinition> reasonAsked,
@@ -1005,7 +1010,7 @@
Set<DexType> neverMerge,
Map<DexDefinition, ProguardMemberRule> noSideEffects,
Map<DexDefinition, ProguardMemberRule> assumedValues,
- Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking,
+ Map<DexDefinition, Map<DexDefinition, Set<ProguardKeepRule>>> dependentNoShrinking,
Set<DexReference> identifierNameStrings,
Set<ProguardIfRule> ifRules) {
this.noShrinking = Collections.unmodifiableMap(noShrinking);
@@ -1030,7 +1035,7 @@
// Add dependent items that depend on -if rules.
void addDependentItems(
- Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentItems) {
+ Map<DexDefinition, Map<DexDefinition, Set<ProguardKeepRule>>> dependentItems) {
dependentItems.forEach(
(def, dependence) ->
dependentNoShrinking
@@ -1038,7 +1043,7 @@
.putAll(dependence));
}
- Map<DexDefinition, ProguardKeepRule> getDependentItems(DexDefinition item) {
+ Map<DexDefinition, Set<ProguardKeepRule>> getDependentItems(DexDefinition item) {
return Collections
.unmodifiableMap(dependentNoShrinking.getOrDefault(item, Collections.emptyMap()));
}
@@ -1224,18 +1229,18 @@
static class ConsequentRootSet {
final Set<DexMethod> neverInline;
final Set<DexType> neverClassInline;
- final Map<DexDefinition, ProguardKeepRule> noShrinking;
+ final Map<DexDefinition, Set<ProguardKeepRule>> noShrinking;
final Set<DexDefinition> noOptimization;
final Set<DexDefinition> noObfuscation;
- final Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking;
+ final Map<DexDefinition, Map<DexDefinition, Set<ProguardKeepRule>>> dependentNoShrinking;
private ConsequentRootSet(
Set<DexMethod> neverInline,
Set<DexType> neverClassInline,
- Map<DexDefinition, ProguardKeepRule> noShrinking,
+ Map<DexDefinition, Set<ProguardKeepRule>> noShrinking,
Set<DexDefinition> noOptimization,
Set<DexDefinition> noObfuscation,
- Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking) {
+ Map<DexDefinition, Map<DexDefinition, Set<ProguardKeepRule>>> dependentNoShrinking) {
this.neverInline = Collections.unmodifiableSet(neverInline);
this.neverClassInline = Collections.unmodifiableSet(neverClassInline);
this.noShrinking = Collections.unmodifiableMap(noShrinking);
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByAnnotatedMethodTestRunner.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByAnnotatedMethodTestRunner.java
index 19cdf04..a7e3e46 100644
--- a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByAnnotatedMethodTestRunner.java
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByAnnotatedMethodTestRunner.java
@@ -86,7 +86,7 @@
.method(fooMethod)
.assertNotRenamed()
.assertNotInvokedFrom(mainMethod)
- // TODO(b/122297131): keepAnnotatedMethods should also be keeping foo alive!
+ .assertKeptBy(keepAnnotatedMethods)
.assertKeptBy(keepClassesOfAnnotatedMethods);
// Check baz is removed.
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByTwoRulesTest.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByTwoRulesTest.java
new file mode 100644
index 0000000..7cb464d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByTwoRulesTest.java
@@ -0,0 +1,18 @@
+// 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.keptgraph;
+
+import com.android.tools.r8.Keep;
+
+@Keep
+public class KeptByTwoRulesTest {
+
+ public static void foo() {
+ System.out.println("called foo");
+ }
+
+ public static void main(String[] args) {
+ foo();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByTwoRulesTestRunner.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByTwoRulesTestRunner.java
new file mode 100644
index 0000000..38c3f00
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByTwoRulesTestRunner.java
@@ -0,0 +1,87 @@
+// 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.keptgraph;
+
+import static com.android.tools.r8.references.Reference.classFromClass;
+import static com.android.tools.r8.references.Reference.methodFromMethod;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.references.MethodReference;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.graphinspector.GraphInspector;
+import com.android.tools.r8.utils.graphinspector.GraphInspector.QueryNode;
+import java.util.Arrays;
+import java.util.Collection;
+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 KeptByTwoRulesTestRunner extends TestBase {
+
+ private static final Class<?> CLASS = KeptByTwoRulesTest.class;
+ private static final Collection<Class<?>> CLASSES = Arrays.asList(CLASS);
+
+ private final String EXPECTED = StringUtils.lines("called foo");
+
+ private final Backend backend;
+
+ @Parameters(name = "{0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public KeptByTwoRulesTestRunner(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void test() throws Exception {
+ MethodReference mainMethod = methodFromMethod(CLASS.getDeclaredMethod("main", String[].class));
+ MethodReference fooMethod = methodFromMethod(CLASS.getDeclaredMethod("foo"));
+
+ if (backend == Backend.CF) {
+ testForJvm().addProgramClasses(CLASSES).run(CLASS).assertSuccessWithOutput(EXPECTED);
+ }
+
+ String keepPublicRule = "-keep @com.android.tools.r8.Keep class * { public *; }";
+ String keepFooRule = "-keep class " + CLASS.getTypeName() + " { public void foo(); }";
+ GraphInspector inspector =
+ testForR8(backend)
+ .enableGraphInspector()
+ .enableInliningAnnotations()
+ .addProgramClasses(CLASSES)
+ .addKeepRules(keepPublicRule, keepFooRule)
+ .run(CLASS)
+ .assertSuccessWithOutput(EXPECTED)
+ .graphInspector();
+
+ assertEquals(2, inspector.getRoots().size());
+ QueryNode keepPublic = inspector.rule(keepPublicRule).assertRoot();
+ QueryNode keepFoo = inspector.rule(keepFooRule).assertRoot();
+
+ inspector
+ .method(mainMethod)
+ .assertNotRenamed()
+ .assertKeptBy(keepPublic)
+ .assertNotKeptBy(keepFoo);
+
+ // Check foo is called from main and kept by two rules.
+ inspector
+ .method(fooMethod)
+ .assertNotRenamed()
+ .assertInvokedFrom(mainMethod)
+ .assertKeptBy(keepPublic)
+ .assertKeptBy(keepFoo);
+
+ // Check the class is also kept by both rules.
+ inspector
+ .clazz(classFromClass(CLASS))
+ .assertNotRenamed()
+ .assertKeptBy(keepPublic)
+ .assertKeptBy(keepFoo);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/graphinspector/GraphInspector.java b/src/test/java/com/android/tools/r8/utils/graphinspector/GraphInspector.java
index 107978b..940482b 100644
--- a/src/test/java/com/android/tools/r8/utils/graphinspector/GraphInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/graphinspector/GraphInspector.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.utils.graphinspector;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -113,16 +114,26 @@
}
public QueryNode assertNotInvokedFrom(MethodReference method) {
- assertTrue(
- errorMessage("no invocation from " + method.toString(), "invoke"),
- !isInvokedFrom(method));
+ assertFalse(
+ errorMessage("no invocation from " + method.toString(), "invoke"), isInvokedFrom(method));
return this;
}
public QueryNode assertKeptBy(QueryNode node) {
- assertTrue("Invalid call to assertKeptBy with: " + node.getNodeDescription(),
- node.isPresent());
- assertTrue(errorMessage("kept by " + node.getNodeDescription(), "none"), isKeptBy(node));
+ assertTrue(
+ "Invalid call to assertKeptBy with: " + node.getNodeDescription(), node.isPresent());
+ assertTrue(
+ errorMessage("kept by " + node.getNodeDescription(), "was not kept by it"),
+ isKeptBy(node));
+ return this;
+ }
+
+ public QueryNode assertNotKeptBy(QueryNode node) {
+ assertTrue(
+ "Invalid call to assertNotKeptBy with: " + node.getNodeDescription(), node.isPresent());
+ assertFalse(
+ errorMessage("not kept by " + node.getNodeDescription(), "was kept by it"),
+ isKeptBy(node));
return this;
}
}
