Account for NoSuchMethodResult when having multiple definitions
Bug: b/226170842
Change-Id: Id9bc61633f57a4ff19b70fcd255f9643827c8d81
diff --git a/src/main/java/com/android/tools/r8/graph/MethodResolution.java b/src/main/java/com/android/tools/r8/graph/MethodResolution.java
index 990f432..983c69e 100644
--- a/src/main/java/com/android/tools/r8/graph/MethodResolution.java
+++ b/src/main/java/com/android/tools/r8/graph/MethodResolution.java
@@ -93,15 +93,12 @@
definitionFor(holder)
.forEachClassResolutionResult(
clazz -> {
- if (clazz.isInterface()) {
- builder.addResolutionResult(
- resolveMethodOnInterface(clazz, method.getProto(), method.getName()));
- } else {
- builder.addResolutionResult(
- resolveMethodOnClass(clazz, method.getProto(), method.getName()));
- }
+ builder.addResolutionResult(
+ clazz.isInterface()
+ ? resolveMethodOnInterface(clazz, method.getProto(), method.getName())
+ : resolveMethodOnClass(clazz, method.getProto(), method.getName()));
});
- return builder.buildOrIfEmpty(ClassNotFoundResult.INSTANCE);
+ return builder.buildOrIfEmpty(ClassNotFoundResult.INSTANCE, holder);
}
/**
@@ -148,7 +145,7 @@
builder.addResolutionResult(resolveMethodOnClass(clazz, methodProto, methodName));
}
});
- return builder.buildOrIfEmpty(ClassNotFoundResult.INSTANCE);
+ return builder.buildOrIfEmpty(ClassNotFoundResult.INSTANCE, holder);
}
public MethodResolutionResult resolveMethodOnClass(
@@ -197,8 +194,8 @@
initialResolutionHolder, clazz, result);
}
// Pt 3: Apply step two to direct superclass of holder.
- MethodResolutionResult.Builder builder = MethodResolutionResult.builder();
if (clazz.superType != null) {
+ MethodResolutionResult.Builder builder = MethodResolutionResult.builder();
definitionFor(clazz.superType)
.forEachClassResolutionResult(
superClass -> {
@@ -212,8 +209,9 @@
resolveMethodOnClassStep2(
superClass, methodProto, methodName, initialResolutionHolder));
});
+ return builder.buildOrIfEmpty(null, clazz.superType);
}
- return builder.buildOrIfEmpty(null);
+ return null;
}
/**
@@ -522,7 +520,7 @@
resolveMethodOnInterface(definition, proto, methodName));
}
});
- return builder.buildOrIfEmpty(ClassNotFoundResult.INSTANCE);
+ return builder.buildOrIfEmpty(ClassNotFoundResult.INSTANCE, holder);
}
public MethodResolutionResult resolveMethodOnInterface(
@@ -552,7 +550,7 @@
resolveMethodStep3(definition, methodProto, methodName));
}
});
- return builder.buildOrIfEmpty(ClassNotFoundResult.INSTANCE);
+ return builder.buildOrIfEmpty(ClassNotFoundResult.INSTANCE, Collections.emptySet());
}
static class MaximallySpecificMethodsBuilder {
@@ -674,7 +672,7 @@
MethodResolutionResult.builder().allowMultipleProgramResults();
if (nonAbstractOnComplete.isEmpty()) {
assert !typesWithMultipleDefinitions.isEmpty();
- builder.addResolutionResult(new NoSuchMethodResult(typesWithMultipleDefinitions));
+ builder.addResolutionResult(NoSuchMethodResult.INSTANCE);
} else {
nonAbstractOnComplete.forEach(
entry ->
@@ -683,7 +681,7 @@
nonAbstractOnIncomplete.forEach(
entry ->
builder.addResolutionResult(singleResultHelper(initialResolutionHolder, entry)));
- return builder.buildOrIfEmpty(NoSuchMethodResult.INSTANCE);
+ return builder.buildOrIfEmpty(NoSuchMethodResult.INSTANCE, typesWithMultipleDefinitions);
}
}
diff --git a/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java b/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java
index a2dd885..6b6991d 100644
--- a/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java
@@ -16,11 +16,14 @@
import com.android.tools.r8.utils.ConsumerUtils;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.OptionalBool;
+import com.android.tools.r8.utils.SetUtils;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Set;
import java.util.function.BiPredicate;
import java.util.function.Consumer;
@@ -68,10 +71,18 @@
return false;
}
+ public boolean isNoSuchMethodResultDueToMultipleClassDefinitions() {
+ return false;
+ }
+
public boolean isNoSuchMethodErrorResult(DexClass context, AppInfoWithClassHierarchy appInfo) {
return false;
}
+ public boolean internalIsInstanceOfNoSuchMethodResult() {
+ return false;
+ }
+
public boolean isIllegalAccessErrorResult(DexClass context, AppInfoWithClassHierarchy appInfo) {
return false;
}
@@ -97,6 +108,10 @@
return null;
}
+ public NoSuchMethodResult asNoSuchMethodResult() {
+ return null;
+ }
+
public DexClass getResolvedHolder() {
return null;
}
@@ -1172,9 +1187,13 @@
public static class NoSuchMethodResult extends FailedResolutionResult {
- static final NoSuchMethodResult INSTANCE = new NoSuchMethodResult(null);
+ static final NoSuchMethodResult INSTANCE = new NoSuchMethodResult();
- public NoSuchMethodResult(Collection<DexType> typesCausingError) {
+ private NoSuchMethodResult() {
+ super(null);
+ }
+
+ protected NoSuchMethodResult(Collection<DexType> typesCausingError) {
super(typesCausingError);
}
@@ -1182,6 +1201,28 @@
public boolean isNoSuchMethodErrorResult(DexClass context, AppInfoWithClassHierarchy appInfo) {
return true;
}
+
+ @Override
+ public boolean internalIsInstanceOfNoSuchMethodResult() {
+ return true;
+ }
+
+ @Override
+ public NoSuchMethodResult asNoSuchMethodResult() {
+ return this;
+ }
+ }
+
+ public static class NoSuchMethodResultDueToMultipleClassDefinitions extends NoSuchMethodResult {
+
+ public NoSuchMethodResultDueToMultipleClassDefinitions(Collection<DexType> typesCausingError) {
+ super(typesCausingError);
+ }
+
+ @Override
+ public boolean isNoSuchMethodResultDueToMultipleClassDefinitions() {
+ return true;
+ }
}
static class IllegalAccessOrNoSuchMethodResult extends FailedResolutionWithCausingMethods {
@@ -1463,7 +1504,15 @@
return this;
}
- public MethodResolutionResult buildOrIfEmpty(MethodResolutionResult emptyResult) {
+ public MethodResolutionResult buildOrIfEmpty(
+ MethodResolutionResult emptyResult, DexType responsibleTypeForNoSuchMethodResult) {
+ return buildOrIfEmpty(
+ emptyResult, Collections.singletonList(responsibleTypeForNoSuchMethodResult));
+ }
+
+ public MethodResolutionResult buildOrIfEmpty(
+ MethodResolutionResult emptyResult,
+ Collection<DexType> responsibleTypesForNoSuchMethodResult) {
if (possiblySingleResult == null) {
return emptyResult;
} else if (allResults == null) {
@@ -1473,6 +1522,7 @@
new ArrayList<>();
List<SingleLibraryResolutionResult> libraryResults = new ArrayList<>();
List<FailedResolutionResult> failedResults = new ArrayList<>();
+ Set<NoSuchMethodResult> noSuchMethodResults = Sets.newIdentityHashSet();
allResults.forEach(
otherResult -> {
otherResult.visitMethodResolutionResults(
@@ -1492,14 +1542,25 @@
},
ConsumerUtils.emptyConsumer(),
newFailedResult -> {
- if (!Iterables.any(
- failedResults,
- existing ->
- existing.isFailedResolution() == newFailedResult.isFailedResolution())) {
+ if (newFailedResult.internalIsInstanceOfNoSuchMethodResult()) {
+ noSuchMethodResults.add(newFailedResult.asNoSuchMethodResult());
+ }
+ if (!Iterables.any(failedResults, existing -> existing == newFailedResult)) {
failedResults.add(newFailedResult);
}
});
});
+ // If we have seen a NoSuchMethod and also a successful result it must be because we have
+ // multiple definitions of a type. Here we compute a single NoSuchMethodResult with root types
+ // that must be preserved to still observe the NoSuchMethodError.
+ if (!noSuchMethodResults.isEmpty()) {
+ if (!libraryResults.isEmpty() || !programOrClasspathResults.isEmpty()) {
+ failedResults.add(
+ mergeNoSuchMethodErrors(noSuchMethodResults, responsibleTypesForNoSuchMethodResult));
+ } else {
+ failedResults.add(NoSuchMethodResult.INSTANCE);
+ }
+ }
if (programOrClasspathResults.isEmpty()) {
if (libraryResults.size() == 1 && failedResults.isEmpty()) {
return libraryResults.get(0);
@@ -1531,5 +1592,21 @@
programOrClasspathResults, libraryResults, failedResults);
}
}
+
+ private NoSuchMethodResult mergeNoSuchMethodErrors(
+ Set<NoSuchMethodResult> noSuchMethodErrors, Collection<DexType> typesCausingErrorsHere) {
+ Set<DexType> typesCausingError = SetUtils.newIdentityHashSet(typesCausingErrorsHere);
+ noSuchMethodErrors.forEach(
+ failedResolutionResult -> {
+ assert failedResolutionResult == NoSuchMethodResult.INSTANCE
+ || failedResolutionResult.isNoSuchMethodResultDueToMultipleClassDefinitions();
+ if (failedResolutionResult.typesCausingError != null) {
+ typesCausingError.addAll(failedResolutionResult.typesCausingError);
+ }
+ });
+ return typesCausingError.isEmpty()
+ ? NoSuchMethodResult.INSTANCE
+ : new NoSuchMethodResultDueToMultipleClassDefinitions(typesCausingError);
+ }
}
}
