Merge "Prevent class merging from hiding IncompatibleClassChangeErrors"
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 c737055..a8fd776 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.shaking;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.graph.AppInfo.ResolutionResult;
import com.android.tools.r8.graph.DefaultUseRegistry;
import com.android.tools.r8.graph.DexAnnotationSet;
import com.android.tools.r8.graph.DexApplication;
@@ -345,6 +346,9 @@
}
continue;
}
+ if (methodResolutionMayChange(clazz, targetClass)) {
+ continue;
+ }
// Field resolution first considers the direct interfaces of [targetClass] before it proceeds
// to the super class.
if (fieldResolutionMayChange(clazz, targetClass)) {
@@ -395,6 +399,49 @@
return renamedMembersLense.build(graphLense);
}
+ private boolean methodResolutionMayChange(DexClass source, DexClass target) {
+ // When merging an interface into a class, all instructions on the form "invoke-interface
+ // [source].m" are changed into "invoke-virtual [target].m". We need to abort the merge if this
+ // transformation could hide IncompatibleClassChangeErrors.
+ if (source.isInterface() && !target.isInterface()) {
+ List<DexEncodedMethod> defaultMethods = new ArrayList<>();
+ for (DexEncodedMethod virtualMethod : source.virtualMethods()) {
+ if (!virtualMethod.accessFlags.isAbstract()) {
+ defaultMethods.add(virtualMethod);
+ }
+ }
+
+ // For each of the default methods, the subclass [target] could inherit another default method
+ // with the same signature from another interface (i.e., there is a conflict). In such cases,
+ // instructions on the form "invoke-interface [source].foo()" will fail with an Incompatible-
+ // ClassChangeError.
+ //
+ // Example:
+ // interface I1 { default void m() {} }
+ // interface I2 { default void m() {} }
+ // class C implements I1, I2 {
+ // ... invoke-interface I1.m ... <- IncompatibleClassChangeError
+ // }
+ for (DexEncodedMethod method : defaultMethods) {
+ // Conservatively find all possible targets for this method.
+ Set<DexEncodedMethod> interfaceTargets = appInfo.lookupInterfaceTargets(method.method);
+
+ // If [method] is not even an interface-target, then we can safely merge it. Otherwise we
+ // need to check for a conflict.
+ if (interfaceTargets.remove(method)) {
+ for (DexEncodedMethod interfaceTarget : interfaceTargets) {
+ DexClass enclosingClass = appInfo.definitionFor(interfaceTarget.method.holder);
+ if (enclosingClass != null && enclosingClass.isInterface()) {
+ // Found another default method that is different from the one in [source], aborting.
+ return true;
+ }
+ }
+ }
+ }
+ }
+ return false;
+ }
+
private boolean fieldResolutionMayChange(DexClass source, DexClass target) {
if (source.type == target.superType) {
// If there is a "iget Target.f" or "iput Target.f" instruction in target, and the class
@@ -657,12 +704,13 @@
// Returns the method that shadows the given method, or null if method is not shadowed.
private DexEncodedMethod findMethodInTarget(DexEncodedMethod method) {
- DexEncodedMethod actual = appInfo.resolveMethod(target.type, method.method).asSingleTarget();
- if (actual == null) {
- // May happen in case of missing classes.
+ ResolutionResult resolutionResult = appInfo.resolveMethod(target.type, method.method);
+ if (!resolutionResult.hasSingleTarget()) {
+ // May happen in case of missing classes, or if multiple implementations were found.
abortMerge = true;
return null;
}
+ DexEncodedMethod actual = resolutionResult.asSingleTarget();
if (actual != method) {
return actual;
}
diff --git a/src/test/java/com/android/tools/r8/resolution/singletarget/one/AbstractTopClass.java b/src/test/java/com/android/tools/r8/resolution/singletarget/one/AbstractTopClass.java
index d9ff675..c745dfa 100644
--- a/src/test/java/com/android/tools/r8/resolution/singletarget/one/AbstractTopClass.java
+++ b/src/test/java/com/android/tools/r8/resolution/singletarget/one/AbstractTopClass.java
@@ -5,33 +5,36 @@
public abstract class AbstractTopClass implements InterfaceWithDefault {
+ // Avoid AbstractTopClass.class.getCanonicalName() as it may change during shrinking.
+ private static final String TAG = "AbstractTopClass";
+
public void singleTargetAtTop() {
- System.out.println(AbstractTopClass.class.getCanonicalName());
+ System.out.println(TAG);
}
public void singleShadowingOverride() {
- System.out.println(AbstractTopClass.class.getCanonicalName());
+ System.out.println(TAG);
}
public abstract void abstractTargetAtTop();
public void overridenInAbstractClassOnly() {
- System.out.println(AbstractTopClass.class.getCanonicalName());
+ System.out.println(TAG);
}
public void overriddenInTwoSubTypes() {
- System.out.println(AbstractTopClass.class.getCanonicalName());
+ System.out.println(TAG);
}
public void definedInTwoSubTypes() {
- System.out.println(AbstractTopClass.class.getCanonicalName());
+ System.out.println(TAG);
}
public static void staticMethod() {
- System.out.println(AbstractTopClass.class.getCanonicalName());
+ System.out.println(TAG);
}
public void overriddenByIrrelevantInterface() {
- System.out.println(AbstractTopClass.class.getCanonicalName());
+ System.out.println(TAG);
}
}
diff --git a/src/test/java/com/android/tools/r8/resolution/singletarget/one/InterfaceWithDefault.java b/src/test/java/com/android/tools/r8/resolution/singletarget/one/InterfaceWithDefault.java
index 1a0c434..f0faedd 100644
--- a/src/test/java/com/android/tools/r8/resolution/singletarget/one/InterfaceWithDefault.java
+++ b/src/test/java/com/android/tools/r8/resolution/singletarget/one/InterfaceWithDefault.java
@@ -5,15 +5,18 @@
public interface InterfaceWithDefault {
+ // Avoid InterfaceWithDefault.class.getCanonicalName() as it may change during shrinking.
+ String TAG = "InterfaceWithDefault";
+
default void defaultMethod() {
- System.out.println(InterfaceWithDefault.class.getCanonicalName());
+ System.out.println(TAG);
}
default void overriddenDefault() {
- System.out.println(InterfaceWithDefault.class.getCanonicalName());
+ System.out.println(TAG);
}
default void overriddenInOtherInterface() {
- System.out.println(InterfaceWithDefault.class.getCanonicalName());
+ System.out.println(TAG);
}
}
diff --git a/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassOne.java b/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassOne.java
index 22be6a4..ff585af 100644
--- a/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassOne.java
+++ b/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassOne.java
@@ -5,18 +5,21 @@
public class SubSubClassOne extends AbstractSubClass implements IrrelevantInterfaceWithDefault {
+ // Avoid SubSubClassOne.class.getCanonicalName() as it may change during shrinking.
+ private static final String TAG = "SubSubClassOne";
+
@Override
public void abstractTargetAtTop() {
- System.out.println(SubSubClassOne.class.getCanonicalName());
+ System.out.println(TAG);
}
@Override
public void overriddenInTwoSubTypes() {
- System.out.println(SubSubClassOne.class.getCanonicalName());
+ System.out.println(TAG);
}
@Override
public void definedInTwoSubTypes() {
- System.out.println(SubSubClassOne.class.getCanonicalName());
+ System.out.println(TAG);
}
}
