Version 2.0.80
Cherry pick: Rewrite invoke-super targeting rewritten desugared library methods.
CL: https://r8-review.googlesource.com/c/r8/+/51960
Bug: 157806261
Change-Id: Ifb1b8e739fdd4161fb259587106fe6bca542644a
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index c918040..6bb88a8 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "2.0.79";
+ public static final String LABEL = "2.0.80";
private Version() {
}
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 7ca1e54..756243a 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
@@ -255,16 +255,14 @@
// exception but we can not report it as error since it can also be the intended
// behavior.
warnMissingType(encodedMethod.method, invokedMethod.holder);
- } else if (clazz.isInterface() && !clazz.isLibraryClass()) {
- // NOTE: we intentionally don't desugar super calls into interface methods
- // coming from android.jar since it is only possible in case v24+ version
- // of android.jar is provided.
- //
- // We assume such calls are properly guarded by if-checks like
- // 'if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.XYZ) { ... }'
- //
- // WARNING: This may result in incorrect code on older platforms!
- // Retarget call to an appropriate method of companion class.
+ continue;
+ }
+ if (!clazz.isInterface()) {
+ // Skip non-interface invokes.
+ continue;
+ }
+ if (!clazz.isLibraryClass()) {
+ // For program and classpath retarget call to an appropriate method of companion class.
DexMethod amendedMethod =
amendDefaultMethod(
appInfo.definitionFor(encodedMethod.method.holder), invokedMethod);
@@ -272,44 +270,32 @@
new InvokeStatic(defaultAsMethodOfCompanionClass(amendedMethod),
invokeSuper.outValue(), invokeSuper.arguments()));
} else {
- DexType dexType = maximallySpecificEmulatedInterfaceOrNull(invokedMethod);
- if (dexType != null) {
- // That invoke super may not resolve since the super method may not be present
- // since it's in the emulated interface. We need to force resolution. If it resolves
- // to a library method, then it needs to be rewritten.
- // If it resolves to a program overrides, the invoke-super can remain.
- DexEncodedMethod dexEncodedMethod =
- appView
- .appInfo()
- .lookupSuperTarget(invokeSuper.getInvokedMethod(), code.method.method.holder);
- if (dexEncodedMethod != null) {
- DexClass dexClass = appView.definitionFor(dexEncodedMethod.method.holder);
- if (dexClass != null && dexClass.isLibraryClass()) {
- // Rewriting is required because the super invoke resolves into a missing
- // method (method is on desugared library). Find out if it needs to be
- // retarget or if it just calls a companion class method and rewrite.
- DexMethod retargetMethod =
- options.desugaredLibraryConfiguration.retargetMethod(
- dexEncodedMethod.method, appView);
- if (retargetMethod == null) {
- DexMethod originalCompanionMethod =
- instanceAsMethodOfCompanionClass(
- dexEncodedMethod.method, DEFAULT_METHOD_PREFIX, factory);
- DexMethod companionMethod =
- factory.createMethod(
- getCompanionClassType(dexType),
- factory.protoWithDifferentFirstParameter(
- originalCompanionMethod.proto, dexType),
- originalCompanionMethod.name);
- instructions.replaceCurrentInstruction(
- new InvokeStatic(
- companionMethod, invokeSuper.outValue(), invokeSuper.arguments()));
- } else {
- instructions.replaceCurrentInstruction(
- new InvokeStatic(
- retargetMethod, invokeSuper.outValue(), invokeSuper.arguments()));
- }
- }
+ // Rewriting is required if the super invoke resolves to a default method on the
+ // desugared library. Retarget or rewrite to the desugared library companion class.
+ DexEncodedMethod dexEncodedMethod =
+ targetForInvokeSuperDispatchToDefaultMethod(
+ invokedMethod, clazz.asLibraryClass(), code.method.method.holder);
+ if (dexEncodedMethod != null) {
+ DexMethod retargetMethod =
+ options.desugaredLibraryConfiguration.retargetMethod(
+ dexEncodedMethod.method, appView);
+ if (retargetMethod == null) {
+ DexMethod originalCompanionMethod =
+ instanceAsMethodOfCompanionClass(
+ dexEncodedMethod.method, DEFAULT_METHOD_PREFIX, factory);
+ DexMethod companionMethod =
+ factory.createMethod(
+ getCompanionClassType(clazz.type),
+ factory.protoWithDifferentFirstParameter(
+ originalCompanionMethod.proto, clazz.type),
+ originalCompanionMethod.name);
+ instructions.replaceCurrentInstruction(
+ new InvokeStatic(
+ companionMethod, invokeSuper.outValue(), invokeSuper.arguments()));
+ } else {
+ instructions.replaceCurrentInstruction(
+ new InvokeStatic(
+ retargetMethod, invokeSuper.outValue(), invokeSuper.arguments()));
}
}
}
@@ -384,6 +370,27 @@
}
}
+ private DexEncodedMethod targetForInvokeSuperDispatchToDefaultMethod(
+ DexMethod invokedSuperMethod, DexLibraryClass holder, DexType context) {
+ assert invokedSuperMethod.holder == holder.type;
+ assert holder.isInterface();
+ DexEncodedMethod definition = holder.lookupMethod(invokedSuperMethod);
+ if (definition == null || !definition.isDefaultMethod()) {
+ return null;
+ }
+ // Only default methods on emulated interfaces or rewritten types need to be dealt with.
+ if (!emulatedMethods.contains(invokedSuperMethod.name)
+ && !appView.rewritePrefix.hasRewrittenType(holder.type)) {
+ return null;
+ }
+ DexEncodedMethod target = appView.appInfo().lookupSuperTarget(invokedSuperMethod, context);
+ DexClass targetHolder = appView.definitionFor(target.method.holder);
+ if (targetHolder == null || !targetHolder.isLibraryClass()) {
+ return null;
+ }
+ return target;
+ }
+
private DexType maximallySpecificEmulatedInterfaceOrNull(DexMethod invokedMethod) {
// Here we try to avoid doing the expensive look-up on all invokes.
if (!emulatedMethods.contains(invokedMethod.name)) {
diff --git a/src/test/java/com/android/tools/r8/D8TestBuilder.java b/src/test/java/com/android/tools/r8/D8TestBuilder.java
index d740e54..5ab2703 100644
--- a/src/test/java/com/android/tools/r8/D8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/D8TestBuilder.java
@@ -67,12 +67,9 @@
@Override
public D8TestBuilder enableCoreLibraryDesugaring(
- AndroidApiLevel minAPILevel, KeepRuleConsumer keepRuleConsumer) {
- if (minAPILevel.getLevel() < AndroidApiLevel.O.getLevel()) {
- // Use P to mimic current Android Studio.
- builder.addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P));
- builder.addDesugaredLibraryConfiguration(
- StringResource.fromFile(ToolHelper.DESUGAR_LIB_JSON_FOR_TESTING));
+ AndroidApiLevel minApiLevel, KeepRuleConsumer keepRuleConsumer) {
+ if (minApiLevel.getLevel() < AndroidApiLevel.O.getLevel()) {
+ super.enableCoreLibraryDesugaring(minApiLevel, keepRuleConsumer);
builder.setDesugaredLibraryKeepRuleConsumer(keepRuleConsumer);
}
return self();
diff --git a/src/test/java/com/android/tools/r8/R8TestBuilder.java b/src/test/java/com/android/tools/r8/R8TestBuilder.java
index 59ae602..52a7c29 100644
--- a/src/test/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -357,12 +357,9 @@
@Override
public T enableCoreLibraryDesugaring(
- AndroidApiLevel minAPILevel, KeepRuleConsumer keepRuleConsumer) {
- if (minAPILevel.getLevel() < AndroidApiLevel.O.getLevel()) {
- // Use P to mimic current Android Studio.
- builder.addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P));
- builder.addDesugaredLibraryConfiguration(
- StringResource.fromFile(ToolHelper.DESUGAR_LIB_JSON_FOR_TESTING));
+ AndroidApiLevel minApiLevel, KeepRuleConsumer keepRuleConsumer) {
+ if (minApiLevel.getLevel() < AndroidApiLevel.O.getLevel()) {
+ super.enableCoreLibraryDesugaring(minApiLevel, keepRuleConsumer);
builder.setDesugaredLibraryKeepRuleConsumer(keepRuleConsumer);
}
return self();
diff --git a/src/test/java/com/android/tools/r8/TestCompilerBuilder.java b/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
index cc8d46a..7627098 100644
--- a/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
@@ -6,7 +6,6 @@
import com.android.tools.r8.TestBase.Backend;
import com.android.tools.r8.debug.DebugTestConfig;
import com.android.tools.r8.desugar.desugaredlibrary.DesugaredLibraryTestBase.KeepRuleConsumer;
-import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.AndroidAppConsumers;
@@ -286,8 +285,16 @@
}
public T enableCoreLibraryDesugaring(
- AndroidApiLevel minAPILevel, KeepRuleConsumer keepRuleConsumer) {
- throw new Unreachable("Should be overridden or is unsupported.");
+ AndroidApiLevel minApiLevel, KeepRuleConsumer keepRuleConsumer) {
+ assert minApiLevel.getLevel() < AndroidApiLevel.O.getLevel();
+ builder.addDesugaredLibraryConfiguration(
+ StringResource.fromFile(ToolHelper.DESUGAR_LIB_JSON_FOR_TESTING));
+ // TODO(b/158543446): This should not be setting an implicit library file. Doing so causes
+ // inconsistent library setup depending on the api level and makes tests hard to read and
+ // reason about.
+ // Use P to mimic current Android Studio.
+ builder.addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P));
+ return self();
}
@Override
diff --git a/src/test/java/com/android/tools/r8/desugar/InvokeSuperToEmulatedDefaultMethodTest.java b/src/test/java/com/android/tools/r8/desugar/InvokeSuperToEmulatedDefaultMethodTest.java
new file mode 100644
index 0000000..5e1a10f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugar/InvokeSuperToEmulatedDefaultMethodTest.java
@@ -0,0 +1,155 @@
+// 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.desugar;
+
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.desugar.desugaredlibrary.DesugaredLibraryTestBase;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class InvokeSuperToEmulatedDefaultMethodTest extends DesugaredLibraryTestBase {
+
+ private static final String EXPECTED = StringUtils.lines("bar", "bar");
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ private final TestParameters parameters;
+
+ public InvokeSuperToEmulatedDefaultMethodTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ private boolean needsDefaultInterfaceMethodDesugaring() {
+ return parameters.isDexRuntime() && parameters.getApiLevel().isLessThan(AndroidApiLevel.N);
+ }
+
+ @Test
+ public void testReference() throws Exception {
+ assumeFalse(needsDefaultInterfaceMethodDesugaring());
+ testForRuntime(parameters)
+ .addInnerClasses(InvokeSuperToEmulatedDefaultMethodTest.class)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test
+ public void testDesugaring() throws Exception {
+ assumeTrue(needsDefaultInterfaceMethodDesugaring());
+ testForD8()
+ .addInnerClasses(InvokeSuperToEmulatedDefaultMethodTest.class)
+ .setMinApi(parameters.getApiLevel())
+ .enableCoreLibraryDesugaring(parameters.getApiLevel())
+ .compile()
+ .addDesugaredCoreLibraryRunClassPath(this::buildDesugaredLibrary, parameters.getApiLevel())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ public interface StringMap extends Map<String, String> {
+
+ @Override
+ default String getOrDefault(Object key, String defaultValue) {
+ // Simple forward that targets the desugared library.
+ return Map.super.getOrDefault(key + "oo", defaultValue);
+ }
+ }
+
+ public interface StringMapIndirection extends StringMap {
+
+ @Override
+ default String getOrDefault(Object key, String defaultValue) {
+ // Simple forward to a program defined default method.
+ return StringMap.super.getOrDefault("f", defaultValue);
+ }
+ }
+
+ public static class StringHashMap implements StringMapIndirection {
+ HashMap<String, String> delegate = new HashMap<>();
+
+ @Override
+ public int size() {
+ return delegate.size();
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return delegate.isEmpty();
+ }
+
+ @Override
+ public boolean containsKey(Object key) {
+ return delegate.containsKey(key);
+ }
+
+ @Override
+ public boolean containsValue(Object value) {
+ return delegate.containsValue(value);
+ }
+
+ @Override
+ public String get(Object key) {
+ return delegate.get(key);
+ }
+
+ @Override
+ public String put(String key, String value) {
+ return delegate.put(key, value);
+ }
+
+ @Override
+ public String remove(Object key) {
+ return delegate.remove(key);
+ }
+
+ @Override
+ public void putAll(Map<? extends String, ? extends String> m) {
+ delegate.putAll(m);
+ }
+
+ @Override
+ public void clear() {
+ delegate.clear();
+ }
+
+ @Override
+ public Set<String> keySet() {
+ return delegate.keySet();
+ }
+
+ @Override
+ public Collection<String> values() {
+ return delegate.values();
+ }
+
+ @Override
+ public Set<Entry<String, String>> entrySet() {
+ return delegate.entrySet();
+ }
+ }
+
+ public static class TestClass {
+
+ public static void main(String[] args) {
+ StringHashMap map = new StringHashMap();
+ map.put("foo", "bar");
+ System.out.println(map.getOrDefault("foo", "not found"));
+ System.out.println(map.getOrDefault("bar", "not found"));
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/desugar/InvokeSuperToRewrittenDefaultMethodTest.java b/src/test/java/com/android/tools/r8/desugar/InvokeSuperToRewrittenDefaultMethodTest.java
new file mode 100644
index 0000000..2d6dc5f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugar/InvokeSuperToRewrittenDefaultMethodTest.java
@@ -0,0 +1,92 @@
+// 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.desugar;
+
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.desugar.desugaredlibrary.DesugaredLibraryTestBase;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+import java.util.function.Consumer;
+import java.util.function.IntConsumer;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class InvokeSuperToRewrittenDefaultMethodTest extends DesugaredLibraryTestBase {
+
+ private static final String EXPECTED = StringUtils.lines("Y", "89");
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ private final TestParameters parameters;
+
+ public InvokeSuperToRewrittenDefaultMethodTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ private boolean needsDefaultInterfaceMethodDesugaring() {
+ return parameters.isDexRuntime() && parameters.getApiLevel().isLessThan(AndroidApiLevel.N);
+ }
+
+ @Test
+ public void testReference() throws Exception {
+ assumeFalse(needsDefaultInterfaceMethodDesugaring());
+ testForRuntime(parameters)
+ .addInnerClasses(InvokeSuperToRewrittenDefaultMethodTest.class)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test
+ public void testDesugaring() throws Exception {
+ assumeTrue(needsDefaultInterfaceMethodDesugaring());
+ testForD8()
+ .addInnerClasses(InvokeSuperToRewrittenDefaultMethodTest.class)
+ .setMinApi(parameters.getApiLevel())
+ .enableCoreLibraryDesugaring(parameters.getApiLevel())
+ .compile()
+ .addDesugaredCoreLibraryRunClassPath(this::buildDesugaredLibrary, parameters.getApiLevel())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @FunctionalInterface
+ public interface CharConsumer extends Consumer<Character>, IntConsumer {
+
+ void accept(char c);
+
+ @Override
+ default void accept(int value) {
+ accept((char) value);
+ }
+
+ @Override
+ default void accept(Character character) {
+ accept(character.charValue());
+ }
+
+ @Override
+ default Consumer<Character> andThen(Consumer<? super Character> after) {
+ // Simple forward to the default method of the parent.
+ // Must be rewritten to target the companion class of the rewritten Consumer type.
+ return Consumer.super.andThen(after);
+ }
+ }
+
+ public static class TestClass {
+
+ public static void main(String[] args) {
+ CharConsumer consumer = System.out::println;
+ consumer.andThen((Consumer<? super Character>) c -> System.out.println((int) c)).accept('Y');
+ }
+ }
+}