Also consider referenced fields in if rule evaluator
Bug: 150189783
Change-Id: If18800c138048a2e803589160f3cb645ea7f346f
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 7d57def..dba8d9c 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -51,6 +51,7 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DirectMappedDexApplication;
import com.android.tools.r8.graph.DirectMappedDexApplication.Builder;
+import com.android.tools.r8.graph.FieldAccessInfoCollection;
import com.android.tools.r8.graph.FieldAccessInfoCollectionImpl;
import com.android.tools.r8.graph.FieldAccessInfoImpl;
import com.android.tools.r8.graph.InnerClassAttribute;
@@ -2040,6 +2041,11 @@
}
}
+ public boolean isFieldReferenced(DexEncodedField field) {
+ FieldAccessInfoImpl info = fieldAccessInfoCollection.get(field.field);
+ return info != null;
+ }
+
public boolean isFieldLive(DexEncodedField field) {
return liveFields.contains(field);
}
@@ -2071,6 +2077,18 @@
return directAndIndirectlyInstantiatedTypes.contains(clazz);
}
+ public boolean isMethodLive(DexEncodedMethod method) {
+ return liveMethods.contains(method);
+ }
+
+ public boolean isMethodTargeted(DexEncodedMethod method) {
+ return targetedMethods.contains(method);
+ }
+
+ public boolean isTypeLive(DexProgramClass clazz) {
+ return liveTypes.contains(clazz);
+ }
+
// Package protected due to entry point from worklist.
void markInstanceFieldAsReachable(DexEncodedField encodedField, KeepReason reason) {
DexField field = encodedField.field;
@@ -2770,14 +2788,10 @@
IfRuleEvaluator ifRuleEvaluator =
new IfRuleEvaluator(
appView,
+ this,
executorService,
activeIfRules,
- liveFields.getItems(),
- liveMethods.getItems(),
- liveTypes.getItems(),
- mode,
- consequentSetBuilder,
- targetedMethods.getItems());
+ consequentSetBuilder);
addConsequentRootSet(ifRuleEvaluator.run(), false);
assert getNumberOfLiveItems() == numberOfLiveItemsAfterProcessing;
if (!workList.isEmpty()) {
diff --git a/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java b/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java
index 951faf1..fe81bb7 100644
--- a/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java
+++ b/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java
@@ -15,7 +15,6 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.shaking.Enqueuer.Mode;
import com.android.tools.r8.shaking.RootSetBuilder.ConsequentRootSet;
import com.android.tools.r8.utils.InternalOptions.TestingOptions.ProguardIfRuleEvaluationData;
import com.android.tools.r8.utils.ThreadUtils;
@@ -37,35 +36,23 @@
public class IfRuleEvaluator {
private final AppView<? extends AppInfoWithSubtyping> appView;
+ private final Enqueuer enqueuer;
private final ExecutorService executorService;
private final List<Future<?>> futures = new ArrayList<>();
private final Map<Wrapper<ProguardIfRule>, Set<ProguardIfRule>> ifRules;
- private final Set<DexEncodedField> liveFields;
- private final Set<DexEncodedMethod> liveMethods;
- private final Set<DexProgramClass> liveTypes;
- private final Mode mode;
private final ConsequentRootSetBuilder rootSetBuilder;
- private final Set<DexEncodedMethod> targetedMethods;
IfRuleEvaluator(
AppView<? extends AppInfoWithSubtyping> appView,
+ Enqueuer enqueuer,
ExecutorService executorService,
Map<Wrapper<ProguardIfRule>, Set<ProguardIfRule>> ifRules,
- Set<DexEncodedField> liveFields,
- Set<DexEncodedMethod> liveMethods,
- Set<DexProgramClass> liveTypes,
- Mode mode,
- ConsequentRootSetBuilder rootSetBuilder,
- Set<DexEncodedMethod> targetedMethods) {
+ ConsequentRootSetBuilder rootSetBuilder) {
this.appView = appView;
+ this.enqueuer = enqueuer;
this.executorService = executorService;
this.ifRules = ifRules;
- this.liveFields = liveFields;
- this.liveMethods = liveMethods;
- this.liveTypes = liveTypes;
- this.mode = mode;
this.rootSetBuilder = rootSetBuilder;
- this.targetedMethods = targetedMethods;
}
public ConsequentRootSet run() throws ExecutionException {
@@ -184,7 +171,7 @@
// A type is effectively live if (1) it is truly live, (2) the value of one of its fields has
// been inlined by the member value propagation, or (3) the return value of one of its methods
// has been forwarded by the member value propagation.
- if (liveTypes.contains(clazz)) {
+ if (enqueuer.isTypeLive(clazz)) {
return true;
}
for (DexEncodedField field : clazz.fields()) {
@@ -236,15 +223,15 @@
filteredMembers,
targetClass.fields(
f ->
- (liveFields.contains(f) || f.getOptimizationInfo().valueHasBeenPropagated())
+ (enqueuer.isFieldReferenced(f) || f.getOptimizationInfo().valueHasBeenPropagated())
&& appView.graphLense().getOriginalFieldSignature(f.field).holder
== sourceClass.type));
Iterables.addAll(
filteredMembers,
targetClass.methods(
m ->
- (liveMethods.contains(m)
- || targetedMethods.contains(m)
+ (enqueuer.isMethodLive(m)
+ || enqueuer.isMethodTargeted(m)
|| m.getOptimizationInfo().returnValueHasBeenPropagated())
&& appView.graphLense().getOriginalMethodSignature(m.method).holder
== sourceClass.type));
@@ -287,7 +274,7 @@
DexItemFactory dexItemFactory = appView.dexItemFactory();
ProguardIfRule materializedRule = rule.materialize(dexItemFactory, preconditions);
- if (mode.isInitialTreeShaking() && !rule.isUsed()) {
+ if (enqueuer.getMode().isInitialTreeShaking() && !rule.isUsed()) {
// We need to abort class inlining of classes that could be matched by the condition of this
// -if rule.
ClassInlineRule neverClassInlineRuleForCondition =
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/KeepClassesWithAnnotatedFieldsTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/KeepClassesWithAnnotatedFieldsTest.java
new file mode 100644
index 0000000..a802383
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/KeepClassesWithAnnotatedFieldsTest.java
@@ -0,0 +1,84 @@
+// Copyright (c) 2020, 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.ifrule;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverPropagateValue;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@RunWith(Parameterized.class)
+public class KeepClassesWithAnnotatedFieldsTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public KeepClassesWithAnnotatedFieldsTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(KeepClassesWithAnnotatedFieldsTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addKeepRules(
+ "-if class * { @" + typeName(MyAnnotation.class) + " <fields>; }",
+ "-keep class <1> { <init>(); }",
+ "-keepclassmembers class * {",
+ " @" + typeName(MyAnnotation.class) + " <fields>;",
+ "}")
+ .addKeepRuntimeVisibleAnnotations()
+ .enableInliningAnnotations()
+ .enableMemberValuePropagationAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) throws Exception {
+ DataClass obj = (DataClass) getDataClass().getDeclaredConstructor().newInstance();
+ setField(obj, "Hello world!");
+ System.out.println(obj.field);
+ }
+
+ @NeverInline
+ @NeverPropagateValue
+ static Class<?> getDataClass() {
+ return DataClass.class;
+ }
+
+ @NeverInline
+ static void setField(Object object, String value) throws Exception {
+ getDataClass().getDeclaredField("field").set(object, value);
+ }
+ }
+
+ static class DataClass {
+
+ @MyAnnotation
+ String field;
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.FIELD)
+ @interface MyAnnotation {}
+}
