Version 2.1.36
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 bc82245..3daa05c 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.1.35";
+ public static final String LABEL = "2.1.36";
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 901517a..4e6d0ab 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
@@ -284,66 +284,52 @@
if (instruction.isInvokeSuper()) {
InvokeSuper invokeSuper = instruction.asInvokeSuper();
DexMethod invokedMethod = invokeSuper.getInvokedMethod();
- DexClass clazz = appInfo.definitionFor(invokedMethod.holder);
+ DexClass clazz = appInfo.definitionForHolder(invokedMethod);
if (clazz == null) {
// NOTE: leave unchanged those calls to undefined targets. This may lead to runtime
// 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.holder()), invokedMethod);
instructions.replaceCurrentInstruction(
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
- .appInfoForDesugaring()
- .lookupSuperTarget(invokeSuper.getInvokedMethod(), code.context());
- if (dexEncodedMethod != null) {
- DexClass dexClass = appView.definitionFor(dexEncodedMethod.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.context());
+ 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()));
}
}
}
@@ -418,6 +404,28 @@
}
}
+ private DexEncodedMethod targetForInvokeSuperDispatchToDefaultMethod(
+ DexMethod invokedSuperMethod, DexLibraryClass holder, ProgramMethod 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, appView)) {
+ return null;
+ }
+ DexEncodedMethod target =
+ appView.appInfoForDesugaring().lookupSuperTarget(invokedSuperMethod, context);
+ DexClass targetHolder = appView.definitionForHolder(target);
+ 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 edfb00c..1496847 100644
--- a/src/test/java/com/android/tools/r8/D8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/D8TestBuilder.java
@@ -74,12 +74,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 61db3e3..2675329 100644
--- a/src/test/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -545,12 +545,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 b7869e4..ce03733 100644
--- a/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
@@ -10,7 +10,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;
@@ -378,8 +377,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..9d3d498
--- /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.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(apiLevelWithDefaultInterfaceMethodsSupport());
+ }
+
+ @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..f63c4d9
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugar/InvokeSuperToRewrittenDefaultMethodTest.java
@@ -0,0 +1,139 @@
+// 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 com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticType;
+import static org.hamcrest.CoreMatchers.allOf;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.StringResource;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.desugar.desugaredlibrary.DesugaredLibraryTestBase;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.ExceptionDiagnostic;
+import com.android.tools.r8.utils.StringUtils;
+import java.util.List;
+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}, old-rt:{1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimesAndApiLevels().build(), BooleanUtils.values());
+ }
+
+ private final TestParameters parameters;
+ private final boolean rtWithoutConsumer;
+
+ public InvokeSuperToRewrittenDefaultMethodTest(
+ TestParameters parameters, boolean rtWithoutConsumer) {
+ this.parameters = parameters;
+ this.rtWithoutConsumer = rtWithoutConsumer;
+ }
+
+ private boolean needsDefaultInterfaceMethodDesugaring() {
+ return parameters.isDexRuntime()
+ && parameters.getApiLevel().isLessThan(apiLevelWithDefaultInterfaceMethodsSupport());
+ }
+
+ @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());
+ try {
+ testForD8()
+ .addInnerClasses(InvokeSuperToRewrittenDefaultMethodTest.class)
+ .setMinApi(parameters.getApiLevel())
+ .apply(
+ b -> {
+ if (rtWithoutConsumer) {
+ b.addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.B));
+ // TODO(b/158543446): Remove this once enableCoreLibraryDesugaring is fixed.
+ b.getBuilder()
+ .addDesugaredLibraryConfiguration(
+ StringResource.fromFile(ToolHelper.DESUGAR_LIB_JSON_FOR_TESTING));
+ } else {
+ // TODO(b/158543446): Move this out to the shared builder once
+ // enableCoreLibraryDesugaring is fixed.
+ b.enableCoreLibraryDesugaring(parameters.getApiLevel());
+ }
+ })
+ .compileWithExpectedDiagnostics(
+ diagnostics -> {
+ if (rtWithoutConsumer) {
+ diagnostics.assertOnlyErrors();
+ // TODO(b/158543011): Should fail with a nice user error for invalid library.
+ diagnostics.assertErrorsMatch(
+ allOf(
+ diagnosticType(ExceptionDiagnostic.class),
+ diagnosticMessage(containsString("AssertionError"))));
+ } else {
+ diagnostics.assertNoMessages();
+ }
+ })
+ .addDesugaredCoreLibraryRunClassPath(
+ this::buildDesugaredLibrary, parameters.getApiLevel())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ assertFalse(rtWithoutConsumer);
+ } catch (CompilationFailedException e) {
+ assertTrue(rtWithoutConsumer);
+ }
+ }
+
+ @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');
+ }
+ }
+}
