Reland "Use instance information to iterate instantiated subtypes."
This relands commit 357a6232744eb8aaa1f896545be45bd5cd4989c1.
CL: https://r8-review.googlesource.com/c/r8/+/50141
Bug: 139464956
Bug: 145344105
Bug: 150277553
Change-Id: I2d0730f918a682b105c1c2055d58c24f23cd0aec
diff --git a/src/main/java/com/android/tools/r8/graph/CfCode.java b/src/main/java/com/android/tools/r8/graph/CfCode.java
index 897763a..93ca81d 100644
--- a/src/main/java/com/android/tools/r8/graph/CfCode.java
+++ b/src/main/java/com/android/tools/r8/graph/CfCode.java
@@ -551,7 +551,7 @@
// We have no debugging info in the code.
return;
}
- int largestPrefix = 1;
+ int largestPrefix = 0;
int existingThisIndex = -1;
for (int i = 0; i < localVariables.size(); i++) {
LocalVariableInfo localVariable = localVariables.get(i);
diff --git a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
index 283777f..fbecfef 100644
--- a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
@@ -388,7 +388,8 @@
}
},
lambda -> {
- assert resolvedHolder.isInterface();
+ assert resolvedHolder.isInterface()
+ || resolvedHolder.type == appInfo.dexItemFactory().objectType;
LookupTarget target = lookupVirtualDispatchTarget(lambda, appInfo);
if (target != null) {
if (target.isLambdaTarget()) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
index 63f6a89..dc319a8 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
@@ -97,11 +97,11 @@
if (lookupResult == null || lookupResult.isEmpty()) {
return null;
}
- assert lookupResult.hasMethodTargets();
if (lookupResult.hasLambdaTargets()) {
// TODO(b/150277553): Support lambda targets.
return null;
}
+ assert lookupResult.hasMethodTargets();
DexType staticReceiverType = getInvokedMethod().holder;
DexType refinedReceiverType =
TypeAnalysis.getRefinedReceiverType(
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java b/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java
index 18b62e6..92b23eb 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java
@@ -218,15 +218,14 @@
resolution.lookupVirtualDispatchTargets(
appView.definitionForProgramType(context), appView.appInfo());
if (lookupResult.isLookupResultSuccess()) {
- // TODO(b/150277553): Add lambda methods to the call graph.
Set<DexEncodedMethod> targets = new HashSet<>();
lookupResult
.asLookupResultSuccess()
.forEach(
methodTarget -> targets.add(methodTarget.getMethod()),
- lambdaTarget -> {
- assert false;
- });
+ lambdaTarget ->
+ // The call target will ultimately be the implementation method.
+ targets.add(lambdaTarget.getImplementationMethod().getMethod()));
return targets;
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
index d6d698e..85b98ba 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
@@ -378,4 +378,8 @@
assert !instructions.hasNext();
nextBlock.copyCatchHandlers(code, blocks, currentBlock, getAppView().options());
}
+
+ public Map<DexType, LambdaClass> getKnownLambdaClasses() {
+ return knownLambdaClasses;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index 78136a9..f0189d3 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -46,7 +46,6 @@
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.PredicateSet;
import com.android.tools.r8.utils.Visibility;
-import com.android.tools.r8.utils.WorkList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.ImmutableSortedSet.Builder;
@@ -1313,19 +1312,8 @@
DexType type,
Consumer<DexProgramClass> subTypeConsumer,
Consumer<LambdaDescriptor> callSiteConsumer) {
- WorkList<DexType> workList = WorkList.newIdentityWorkList();
- workList.addIfNotSeen(type);
- while (workList.hasNext()) {
- DexType subType = workList.next();
- DexProgramClass clazz = definitionForProgramType(subType);
- workList.addIfNotSeen(allImmediateSubtypes(subType));
- if (clazz == null) {
- continue;
- }
- if (isInstantiatedOrPinned(clazz)) {
- subTypeConsumer.accept(clazz);
- }
- }
+ objectAllocationInfoCollection.forEachInstantiatedSubType(
+ type, subTypeConsumer, callSiteConsumer, this);
}
public void forEachInstantiatedSubTypeInChain(
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 f636d05..1d712b5 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -2726,18 +2726,11 @@
DexEncodedMethod context = lambdaClassAndContext.getSecond();
DexProgramClass programClass = lambdaClass.getOrCreateLambdaClass();
additions.addInstantiatedClass(programClass, context, lambdaClass.addToMainDexList.get());
-
- // Mark all methods on the desugared lambda classes as live.
- for (DexEncodedMethod method : programClass.methods()) {
- additions.addLiveMethod(new ProgramMethod(programClass, method));
- }
-
- // Ensure accessors if needed and mark them live too.
- DexEncodedMethod accessor = lambdaClass.target.ensureAccessibilityIfNeeded(false);
- if (accessor != null) {
- DexProgramClass clazz = getProgramClassOrNull(accessor.holder());
- additions.addLiveMethod(new ProgramMethod(clazz, accessor));
- }
+ // Mark the instance constructor targeted and live.
+ DexEncodedMethod constructor = programClass.lookupDirectMethod(lambdaClass.constructor);
+ KeepReason reason = KeepReason.instantiatedIn(context);
+ markMethodAsTargeted(programClass, constructor, reason);
+ markDirectStaticOrConstructorMethodAsLive(programClass, constructor, reason);
}
// Rewrite all of the invoke-dynamic instructions to lambda class instantiations.
@@ -2774,6 +2767,12 @@
}
private AppInfoWithLiveness createAppInfo(AppInfoWithSubtyping appInfo) {
+ // Once all tracing is done, we generate accessor methods for lambdas.
+ // These are assumed to be simple forwarding or access flag updates, thus no further tracing
+ // is needed. These cannot be generated as part of lambda synthesis as changing a direct method
+ // to a static method will invalidate the reachable method sets for tracing methods.
+ ensureLambdaAccessibility();
+
// Compute the set of dead proto types.
deadProtoTypeCandidates.removeIf(this::isTypeLive);
@@ -2866,6 +2865,36 @@
return appInfoWithLiveness;
}
+ private void ensureLambdaAccessibility() {
+ if (lambdaRewriter == null) {
+ return;
+ }
+ lambdaRewriter
+ .getKnownLambdaClasses()
+ .forEach(
+ (type, lambda) -> {
+ DexProgramClass synthesizedClass = getProgramClassOrNull(type);
+ assert synthesizedClass != null;
+ assert liveTypes.contains(synthesizedClass);
+ if (synthesizedClass == null) {
+ return;
+ }
+ DexMethod method = lambda.descriptor.getMainMethod();
+ if (!liveMethods.contains(synthesizedClass.lookupMethod(method))) {
+ return;
+ }
+ DexEncodedMethod accessor = lambda.target.ensureAccessibilityIfNeeded(false);
+ if (accessor == null) {
+ return;
+ }
+ DexProgramClass accessorClass = getProgramClassOrNull(accessor.holder());
+ assert accessorClass != null;
+ if (accessorClass != null) {
+ liveMethods.add(accessorClass, accessor, graphReporter.fakeReportShouldNotBeUsed());
+ }
+ });
+ }
+
private boolean verifyReferences(DexApplication app) {
WorkList<DexClass> worklist = WorkList.newIdentityWorkList();
for (DexProgramClass clazz : liveTypes.getItems()) {
@@ -3668,12 +3697,16 @@
if (clazz != null && clazz.isInterface()) {
// Add this interface to the set of pinned items to ensure that we do not merge the
// interface into its unique subtype, if any.
+ // TODO(b/145344105): This should be superseded by the unknown interface hierarchy.
pinnedItems.add(clazz.type);
+ KeepReason reason = KeepReason.reflectiveUseIn(method);
+ markInterfaceAsInstantiated(clazz, graphReporter.registerClass(clazz, reason));
// Also pin all of its virtual methods to ensure that the devirtualizer does not perform
// illegal rewritings of invoke-interface instructions into invoke-virtual instructions.
for (DexEncodedMethod virtualMethod : clazz.virtualMethods()) {
pinnedItems.add(virtualMethod.method);
+ markVirtualMethodAsReachable(virtualMethod.method, true, null, reason);
}
}
}
diff --git a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
index b8256fe..a642002 100644
--- a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
@@ -102,7 +102,7 @@
.withOptionConsumer(opts -> opts.enableClassInlining = false)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 116, "lambdadesugaring"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 115, "lambdadesugaring"))
.run();
test("lambdadesugaring", "lambdadesugaring", "LambdaDesugaring")
@@ -142,7 +142,7 @@
.withOptionConsumer(opts -> opts.enableClassInlining = false)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 116, "lambdadesugaring"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 115, "lambdadesugaring"))
.run();
test("lambdadesugaring", "lambdadesugaring", "LambdaDesugaring")
diff --git a/src/test/java/com/android/tools/r8/classmerging/InterfaceWithProxyTest.java b/src/test/java/com/android/tools/r8/classmerging/InterfaceWithProxyTest.java
index 03f5803..ba16ddc 100644
--- a/src/test/java/com/android/tools/r8/classmerging/InterfaceWithProxyTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/InterfaceWithProxyTest.java
@@ -18,6 +18,8 @@
@RunWith(Parameterized.class)
public class InterfaceWithProxyTest extends TestBase {
+ private static final String EXPECTED = StringUtils.lines("Hello world!");
+
private final TestParameters parameters;
@Parameterized.Parameters(name = "{0}")
@@ -30,8 +32,15 @@
}
@Test
- public void test() throws Exception {
- String expectedOutput = StringUtils.lines("Hello world!");
+ public void testReference() throws Exception {
+ testForRuntime(parameters)
+ .addInnerClasses(InterfaceWithProxyTest.class)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test
+ public void testR8() throws Exception {
testForR8(parameters.getBackend())
.addInnerClasses(InterfaceWithProxyTest.class)
.addKeepMainRule(TestClass.class)
@@ -39,7 +48,7 @@
.enableInliningAnnotations()
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutput(expectedOutput);
+ .assertSuccessWithOutput(EXPECTED);
}
static class TestClass {
diff --git a/src/test/java/com/android/tools/r8/desugar/DefaultLambdaWithSelfReferenceTestRunner.java b/src/test/java/com/android/tools/r8/desugar/DefaultLambdaWithSelfReferenceTestRunner.java
index e209e44..7a97972 100644
--- a/src/test/java/com/android/tools/r8/desugar/DefaultLambdaWithSelfReferenceTestRunner.java
+++ b/src/test/java/com/android/tools/r8/desugar/DefaultLambdaWithSelfReferenceTestRunner.java
@@ -6,19 +6,21 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assume.assumeTrue;
import com.android.tools.r8.D8TestCompileResult;
import com.android.tools.r8.Disassemble;
import com.android.tools.r8.Disassemble.DisassembleCommand;
import com.android.tools.r8.JvmTestBuilder;
import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.debug.DebugTestBase;
import com.android.tools.r8.debug.DebugTestBase.JUnit3Wrapper.Command;
import com.android.tools.r8.debug.DebugTestConfig;
import com.android.tools.r8.references.MethodReference;
import com.android.tools.r8.references.Reference;
-import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.ImmutableList;
import java.nio.file.Files;
@@ -28,57 +30,85 @@
import java.util.Collections;
import java.util.List;
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 DefaultLambdaWithSelfReferenceTestRunner extends DebugTestBase {
- final Class<?> CLASS = DefaultLambdaWithSelfReferenceTest.class;
- final String EXPECTED = StringUtils.lines("stateful(stateless)");
+ private static final Class<?> CLASS = DefaultLambdaWithSelfReferenceTest.class;
+ private static final String EXPECTED = StringUtils.lines("stateful(stateless)");
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public DefaultLambdaWithSelfReferenceTestRunner(TestParameters parameters) {
+ this.parameters = parameters;
+ }
private void runDebugger(DebugTestConfig config) throws Throwable {
MethodReference main = Reference.methodFromMethod(CLASS.getMethod("main", String[].class));
- Command checkThis = conditional((state) ->
- state.isCfRuntime()
- ? Collections.singletonList(checkLocal("this"))
- : ImmutableList.of(
- checkNoLocal("this"),
- checkLocal("_this")));
+ Command checkThisLambda =
+ conditional(
+ (state) ->
+ parameters.isCfRuntime()
+ ? Collections.singletonList(checkLocal("this"))
+ : ImmutableList.of(checkNoLocal("this"), checkLocal("_this")));
- runDebugTest(config, CLASS,
+ Command checkThisDefaultMethod =
+ conditional(
+ (state) ->
+ parameters.canUseDefaultAndStaticInterfaceMethods()
+ ? Collections.singletonList(checkLocal("this"))
+ : ImmutableList.of(checkNoLocal("this"), checkLocal("_this")));
+
+ runDebugTest(
+ config,
+ CLASS,
breakpoint(main, 26),
run(),
checkLine(26),
stepInto(INTELLIJ_FILTER),
checkLine(16),
// When desugaring, the InterfaceProcessor makes this static on the companion class.
- checkThis,
+ checkThisDefaultMethod,
breakpoint(main, 27),
run(),
checkLine(27),
stepInto(INTELLIJ_FILTER),
checkLine(17),
// When desugaring, the LambdaClass will change this to a static (later moved to companion).
- checkThis,
+ checkThisLambda,
run());
}
@Test
public void testJvm() throws Throwable {
+ assumeTrue(parameters.isCfRuntime());
JvmTestBuilder builder = testForJvm().addTestClasspath();
- builder.run(CLASS).assertSuccessWithOutput(EXPECTED);
+ builder.run(parameters.getRuntime(), CLASS).assertSuccessWithOutput(EXPECTED);
runDebugger(builder.debugConfig());
}
@Test
- public void testR8Cf() throws Throwable {
- R8TestCompileResult compileResult = testForR8(Backend.CF)
- .addProgramClassesAndInnerClasses(CLASS)
- .noMinification()
- .noTreeShaking()
- .debug()
- .compile();
+ public void testR8() throws Throwable {
+ R8TestCompileResult compileResult =
+ testForR8(parameters.getBackend())
+ .addProgramClassesAndInnerClasses(CLASS)
+ .setMinApi(parameters.getApiLevel())
+ .noMinification()
+ .noTreeShaking()
+ .addKeepAllAttributes()
+ .debug()
+ .compile()
+ .assertNoMessages();
compileResult
- // TODO(b/123506120): Add .assertNoMessages()
- .run(CLASS)
+ .run(parameters.getRuntime(), CLASS)
.assertSuccessWithOutput(EXPECTED)
.inspect(inspector -> assertThat(inspector.clazz(CLASS), isPresent()));
runDebugger(compileResult.debugConfig());
@@ -86,14 +116,15 @@
@Test
public void testD8() throws Throwable {
+ assumeTrue(parameters.isDexRuntime());
Path out1 = temp.newFolder().toPath().resolve("out1.zip");
testForD8()
.addProgramClassesAndInnerClasses(CLASS)
- .setMinApi(AndroidApiLevel.K)
+ .setMinApi(parameters.getApiLevel())
.compile()
- // TODO(b/123506120): Add .assertNoMessages()
+ .assertNoMessages()
.writeToZip(out1)
- .run(CLASS)
+ .run(parameters.getRuntime(), CLASS)
.assertSuccessWithOutput(EXPECTED);
Path outPerClassDir = temp.newFolder().toPath();
@@ -109,9 +140,9 @@
.addProgramClasses(CLASS)
.addClasspathFiles(ToolHelper.getClassPathForTests())
.setIntermediate(true)
- .setMinApi(AndroidApiLevel.K)
+ .setMinApi(parameters.getApiLevel())
.compile()
- // TODO(b/123506120): Add .assertNoMessages()
+ .assertNoMessages()
.writeToZip(mainOut);
}
for (Path innerClass : innerClasses) {
@@ -121,21 +152,20 @@
.addProgramFiles(innerClass)
.addClasspathFiles(ToolHelper.getClassPathForTests())
.setIntermediate(true)
- .setMinApi(AndroidApiLevel.K)
+ .setMinApi(parameters.getApiLevel())
.compile()
- // TODO(b/123506120): Add .assertNoMessages()
+ .assertNoMessages()
.writeToZip(out);
}
Path out2 = temp.newFolder().toPath().resolve("out2.zip");
- D8TestCompileResult compiledResult = testForD8()
- .addProgramFiles(outs)
- .compile();
+ D8TestCompileResult compiledResult =
+ testForD8().addProgramFiles(outs).setMinApi(parameters.getApiLevel()).compile();
compiledResult
- // TODO(b/123506120): Add .assertNoMessages()
+ .assertNoMessages()
.writeToZip(out2)
- .run(CLASS)
+ .run(parameters.getRuntime(), CLASS)
.assertSuccessWithOutput(EXPECTED);
runDebugger(compiledResult.debugConfig());
diff --git a/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java b/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
index 530b5b5..7bc8279 100644
--- a/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
+++ b/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.graph.AccessFlags;
+import com.android.tools.r8.graph.ClassAccessFlags;
import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.references.ClassReference;
@@ -228,6 +229,25 @@
});
}
+ public ClassFileTransformer setAccessFlags(Consumer<ClassAccessFlags> fn) {
+ return addClassTransformer(
+ new ClassTransformer() {
+ @Override
+ public void visit(
+ int version,
+ int access,
+ String name,
+ String signature,
+ String superName,
+ String[] interfaces) {
+ ClassAccessFlags flags = ClassAccessFlags.fromCfAccessFlags(access);
+ fn.accept(flags);
+ super.visit(
+ version, flags.getAsCfAccessFlags(), name, signature, superName, interfaces);
+ }
+ });
+ }
+
public ClassFileTransformer setNest(Class<?> host, Class<?>... members) {
assert !Arrays.asList(members).contains(host);
return setMinVersion(CfVm.JDK11)
