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'); + } + } +}