Fixing super calls to default interface methods in desugaring.
When interface or class calls default interface method via supercall
invokespecial instruction must point to an interface which is directly
implemented/extended by the class/interface performing the call. In
this case, invokespecial instruction may point to a default method
not directly implemented by the interface, but inherited by it.
We try to make our best attempt to find proper method to call, and
only use the method specified if we fail to do so.
Bug:
Change-Id: I52498aff4cf508bd4815dc4a3fac5ff24c7413af
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java b/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
index c752fb5..2152364 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
@@ -32,8 +32,6 @@
private final Set<DexClass> processedClasses = Sets.newIdentityHashSet();
// Maps already created methods into default methods they were generated based on.
private final Map<DexEncodedMethod, DexEncodedMethod> createdMethods = new IdentityHashMap<>();
- // Caches default interface method info for already processed interfaces.
- private final Map<DexType, DefaultMethodsHelper.Collection> cache = new IdentityHashMap<>();
ClassProcessor(InterfaceMethodRewriter rewriter) {
this.rewriter = rewriter;
@@ -67,7 +65,7 @@
if (superClass != null && superType != rewriter.factory.objectType) {
if (superClass.isInterface()) {
throw new CompilationError("Interface `" + superClass.toSourceString()
- + "` used as super class of `" + clazz.toSourceString() + "`.");
+ + "` used as super class of `" + clazz.toSourceString() + "`.");
}
process(superClass);
}
@@ -136,7 +134,7 @@
// the future.
while (current.type != rewriter.factory.objectType) {
for (DexType type : current.interfaces.values) {
- helper.merge(getOrCreateInterfaceInfo(clazz, current, type));
+ helper.merge(rewriter.getOrCreateInterfaceInfo(clazz, current, type));
}
accumulatedVirtualMethods.addAll(Arrays.asList(clazz.virtualMethods()));
@@ -168,8 +166,8 @@
} else {
String message = "Default method desugaring of `" + clazz.toSourceString() + "` failed";
if (current == clazz) {
- message += " because its super class `" + clazz.superType.toSourceString()
- + "` is missing";
+ message += " because its super class `" +
+ clazz.superType.toSourceString() + "` is missing";
} else {
message +=
" because it's hierarchy is incomplete. The class `"
@@ -244,62 +242,4 @@
}
}
}
-
- private DefaultMethodsHelper.Collection getOrCreateInterfaceInfo(
- DexClass classToDesugar,
- DexClass implementing,
- DexType iface) {
- DefaultMethodsHelper.Collection collection = cache.get(iface);
- if (collection != null) {
- return collection;
- }
- collection = createInterfaceInfo(classToDesugar, implementing, iface);
- cache.put(iface, collection);
- return collection;
- }
-
- private DefaultMethodsHelper.Collection createInterfaceInfo(
- DexClass classToDesugar,
- DexClass implementing,
- DexType iface) {
- DefaultMethodsHelper helper = new DefaultMethodsHelper();
- DexClass definedInterface = rewriter.findDefinitionFor(iface);
- if (definedInterface == null) {
- rewriter.warnMissingInterface(classToDesugar, implementing, iface);
- return helper.wrapInCollection();
- }
-
- if (!definedInterface.isInterface()) {
- throw new CompilationError(
- "Type " + iface.toSourceString() + " is referenced as an interface of `"
- + implementing.toString() + "`.");
- }
-
- if (definedInterface.isLibraryClass()) {
- // NOTE: We intentionally ignore all candidates coming from android.jar
- // since it is only possible in case v24+ version of android.jar is provided.
- // WARNING: This may result in incorrect code if something else than Android bootclasspath
- // classes are given as libraries!
- return helper.wrapInCollection();
- }
-
- // Merge information from all superinterfaces.
- for (DexType superinterface : definedInterface.interfaces.values) {
- helper.merge(getOrCreateInterfaceInfo(classToDesugar, definedInterface, superinterface));
- }
-
- // Hide by virtual methods of this interface.
- for (DexEncodedMethod virtual : definedInterface.virtualMethods()) {
- helper.hideMatches(virtual.method);
- }
-
- // Add all default methods of this interface.
- for (DexEncodedMethod encoded : definedInterface.virtualMethods()) {
- if (rewriter.isDefaultMethod(encoded)) {
- helper.addDefaultMethod(encoded);
- }
- }
-
- return helper.wrapInCollection();
- }
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/DefaultMethodsHelper.java b/src/main/java/com/android/tools/r8/ir/desugar/DefaultMethodsHelper.java
index df65e7e..fb051aa 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/DefaultMethodsHelper.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/DefaultMethodsHelper.java
@@ -40,6 +40,22 @@
this.live = live;
this.hidden = hidden;
}
+
+ // If there is just one live method having specified
+ // signature return it, otherwise return null.
+ DexMethod getSingleCandidate(DexMethod method) {
+ DexMethod candidate = null;
+ for (DexEncodedMethod encodedMethod : live) {
+ DexMethod current = encodedMethod.method;
+ if (current.proto == method.proto && current.name == method.name) {
+ if (candidate != null) {
+ return null;
+ }
+ candidate = current;
+ }
+ }
+ return candidate;
+ }
}
final void merge(Collection collection) {
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
index 9379a83..9b8b8e2 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
@@ -26,6 +26,7 @@
import com.android.tools.r8.ir.code.InvokeStatic;
import com.android.tools.r8.ir.code.InvokeSuper;
import com.android.tools.r8.ir.conversion.IRConverter;
+import com.android.tools.r8.ir.desugar.DefaultMethodsHelper.Collection;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.StringDiagnostic;
@@ -33,6 +34,7 @@
import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
//
// Default and static interface method desugaring rewriter (note that lambda
@@ -75,6 +77,9 @@
// to this collection since it is only filled in ClassProcessor running synchronously.
private final Set<DexEncodedMethod> forwardingMethods = Sets.newIdentityHashSet();
+ // Caches default interface method info for already processed interfaces.
+ private final Map<DexType, DefaultMethodsHelper.Collection> cache = new ConcurrentHashMap<>();
+
/**
* A set of dexitems we have reported missing to dedupe warnings.
*/
@@ -166,8 +171,10 @@
// of android.jar is provided.
// WARNING: This may result in incorrect code on older platforms!
// Retarget call to an appropriate method of companion class.
+ DexMethod amendedMethod = amendDefaultMethod(
+ findDefinitionFor(encodedMethod.method.holder), method);
instructions.replaceCurrentInstruction(
- new InvokeStatic(defaultAsMethodOfCompanionClass(method),
+ new InvokeStatic(defaultAsMethodOfCompanionClass(amendedMethod),
invokeSuper.outValue(), invokeSuper.arguments()));
}
continue;
@@ -294,6 +301,15 @@
factory.createString(prefix + method.name.toString()));
}
+ // It is possible that referenced method actually points to an interface which does
+ // not define this default methods, but inherits it. We are making our best effort
+ // to find an appropriate method, but still use the original one in case we fail.
+ private DexMethod amendDefaultMethod(DexClass classToDesugar, DexMethod method) {
+ DexMethod singleCandidate = getOrCreateInterfaceInfo(
+ classToDesugar, classToDesugar, method.holder).getSingleCandidate(method);
+ return singleCandidate != null ? singleCandidate : method;
+ }
+
// Represent a default interface method as a method of companion class.
final DexMethod defaultAsMethodOfCompanionClass(DexMethod method) {
return instanceAsMethodOfCompanionClass(method, DEFAULT_METHOD_PREFIX);
@@ -329,6 +345,14 @@
for (DexEncodedMethod method : forwardingMethods) {
converter.optimizeSynthesizedMethod(method);
}
+
+ // Cached data is not needed any more.
+ clear();
+ }
+
+ private void clear() {
+ this.cache.clear();
+ this.forwardingMethods.clear();
}
private static boolean shouldProcess(
@@ -426,4 +450,62 @@
DexClass clazz = converter.appInfo.definitionFor(holder);
return clazz == null ? Origin.unknown() : clazz.getOrigin();
}
+
+ final DefaultMethodsHelper.Collection getOrCreateInterfaceInfo(
+ DexClass classToDesugar,
+ DexClass implementing,
+ DexType iface) {
+ DefaultMethodsHelper.Collection collection = cache.get(iface);
+ if (collection != null) {
+ return collection;
+ }
+ collection = createInterfaceInfo(classToDesugar, implementing, iface);
+ Collection existing = cache.putIfAbsent(iface, collection);
+ return existing != null ? existing : collection;
+ }
+
+ private DefaultMethodsHelper.Collection createInterfaceInfo(
+ DexClass classToDesugar,
+ DexClass implementing,
+ DexType iface) {
+ DefaultMethodsHelper helper = new DefaultMethodsHelper();
+ DexClass definedInterface = findDefinitionFor(iface);
+ if (definedInterface == null) {
+ warnMissingInterface(classToDesugar, implementing, iface);
+ return helper.wrapInCollection();
+ }
+
+ if (!definedInterface.isInterface()) {
+ throw new CompilationError(
+ "Type " + iface.toSourceString() + " is referenced as an interface from `"
+ + implementing.toString() + "`.");
+ }
+
+ if (definedInterface.isLibraryClass()) {
+ // NOTE: We intentionally ignore all candidates coming from android.jar
+ // since it is only possible in case v24+ version of android.jar is provided.
+ // WARNING: This may result in incorrect code if something else than Android bootclasspath
+ // classes are given as libraries!
+ return helper.wrapInCollection();
+ }
+
+ // Merge information from all superinterfaces.
+ for (DexType superinterface : definedInterface.interfaces.values) {
+ helper.merge(getOrCreateInterfaceInfo(classToDesugar, definedInterface, superinterface));
+ }
+
+ // Hide by virtual methods of this interface.
+ for (DexEncodedMethod virtual : definedInterface.virtualMethods()) {
+ helper.hideMatches(virtual.method);
+ }
+
+ // Add all default methods of this interface.
+ for (DexEncodedMethod encoded : definedInterface.virtualMethods()) {
+ if (isDefaultMethod(encoded)) {
+ helper.addDefaultMethod(encoded);
+ }
+ }
+
+ return helper.wrapInCollection();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/desugaring/interfacemethods/InterfaceMethodDesugaringTests.java b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/InterfaceMethodDesugaringTests.java
index e07f319..d2cf708 100644
--- a/src/test/java/com/android/tools/r8/desugaring/interfacemethods/InterfaceMethodDesugaringTests.java
+++ b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/InterfaceMethodDesugaringTests.java
@@ -4,8 +4,7 @@
package com.android.tools.r8.desugaring.interfacemethods;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
import com.android.tools.r8.AsmTestBase;
import com.android.tools.r8.ToolHelper;
@@ -33,7 +32,7 @@
ToolHelper.getMinApiLevelForDexVm(),
ToolHelper.getClassAsBytes(
com.android.tools.r8.desugaring.interfacemethods.test0.TestMain.class),
- introduceInvokeSpecial(ToolHelper.getClassAsBytes(
+ patchInterfaceWithDefaults(ToolHelper.getClassAsBytes(
com.android.tools.r8.desugaring.interfacemethods.test0.InterfaceWithDefaults.class)));
}
@@ -47,16 +46,33 @@
ToolHelper.getMinApiLevelForDexVm(),
ToolHelper.getClassAsBytes(
com.android.tools.r8.desugaring.interfacemethods.test1.TestMain.class),
- introduceInvokeSpecial(ToolHelper.getClassAsBytes(
+ patchInterfaceWithDefaults(ToolHelper.getClassAsBytes(
com.android.tools.r8.desugaring.interfacemethods.test1.InterfaceWithDefaults.class)));
}
- private class MutableBoolean {
- boolean value;
+ @Test
+ public void testInvokeSpecialToInheritedDefaultMethod() throws Exception {
+ ensureSameOutput(
+ com.android.tools.r8.desugaring.interfacemethods.test2.TestMain.class.getCanonicalName(),
+ ToolHelper.getMinApiLevelForDexVm(),
+ ToolHelper.getClassAsBytes(
+ com.android.tools.r8.desugaring.interfacemethods.test2.TestMain.class),
+ ToolHelper.getClassAsBytes(
+ com.android.tools.r8.desugaring.interfacemethods.test2.Test.class),
+ ToolHelper.getClassAsBytes(
+ com.android.tools.r8.desugaring.interfacemethods.test2.LeftTest.class),
+ ToolHelper.getClassAsBytes(
+ com.android.tools.r8.desugaring.interfacemethods.test2.RightTest.class),
+ ToolHelper.getClassAsBytes(
+ com.android.tools.r8.desugaring.interfacemethods.test2.Test2.class));
}
- private byte[] introduceInvokeSpecial(byte[] classBytes) throws IOException {
- MutableBoolean patched = new MutableBoolean();
+ private static class MutableInteger {
+ int value;
+ }
+
+ private byte[] patchInterfaceWithDefaults(byte[] classBytes) throws IOException {
+ MutableInteger patched = new MutableInteger();
try (InputStream input = new ByteArrayInputStream(classBytes)) {
ClassReader cr = new ClassReader(input);
ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES);
@@ -73,9 +89,9 @@
if (opcode == Opcodes.INVOKEINTERFACE &&
owner.endsWith("InterfaceWithDefaults") &&
name.equals("foo")) {
- assertFalse(patched.value);
+ assertEquals(0, patched.value);
super.visitMethodInsn(Opcodes.INVOKESPECIAL, owner, name, desc, itf);
- patched.value = true;
+ patched.value++;
} else {
super.visitMethodInsn(opcode, owner, name, desc, itf);
@@ -84,7 +100,7 @@
};
}
}, 0);
- assertTrue(patched.value);
+ assertEquals(1, patched.value);
return cw.toByteArray();
}
}
diff --git a/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/LeftTest.java b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/LeftTest.java
new file mode 100644
index 0000000..4a5b2de
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/LeftTest.java
@@ -0,0 +1,8 @@
+// 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.desugaring.interfacemethods.test2;
+
+public interface LeftTest extends Test {
+}
diff --git a/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/RightTest.java b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/RightTest.java
new file mode 100644
index 0000000..6c6ea7e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/RightTest.java
@@ -0,0 +1,8 @@
+// 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.desugaring.interfacemethods.test2;
+
+public interface RightTest extends Test {
+}
diff --git a/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/Test.java b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/Test.java
new file mode 100644
index 0000000..1bf3295
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/Test.java
@@ -0,0 +1,11 @@
+// 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.desugaring.interfacemethods.test2;
+
+public interface Test {
+ default String foo(String a) {
+ return "Test::foo(" + a + ")";
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/Test2.java b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/Test2.java
new file mode 100644
index 0000000..b076cde
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/Test2.java
@@ -0,0 +1,11 @@
+// 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.desugaring.interfacemethods.test2;
+
+public interface Test2 extends LeftTest, RightTest {
+ default String bar(String a) {
+ return "Test2::bar(" + LeftTest.super.foo(a) + " + " + RightTest.super.foo(a) + ")";
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/TestMain.java b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/TestMain.java
new file mode 100644
index 0000000..ca86963
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugaring/interfacemethods/test2/TestMain.java
@@ -0,0 +1,19 @@
+// 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.desugaring.interfacemethods.test2;
+
+public class TestMain implements Test2 {
+ public static void main(String... args) {
+ TestMain m = new TestMain();
+ System.out.println(m.bar("first"));
+ System.out.println(m.foo("second"));
+ System.out.println(m.fooDelegate("third"));
+ }
+
+ private String fooDelegate(String a) {
+ return "TestMain::fooDelegate(" + Test2.super.foo(a) + ")";
+ }
+}
+
