Record all reachable field edges.
Bug: 123262024, 124428834
Change-Id: Ib6ddc9f4dfece7f44119104e79957528c2ce515e
diff --git a/src/main/java/com/android/tools/r8/references/Reference.java b/src/main/java/com/android/tools/r8/references/Reference.java
index 0e3d988..decd262 100644
--- a/src/main/java/com/android/tools/r8/references/Reference.java
+++ b/src/main/java/com/android/tools/r8/references/Reference.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.utils.DescriptorUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.MapMaker;
+import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.List;
@@ -150,6 +151,17 @@
returnType == Void.TYPE ? null : typeFromClass(returnType));
}
+ /** Get a method reference from a Java reflection constructor. */
+ public static MethodReference methodFromMethod(Constructor<?> method) {
+ Class<?> holderClass = method.getDeclaringClass();
+ Class<?>[] parameterTypes = method.getParameterTypes();
+ ImmutableList.Builder<TypeReference> builder = ImmutableList.builder();
+ for (Class<?> parameterType : parameterTypes) {
+ builder.add(typeFromClass(parameterType));
+ }
+ return method(classFromClass(holderClass), "<init>", builder.build(), null);
+ }
+
/** Get a field reference from its full reference specification. */
public static FieldReference field(
ClassReference holderClass, String fieldName, TypeReference fieldType) {
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 f011931..6d24a8c 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -307,6 +307,10 @@
this.options = options;
}
+ private static <T> SetWithReason<T> newSetWithoutReasonReporter() {
+ return new SetWithReason<>((f, r) -> {});
+ }
+
private void enqueueRootItems(Map<DexReference, Set<ProguardKeepRule>> items) {
items.entrySet().forEach(this::enqueueRootItem);
}
@@ -1097,6 +1101,7 @@
SetWithReason<DexEncodedField> reachableFields = reachableInstanceFields.get(type);
if (reachableFields != null) {
for (DexEncodedField field : reachableFields.getItems()) {
+ // TODO(b/120959039): Should the reason this field is reachable come from the set?
markInstanceFieldAsLive(field, KeepReason.reachableFromLiveType(type));
}
}
@@ -1241,14 +1246,14 @@
if (encodedField.accessFlags.isStatic()) {
markStaticFieldAsLive(encodedField.field, reason);
} else {
- SetWithReason<DexEncodedField> reachable =
- reachableInstanceFields.computeIfAbsent(
- encodedField.field.clazz, ignore -> new SetWithReason<>((f, r) -> {}));
- // TODO(b/120959039): The reachable.add test might be hiding other paths to the field.
- if (reachable.add(encodedField, reason)
- && isInstantiatedOrHasInstantiatedSubtype(encodedField.field.clazz)) {
+ if (isInstantiatedOrHasInstantiatedSubtype(encodedField.field.clazz)) {
// We have at least one live subtype, so mark it as live.
markInstanceFieldAsLive(encodedField, reason);
+ } else {
+ // Add the field to the reachable set if the type later becomes instantiated.
+ reachableInstanceFields
+ .computeIfAbsent(encodedField.field.clazz, ignore -> newSetWithoutReasonReporter())
+ .add(encodedField, reason);
}
}
}
@@ -1292,7 +1297,7 @@
// TODO(b/120959039): The reachable.add test might be hiding other paths to the method.
SetWithReason<DexEncodedMethod> reachable =
reachableVirtualMethods.computeIfAbsent(
- encodedMethod.method.holder, (ignore) -> new SetWithReason<>((m, r) -> {}));
+ encodedMethod.method.holder, ignore -> newSetWithoutReasonReporter());
if (reachable.add(encodedMethod, reason)) {
// Abstract methods cannot be live.
if (!encodedMethod.accessFlags.isAbstract()) {
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java
index e518181..3ba9b99 100644
--- a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java
@@ -8,9 +8,7 @@
@Keep
public class KeptByFieldReflectionTest {
- // TODO(b/123262024): This field must be kept un-initialized. Otherwise the "-whyareyoukeeping"
- // output tested will hit the initialization in <init> and not the reflective access.
- public int foo;
+ public int foo = 42;
public static void main(String[] args) throws Exception {
// Due to b/123210548 the object cannot be created by a reflective newInstance call.
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java
index 62dcebe..6ba1f08 100644
--- a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java
@@ -30,7 +30,7 @@
private static final Class<?> CLASS = KeptByFieldReflectionTest.class;
private static final Collection<Class<?>> CLASSES = Arrays.asList(CLASS);
- private final String EXPECTED_STDOUT = StringUtils.lines("got foo: 0");
+ private final String EXPECTED_STDOUT = StringUtils.lines("got foo: 42");
private final String EXPECTED_WHYAREYOUKEEPING =
StringUtils.lines(
@@ -55,6 +55,7 @@
public void test() throws Exception {
MethodReference mainMethod = methodFromMethod(CLASS.getDeclaredMethod("main", String[].class));
FieldReference fooField = fieldFromField(CLASS.getDeclaredField("foo"));
+ MethodReference fooInit = methodFromMethod(CLASS.getDeclaredConstructor());
if (backend == Backend.CF) {
testForJvm().addProgramClasses(CLASSES).run(CLASS).assertSuccessWithOutput(EXPECTED_STDOUT);
@@ -80,6 +81,11 @@
inspector.method(mainMethod).assertNotRenamed().assertKeptBy(keepMain);
+ // The field is primarily kept by the reflective lookup in main.
inspector.field(fooField).assertRenamed().assertReflectedFrom(mainMethod);
+
+ // The field is also kept by the write in Foo.<init>.
+ // We may want to change that behavior. See b/124428834.
+ inspector.field(fooField).assertRenamed().assertKeptBy(inspector.method(fooInit));
}
}
