Don't add keep info to items until they become live
Change-Id: Id717c915f99519e4bc9a2e12dcccc4a4bbc63ddf
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 ecc5194..c93bdb3 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -944,7 +944,10 @@
workList.enqueueMarkFieldKeptAction(
field,
graphReporter.reportKeepField(
- precondition, minimumKeepInfo.getRules(), field.getDefinition()));
+ precondition,
+ minimumKeepInfo.getReasons(),
+ minimumKeepInfo.getRules(),
+ field.getDefinition()));
}
private void enqueueMethodDueToNoShrinkingRule(
@@ -3543,7 +3546,7 @@
(clazz, minimumKeepInfo) ->
applyMinimumKeepInfoWhenLive(clazz, preconditionEvent, minimumKeepInfo),
(field, minimumKeepInfo) ->
- applyMinimumKeepInfoWhenLive(field, preconditionEvent, minimumKeepInfo),
+ applyMinimumKeepInfoWhenLive(field, minimumKeepInfo, preconditionEvent),
this::applyMinimumKeepInfoWhenLiveOrTargeted);
}
enqueueAllIfNotShrinking();
@@ -3637,7 +3640,12 @@
}
private void applyMinimumKeepInfoWhenLive(
- ProgramField field, EnqueuerEvent preconditionEvent, KeepFieldInfo.Joiner minimumKeepInfo) {
+ ProgramField field, KeepFieldInfo.Joiner minimumKeepInfo) {
+ applyMinimumKeepInfoWhenLive(field, minimumKeepInfo, EnqueuerEvent.unconditional());
+ }
+
+ private void applyMinimumKeepInfoWhenLive(
+ ProgramField field, KeepFieldInfo.Joiner minimumKeepInfo, EnqueuerEvent preconditionEvent) {
if (liveFields.contains(field)) {
keepInfo.joinField(field, info -> info.merge(minimumKeepInfo));
} else {
@@ -3660,7 +3668,7 @@
private void recordDependentMinimumKeepInfo(
EnqueuerEvent preconditionEvent, ProgramField field, KeepFieldInfo.Joiner minimumKeepInfo) {
if (isPreconditionForMinimumKeepInfoSatisfied(preconditionEvent)) {
- applyMinimumKeepInfoWhenLive(field, preconditionEvent, minimumKeepInfo);
+ applyMinimumKeepInfoWhenLive(field, minimumKeepInfo, preconditionEvent);
} else {
dependentMinimumKeepInfo
.getOrCreateMinimumKeepInfoFor(preconditionEvent)
@@ -3741,7 +3749,7 @@
(clazz, minimumKeepInfoForClass) ->
applyMinimumKeepInfoWhenLive(clazz, preconditionEvent, minimumKeepInfoForClass),
(field, minimumKeepInfoForField) ->
- applyMinimumKeepInfoWhenLive(field, preconditionEvent, minimumKeepInfoForField),
+ applyMinimumKeepInfoWhenLive(field, minimumKeepInfoForField, preconditionEvent),
(method, minimumKeepInfoForMethod) ->
applyMinimumKeepInfoWhenLiveOrTargeted(
method, minimumKeepInfoForMethod, preconditionEvent));
@@ -4841,9 +4849,12 @@
}
if (keepInfo.getFieldInfo(encodedField, clazz).isShrinkingAllowed(options)) {
ProgramField programField = new ProgramField(clazz, encodedField);
- keepInfo.joinField(
- programField, joiner -> joiner.disallowOptimization().disallowShrinking());
- markFieldAsKept(programField, KeepReason.reflectiveUseIn(method));
+ applyMinimumKeepInfoWhenLive(
+ programField,
+ KeepFieldInfo.newEmptyJoiner()
+ .disallowOptimization()
+ .disallowShrinking()
+ .addReason(KeepReason.reflectiveUseIn(method)));
}
} else {
assert referencedItem.isDexMethod();
diff --git a/src/main/java/com/android/tools/r8/shaking/GraphReporter.java b/src/main/java/com/android/tools/r8/shaking/GraphReporter.java
index 2fe6b81..c25095b 100644
--- a/src/main/java/com/android/tools/r8/shaking/GraphReporter.java
+++ b/src/main/java/com/android/tools/r8/shaking/GraphReporter.java
@@ -162,9 +162,15 @@
}
KeepReasonWitness reportKeepField(
- DexDefinition precondition, Collection<ProguardKeepRuleBase> rules, DexEncodedField field) {
- assert !rules.isEmpty() || !options.isShrinking();
+ DexDefinition precondition,
+ Collection<KeepReason> reasons,
+ Collection<ProguardKeepRuleBase> rules,
+ DexEncodedField field) {
+ assert !reasons.isEmpty() || !rules.isEmpty() || !options.isShrinking();
if (keptGraphConsumer != null) {
+ for (KeepReason reason : reasons) {
+ registerField(field, reason);
+ }
for (ProguardKeepRuleBase rule : rules) {
reportKeepField(precondition, rule, field);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
index b45eb72..f85dc0f 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.shaking.KeepInfo.Builder;
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.Sets;
+import java.util.HashSet;
import java.util.Set;
import java.util.function.Consumer;
@@ -477,6 +478,7 @@
// The set of rules that have contributed to this joiner. These are only needed for the
// interpretation of keep rules into keep info, and is therefore not stored in the keep info
// builder above.
+ final Set<KeepReason> reasons = new HashSet<>();
final Set<ProguardKeepRuleBase> rules = Sets.newIdentityHashSet();
Joiner(B builder) {
@@ -506,6 +508,10 @@
return null;
}
+ public Set<KeepReason> getReasons() {
+ return reasons;
+ }
+
public Set<ProguardKeepRuleBase> getRules() {
return rules;
}
@@ -535,6 +541,11 @@
return self();
}
+ public J addReason(KeepReason reason) {
+ reasons.add(reason);
+ return self();
+ }
+
public J addRule(ProguardKeepRuleBase rule) {
rules.add(rule);
return self();
@@ -592,6 +603,7 @@
applyIf(
builder.isAccessModificationRequiredForRepackaging(),
Joiner::requireAccessModificationForRepackaging);
+ reasons.addAll(joiner.reasons);
rules.addAll(joiner.rules);
return self();
}
@@ -610,7 +622,7 @@
public boolean verifyShrinkingDisallowedWithRule(InternalOptions options) {
assert !isShrinkingAllowed();
- assert !getRules().isEmpty() || !options.isShrinking();
+ assert !getReasons().isEmpty() || !getRules().isEmpty() || !options.isShrinking();
return true;
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/reflection/FieldAccessTest.java b/src/test/java/com/android/tools/r8/shaking/reflection/FieldAccessTest.java
index dd8dd90..9d2b858 100644
--- a/src/test/java/com/android/tools/r8/shaking/reflection/FieldAccessTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/reflection/FieldAccessTest.java
@@ -8,7 +8,8 @@
import static org.junit.Assert.assertEquals;
import com.android.tools.r8.TestBase;
-import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.references.FieldReference;
import com.android.tools.r8.references.MethodReference;
@@ -18,20 +19,18 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
import org.junit.runners.Parameterized.Parameters;
@RunWith(Parameterized.class)
public class FieldAccessTest extends TestBase {
- private final Backend backend;
+ @Parameter(0)
+ public TestParameters parameters;
@Parameters(name = "{0}")
- public static Backend[] data() {
- return ToolHelper.getBackends();
- }
-
- public FieldAccessTest(Backend backend) {
- this.backend = backend;
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
}
private boolean isInvokeGetField(InstructionSubject instruction) {
@@ -45,27 +44,30 @@
methodFromMethod(testClass.getDeclaredMethod("main", String[].class));
FieldReference fooField = fieldFromField(testClass.getDeclaredField("foo"));
- testForR8(Backend.DEX)
+ testForR8(parameters.getBackend())
.enableGraphInspector()
.addProgramClasses(testClass)
.addKeepMainRule(testClass)
- .run(testClass)
- .inspectGraph(inspector -> {
- // The only root should be the keep annotation rule.
- assertEquals(1, inspector.getRoots().size());
- QueryNode root = inspector.rule(Origin.unknown(), 1, 1).assertRoot();
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), testClass)
+ .inspectGraph(
+ inspector -> {
+ // The only root should be the keep annotation rule.
+ assertEquals(1, inspector.getRoots().size());
+ QueryNode root = inspector.rule(Origin.unknown(), 1, 1).assertRoot();
- inspector.method(mainMethod).assertNotRenamed().assertKeptBy(root);
- inspector.field(fooField).assertRenamed();
- })
- .inspect(inspector -> {
- Assert.assertTrue(
- inspector
- .clazz(testClass)
- .uniqueMethodWithName("main")
- .streamInstructions()
- .anyMatch(this::isInvokeGetField));
- })
+ inspector.method(mainMethod).assertNotRenamed().assertKeptBy(root);
+ inspector.field(fooField).assertRenamed();
+ })
+ .inspect(
+ inspector -> {
+ Assert.assertTrue(
+ inspector
+ .clazz(testClass)
+ .uniqueMethodWithName("main")
+ .streamInstructions()
+ .anyMatch(this::isInvokeGetField));
+ })
.assertSuccessWithOutput("42");
}
