Don't vertical merge classes that lead to resolution error changes.
The workaround for the Dalvik verifier issue of not allowing an abstract method
on a non-abstract class did not preserve the expected AbstractMethodError.
Bug: 144085169
Change-Id: I5ec96dba37e0620e0ba05f27de02d2a4381a2174
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 b51c78d..f07499b 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -994,15 +994,15 @@
// Non-abstract methods are handled below (they cannot simply be moved to the subclass as
// a virtual method, because they might be the target of an invoke-super instruction).
if (virtualMethod.accessFlags.isAbstract()) {
+ // Abort if target is non-abstract and does not override the abstract method.
+ if (!target.isAbstract()) {
+ assert appView.options().testing.allowNonAbstractClassesWithAbstractMethods;
+ abortMerge = true;
+ return false;
+ }
// Update the holder of [virtualMethod] using renameMethod().
DexEncodedMethod resultingVirtualMethod =
renameMethod(virtualMethod, availableMethodSignatures, Rename.NEVER);
- if (appView.options().canHaveDalvikAbstractMethodOnNonAbstractClassVerificationBug()
- && !target.isAbstract()) {
- resultingVirtualMethod.accessFlags.unsetAbstract();
- resultingVirtualMethod =
- resultingVirtualMethod.toEmptyThrowingMethod(appView.options());
- }
deferredRenamings.map(virtualMethod.method, resultingVirtualMethod.method);
deferredRenamings.recordMove(virtualMethod.method, resultingVirtualMethod.method);
add(virtualMethods, resultingVirtualMethod, MethodSignatureEquivalence.get());
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 8faa274..cd14b54 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1005,6 +1005,7 @@
public boolean trackDesugaredAPIConversions =
System.getProperty("com.android.tools.r8.trackDesugaredAPIConversions") != null;
public boolean keepInheritedInterfaceMethods = false;
+ public boolean allowNonAbstractClassesWithAbstractMethods = false;
// Flag to turn on/off JDK11+ nest-access control even when not required (Cf backend)
public boolean enableForceNestBasedAccessDesugaringForTest = false;
diff --git a/src/test/java/com/android/tools/r8/resolution/interfacediamonds/DefaultTopAbstractLeftTest.java b/src/test/java/com/android/tools/r8/resolution/interfacediamonds/DefaultTopAbstractLeftTest.java
index f5a99fa..30295fd 100644
--- a/src/test/java/com/android/tools/r8/resolution/interfacediamonds/DefaultTopAbstractLeftTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/interfacediamonds/DefaultTopAbstractLeftTest.java
@@ -17,7 +17,6 @@
import com.android.tools.r8.graph.ResolutionResult;
import com.android.tools.r8.resolution.SingleTargetLookupTest;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
-import com.android.tools.r8.utils.AndroidApiLevel;
import com.google.common.collect.ImmutableList;
import java.util.Collection;
import java.util.Collections;
@@ -65,7 +64,7 @@
.addProgramClasses(CLASSES)
.addProgramClassFileData(transformB())
.run(parameters.getRuntime(), Main.class)
- .assertFailureWithErrorThatMatches(getExpectedErrorMatcher(false));
+ .assertFailureWithErrorThatMatches(getExpectedErrorMatcher());
}
@Test
@@ -75,16 +74,13 @@
.addProgramClassFileData(transformB())
.addKeepMainRule(Main.class)
.setMinApi(parameters.getApiLevel())
+ .addOptionsModification(
+ options -> options.testing.allowNonAbstractClassesWithAbstractMethods = true)
.run(parameters.getRuntime(), Main.class)
- .assertFailureWithErrorThatMatches(getExpectedErrorMatcher(true));
+ .assertFailureWithErrorThatMatches(containsString("AbstractMethodError"));
}
- private Matcher<String> getExpectedErrorMatcher(boolean isR8) {
- if (isR8
- && (parameters.isCfRuntime() || parameters.getApiLevel().isLessThan(AndroidApiLevel.L))) {
- // TODO(b/144085169): R8 replaces the entire main method by 'throw null', why?
- return containsString("NullPointerException");
- }
+ private Matcher<String> getExpectedErrorMatcher() {
if (parameters.isDexRuntime()
&& parameters
.getRuntime()
diff --git a/src/test/java/com/android/tools/r8/resolution/interfacediamonds/DefaultTopAbstractRightTest.java b/src/test/java/com/android/tools/r8/resolution/interfacediamonds/DefaultTopAbstractRightTest.java
index bd2d4ac..6c4070e 100644
--- a/src/test/java/com/android/tools/r8/resolution/interfacediamonds/DefaultTopAbstractRightTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/interfacediamonds/DefaultTopAbstractRightTest.java
@@ -17,7 +17,6 @@
import com.android.tools.r8.graph.ResolutionResult;
import com.android.tools.r8.resolution.SingleTargetLookupTest;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
-import com.android.tools.r8.utils.AndroidApiLevel;
import com.google.common.collect.ImmutableList;
import java.util.Collection;
import java.util.Collections;
@@ -65,7 +64,7 @@
.addProgramClasses(CLASSES)
.addProgramClassFileData(transformB())
.run(parameters.getRuntime(), Main.class)
- .assertFailureWithErrorThatMatches(getExpectedErrorMatcher(false));
+ .assertFailureWithErrorThatMatches(getExpectedErrorMatcher());
}
@Test
@@ -75,16 +74,13 @@
.addProgramClassFileData(transformB())
.addKeepMainRule(Main.class)
.setMinApi(parameters.getApiLevel())
+ .addOptionsModification(
+ options -> options.testing.allowNonAbstractClassesWithAbstractMethods = true)
.run(parameters.getRuntime(), Main.class)
- .assertFailureWithErrorThatMatches(getExpectedErrorMatcher(true));
+ .assertFailureWithErrorThatMatches(containsString("AbstractMethodError"));
}
- private Matcher<String> getExpectedErrorMatcher(boolean isR8) {
- if (isR8
- && (parameters.isCfRuntime() || parameters.getApiLevel().isLessThan(AndroidApiLevel.L))) {
- // TODO(b/144085169): R8 replaces the entire main method by 'throw null', why?
- return containsString("NullPointerException");
- }
+ private Matcher<String> getExpectedErrorMatcher() {
if (parameters.isDexRuntime()
&& parameters
.getRuntime()
