Redirection of super calls to default interface methods
Change-Id: I08851eb9fd21cd293af8f803d049599877ecddbe
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 38473f3..4d072c4 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -895,50 +895,61 @@
}
private void redirectSuperCallsInTarget(DexMethod oldTarget, DexMethod newTarget) {
- // If we merge class B into class C, and class C contains an invocation super.m(), then it
- // is insufficient to rewrite "invoke-super B.m()" to "invoke-direct C.m$B()" (the method
- // C.m$B denotes the direct method that has been created in C for B.m). In particular, there
- // might be an instruction "invoke-super A.m()" in C that resolves to B.m at runtime (A is
- // a superclass of B), which also needs to be rewritten to "invoke-direct C.m$B()".
- //
- // We handle this by adding a mapping for [target] and all of its supertypes.
- DexClass holder = target;
- while (holder != null && holder.isProgramClass()) {
- DexMethod signatureInHolder =
- application.dexItemFactory.createMethod(holder.type, oldTarget.proto, oldTarget.name);
- // Only rewrite the invoke-super call if it does not lead to a NoSuchMethodError.
- boolean resolutionSucceeds =
- holder.lookupVirtualMethod(signatureInHolder) != null
- || appInfo.lookupSuperTarget(signatureInHolder, holder.type) != null;
- if (resolutionSucceeds) {
- deferredRenamings.mapVirtualMethodToDirectInType(
- signatureInHolder, newTarget, target.type);
- } else {
- break;
- }
+ if (source.accessFlags.isInterface()) {
+ // If we merge a default interface method from interface I to its subtype C, then we need
+ // to rewrite invocations on the form "invoke-super I.m()" to "invoke-direct C.m$I()".
+ //
+ // Unlike when we merge a class into its subclass (the else-branch below), we should *not*
+ // rewrite any invocations on the form "invoke-super J.m()" to "invoke-direct C.m$I()",
+ // if I has a supertype J. This is due to the fact that invoke-super instructions that
+ // resolve to a method on an interface never hit an implementation below that interface.
+ deferredRenamings.mapVirtualMethodToDirectInType(oldTarget, newTarget, target.type);
+ } else {
+ // If we merge class B into class C, and class C contains an invocation super.m(), then it
+ // is insufficient to rewrite "invoke-super B.m()" to "invoke-direct C.m$B()" (the method
+ // C.m$B denotes the direct method that has been created in C for B.m). In particular, there
+ // might be an instruction "invoke-super A.m()" in C that resolves to B.m at runtime (A is
+ // a superclass of B), which also needs to be rewritten to "invoke-direct C.m$B()".
+ //
+ // We handle this by adding a mapping for [target] and all of its supertypes.
+ DexClass holder = target;
+ while (holder != null && holder.isProgramClass()) {
+ DexMethod signatureInHolder =
+ application.dexItemFactory.createMethod(holder.type, oldTarget.proto, oldTarget.name);
+ // Only rewrite the invoke-super call if it does not lead to a NoSuchMethodError.
+ boolean resolutionSucceeds =
+ holder.lookupVirtualMethod(signatureInHolder) != null
+ || appInfo.lookupSuperTarget(signatureInHolder, holder.type) != null;
+ if (resolutionSucceeds) {
+ deferredRenamings.mapVirtualMethodToDirectInType(
+ signatureInHolder, newTarget, target.type);
+ } else {
+ break;
+ }
- // Consider that A gets merged into B and B's subclass C gets merged into D. Instructions
- // on the form "invoke-super {B,C,D}.m()" in D are changed into "invoke-direct D.m$C()" by
- // the code above. However, instructions on the form "invoke-super A.m()" should also be
- // changed into "invoke-direct D.m$C()". This is achieved by also considering the classes
- // that have been merged into [holder].
- Set<DexType> mergedTypes = mergedClassesInverse.get(holder.type);
- if (mergedTypes != null) {
- for (DexType type : mergedTypes) {
- DexMethod signatureInType =
- application.dexItemFactory.createMethod(type, oldTarget.proto, oldTarget.name);
- // Resolution would have succeeded if the method used to be in [type], or if one of
- // its super classes declared the method.
- boolean resolutionSucceededBeforeMerge =
- renamedMembersLense.hasMappingForSignatureInContext(holder.type, signatureInType)
- || appInfo.lookupSuperTarget(signatureInHolder, holder.type) != null;
- if (resolutionSucceededBeforeMerge) {
- deferredRenamings.mapVirtualMethodToDirectInType(
- signatureInType, newTarget, target.type);
+ // Consider that A gets merged into B and B's subclass C gets merged into D. Instructions
+ // on the form "invoke-super {B,C,D}.m()" in D are changed into "invoke-direct D.m$C()" by
+ // the code above. However, instructions on the form "invoke-super A.m()" should also be
+ // changed into "invoke-direct D.m$C()". This is achieved by also considering the classes
+ // that have been merged into [holder].
+ Set<DexType> mergedTypes = mergedClassesInverse.get(holder.type);
+ if (mergedTypes != null) {
+ for (DexType type : mergedTypes) {
+ DexMethod signatureInType =
+ application.dexItemFactory.createMethod(type, oldTarget.proto, oldTarget.name);
+ // Resolution would have succeeded if the method used to be in [type], or if one of
+ // its super classes declared the method.
+ boolean resolutionSucceededBeforeMerge =
+ renamedMembersLense.hasMappingForSignatureInContext(holder.type, signatureInType)
+ || appInfo.lookupSuperTarget(signatureInHolder, holder.type) != null;
+ if (resolutionSucceededBeforeMerge) {
+ deferredRenamings.mapVirtualMethodToDirectInType(
+ signatureInType, newTarget, target.type);
+ }
}
}
+ holder = holder.superType != null ? appInfo.definitionFor(holder.superType) : null;
}
- holder = holder.superType != null ? appInfo.definitionFor(holder.superType) : null;
}
}
diff --git a/src/test/examplesAndroidO/classmerging/NestedDefaultInterfaceMethodsTest.java b/src/test/examplesAndroidO/classmerging/NestedDefaultInterfaceMethodsTest.java
new file mode 100644
index 0000000..7a0e325
--- /dev/null
+++ b/src/test/examplesAndroidO/classmerging/NestedDefaultInterfaceMethodsTest.java
@@ -0,0 +1,37 @@
+// Copyright (c) 2018, 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 classmerging;
+
+public class NestedDefaultInterfaceMethodsTest {
+
+ public static void main(String[] args) {
+ new C().m();
+ }
+
+ public interface A {
+
+ default void m() {
+ System.out.println("In A.m()");
+ }
+ }
+
+ public interface B extends A {
+
+ @Override
+ default void m() {
+ System.out.println("In B.m()");
+ A.super.m();
+ }
+ }
+
+ public static class C implements B {
+
+ @Override
+ public void m() {
+ System.out.println("In C.m()");
+ B.super.m();
+ }
+ }
+}
diff --git a/src/test/examplesAndroidO/classmerging/keep-rules.txt b/src/test/examplesAndroidO/classmerging/keep-rules.txt
index fc91808..4df182e 100644
--- a/src/test/examplesAndroidO/classmerging/keep-rules.txt
+++ b/src/test/examplesAndroidO/classmerging/keep-rules.txt
@@ -10,6 +10,9 @@
-keep public class classmerging.MergeDefaultMethodIntoClassTest {
public static void main(...);
}
+-keep public class classmerging.NestedDefaultInterfaceMethodsTest {
+ public static void main(...);
+}
# TODO(herhut): Consider supporting merging of inner-class attributes.
# -keepattributes *
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
index f28c86f..b6489b0 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -251,6 +251,50 @@
}
@Test
+ public void testNestedDefaultInterfaceMethodsTest() throws Exception {
+ String main = "classmerging.NestedDefaultInterfaceMethodsTest";
+ Path[] programFiles =
+ new Path[] {
+ JAVA8_CF_DIR.resolve("NestedDefaultInterfaceMethodsTest.class"),
+ JAVA8_CF_DIR.resolve("NestedDefaultInterfaceMethodsTest$A.class"),
+ JAVA8_CF_DIR.resolve("NestedDefaultInterfaceMethodsTest$B.class"),
+ JAVA8_CF_DIR.resolve("NestedDefaultInterfaceMethodsTest$C.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.NestedDefaultInterfaceMethodsTest",
+ "classmerging.NestedDefaultInterfaceMethodsTest$B",
+ "classmerging.NestedDefaultInterfaceMethodsTest$C");
+ runTest(
+ main, programFiles, preservedClassNames::contains, getProguardConfig(JAVA8_EXAMPLE_KEEP));
+ }
+
+ @Test
+ public void testNestedDefaultInterfaceMethodsWithCDumpTest() throws Exception {
+ String main = "classmerging.NestedDefaultInterfaceMethodsTest";
+ Path[] programFiles =
+ new Path[] {
+ JAVA8_CF_DIR.resolve("NestedDefaultInterfaceMethodsTest.class"),
+ JAVA8_CF_DIR.resolve("NestedDefaultInterfaceMethodsTest$A.class"),
+ JAVA8_CF_DIR.resolve("NestedDefaultInterfaceMethodsTest$B.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.NestedDefaultInterfaceMethodsTest",
+ "classmerging.NestedDefaultInterfaceMethodsTest$B",
+ "classmerging.NestedDefaultInterfaceMethodsTest$C");
+ runTestOnInput(
+ main,
+ AndroidApp.builder()
+ .addProgramFiles(programFiles)
+ .addClassProgramData(
+ NestedDefaultInterfaceMethodsTestDump.CDump.dump(), Origin.unknown())
+ .build(),
+ preservedClassNames::contains,
+ getProguardConfig(JAVA8_EXAMPLE_KEEP));
+ }
+
+ @Test
public void testPinnedParameterTypes() throws Exception {
String main = "classmerging.PinnedParameterTypesTest";
Path[] programFiles =
diff --git a/src/test/java/com/android/tools/r8/classmerging/NestedDefaultInterfaceMethodsTestDump.java b/src/test/java/com/android/tools/r8/classmerging/NestedDefaultInterfaceMethodsTestDump.java
new file mode 100644
index 0000000..e55802d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/NestedDefaultInterfaceMethodsTestDump.java
@@ -0,0 +1,76 @@
+// Copyright (c) 2018, 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;
+
+import org.objectweb.asm.AnnotationVisitor;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.FieldVisitor;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+
+public class NestedDefaultInterfaceMethodsTestDump {
+
+ // Generated by running ./tools/asmifier.py build/test/examplesAndroidO/classes/classmerging/-
+ // NestedDefaultInterfaceMethodsTest\$C.class, and changing "invoke-special B.m()" to "invoke-
+ // special A.m()".
+ public static class CDump implements Opcodes {
+
+ public static byte[] dump() {
+ ClassWriter cw = new ClassWriter(0);
+ FieldVisitor fv;
+ MethodVisitor mv;
+ AnnotationVisitor av0;
+
+ cw.visit(
+ V1_8,
+ ACC_PUBLIC + ACC_SUPER,
+ "classmerging/NestedDefaultInterfaceMethodsTest$C",
+ null,
+ "java/lang/Object",
+ new String[] {"classmerging/NestedDefaultInterfaceMethodsTest$B"});
+
+ cw.visitInnerClass(
+ "classmerging/NestedDefaultInterfaceMethodsTest$C",
+ "classmerging/NestedDefaultInterfaceMethodsTest",
+ "C",
+ ACC_PUBLIC + ACC_STATIC);
+
+ cw.visitInnerClass(
+ "classmerging/NestedDefaultInterfaceMethodsTest$B",
+ "classmerging/NestedDefaultInterfaceMethodsTest",
+ "B",
+ ACC_PUBLIC + ACC_STATIC + ACC_ABSTRACT + ACC_INTERFACE);
+
+ {
+ mv = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
+ mv.visitCode();
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(1, 1);
+ mv.visitEnd();
+ }
+ {
+ mv = cw.visitMethod(ACC_PUBLIC, "m", "()V", null, null);
+ mv.visitCode();
+ mv.visitFieldInsn(GETSTATIC, "java/lang/System", "out", "Ljava/io/PrintStream;");
+ mv.visitLdcInsn("In C.m()");
+ mv.visitMethodInsn(
+ INVOKEVIRTUAL, "java/io/PrintStream", "println", "(Ljava/lang/String;)V", false);
+ mv.visitVarInsn(ALOAD, 0);
+ // The signature "classmerging/NestedDefaultInterfaceMethodsTest$B" has been changed to
+ // "classmerging/NestedDefaultInterfaceMethodsTest$A".
+ mv.visitMethodInsn(
+ INVOKESPECIAL, "classmerging/NestedDefaultInterfaceMethodsTest$A", "m", "()V", true);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(2, 1);
+ mv.visitEnd();
+ }
+ cw.visitEnd();
+
+ return cw.toByteArray();
+ }
+ }
+}
