Revert "Synthesize null-throwing body for abstract methods on non-abstract classes"
This reverts commit 28e9332ccce95f3bc6bdd717e292f76860ae1c91.
Reason for revert: The 800-smali test is failing because we are setting an API level.
Change-Id: I325f1dcbe8f1a22961adec4ac656d7b70fdb8abd
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index a1ca852..43bc47d 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -763,7 +763,7 @@
}
// Validity checks.
- assert application.classes().stream().allMatch(clazz -> clazz.isValid(options));
+ assert application.classes().stream().allMatch(DexClass::isValid);
assert appView.rootSet().verifyKeptItemsAreKept(application, appView.appInfo());
assert appView
.graphLense()
diff --git a/src/main/java/com/android/tools/r8/dex/DexParser.java b/src/main/java/com/android/tools/r8/dex/DexParser.java
index df86e96..3ef485f 100644
--- a/src/main/java/com/android/tools/r8/dex/DexParser.java
+++ b/src/main/java/com/android/tools/r8/dex/DexParser.java
@@ -600,12 +600,8 @@
return fields;
}
- private DexEncodedMethod[] readMethods(
- int size,
- DexMethodAnnotation[] annotations,
- DexParameterAnnotation[] parameters,
- boolean skipCodes,
- boolean ensureNonAbstract) {
+ private DexEncodedMethod[] readMethods(int size, DexMethodAnnotation[] annotations,
+ DexParameterAnnotation[] parameters, boolean skipCodes) {
DexEncodedMethod[] methods = new DexEncodedMethod[size];
int methodIndex = 0;
MemberAnnotationIterator<DexMethod, DexAnnotationSet> annotationIterator =
@@ -622,21 +618,8 @@
code = codes.get(codeOff);
}
DexMethod method = indexedItems.getMethod(methodIndex);
- DexEncodedMethod encodedMethod =
- new DexEncodedMethod(
- method,
- accessFlags,
- annotationIterator.getNextFor(method),
- parameterAnnotationsIterator.getNextFor(method),
- code);
- if (accessFlags.isAbstract() && ensureNonAbstract) {
- accessFlags.unsetAbstract();
- encodedMethod =
- options.isGeneratingClassFiles()
- ? encodedMethod.toEmptyThrowingMethodCf()
- : encodedMethod.toEmptyThrowingMethodDex();
- }
- methods[i] = encodedMethod;
+ methods[i] = new DexEncodedMethod(method, accessFlags, annotationIterator.getNextFor(method),
+ parameterAnnotationsIterator.getNextFor(method), code);
}
return methods;
}
@@ -709,17 +692,13 @@
directMethodsSize,
annotationsDirectory.methods,
annotationsDirectory.parameters,
- classKind != ClassKind.PROGRAM,
- options.canHaveDalvikAbstractMethodOnNonAbstractClassVerificationBug()
- && !flags.isAbstract());
+ classKind != ClassKind.PROGRAM);
virtualMethods =
readMethods(
virtualMethodsSize,
annotationsDirectory.methods,
annotationsDirectory.parameters,
- classKind != ClassKind.PROGRAM,
- options.canHaveDalvikAbstractMethodOnNonAbstractClassVerificationBug()
- && !flags.isAbstract());
+ classKind != ClassKind.PROGRAM);
}
AttributesAndAnnotations attrs =
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index 4a2e1e3..9a1ea4c 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -262,19 +262,6 @@
return true;
}
- private boolean verifyNoAbstractMethodsOnNonAbstractClasses(
- Iterable<DexEncodedMethod> methods, InternalOptions options) {
- if (options.canHaveDalvikAbstractMethodOnNonAbstractClassVerificationBug()) {
- if (!isAbstract()) {
- for (DexEncodedMethod method : methods) {
- assert !method.isAbstract()
- : "Non-abstract method on abstract class: `" + method.method.toSourceString() + "`";
- }
- }
- }
- return true;
- }
-
private boolean verifyNoDuplicateMethods() {
Set<DexMethod> unique = Sets.newIdentityHashSet();
for (DexEncodedMethod method : methods()) {
@@ -585,10 +572,7 @@
return null;
}
- public boolean isAbstract() {
- return accessFlags.isAbstract();
- }
-
+ // Tells whether this is an interface.
public boolean isInterface() {
return accessFlags.isInterface();
}
@@ -892,9 +876,9 @@
return getKotlinInfo() != null;
}
- public boolean isValid(InternalOptions options) {
- assert verifyNoAbstractMethodsOnNonAbstractClasses(virtualMethods(), options);
- assert !isInterface() || virtualMethods().stream().noneMatch(DexEncodedMethod::isFinal);
+ public boolean isValid() {
+ assert !isInterface()
+ || Arrays.stream(virtualMethods).noneMatch(method -> method.accessFlags.isFinal());
assert verifyCorrectnessOfFieldHolders(fields());
assert verifyNoDuplicateFields();
assert verifyCorrectnessOfMethodHolders(methods());
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 b44ad18..cb90f1f 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -231,14 +231,6 @@
return compilationState != CompilationState.NOT_PROCESSED;
}
- public boolean isAbstract() {
- return accessFlags.isAbstract();
- }
-
- public boolean isFinal() {
- return accessFlags.isFinal();
- }
-
public boolean isInitializer() {
checkIfObsolete();
return isInstanceInitializer() || isClassInitializer();
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 a65dd20..0d42996 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1250,12 +1250,4 @@
public boolean canHaveDalvikCatchHandlerVerificationBug() {
return minApiLevel < AndroidApiLevel.L.getLevel();
}
-
- // Having an invoke instruction that targets an abstract method on a non-abstract class will fail
- // with a verification error.
- //
- // See b/132953944.
- public boolean canHaveDalvikAbstractMethodOnNonAbstractClassVerificationBug() {
- return minApiLevel < AndroidApiLevel.L.getLevel();
- }
}
diff --git a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
index ca01e2d..2b32d66 100644
--- a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
@@ -118,8 +118,6 @@
private static Map<String, AndroidApiLevel> needMinSdkVersion =
new ImmutableMap.Builder<String, AndroidApiLevel>()
.put("004-JniTest", AndroidApiLevel.N)
- // Has a non-abstract class with an abstract method.
- .put("800-smali", AndroidApiLevel.L)
// Android O
.put("952-invoke-custom", AndroidApiLevel.O)
.put("952-invoke-custom-kinds", AndroidApiLevel.O)
diff --git a/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java b/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java
index e9368e9..dc980d7 100644
--- a/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java
+++ b/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java
@@ -85,23 +85,13 @@
public class ClassBuilder extends Builder {
- private boolean isAbstract = false;
-
ClassBuilder(String name, String superName, List<String> implementedInterfaces) {
super(name, superName, implementedInterfaces);
}
- public ClassBuilder setAbstract() {
- isAbstract = true;
- return this;
- }
-
public String toString() {
StringBuilder builder = new StringBuilder();
builder.append(".class public ");
- if (isAbstract) {
- builder.append("abstract ");
- }
builder.append(DescriptorUtils.javaTypeToDescriptor(name));
builder.append("\n");
appendSuper(builder);
@@ -174,12 +164,10 @@
addClass(name, superName, ImmutableList.of());
}
- public ClassBuilder addClass(String name, String superName, List<String> implementedInterfaces) {
+ public void addClass(String name, String superName, List<String> implementedInterfaces) {
assert !classes.containsKey(name);
currentClassName = name;
- ClassBuilder classBuilder = new ClassBuilder(name, superName, implementedInterfaces);
- classes.put(name, classBuilder);
- return classBuilder;
+ classes.put(name, new ClassBuilder(name, superName, implementedInterfaces));
}
public void addInterface(String name) {
diff --git a/src/test/java/com/android/tools/r8/smali/SmaliDisassembleTest.java b/src/test/java/com/android/tools/r8/smali/SmaliDisassembleTest.java
index bbeed31..2297ebd 100644
--- a/src/test/java/com/android/tools/r8/smali/SmaliDisassembleTest.java
+++ b/src/test/java/com/android/tools/r8/smali/SmaliDisassembleTest.java
@@ -380,20 +380,20 @@
@Test
public void implementsInterface() {
SmaliBuilder builder = new SmaliBuilder();
- builder.addClass("Test", "java.lang.Object", ImmutableList.of("java.util.List")).setAbstract();
+ builder.addClass("Test", "java.lang.Object", ImmutableList.of("java.util.List"));
builder.addAbstractMethod("int", "test", ImmutableList.of());
AndroidApp application = buildApplication(builder);
String expected =
- ".class public abstract LTest;\n"
- + "\n"
- + ".super Ljava/lang/Object;\n"
- + ".implements Ljava/util/List;\n"
- + "\n"
- + ".method public abstract test()I\n"
- + ".end method\n"
- + "\n"
- + "# End of class LTest;\n";
+ ".class public LTest;\n" +
+ "\n" +
+ ".super Ljava/lang/Object;\n" +
+ ".implements Ljava/util/List;\n" +
+ "\n" +
+ ".method public abstract test()I\n" +
+ ".end method\n" +
+ "\n" +
+ "# End of class LTest;\n";
assertEquals(expected, SmaliWriter.smali(application, new InternalOptions()));
