Move utilities for looking up virtual targets to resolution result.
Bug: 140016938
Change-Id: I9d0a46b5d4440c99df1401b0a8e6db3f85d5db64
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfo.java b/src/main/java/com/android/tools/r8/graph/AppInfo.java
index 543326a..393a4fd 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -9,10 +9,12 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMap.Builder;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
@@ -299,6 +301,9 @@
*/
public ResolutionResult resolveMethodOnClass(DexType holder, DexMethod method) {
assert checkIfObsolete();
+ if (holder.isArrayType()) {
+ return resolveMethodOnArray(holder, method);
+ }
DexClass clazz = definitionFor(holder);
// Step 1: If holder is an interface, resolution fails with an ICCE. We return null.
if (clazz == null || clazz.isInterface()) {
@@ -605,6 +610,96 @@
List<DexEncodedMethod> asListOfTargets();
void forEachTarget(Consumer<DexEncodedMethod> consumer);
+
+ default Set<DexEncodedMethod> lookupVirtualTargets(AppInfoWithSubtyping appInfo) {
+ // TODO(b/140016938): Don't allow this lookup on non-virtual resolutions.
+ // First add the target for receiver type method.type.
+ Set<DexEncodedMethod> result = Sets.newIdentityHashSet();
+ forEachTarget(result::add);
+ // Add all matching targets from the subclass hierarchy.
+ DexEncodedMethod encodedMethod = asResultOfResolve();
+ DexMethod method = encodedMethod.method;
+ for (DexType type : appInfo.subtypes(method.holder)) {
+ DexClass clazz = appInfo.definitionFor(type);
+ if (!clazz.isInterface()) {
+ ResolutionResult methods = appInfo.resolveMethodOnClass(type, method);
+ methods.forEachTarget(
+ target -> {
+ if (target.isVirtualMethod()) {
+ result.add(target);
+ }
+ });
+ }
+ }
+ return result;
+ }
+
+ default Set<DexEncodedMethod> lookupInterfaceTargets(AppInfoWithSubtyping appInfo) {
+ // TODO(b/140016938): Don't allow this lookup on non-virtual resolutions.
+ Set<DexEncodedMethod> result = Sets.newIdentityHashSet();
+ if (hasSingleTarget()) {
+ // Add default interface methods to the list of targets.
+ //
+ // This helps to make sure we take into account synthesized lambda classes
+ // that we are not aware of. Like in the following example, we know that all
+ // classes, XX in this case, override B::bar(), but there are also synthesized
+ // classes for lambda which don't, so we still need default method to be live.
+ //
+ // public static void main(String[] args) {
+ // X x = () -> {};
+ // x.bar();
+ // }
+ //
+ // interface X {
+ // void foo();
+ // default void bar() { }
+ // }
+ //
+ // class XX implements X {
+ // public void foo() { }
+ // public void bar() { }
+ // }
+ //
+ DexEncodedMethod singleTarget = asSingleTarget();
+ if (singleTarget.getCode() != null
+ && appInfo.hasAnyInstantiatedLambdas(singleTarget.method.holder)) {
+ result.add(singleTarget);
+ }
+ }
+
+ DexEncodedMethod encodedMethod = asResultOfResolve();
+ DexMethod method = encodedMethod.method;
+ Consumer<DexEncodedMethod> addIfNotAbstract =
+ m -> {
+ if (!m.accessFlags.isAbstract()) {
+ result.add(m);
+ }
+ };
+ // Default methods are looked up when looking at a specific subtype that does not override
+ // them.
+ // Otherwise, we would look up default methods that are actually never used. However, we have
+ // to
+ // add bridge methods, otherwise we can remove a bridge that will be used.
+ Consumer<DexEncodedMethod> addIfNotAbstractAndBridge =
+ m -> {
+ if (!m.accessFlags.isAbstract() && m.accessFlags.isBridge()) {
+ result.add(m);
+ }
+ };
+
+ Set<DexType> set = appInfo.subtypes(method.holder);
+ for (DexType type : set) {
+ DexClass clazz = appInfo.definitionFor(type);
+ if (clazz.isInterface()) {
+ ResolutionResult targetMethods = appInfo.resolveMethodOnInterface(type, method);
+ targetMethods.forEachTarget(addIfNotAbstractAndBridge);
+ } else {
+ ResolutionResult targetMethods = appInfo.resolveMethodOnClass(type, method);
+ targetMethods.forEachTarget(addIfNotAbstract);
+ }
+ }
+ return result;
+ }
}
private static class MultiResultBuilder {
@@ -706,5 +801,15 @@
public void forEachTarget(Consumer<DexEncodedMethod> consumer) {
// Intentionally left empty.
}
+
+ @Override
+ public Set<DexEncodedMethod> lookupVirtualTargets(AppInfoWithSubtyping appInfo) {
+ return null;
+ }
+
+ @Override
+ public Set<DexEncodedMethod> lookupInterfaceTargets(AppInfoWithSubtyping appInfo) {
+ return null;
+ }
}
}
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
index e143dee..82dbe10 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
@@ -254,43 +254,6 @@
return true;
}
- // For mapping invoke virtual instruction to target methods.
- public Set<DexEncodedMethod> lookupVirtualTargets(DexMethod method) {
- assert checkIfObsolete();
- if (method.holder.isArrayType()) {
- // For javac output this will only be clone(), but in general the methods from Object can
- // be invoked with an array type holder.
- return null;
- }
- DexClass root = definitionFor(method.holder);
- if (root == null) {
- // type specified in method does not have a materialized class.
- return null;
- }
- ResolutionResult topTargets = resolveMethodOnClass(method.holder, method);
- if (topTargets.asResultOfResolve() == null) {
- // This will fail at runtime.
- return null;
- }
- // First add the target for receiver type method.type.
- Set<DexEncodedMethod> result = new HashSet<>();
- topTargets.forEachTarget(result::add);
- // Add all matching targets from the subclass hierarchy.
- for (DexType type : subtypes(method.holder)) {
- DexClass clazz = definitionFor(type);
- if (!clazz.isInterface()) {
- ResolutionResult methods = resolveMethodOnClass(type, method);
- methods.forEachTarget(
- target -> {
- if (target.isVirtualMethod()) {
- result.add(target);
- }
- });
- }
- }
- return result;
- }
-
/**
* Lookup super method following the super chain from the holder of {@code method}.
*
@@ -363,76 +326,6 @@
return false;
}
- // For mapping invoke interface instruction to target methods.
- public Set<DexEncodedMethod> lookupInterfaceTargets(DexMethod method) {
- assert checkIfObsolete();
- // First check that there is a target for this invoke-interface to hit. If there is none,
- // this will fail at runtime.
- ResolutionResult topTarget = resolveMethodOnInterface(method.holder, method);
- if (topTarget.asResultOfResolve() == null) {
- return null;
- }
-
- Set<DexEncodedMethod> result = new HashSet<>();
- if (topTarget.hasSingleTarget()) {
- // Add default interface methods to the list of targets.
- //
- // This helps to make sure we take into account synthesized lambda classes
- // that we are not aware of. Like in the following example, we know that all
- // classes, XX in this case, override B::bar(), but there are also synthesized
- // classes for lambda which don't, so we still need default method to be live.
- //
- // public static void main(String[] args) {
- // X x = () -> {};
- // x.bar();
- // }
- //
- // interface X {
- // void foo();
- // default void bar() { }
- // }
- //
- // class XX implements X {
- // public void foo() { }
- // public void bar() { }
- // }
- //
- DexEncodedMethod singleTarget = topTarget.asSingleTarget();
- if (singleTarget.getCode() != null && hasAnyInstantiatedLambdas(singleTarget.method.holder)) {
- result.add(singleTarget);
- }
- }
-
- Consumer<DexEncodedMethod> addIfNotAbstract =
- m -> {
- if (!m.accessFlags.isAbstract()) {
- result.add(m);
- }
- };
- // Default methods are looked up when looking at a specific subtype that does not override them.
- // Otherwise, we would look up default methods that are actually never used. However, we have to
- // add bridge methods, otherwise we can remove a bridge that will be used.
- Consumer<DexEncodedMethod> addIfNotAbstractAndBridge =
- m -> {
- if (!m.accessFlags.isAbstract() && m.accessFlags.isBridge()) {
- result.add(m);
- }
- };
-
- Set<DexType> set = subtypes(method.holder);
- for (DexType type : set) {
- DexClass clazz = definitionFor(type);
- if (clazz.isInterface()) {
- ResolutionResult targetMethods = resolveMethodOnInterface(type, method);
- targetMethods.forEachTarget(addIfNotAbstractAndBridge);
- } else {
- ResolutionResult targetMethods = resolveMethodOnClass(type, method);
- targetMethods.forEachTarget(addIfNotAbstract);
- }
- }
- return result;
- }
-
/**
* Resolve the methods implemented by the lambda expression that created the {@code callSite}.
*
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 8bd6e4b..5a223ac 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -1855,5 +1855,4 @@
checkIfObsolete();
consumer.accept(this);
}
-
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeInterface.java b/src/main/java/com/android/tools/r8/ir/code/InvokeInterface.java
index 6a6d5e3..45716d0 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeInterface.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeInterface.java
@@ -100,7 +100,11 @@
@Override
public Collection<DexEncodedMethod> lookupTargets(
AppView<? extends AppInfoWithSubtyping> appView, DexType invocationContext) {
- return appView.appInfo().lookupInterfaceTargets(getInvokedMethod());
+ DexMethod method = getInvokedMethod();
+ return appView
+ .appInfo()
+ .resolveMethodOnInterface(method.holder, method)
+ .lookupInterfaceTargets(appView.appInfo());
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java b/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java
index a57c585..8a9caba 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java
@@ -103,7 +103,11 @@
@Override
public Collection<DexEncodedMethod> lookupTargets(
AppView<? extends AppInfoWithSubtyping> appView, DexType invocationContext) {
- return appView.appInfo().lookupVirtualTargets(getInvokedMethod());
+ DexMethod method = getInvokedMethod();
+ return appView
+ .appInfo()
+ .resolveMethodOnClass(method.holder, method)
+ .lookupVirtualTargets(appView.appInfo());
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilder.java
index d2aa749..dfaa803 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilder.java
@@ -189,8 +189,14 @@
target,
method ->
type == Type.INTERFACE
- ? appView.appInfo().lookupInterfaceTargets(method)
- : appView.appInfo().lookupVirtualTargets(method));
+ ? appView
+ .appInfo()
+ .resolveMethodOnInterface(method.holder, method)
+ .lookupInterfaceTargets(appView.appInfo())
+ : appView
+ .appInfo()
+ .resolveMethodOnClass(method.holder, method)
+ .lookupVirtualTargets(appView.appInfo()));
if (possibleTargets != null) {
boolean likelySpuriousCallEdge =
possibleTargets.size() >= appView.options().callGraphLikelySpuriousCallEdgeThreshold;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
index 8dc2df2..9e69123 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
@@ -163,7 +163,12 @@
public ConstraintWithTarget forInvokeInterface(DexMethod method, DexType invocationContext) {
DexMethod lookup = graphLense.lookupMethod(method);
return forVirtualInvoke(
- lookup, appView.appInfo().lookupInterfaceTargets(lookup), invocationContext);
+ lookup,
+ appView
+ .appInfo()
+ .resolveMethodOnInterface(lookup.holder, lookup)
+ .lookupInterfaceTargets(appView.appInfo()),
+ invocationContext);
}
public ConstraintWithTarget forInvokeMultiNewArray(DexType type, DexType invocationContext) {
@@ -192,7 +197,12 @@
public ConstraintWithTarget forInvokeVirtual(DexMethod method, DexType invocationContext) {
DexMethod lookup = graphLense.lookupMethod(method);
return forVirtualInvoke(
- lookup, appView.appInfo().lookupVirtualTargets(lookup), invocationContext);
+ lookup,
+ appView
+ .appInfo()
+ .resolveMethodOnClass(lookup.holder, lookup)
+ .lookupVirtualTargets(appView.appInfo()),
+ invocationContext);
}
public ConstraintWithTarget forJumpInstruction() {
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 90dbcff5..ac9f78e 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1623,12 +1623,11 @@
return;
}
- // Lookup the possible targets starting from the declared type.
assert interfaceInvoke == holder.isInterface();
Set<DexEncodedMethod> possibleTargets =
holder.isInterface()
- ? appInfo.lookupInterfaceTargets(method)
- : appInfo.lookupVirtualTargets(method);
+ ? resolutionTarget.lookupInterfaceTargets(appInfo)
+ : resolutionTarget.lookupVirtualTargets(appInfo);
if (possibleTargets == null || possibleTargets.isEmpty()) {
return;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 7892482..1dca48a 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -753,7 +753,10 @@
// }
for (DexEncodedMethod method : defaultMethods) {
// Conservatively find all possible targets for this method.
- Set<DexEncodedMethod> interfaceTargets = appInfo.lookupInterfaceTargets(method.method);
+ Set<DexEncodedMethod> interfaceTargets =
+ appInfo
+ .resolveMethodOnInterface(method.method.holder, method.method)
+ .lookupInterfaceTargets(appInfo);
// If [method] is not even an interface-target, then we can safely merge it. Otherwise we
// need to check for a conflict.
diff --git a/src/test/java/com/android/tools/r8/internal/R8GMSCoreLookupTest.java b/src/test/java/com/android/tools/r8/internal/R8GMSCoreLookupTest.java
index b451b48..e9e836b 100644
--- a/src/test/java/com/android/tools/r8/internal/R8GMSCoreLookupTest.java
+++ b/src/test/java/com/android/tools/r8/internal/R8GMSCoreLookupTest.java
@@ -59,12 +59,18 @@
assertEquals(appInfo.lookupVirtualTarget(id.holder, method.method), method);
// Check lookup targets with include method.
- Set<DexEncodedMethod> targets = appInfo.lookupVirtualTargets(method.method);
+ Set<DexEncodedMethod> targets =
+ appInfo
+ .resolveMethodOnClass(method.method.holder, method.method)
+ .lookupVirtualTargets(appInfo);
assertTrue(targets.contains(method));
}
private void testInterfaceLookup(DexProgramClass clazz, DexEncodedMethod method) {
- Set<DexEncodedMethod> targets = appInfo.lookupInterfaceTargets(method.method);
+ Set<DexEncodedMethod> targets =
+ appInfo
+ .resolveMethodOnInterface(method.method.holder, method.method)
+ .lookupInterfaceTargets(appInfo);
if (appInfo.subtypes(method.method.holder).stream()
.allMatch(t -> appInfo.definitionFor(t).isInterface())) {
assertEquals(
diff --git a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
index 47aa023..11265a9 100644
--- a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
@@ -247,7 +247,8 @@
DexMethod method = buildMethod(invokeReceiver, methodName, appInfo);
Assert.assertNotNull(
appInfo.resolveMethod(toType(invokeReceiver, appInfo), method).asResultOfResolve());
- Set<DexEncodedMethod> targets = appInfo.lookupVirtualTargets(method);
+ Set<DexEncodedMethod> targets =
+ appInfo.resolveMethod(method.holder, method).lookupVirtualTargets(appInfo);
Set<DexType> targetHolders = targets.stream().map(m -> m.method.holder)
.collect(Collectors.toSet());
Assert.assertEquals(allTargetHolders.size(), targetHolders.size());
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java
index 8b71aaf..c565a43 100644
--- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper.DexVm;
+import com.android.tools.r8.graph.AppInfo.ResolutionResult;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
@@ -92,13 +93,13 @@
@Test
public void lookupVirtualTargets() {
- DexEncodedMethod resolved =
- appInfo.resolveMethod(methodOnB.holder, methodOnB).asResultOfResolve();
+ ResolutionResult resolutionResult = appInfo.resolveMethod(methodOnB.holder, methodOnB);
+ DexEncodedMethod resolved = resolutionResult.asResultOfResolve();
assertEquals(methodOnA, resolved.method);
// This behavior is up for debate as the resolution target is A.f which is private static, thus
// the runtime behavior will be to throw a runtime exception. Thus it would be reasonable to
// return the empty set as no targets can actually be hit at runtime.
- Set<DexEncodedMethod> targets = appInfo.lookupVirtualTargets(methodOnB);
+ Set<DexEncodedMethod> targets = resolutionResult.lookupVirtualTargets(appInfo);
assertTrue("Expected " + methodOnA, targets.stream().anyMatch(m -> m.method == methodOnA));
assertTrue("Expected " + methodOnC, targets.stream().anyMatch(m -> m.method == methodOnC));
}
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java
index b7b328e..0ae734d 100644
--- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.TestRunResult;
import com.android.tools.r8.TestRuntime;
import com.android.tools.r8.ToolHelper.DexVm;
+import com.android.tools.r8.graph.AppInfo.ResolutionResult;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
@@ -142,11 +143,11 @@
@Test
public void lookupVirtualTargets() {
- DexEncodedMethod resolved =
- appInfo.resolveMethod(methodOnB.holder, methodOnB).asResultOfResolve();
+ ResolutionResult resolutionResult = appInfo.resolveMethod(methodOnB.holder, methodOnB);
+ DexEncodedMethod resolved = resolutionResult.asResultOfResolve();
assertEquals(methodOnA, resolved.method);
// See comment in VirtualOverrideOfPrivateStaticMethodTest.lookupVirtualTargets().
- Set<DexEncodedMethod> targets = appInfo.lookupVirtualTargets(methodOnB);
+ Set<DexEncodedMethod> targets = resolutionResult.lookupVirtualTargets(appInfo);
assertTrue("Expected " + methodOnA, targets.stream().anyMatch(m -> m.method == methodOnA));
assertTrue("Expected " + methodOnC, targets.stream().anyMatch(m -> m.method == methodOnC));
}
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
index d93d684..f7b72e9 100644
--- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.TestRunResult;
import com.android.tools.r8.TestRuntime;
import com.android.tools.r8.ToolHelper.DexVm;
+import com.android.tools.r8.graph.AppInfo.ResolutionResult;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
@@ -176,11 +177,11 @@
@Test
public void lookupVirtualTargets() {
- DexEncodedMethod resolved =
- appInfo.resolveMethod(methodOnB.holder, methodOnB).asResultOfResolve();
+ ResolutionResult resolutionResult = appInfo.resolveMethod(methodOnB.holder, methodOnB);
+ DexEncodedMethod resolved = resolutionResult.asResultOfResolve();
assertEquals(methodOnA, resolved.method);
// See comment in VirtualOverrideOfPrivateStaticMethodTest.lookupVirtualTargets().
- Set<DexEncodedMethod> targets = appInfo.lookupVirtualTargets(methodOnB);
+ Set<DexEncodedMethod> targets = resolutionResult.lookupVirtualTargets(appInfo);
assertTrue("Expected " + methodOnA, targets.stream().anyMatch(m -> m.method == methodOnA));
assertTrue("Expected " + methodOnC, targets.stream().anyMatch(m -> m.method == methodOnC));
}
