Record static edge from the context of static invokes to the static receiver.
This deals with one of the two issues in b/135474075, where a singleton pattern
(the type Absent) was reported to have only a cyclic reason for being kept. A
regression test is added for the remaining issue: the super type (the type
Optional in the report) is reported as not being kept despite being in the
output.
Bug: 124501298
Bug: 135474075
Change-Id: I7c4f08e8f7a16e364b72b577a2c8154ad97f82a8
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 decd262..8a3d278 100644
--- a/src/main/java/com/android/tools/r8/references/Reference.java
+++ b/src/main/java/com/android/tools/r8/references/Reference.java
@@ -10,6 +10,7 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
+import java.util.Collections;
import java.util.List;
import java.util.concurrent.ConcurrentMap;
@@ -162,6 +163,10 @@
return method(classFromClass(holderClass), "<init>", builder.build(), null);
}
+ public static MethodReference classConstructor(ClassReference type) {
+ return method(type, "<clinit>", Collections.emptyList(), 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 175a41d..093adaa 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1080,10 +1080,18 @@
// Only mark methods for which invocation will succeed at runtime live.
DexEncodedMethod targetMethod = appInfo.dispatchStaticInvoke(resolutionResult);
if (targetMethod != null) {
+ registerClassInitializer(targetMethod.method.holder, reason);
markDirectStaticOrConstructorMethodAsLive(targetMethod, reason);
}
}
+ private void registerClassInitializer(DexType holder, KeepReason reason) {
+ DexClass definition = appView.definitionFor(holder);
+ if (definition != null && definition.hasClassInitializer()) {
+ registerMethod(definition.getClassInitializer(), reason);
+ }
+ }
+
private void handleInvokeOfDirectTarget(DexMethod method, KeepReason reason) {
// We have to mark the resolved method as targeted even if it cannot actually be invoked
// to make sure the invocation will keep failing in the appropriate way.
@@ -1337,6 +1345,7 @@
private void markStaticFieldAsLive(
DexField field, KeepReason reason, DexEncodedField encodedField) {
// Mark the type live here, so that the class exists at runtime.
+ registerClassInitializer(field.holder, reason);
markTypeAsLive(field.holder);
markTypeAsLive(field.type);
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptSingletonIsNotCyclicTest.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptSingletonIsNotCyclicTest.java
new file mode 100644
index 0000000..8ff2539
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptSingletonIsNotCyclicTest.java
@@ -0,0 +1,158 @@
+// 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 org.hamcrest.CoreMatchers.anyOf;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersBuilder;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.references.ClassReference;
+import com.android.tools.r8.references.MethodReference;
+import com.android.tools.r8.references.Reference;
+import com.android.tools.r8.shaking.WhyAreYouKeepingConsumer;
+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.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+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 KeptSingletonIsNotCyclicTest extends TestBase {
+
+ private static final String EXPECTED = StringUtils.lines("Foo!");
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return TestParametersBuilder.builder().withCfRuntimes().build();
+ }
+
+ public KeptSingletonIsNotCyclicTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testStaticMethod() throws Exception {
+ test(FooStaticMethod.class, TestStaticMethod.class);
+ }
+
+ @Test
+ public void testStaticField() throws Exception {
+ test(FooStaticField.class, TestStaticField.class);
+ }
+
+ private void test(Class<?> fooClass, Class<?> testClass) throws Exception {
+ WhyAreYouKeepingConsumer whyAreYouKeepingConsumer = new WhyAreYouKeepingConsumer(null);
+ GraphInspector inspector =
+ testForR8(parameters.getBackend())
+ .enableClassInliningAnnotations()
+ .enableGraphInspector(whyAreYouKeepingConsumer)
+ .addProgramClasses(testClass, fooClass)
+ .addKeepMainRule(testClass)
+ .run(parameters.getRuntime(), testClass)
+ .assertSuccessWithOutput(EXPECTED)
+ .graphInspector();
+
+ ClassReference fooClassRef = Reference.classFromClass(fooClass);
+ MethodReference fooClInitRef = Reference.classConstructor(fooClassRef);
+ MethodReference fooInitRef = Reference.methodFromMethod(fooClass.getDeclaredConstructor());
+
+ MethodReference testInitRef =
+ Reference.methodFromMethod(testClass.getDeclaredConstructor());
+
+ MethodReference mainMethodRef =
+ Reference.methodFromMethod(testClass.getDeclaredMethod("main", String[].class));
+
+ // Check that whyareyoukeeping does not mention cycles.
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ whyAreYouKeepingConsumer.printWhyAreYouKeeping(fooClassRef, new PrintStream(baos));
+ assertThat(
+ baos.toString().replace(getClass().getTypeName(), "<test>").toLowerCase(),
+ not(anyOf(containsString("cyclic"), containsString("cycle"))));
+
+ // The only root should be the keep main-method rule.
+ assertEquals(1, inspector.getRoots().size());
+ QueryNode root = inspector.rule(Origin.unknown(), 1, 1).assertRoot();
+
+ // TestClass.main is kept by the root rule.
+ QueryNode mainMethod = inspector.method(mainMethodRef).assertNotRenamed().assertKeptBy(root);
+ // TestClass.<init> is kept by TestClass.main.
+ QueryNode testInit = inspector.method(testInitRef).assertPresent().assertKeptBy(mainMethod);
+ // Foo.<clinit> is kept by TestClass.<init>
+ QueryNode fooClInit = inspector.method(fooClInitRef).assertPresent().assertKeptBy(testInit);
+ // Foo.<init> is kept by Foo.<clinit>
+ QueryNode fooInit = inspector.method(fooInitRef).assertPresent().assertKeptBy(fooClInit);
+ // TODO(b/139273002): It should be the class constructor of Foo that mark Foo as instantiated.
+ inspector.clazz(fooClassRef).assertRenamed().assertKeptBy(fooClInit);
+ assertFalse(inspector.clazz(fooClassRef).assertRenamed().isKeptBy(fooInit));
+ }
+
+ public static final class FooStaticMethod {
+
+ static final FooStaticMethod INSTANCE = new FooStaticMethod();
+
+ public static FooStaticMethod getInstance() {
+ return INSTANCE;
+ }
+
+ private FooStaticMethod() {}
+
+ @Override
+ public String toString() {
+ return "Foo!";
+ }
+ }
+
+ @NeverClassInline
+ public static class TestStaticMethod {
+ public FooStaticMethod foo;
+
+ public TestStaticMethod() {
+ this.foo = FooStaticMethod.getInstance();
+ }
+
+ public static void main(String[] args) {
+ System.out.println(new TestStaticMethod().foo.toString());
+ }
+ }
+
+ public static final class FooStaticField {
+
+ static final FooStaticField INSTANCE = new FooStaticField();
+
+ private FooStaticField() {}
+
+ @Override
+ public String toString() {
+ return "Foo!";
+ }
+ }
+
+ @NeverClassInline
+ public static class TestStaticField {
+ public FooStaticField foo;
+
+ public TestStaticField() {
+ this.foo = FooStaticField.INSTANCE;
+ }
+
+ public static void main(String[] args) {
+ System.out.println(new TestStaticField().foo.toString());
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptSubclassKeepsSuperTest.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptSubclassKeepsSuperTest.java
new file mode 100644
index 0000000..6bc7cb5
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptSubclassKeepsSuperTest.java
@@ -0,0 +1,97 @@
+// 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 org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersBuilder;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.references.ClassReference;
+import com.android.tools.r8.references.Reference;
+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 org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class KeptSubclassKeepsSuperTest extends TestBase {
+
+ private static final Class<?> CLASS = TestClass.class;
+ private static final String EXPECTED = StringUtils.lines("Foo!");
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return TestParametersBuilder.builder().withCfRuntimes().build();
+ }
+
+ public KeptSubclassKeepsSuperTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ GraphInspector inspector =
+ testForR8(parameters.getBackend())
+ .enableGraphInspector()
+ .enableMergeAnnotations()
+ .addProgramClasses(CLASS, Foo.class, Bar.class)
+ .addKeepMainRule(CLASS)
+ .noMinification()
+ .run(parameters.getRuntime(), CLASS)
+ .assertSuccessWithOutput(EXPECTED)
+ .graphInspector();
+
+ // The only root should be the keep main-method rule.
+ assertEquals(1, inspector.getRoots().size());
+ inspector.rule(Origin.unknown(), 1, 1).assertRoot();
+
+ QueryNode fooClass = inspector.clazz(Reference.classFromClass(Foo.class));
+ fooClass.assertPresent();
+
+ ClassReference barClassRef = Reference.classFromClass(Bar.class);
+ QueryNode barClass = inspector.clazz(barClassRef);
+ // TODO(b/135474075): No edge is reported for the super class!
+ barClass.assertAbsent();
+ }
+
+ @NeverMerge
+ public abstract static class Bar {}
+
+ public static final class Foo extends Bar {
+
+ static final Foo INSTANCE = new Foo();
+
+ public static Foo getInstance() {
+ return INSTANCE;
+ }
+
+ private Foo() {}
+
+ @Override
+ public String toString() {
+ return "Foo!";
+ }
+ }
+
+ public static class TestClass {
+ public Bar bar;
+
+ public TestClass() {
+ this.bar = Foo.getInstance();
+ }
+
+ public static void main(String[] args) {
+ System.out.println(new TestClass().bar.toString());
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptViaClassInitializerTestRunner.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptViaClassInitializerTestRunner.java
index ddf82f0..3c5c230 100644
--- a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptViaClassInitializerTestRunner.java
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptViaClassInitializerTestRunner.java
@@ -67,6 +67,7 @@
}
}
+ private static final String CLASS_NAME = KeptViaClassInitializerTestRunner.class.getTypeName();
private static final String EXPECTED = StringUtils.lines("I'm an A");
private final Backend backend;
@@ -80,6 +81,15 @@
this.backend = backend;
}
+ public static String KEPT_REASON_SUFFIX = StringUtils.lines(
+ // The full reason is not shared between CF and DEX due to desugaring.
+ "| void " + CLASS_NAME + "$T.<clinit>()",
+ "|- is referenced from:",
+ "| void " + CLASS_NAME + "$Main.main(java.lang.String[])",
+ "|- is referenced in keep rule:",
+ "| -keep class " + CLASS_NAME + "$Main { void main(java.lang.String[]); }"
+ );
+
@Test
public void testKeptMethod() throws Exception {
assumeTrue(ToolHelper.getDexVm().getVersion().isAtLeast(Version.V7_0_0));
@@ -110,9 +120,7 @@
ByteArrayOutputStream baos = new ByteArrayOutputStream();
consumer.printWhyAreYouKeeping(classFromClass(A.class), new PrintStream(baos));
-
- // TODO(b/124501298): Currently the rooted path is not found.
- assertThat(baos.toString(), containsString("is kept for unknown reason"));
+ assertThat(baos.toString(), containsString(KEPT_REASON_SUFFIX));
// TODO(b/124499108): Currently synthetic lambda classes are referenced,
// should be their originating context.
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/WhyAreYouKeepingAllTest.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/WhyAreYouKeepingAllTest.java
index da94e04..4f0ba9e 100644
--- a/src/test/java/com/android/tools/r8/shaking/keptgraph/WhyAreYouKeepingAllTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/WhyAreYouKeepingAllTest.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.shaking.keptgraph;
import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.TestBase;
@@ -38,7 +39,10 @@
.redirectStdOut(new PrintStream(baos))
.compile();
assertThat(baos.toString(), containsString("referenced in keep rule"));
+
// TODO(b/124655065): We should always know the reason for keeping.
- assertThat(baos.toString(), containsString("kept for unknown reasons"));
+ // It is OK if this starts failing while the kept-graph API is incomplete, in which case replace
+ // the 'not(containsString(' by just 'containsString('.
+ assertThat(baos.toString(), not(containsString("kept for unknown reasons")));
}
}
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 3d155db..4729766 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
@@ -27,10 +27,10 @@
import com.android.tools.r8.references.MethodReference;
import com.android.tools.r8.shaking.CollectingGraphConsumer;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.google.common.collect.ImmutableSet;
import java.util.Collections;
import java.util.HashSet;
import java.util.IdentityHashMap;
-import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
@@ -225,6 +225,8 @@
isSatisfiedBy(node));
return this;
}
+
+ public abstract String getKeptGraphString();
}
private static class AbsentQueryNode extends QueryNode {
@@ -236,6 +238,11 @@
}
@Override
+ public String getKeptGraphString() {
+ return "<not kept>";
+ }
+
+ @Override
public boolean equals(Object obj) {
return obj instanceof AbsentQueryNode
&& failedQueryNodeDescription.equals(((AbsentQueryNode) obj).failedQueryNodeDescription);
@@ -393,6 +400,39 @@
return filterSources((source, infos) -> impl.graphNode == source).findFirst().isPresent();
}
+ @Override
+ public String getKeptGraphString() {
+ StringBuilder builder = new StringBuilder();
+ getKeptGraphString(graphNode, inspector, builder, "", ImmutableSet.of());
+ return builder.toString();
+ }
+
+ private static void getKeptGraphString(
+ GraphNode graphNode,
+ GraphInspector inspector,
+ StringBuilder builder,
+ String indent,
+ Set<GraphNode> seen) {
+ builder.append(graphNode);
+ if (seen.contains(graphNode)) {
+ builder.append(" <CYCLE>");
+ return;
+ }
+ seen = ImmutableSet.<GraphNode>builder().addAll(seen).add(graphNode).build();
+ Map<GraphNode, Set<GraphEdgeInfo>> sources =
+ inspector.consumer.getSourcesTargeting(graphNode);
+ if (sources == null) {
+ builder.append(" <ROOT>");
+ return;
+ }
+ for (Entry<GraphNode, Set<GraphEdgeInfo>> entry : sources.entrySet()) {
+ GraphNode source = entry.getKey();
+ Set<GraphEdgeInfo> reasons = entry.getValue();
+ builder.append('\n').append(indent).append("<- ");
+ getKeptGraphString(source, inspector, builder, indent + " ", seen);
+ }
+ }
+
private Stream<GraphNode> filterSources(BiPredicate<GraphNode, Set<GraphEdgeInfo>> test) {
Map<GraphNode, Set<GraphEdgeInfo>> sources =
inspector.consumer.getSourcesTargeting(graphNode);
