Refactor helpers for obtaining the maximally specific interface method.
Change-Id: Ifa226c7685ca777acd03df0bb5f9578c9fc634a8
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 141f742..eb972ca 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMap.Builder;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
@@ -402,16 +403,32 @@
* 5.4.3.3 of the JVM Spec</a>. As this is the same for interfaces and classes, we share one
* implementation.
*/
- ResolutionResult resolveMethodStep3(DexClass clazz, DexMethod method) {
- MaximallySpecificMethodsBuilder builder = new MaximallySpecificMethodsBuilder(clazz);
- resolveMethodStep3Helper(clazz, method, builder);
- return builder.resolve();
+ private ResolutionResult resolveMethodStep3(DexClass clazz, DexMethod method) {
+ MaximallySpecificMethodsBuilder builder = new MaximallySpecificMethodsBuilder();
+ resolveMethodStep3Helper(method, clazz, builder);
+ return builder.resolve(clazz);
+ }
+
+ // Non-private lookup (ie, not resolution) to find interface targets.
+ DexClassAndMethod lookupMaximallySpecificTarget(DexClass clazz, DexMethod method) {
+ MaximallySpecificMethodsBuilder builder = new MaximallySpecificMethodsBuilder();
+ resolveMethodStep3Helper(method, clazz, builder);
+ return builder.lookup();
}
/** Helper method that builds the set of maximally specific methods. */
private void resolveMethodStep3Helper(
- DexClass clazz, DexMethod method, MaximallySpecificMethodsBuilder builder) {
- for (DexType iface : clazz.interfaces.values) {
+ DexMethod method, DexClass clazz, MaximallySpecificMethodsBuilder builder) {
+ resolveMethodStep3Helper(
+ method, clazz.superType, Arrays.asList(clazz.interfaces.values), builder);
+ }
+
+ private void resolveMethodStep3Helper(
+ DexMethod method,
+ DexType superType,
+ List<DexType> interfaces,
+ MaximallySpecificMethodsBuilder builder) {
+ for (DexType iface : interfaces) {
DexClass definiton = definitionFor(iface);
if (definiton == null) {
// Ignore missing interface definitions.
@@ -424,14 +441,14 @@
builder.addCandidate(definiton, result, this);
} else {
// Look at the super-interfaces of this class and keep searching.
- resolveMethodStep3Helper(definiton, method, builder);
+ resolveMethodStep3Helper(method, definiton, builder);
}
}
// Now look at indirect super interfaces.
- if (clazz.superType != null) {
- DexClass superClass = definitionFor(clazz.superType);
+ if (superType != null) {
+ DexClass superClass = definitionFor(superType);
if (superClass != null) {
- resolveMethodStep3Helper(superClass, method, builder);
+ resolveMethodStep3Helper(method, superClass, builder);
}
}
}
@@ -579,8 +596,6 @@
private static class MaximallySpecificMethodsBuilder {
- private final DexClass initialResolutionHolder;
-
// The set of actual maximally specific methods.
// This set is linked map so that in the case where a number of methods remain a deterministic
// choice can be made. The map is from definition classes to their maximally specific method, or
@@ -589,10 +604,6 @@
// prior to writing.
LinkedHashMap<DexClass, DexEncodedMethod> maximallySpecificMethods = new LinkedHashMap<>();
- public MaximallySpecificMethodsBuilder(DexClass initialResolutionHolder) {
- this.initialResolutionHolder = initialResolutionHolder;
- }
-
void addCandidate(DexClass holder, DexEncodedMethod method, AppInfo appInfo) {
// If this candidate is already a candidate or it is shadowed, then no need to continue.
if (maximallySpecificMethods.containsKey(holder)) {
@@ -629,16 +640,26 @@
}
}
- ResolutionResult resolve() {
+ DexClassAndMethod lookup() {
+ SingleResolutionResult result = internalResolve(null).asSingleResolution();
+ return result != null
+ ? DexClassAndMethod.create(result.getResolvedHolder(), result.getResolvedMethod())
+ : null;
+ }
+
+ ResolutionResult resolve(DexClass initialResolutionHolder) {
+ assert initialResolutionHolder != null;
+ return internalResolve(initialResolutionHolder);
+ }
+
+ private ResolutionResult internalResolve(DexClass initialResolutionHolder) {
if (maximallySpecificMethods.isEmpty()) {
return NoSuchMethodResult.INSTANCE;
}
// Fast path in the common case of a single method.
if (maximallySpecificMethods.size() == 1) {
- Entry<DexClass, DexEncodedMethod> first =
- maximallySpecificMethods.entrySet().iterator().next();
- return new SingleResolutionResult(
- initialResolutionHolder, first.getKey(), first.getValue());
+ return singleResultHelper(
+ initialResolutionHolder, maximallySpecificMethods.entrySet().iterator().next());
}
Entry<DexClass, DexEncodedMethod> firstMaximallySpecificMethod = null;
List<Entry<DexClass, DexEncodedMethod>> nonAbstractMethods =
@@ -659,21 +680,24 @@
// If there are no non-abstract methods, then any candidate will suffice as a target.
// For deterministic resolution, we return the first mapped method (of the linked map).
if (nonAbstractMethods.isEmpty()) {
- return new SingleResolutionResult(
- initialResolutionHolder,
- firstMaximallySpecificMethod.getKey(),
- firstMaximallySpecificMethod.getValue());
+ return singleResultHelper(initialResolutionHolder, firstMaximallySpecificMethod);
}
// If there is exactly one non-abstract method (a default method) it is the resolution target.
if (nonAbstractMethods.size() == 1) {
- Entry<DexClass, DexEncodedMethod> entry = nonAbstractMethods.get(0);
- return new SingleResolutionResult(
- initialResolutionHolder, entry.getKey(), entry.getValue());
+ return singleResultHelper(initialResolutionHolder, nonAbstractMethods.get(0));
}
return IncompatibleClassResult.create(ListUtils.map(nonAbstractMethods, Entry::getValue));
}
}
+ private static SingleResolutionResult singleResultHelper(
+ DexClass initialResolutionResult, Entry<DexClass, DexEncodedMethod> entry) {
+ return new SingleResolutionResult(
+ initialResolutionResult != null ? initialResolutionResult : entry.getKey(),
+ entry.getKey(),
+ entry.getValue());
+ }
+
// TODO(b/149190785): Remove once fixed.
public void enableDefinitionForAssert() {}
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
index cefb55a..e0e8269 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
@@ -7,7 +7,6 @@
import static com.android.tools.r8.utils.TraversalContinuation.BREAK;
import static com.android.tools.r8.utils.TraversalContinuation.CONTINUE;
-import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult;
import com.android.tools.r8.utils.TraversalContinuation;
import com.google.common.collect.Sets;
import java.util.ArrayDeque;
@@ -207,16 +206,8 @@
* Helper method used for emulated interface resolution (not in JVM specifications). The result
* may be abstract.
*/
- public ResolutionResult resolveMaximallySpecificMethods(DexClass clazz, DexMethod method) {
- assert !clazz.type.isArrayType();
- if (clazz.isInterface()) {
- // Look for exact method on interface.
- DexEncodedMethod result = clazz.lookupMethod(method);
- if (result != null) {
- return new SingleResolutionResult(clazz, clazz, result);
- }
- }
- return resolveMethodStep3(clazz, method);
+ public DexClassAndMethod lookupMaximallySpecificMethod(DexClass clazz, DexMethod method) {
+ return lookupMaximallySpecificTarget(clazz, method);
}
/**
diff --git a/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java b/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java
index 281b58a..ded00bd 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java
@@ -11,7 +11,7 @@
private final DexClass holder;
private final DexEncodedMethod method;
- protected DexClassAndMethod(DexClass holder, DexEncodedMethod method) {
+ DexClassAndMethod(DexClass holder, DexEncodedMethod method) {
assert holder.type == method.method.holder;
this.holder = holder;
this.method = method;
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 1002cd5..4c5e239 100644
--- a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
@@ -286,7 +286,10 @@
}
// 4. Otherwise, it is the single maximally specific method:
if (target == null) {
- target = appInfo.resolveMaximallySpecificMethods(initialType, method).getSingleTarget();
+ DexClassAndMethod result = appInfo.lookupMaximallySpecificMethod(initialType, method);
+ if (result != null) {
+ target = result.getMethod();
+ }
}
if (target == null) {
return null;
@@ -439,22 +442,14 @@
if (!resolvedHolder.isInterface()) {
return null;
}
- DexEncodedMethod maximalSpecific =
- lookupMaximallySpecificDispatchTarget(dynamicInstance, appView);
- return maximalSpecific == null
- ? null
- : DexClassAndMethod.create(
- appView.definitionFor(maximalSpecific.method.holder), maximalSpecific);
+ return lookupMaximallySpecificDispatchTarget(dynamicInstance, appView);
}
- private DexEncodedMethod lookupMaximallySpecificDispatchTarget(
+ private DexClassAndMethod lookupMaximallySpecificDispatchTarget(
DexProgramClass dynamicInstance, AppView<? extends AppInfoWithClassHierarchy> appView) {
- ResolutionResult maximallySpecificResult =
- appView.appInfo().resolveMaximallySpecificMethods(dynamicInstance, resolvedMethod.method);
- if (maximallySpecificResult.isSingleResolution()) {
- return maximallySpecificResult.asSingleResolution().resolvedMethod;
- }
- return null;
+ return appView
+ .appInfo()
+ .lookupMaximallySpecificMethod(dynamicInstance, resolvedMethod.method);
}
/**
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java b/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
index 2f54714..76e97df 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexAnnotationSet;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexLibraryClass;
@@ -319,14 +320,10 @@
// If target is a non-interface library class it may be an emulated interface.
if (!libraryHolder.isInterface()) {
// Here we use step-3 of resolution to find a maximally specific default interface method.
- target =
- appView
- .appInfo()
- .resolveMaximallySpecificMethods(libraryHolder, method)
- .getSingleTarget();
- if (target != null && rewriter.isEmulatedInterface(target.method.holder)) {
- targetHolder = appView.definitionFor(target.method.holder);
- addForward.accept(targetHolder, target);
+ DexClassAndMethod result =
+ appView.appInfo().lookupMaximallySpecificMethod(libraryHolder, method);
+ if (result != null && rewriter.isEmulatedInterface(result.getHolder().type)) {
+ addForward.accept(result.getHolder(), result.getMethod());
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
index 0e8cbae..f6693eb 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.graph.DexApplication.Builder;
import com.android.tools.r8.graph.DexCallSite;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
@@ -399,11 +400,18 @@
if (appView.rewritePrefix.hasRewrittenType(dexClass.type, appView)) {
return null;
}
- DexEncodedMethod singleTarget =
- appView
- .appInfo()
- .resolveMaximallySpecificMethods(dexClass, invokedMethod)
- .getSingleTarget();
+ DexEncodedMethod singleTarget = null;
+ if (dexClass.isInterface()) {
+ // Look for exact method on the interface.
+ singleTarget = dexClass.lookupMethod(invokedMethod);
+ }
+ if (singleTarget == null) {
+ DexClassAndMethod result =
+ appView.appInfo().lookupMaximallySpecificMethod(dexClass, invokedMethod);
+ if (result != null) {
+ singleTarget = result.getMethod();
+ }
+ }
if (singleTarget == null) {
// At this point we are in a library class. Failures can happen with NoSuchMethod if a
// library class implement a method with same signature but not related to emulated
diff --git a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
index 68f1749..6bce9c1 100644
--- a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
+++ b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.graph.Code;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexCode;
import com.android.tools.r8.graph.DexDebugEvent;
import com.android.tools.r8.graph.DexDebugEvent.AdvancePC;
@@ -35,7 +36,6 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DexValue.DexValueString;
import com.android.tools.r8.graph.GraphLense;
-import com.android.tools.r8.graph.ResolutionResult;
import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.kotlin.KotlinSourceDebugExtensionParser;
import com.android.tools.r8.kotlin.KotlinSourceDebugExtensionParser.Result;
@@ -309,7 +309,7 @@
// they may be bridges for interface methods with covariant return types.
sortMethods(methods);
// TODO(b/149360203): Reenable assert.
- // assert verifyMethodsAreKeptDirectlyOrIndirectly(appView, methods);
+ assert true || verifyMethodsAreKeptDirectlyOrIndirectly(appView, methods);
}
boolean identityMapping =
@@ -457,15 +457,14 @@
}
// We use the same name for interface names even if it has different types.
DexProgramClass clazz = appView.definitionForProgramType(method.method.holder);
- ResolutionResult resolutionResult =
- appView.appInfo().resolveMaximallySpecificMethods(clazz, method.method);
- if (resolutionResult.isFailedResolution()) {
+ DexClassAndMethod lookupResult =
+ appView.appInfo().lookupMaximallySpecificMethod(clazz, method.method);
+ if (lookupResult == null) {
// We cannot rename methods we cannot look up.
continue;
}
String errorString = method.method.qualifiedName() + " is not kept but is overloaded";
- assert resolutionResult.isSingleResolution() : errorString;
- assert resolutionResult.asSingleResolution().getResolvedHolder().isInterface() : errorString;
+ assert lookupResult.getHolder().isInterface() : errorString;
assert originalName == null || originalName.equals(method.method.name) : errorString;
originalName = method.method.name;
}
diff --git a/src/test/java/com/android/tools/r8/desugar/LambdaWithDefaultMethodsTest.java b/src/test/java/com/android/tools/r8/desugar/LambdaWithDefaultMethodsTest.java
index 3b4f5f1..ce7a552 100644
--- a/src/test/java/com/android/tools/r8/desugar/LambdaWithDefaultMethodsTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/LambdaWithDefaultMethodsTest.java
@@ -79,7 +79,8 @@
public static void main(String[] args) {
// Target the default method, causing it to be marked reachable.
// This is done directly in main to ensure that it is the first thing hit in tracing.
- new A().bar();
+ I i = new A();
+ i.bar();
// Create a call-site instance that will need to identify the default method as live.
// The creation is outlined to ensure that it is not hit before the method is reachable.
runDefault(createLambda());
