Fix merging of non-final classes into final classes
Change-Id: I47fc052d2a34f4456f4dc3518efcd7a6c4c04b1d
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
index f4158c5..cfd5182 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
@@ -40,7 +40,6 @@
public class ClassMerger {
public static final String CLASS_ID_FIELD_NAME = "$r8$classId";
- private final AppView<AppInfoWithLiveness> appView;
private final DexProgramClass target;
private final Collection<DexProgramClass> toMergeGroup;
private final DexItemFactory dexItemFactory;
@@ -63,7 +62,6 @@
DexField classIdField,
Collection<VirtualMethodMerger> virtualMethodMergers,
Collection<ConstructorMerger> constructorMergers) {
- this.appView = appView;
this.lensBuilder = lensBuilder;
this.mergedClassesBuilder = mergedClassesBuilder;
this.fieldAccessChangesBuilder = fieldAccessChangesBuilder;
@@ -99,6 +97,10 @@
}
void merge(DexProgramClass toMerge) {
+ if (!toMerge.isFinal()) {
+ target.getAccessFlags().demoteFromFinal();
+ }
+
toMerge.forEachProgramDirectMethod(
method -> {
DexEncodedMethod definition = method.getDefinition();
diff --git a/src/test/java/com/android/tools/r8/cf/bootstrap/BootstrapTest.java b/src/test/java/com/android/tools/r8/cf/bootstrap/BootstrapTest.java
index 1c7a809..722e4f6 100644
--- a/src/test/java/com/android/tools/r8/cf/bootstrap/BootstrapTest.java
+++ b/src/test/java/com/android/tools/r8/cf/bootstrap/BootstrapTest.java
@@ -73,7 +73,6 @@
@Test
public void test() throws Exception {
- expectThrowsWithHorizontalClassMerging();
// Run hello.jar to ensure it exists and is valid.
Path hello = Paths.get(ToolHelper.EXAMPLES_BUILD_DIR, "hello" + JAR_EXTENSION);
ProcessResult runHello = ToolHelper.runJava(hello, "hello.Hello");
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/MergeNonFinalAndFinalClassTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/MergeNonFinalAndFinalClassTest.java
new file mode 100644
index 0000000..a5eb885
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/MergeNonFinalAndFinalClassTest.java
@@ -0,0 +1,72 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.classmerging.horizontal;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isFinal;
+import static com.android.tools.r8.utils.codeinspector.Matchers.notIf;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NoVerticalClassMerging;
+import com.android.tools.r8.TestParameters;
+import org.junit.Test;
+
+public class MergeNonFinalAndFinalClassTest extends HorizontalClassMergingTestBase {
+ public MergeNonFinalAndFinalClassTest(
+ TestParameters parameters, boolean enableHorizontalClassMerging) {
+ super(parameters, enableHorizontalClassMerging);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addOptionsModification(
+ options -> options.enableHorizontalClassMerging = enableHorizontalClassMerging)
+ .addHorizontallyMergedClassesInspectorIf(
+ enableHorizontalClassMerging, inspector -> inspector.assertMergedInto(B.class, A.class))
+ .enableNeverClassInliningAnnotations()
+ .enableNoVerticalClassMergingAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector ->
+ assertThat(
+ inspector.clazz(A.class), notIf(isFinal(), enableHorizontalClassMerging)))
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("a", "b", "b", "c");
+ }
+
+ public static class Main {
+ public static void main(String[] args) {
+ new A();
+ new B();
+ new C();
+ }
+ }
+
+ @NeverClassInline
+ public static final class A {
+ public A() {
+ System.out.println("a");
+ }
+ }
+
+ @NeverClassInline
+ @NoVerticalClassMerging
+ public static class B {
+ public B() {
+ System.out.println("b");
+ }
+ }
+
+ @NeverClassInline
+ public static class C extends B {
+ public C() {
+ System.out.println("c");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java
index 6160dc6..ee4fe7b 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java
@@ -21,6 +21,11 @@
}
@Override
+ public boolean isFinal() {
+ throw new Unreachable("Cannot determine if an absent class is final");
+ }
+
+ @Override
public boolean isPresent() {
return false;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/ClassOrMemberSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/ClassOrMemberSubject.java
new file mode 100644
index 0000000..14666f1
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/ClassOrMemberSubject.java
@@ -0,0 +1,10 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.utils.codeinspector;
+
+public abstract class ClassOrMemberSubject extends Subject {
+
+ public abstract boolean isFinal();
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java
index 60203c1..54e3d63 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java
@@ -27,7 +27,7 @@
import kotlinx.metadata.jvm.KotlinClassMetadata;
import org.junit.rules.TemporaryFolder;
-public abstract class ClassSubject extends Subject {
+public abstract class ClassSubject extends ClassOrMemberSubject {
protected final ClassReference reference;
protected final CodeInspector codeInspector;
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java
index 3192f8b..bcedc29 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java
@@ -63,6 +63,11 @@
}
@Override
+ public boolean isFinal() {
+ return dexClass.isFinal();
+ }
+
+ @Override
public boolean isPresent() {
return true;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
index 6f11640..98c8562 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
@@ -273,24 +273,21 @@
};
}
- public static Matcher<MethodSubject> isFinal() {
- return new TypeSafeMatcher<MethodSubject>() {
+ public static Matcher<ClassOrMemberSubject> isFinal() {
+ return new TypeSafeMatcher<ClassOrMemberSubject>() {
@Override
- public boolean matchesSafely(final MethodSubject method) {
- return method.isPresent() && method.isFinal();
+ public boolean matchesSafely(ClassOrMemberSubject subject) {
+ return subject.isPresent() && subject.isFinal();
}
@Override
- public void describeTo(final Description description) {
+ public void describeTo(Description description) {
description.appendText("is final");
}
@Override
- public void describeMismatchSafely(final MethodSubject method, Description description) {
- description
- .appendText("method ")
- .appendValue(method.getOriginalName())
- .appendText(" was not");
+ public void describeMismatchSafely(ClassOrMemberSubject subject, Description description) {
+ description.appendText("subject ").appendValue(subject.toString()).appendText(" was not");
}
};
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
index 77172c3..7afc9f4 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
@@ -6,7 +6,7 @@
import com.android.tools.r8.naming.MemberNaming.Signature;
-public abstract class MemberSubject extends Subject {
+public abstract class MemberSubject extends ClassOrMemberSubject {
public abstract boolean isPublic();
@@ -18,8 +18,6 @@
public abstract boolean isStatic();
- public abstract boolean isFinal();
-
public abstract Signature getOriginalSignature();
public abstract Signature getFinalSignature();
