Fix invoke-super of a default method in desugared library for cf to cf
This forces IR processing for a method when there is an invoke-super
targeting a default method in the desugared library. The rewriting
of that currently requires IR processing.
Bug: 177221295
Change-Id: I8f15adde05f0b348517fc48e49fd230a64d29889
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/NeedsIRDesugarUseRegistry.java b/src/main/java/com/android/tools/r8/ir/conversion/NeedsIRDesugarUseRegistry.java
index a9bbb90..a89e3f9 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/NeedsIRDesugarUseRegistry.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/NeedsIRDesugarUseRegistry.java
@@ -56,14 +56,14 @@
public void registerInvokeVirtual(DexMethod method) {
registerBackportedMethodRewriting(method);
registerLibraryRetargeting(method, false);
- registerInterfaceMethodRewriting(method);
+ registerInterfaceMethodRewriting(method, false);
registerDesugaredLibraryAPIConverter(method);
}
@Override
public void registerInvokeDirect(DexMethod method) {
registerLibraryRetargeting(method, false);
- registerInterfaceMethodRewriting(method);
+ registerInterfaceMethodRewriting(method, false);
registerDesugaredLibraryAPIConverter(method);
}
@@ -80,10 +80,11 @@
}
}
- private void registerInterfaceMethodRewriting(DexMethod method) {
+ private void registerInterfaceMethodRewriting(DexMethod method, boolean isInvokeSuper) {
if (!needsDesugarging) {
needsDesugarging =
- interfaceMethodRewriter != null && interfaceMethodRewriter.needsRewriting(method);
+ interfaceMethodRewriter != null
+ && interfaceMethodRewriter.needsRewriting(method, isInvokeSuper, appView);
}
}
@@ -108,14 +109,14 @@
registerTwrCloseResourceRewriting(method);
registerBackportedMethodRewriting(method);
registerLibraryRetargeting(method, false);
- registerInterfaceMethodRewriting(method);
+ registerInterfaceMethodRewriting(method, false);
registerDesugaredLibraryAPIConverter(method);
}
@Override
public void registerInvokeInterface(DexMethod method) {
registerLibraryRetargeting(method, true);
- registerInterfaceMethodRewriting(method);
+ registerInterfaceMethodRewriting(method, false);
registerDesugaredLibraryAPIConverter(method);
}
@@ -136,7 +137,7 @@
@Override
public void registerInvokeSuper(DexMethod method) {
registerLibraryRetargeting(method, false);
- registerInterfaceMethodRewriting(method);
+ registerInterfaceMethodRewriting(method, true);
registerDesugaredLibraryAPIConverter(method);
}
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 27216c1..9ea3aa3 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
@@ -216,7 +216,16 @@
return emulatedInterfaces.containsKey(itf);
}
- public boolean needsRewriting(DexMethod method) {
+ public boolean needsRewriting(DexMethod method, boolean isInvokeSuper, AppView<?> appView) {
+ if (isInvokeSuper) {
+ DexClass clazz = appView.appInfo().definitionFor(method.getHolderType());
+ if (clazz != null
+ && clazz.isLibraryClass()
+ && clazz.isInterface()
+ && appView.rewritePrefix.hasRewrittenType(clazz.type, appView)) {
+ return true;
+ }
+ }
return emulatedMethods.contains(method.getName());
}
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/IterableTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/IterableTest.java
index f6e0d0a..f998b89 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/IterableTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/IterableTest.java
@@ -4,16 +4,23 @@
package com.android.tools.r8.desugar.desugaredlibrary;
+import static org.hamcrest.MatcherAssert.assertThat;
+
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.CodeMatchers;
+import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
+import java.util.function.Consumer;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
import org.jetbrains.annotations.NotNull;
+import org.junit.Assume;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -25,12 +32,13 @@
private final TestParameters parameters;
private final boolean shrinkDesugaredLibrary;
private static final String EXPECTED_OUTPUT =
- StringUtils.lines("1", "2", "3", "4", "5", "Count: 4");
+ StringUtils.lines("1", "2", "3", "4", "5", "Count: 4", "1", "2", "3", "4", "5");
@Parameters(name = "{1}, shrinkDesugaredLibrary: {0}")
public static List<Object[]> data() {
return buildParameters(
- BooleanUtils.values(), getTestParameters().withAllRuntimes().withAllApiLevels().build());
+ BooleanUtils.values(),
+ getTestParameters().withAllRuntimes().withAllApiLevelsAlsoForCf().build());
}
public IterableTest(boolean shrinkDesugaredLibrary, TestParameters parameters) {
@@ -38,6 +46,54 @@
this.parameters = parameters;
}
+ private void inspect(CodeInspector inspector) {
+ if (parameters
+ .getApiLevel()
+ .isGreaterThanOrEqualTo(apiLevelWithDefaultInterfaceMethodsSupport())) {
+ assertThat(
+ inspector.clazz(MyIterableSub.class).uniqueMethodWithFinalName("myForEach"),
+ CodeMatchers.invokesMethod(null, MyIterable.class.getTypeName(), "forEach", null));
+ } else {
+ assertThat(
+ inspector.clazz(MyIterableSub.class).uniqueMethodWithFinalName("myForEach"),
+ CodeMatchers.invokesMethod(null, "j$.lang.Iterable$-CC", "$default$forEach", null));
+ }
+ }
+
+ @Test
+ public void testIterableD8Cf() throws Exception {
+ // Only test without shrinking desugared library.
+ Assume.assumeFalse(shrinkDesugaredLibrary);
+ // Use D8 to desugar with Java classfile output.
+ Path jar =
+ testForD8(Backend.CF)
+ .addInnerClasses(IterableTest.class)
+ .setMinApi(parameters.getApiLevel())
+ .enableCoreLibraryDesugaring(parameters.getApiLevel())
+ .compile()
+ .inspect(this::inspect)
+ .writeToZip();
+
+ if (parameters.getRuntime().isDex()) {
+ // Convert to DEX without desugaring and run.
+ testForD8()
+ .addProgramFiles(jar)
+ .setMinApi(parameters.getApiLevel())
+ .disableDesugaring()
+ .compile()
+ .addRunClasspathFiles(buildDesugaredLibrary(parameters.getApiLevel()))
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ } else {
+ // Run on the JVM with desugared library on classpath.
+ testForJvm()
+ .addProgramFiles(jar)
+ .addRunClasspathFiles(buildDesugaredLibraryClassFile(parameters.getApiLevel()))
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+ }
+
@Test
public void testIterable() throws Exception {
if (parameters.isCfRuntime()) {
@@ -69,6 +125,8 @@
iterable.forEach(System.out::println);
Stream<Integer> stream = StreamSupport.stream(iterable.spliterator(), false);
System.out.println("Count: " + stream.filter(x -> x != 3).count());
+ MyIterableSub<Integer> iterableSub = new MyIterableSub<>(Arrays.asList(1, 2, 3, 4, 5));
+ iterableSub.myForEach(System.out::println);
}
}
@@ -86,4 +144,15 @@
return collection.iterator();
}
}
+
+ static class MyIterableSub<E> extends MyIterable<E> {
+
+ public MyIterableSub(Collection<E> collection) {
+ super(collection);
+ }
+
+ public void myForEach(Consumer<E> consumer) {
+ super.forEach(consumer);
+ }
+ }
}
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/JavaTimeTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/JavaTimeTest.java
index efb77c9..31bbcbd 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/JavaTimeTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/JavaTimeTest.java
@@ -5,15 +5,15 @@
package com.android.tools.r8.desugar.desugaredlibrary;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static org.hamcrest.CoreMatchers.anyOf;
-import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoVerticalClassMerging;
+import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestCompileResult;
import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestRunResult;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.BooleanUtils;
@@ -22,6 +22,7 @@
import com.android.tools.r8.utils.codeinspector.CheckCastInstructionSubject;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.CodeMatchers;
import com.android.tools.r8.utils.codeinspector.InstructionSubject;
import com.android.tools.r8.utils.codeinspector.InvokeInstructionSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
@@ -58,8 +59,8 @@
"GMT",
"GMT",
"true",
+ "true",
"Hello, world");
- boolean canUseDefaultAndStaticInterfaceMethods;
@Parameters(name = "{2}, shrinkDesugaredLibrary: {0}, traceReferencesKeepRules {1}")
public static List<Object[]> data() {
@@ -78,13 +79,17 @@
this.shrinkDesugaredLibrary = shrinkDesugaredLibrary;
this.traceReferencesKeepRules = traceReferencesKeepRules;
this.parameters = parameters;
- this.canUseDefaultAndStaticInterfaceMethods =
- parameters
- .getApiLevel()
- .isGreaterThanOrEqualTo(apiLevelWithDefaultInterfaceMethodsSupport());
}
- private void checkRewrittenInvokes(CodeInspector inspector) {
+ private void checkRewrittenInvokesForD8(CodeInspector inspector) {
+ checkRewrittenInvokes(inspector, false);
+ }
+
+ private void checkRewrittenInvokesForR8(CodeInspector inspector) {
+ checkRewrittenInvokes(inspector, true);
+ }
+
+ private void checkRewrittenInvokes(CodeInspector inspector, boolean isR8) {
Set<String> expectedInvokeHolders;
Set<String> expectedCatchGuards;
Set<String> expectedCheckCastType;
@@ -133,6 +138,31 @@
.map(TypeSubject::toString)
.collect(Collectors.toSet());
assertEquals(expectedCatchGuards, foundCatchGuards);
+
+ if (isR8) {
+ assertThat(
+ inspector.clazz(TemporalAccessorImpl.class).uniqueMethodWithFinalName("query"),
+ not(isPresent()));
+ } else {
+ assertThat(
+ inspector.clazz(TemporalAccessorImplSub.class).uniqueMethodWithFinalName("query"),
+ CodeMatchers.invokesMethod(
+ null, TemporalAccessorImpl.class.getTypeName(), "query", null));
+ }
+ if (parameters
+ .getApiLevel()
+ .isGreaterThanOrEqualTo(TestBase.apiLevelWithDefaultInterfaceMethodsSupport())) {
+ assertThat(
+ inspector.clazz(TemporalAccessorImpl.class).uniqueMethodWithName("query"),
+ not(isPresent()));
+ } else {
+ assertThat(
+ inspector
+ .clazz(isR8 ? TemporalAccessorImplSub.class : TemporalAccessorImpl.class)
+ .uniqueMethodWithFinalName("query"),
+ CodeMatchers.invokesMethod(
+ null, "j$.time.temporal.TemporalAccessor$-CC", "$default$query", null));
+ }
}
private String desugaredLibraryKeepRules(
@@ -164,7 +194,7 @@
.setMinApi(parameters.getApiLevel())
.enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer)
.compile()
- .inspect(this::checkRewrittenInvokes)
+ .inspect(this::checkRewrittenInvokesForD8)
.writeToZip();
String desugaredLibraryKeepRules;
@@ -177,37 +207,27 @@
}
// Determine desugared library keep rules.
- TestRunResult<?> result;
if (parameters.getRuntime().isDex()) {
// Convert to DEX without desugaring and run.
- result =
- testForD8()
- .addProgramFiles(jar)
- .setMinApi(parameters.getApiLevel())
- .disableDesugaring()
- .compile()
- .addDesugaredCoreLibraryRunClassPath(
- this::buildDesugaredLibrary,
- parameters.getApiLevel(),
- desugaredLibraryKeepRules,
- shrinkDesugaredLibrary)
- .run(parameters.getRuntime(), TestClass.class);
+ testForD8()
+ .addProgramFiles(jar)
+ .setMinApi(parameters.getApiLevel())
+ .disableDesugaring()
+ .compile()
+ .addDesugaredCoreLibraryRunClassPath(
+ this::buildDesugaredLibrary,
+ parameters.getApiLevel(),
+ desugaredLibraryKeepRules,
+ shrinkDesugaredLibrary)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(expectedOutput);
} else {
// Run on the JVM with desugared library on classpath.
- result =
- testForJvm()
- .addProgramFiles(jar)
- .addRunClasspathFiles(buildDesugaredLibraryClassFile(parameters.getApiLevel()))
- .run(parameters.getRuntime(), TestClass.class);
- }
- if (canUseDefaultAndStaticInterfaceMethods) {
- result.assertSuccessWithOutput(expectedOutput);
- } else {
- result.assertFailureWithErrorThatMatches(
- anyOf(
- containsString(VerifyError.class.getName()),
- containsString(IncompatibleClassChangeError.class.getName()),
- containsString(AbstractMethodError.class.getName())));
+ testForJvm()
+ .addProgramFiles(jar)
+ .addRunClasspathFiles(buildDesugaredLibraryClassFile(parameters.getApiLevel()))
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(expectedOutput);
}
}
@@ -223,7 +243,7 @@
.setMinApi(parameters.getApiLevel())
.enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer)
.compile()
- .inspect(this::checkRewrittenInvokes);
+ .inspect(this::checkRewrittenInvokesForD8);
result
.addDesugaredCoreLibraryRunClassPath(
this::buildDesugaredLibrary,
@@ -244,11 +264,12 @@
testForR8(parameters.getBackend())
.addInnerClasses(JavaTimeTest.class)
.addKeepMainRule(TestClass.class)
+ .enableNoVerticalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
.enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer)
.enableInliningAnnotations()
.compile()
- .inspect(this::checkRewrittenInvokes);
+ .inspect(this::checkRewrittenInvokesForR8);
result
.addDesugaredCoreLibraryRunClassPath(
this::buildDesugaredLibrary,
@@ -259,6 +280,31 @@
.assertSuccessWithOutput(expectedOutput);
}
+ @NoVerticalClassMerging
+ static class TemporalAccessorImpl implements TemporalAccessor {
+ @Override
+ public boolean isSupported(TemporalField field) {
+ return false;
+ }
+
+ @Override
+ public long getLong(TemporalField field) {
+ throw new DateTimeException("Mock");
+ }
+ }
+
+ @NoVerticalClassMerging
+ static class TemporalAccessorImplSub extends TemporalAccessorImpl {
+ @SuppressWarnings("unchecked")
+ @Override
+ public <R> R query(TemporalQuery<R> query) {
+ if (query == TemporalQueries.zoneId()) {
+ return (R) ZoneId.of("GMT");
+ }
+ return super.query(query);
+ }
+ }
+
static class TestClass {
@NeverInline
@@ -296,6 +342,11 @@
System.out.println(ZoneId.from(mock).equals(ZoneId.of("GMT")));
}
+ public static void superInvokeOnLibraryDesugaredDefaultMethodFromSubclass() {
+ TemporalAccessor mock = new TemporalAccessorImplSub();
+ System.out.println(ZoneId.from(mock).equals(ZoneId.of("GMT")));
+ }
+
public static void main(String[] args) {
java.time.Clock.systemDefaultZone();
try {
@@ -317,6 +368,7 @@
System.out.println(timeZone.toZoneId().getId());
superInvokeOnLibraryDesugaredDefaultMethod();
+ superInvokeOnLibraryDesugaredDefaultMethodFromSubclass();
System.out.println("Hello, world");
}
