Extend profile rewriting to covariant return type annotations
Bug: b/265729283
Change-Id: I1084ad589809911d7bb1016e08ee81643ae2207b
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index e607b66..64734f8 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -220,7 +220,7 @@
: CfInstructionDesugaringCollection.create(appView, appView.apiLevelCompute());
this.covariantReturnTypeAnnotationTransformer =
options.processCovariantReturnTypeAnnotations
- ? new CovariantReturnTypeAnnotationTransformer(this, appView.dexItemFactory())
+ ? new CovariantReturnTypeAnnotationTransformer(appView, this)
: null;
if (appView.options().desugarState.isOn()
&& appView.options().apiModelingOptions().enableOutliningOfMethods) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/PrimaryD8L8IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/PrimaryD8L8IRConverter.java
index 1d06a25..0092f10 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/PrimaryD8L8IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/PrimaryD8L8IRConverter.java
@@ -22,6 +22,7 @@
import com.android.tools.r8.ir.desugar.CfInstructionDesugaringEventConsumer;
import com.android.tools.r8.ir.desugar.CfPostProcessingDesugaringCollection;
import com.android.tools.r8.ir.desugar.CfPostProcessingDesugaringEventConsumer;
+import com.android.tools.r8.ir.desugar.CovariantReturnTypeAnnotationTransformerEventConsumer;
import com.android.tools.r8.ir.desugar.ProgramAdditions;
import com.android.tools.r8.ir.desugar.desugaredlibrary.apiconversion.DesugaredLibraryAPIConverter;
import com.android.tools.r8.ir.desugar.itf.EmulatedInterfaceApplicationRewriter;
@@ -91,7 +92,7 @@
new L8InnerOuterAttributeEraser(appView).run();
}
- processCovariantReturnTypeAnnotations(builder);
+ processCovariantReturnTypeAnnotations(builder, artProfileCollectionAdditions);
timing.end();
@@ -338,9 +339,13 @@
programAdditions.apply(executorService);
}
- private void processCovariantReturnTypeAnnotations(Builder<?> builder) {
+ private void processCovariantReturnTypeAnnotations(
+ Builder<?> builder, ArtProfileCollectionAdditions artProfileCollectionAdditions) {
if (covariantReturnTypeAnnotationTransformer != null) {
- covariantReturnTypeAnnotationTransformer.process(builder);
+ covariantReturnTypeAnnotationTransformer.process(
+ builder,
+ CovariantReturnTypeAnnotationTransformerEventConsumer.create(
+ artProfileCollectionAdditions));
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/CovariantReturnTypeAnnotationTransformer.java b/src/main/java/com/android/tools/r8/ir/desugar/CovariantReturnTypeAnnotationTransformer.java
index 1143afd..c9ef3ed 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/CovariantReturnTypeAnnotationTransformer.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/CovariantReturnTypeAnnotationTransformer.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.ir.desugar;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexAnnotation;
import com.android.tools.r8.graph.DexAnnotationElement;
import com.android.tools.r8.graph.DexApplication;
@@ -53,23 +54,31 @@
// the contained CovariantReturnType annotations.
public final class CovariantReturnTypeAnnotationTransformer {
+ private final AppView<?> appView;
private final IRConverter converter;
- private final MethodProcessorEventConsumer eventConsumer = MethodProcessorEventConsumer.empty();
+ private final MethodProcessorEventConsumer methodProcessorEventConsumer =
+ MethodProcessorEventConsumer.empty();
private final DexItemFactory factory;
- public CovariantReturnTypeAnnotationTransformer(IRConverter converter, DexItemFactory factory) {
+ public CovariantReturnTypeAnnotationTransformer(AppView<?> appView, IRConverter converter) {
+ this.appView = appView;
this.converter = converter;
- this.factory = factory;
+ this.factory = appView.dexItemFactory();
}
- public void process(DexApplication.Builder<?> builder) {
+ public void process(
+ DexApplication.Builder<?> builder,
+ CovariantReturnTypeAnnotationTransformerEventConsumer eventConsumer) {
// List of methods that should be added to the next class.
List<DexEncodedMethod> methodsWithCovariantReturnTypeAnnotation = new LinkedList<>();
List<DexEncodedMethod> covariantReturnTypeMethods = new LinkedList<>();
for (DexProgramClass clazz : builder.getProgramClasses()) {
// Construct the methods that should be added to clazz.
buildCovariantReturnTypeMethodsForClass(
- clazz, methodsWithCovariantReturnTypeAnnotation, covariantReturnTypeMethods);
+ clazz,
+ methodsWithCovariantReturnTypeAnnotation,
+ covariantReturnTypeMethods,
+ eventConsumer);
if (covariantReturnTypeMethods.isEmpty()) {
continue;
}
@@ -79,6 +88,7 @@
methodsWithCovariantReturnTypeAnnotation.clear();
covariantReturnTypeMethods.clear();
}
+ eventConsumer.finished(appView);
}
private void updateClass(
@@ -111,12 +121,14 @@
private void buildCovariantReturnTypeMethodsForClass(
DexProgramClass clazz,
List<DexEncodedMethod> methodsWithCovariantReturnTypeAnnotation,
- List<DexEncodedMethod> covariantReturnTypeMethods) {
+ List<DexEncodedMethod> covariantReturnTypeMethods,
+ CovariantReturnTypeAnnotationTransformerEventConsumer eventConsumer) {
clazz.forEachProgramVirtualMethod(
method -> {
if (methodHasCovariantReturnTypeAnnotation(method.getDefinition())) {
methodsWithCovariantReturnTypeAnnotation.add(method.getDefinition());
- buildCovariantReturnTypeMethodsForMethod(method, covariantReturnTypeMethods);
+ buildCovariantReturnTypeMethodsForMethod(
+ method, covariantReturnTypeMethods, eventConsumer);
}
});
}
@@ -134,11 +146,13 @@
// variantReturnTypes annotations on the given method. Adds the newly constructed, synthetic
// methods to the list covariantReturnTypeMethods.
private void buildCovariantReturnTypeMethodsForMethod(
- ProgramMethod method, List<DexEncodedMethod> covariantReturnTypeMethods) {
+ ProgramMethod method,
+ List<DexEncodedMethod> covariantReturnTypeMethods,
+ CovariantReturnTypeAnnotationTransformerEventConsumer eventConsumer) {
assert methodHasCovariantReturnTypeAnnotation(method.getDefinition());
for (DexType covariantReturnType : getCovariantReturnTypes(method)) {
DexEncodedMethod covariantReturnTypeMethod =
- buildCovariantReturnTypeMethod(method, covariantReturnType);
+ buildCovariantReturnTypeMethod(method, covariantReturnType, eventConsumer);
covariantReturnTypeMethods.add(covariantReturnTypeMethod);
}
}
@@ -149,7 +163,9 @@
//
// Note: any "synchronized" or "strictfp" modifier could be dropped safely.
private DexEncodedMethod buildCovariantReturnTypeMethod(
- ProgramMethod method, DexType covariantReturnType) {
+ ProgramMethod method,
+ DexType covariantReturnType,
+ CovariantReturnTypeAnnotationTransformerEventConsumer eventConsumer) {
DexProgramClass methodHolder = method.getHolder();
DexMethod methodReference = method.getReference();
DexEncodedMethod methodDefinition = method.getDefinition();
@@ -183,7 +199,8 @@
.build();
// Optimize to generate DexCode instead of CfCode.
ProgramMethod programMethod = new ProgramMethod(methodHolder, newVirtualMethod);
- converter.optimizeSynthesizedMethod(programMethod, eventConsumer);
+ converter.optimizeSynthesizedMethod(programMethod, methodProcessorEventConsumer);
+ eventConsumer.acceptCovariantReturnTypeBridgeMethod(programMethod, method);
return newVirtualMethod;
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/CovariantReturnTypeAnnotationTransformerEventConsumer.java b/src/main/java/com/android/tools/r8/ir/desugar/CovariantReturnTypeAnnotationTransformerEventConsumer.java
new file mode 100644
index 0000000..a639916
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/desugar/CovariantReturnTypeAnnotationTransformerEventConsumer.java
@@ -0,0 +1,53 @@
+// Copyright (c) 2023, 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.ir.desugar;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.profile.art.rewriting.ArtProfileCollectionAdditions;
+import com.android.tools.r8.profile.art.rewriting.ArtProfileRewritingCovariantReturnTypeAnnotationTransformerEventConsumer;
+
+public interface CovariantReturnTypeAnnotationTransformerEventConsumer {
+
+ void acceptCovariantReturnTypeBridgeMethod(ProgramMethod bridge, ProgramMethod target);
+
+ void finished(AppView<?> appView);
+
+ static CovariantReturnTypeAnnotationTransformerEventConsumer create(
+ ArtProfileCollectionAdditions artProfileCollectionAdditions) {
+ if (artProfileCollectionAdditions.isNop()) {
+ return empty();
+ }
+ return ArtProfileRewritingCovariantReturnTypeAnnotationTransformerEventConsumer.attach(
+ artProfileCollectionAdditions, empty());
+ }
+
+ static EmptyCovariantReturnTypeAnnotationTransformerEventConsumer empty() {
+ return EmptyCovariantReturnTypeAnnotationTransformerEventConsumer.getInstance();
+ }
+
+ class EmptyCovariantReturnTypeAnnotationTransformerEventConsumer
+ implements CovariantReturnTypeAnnotationTransformerEventConsumer {
+
+ private static final EmptyCovariantReturnTypeAnnotationTransformerEventConsumer INSTANCE =
+ new EmptyCovariantReturnTypeAnnotationTransformerEventConsumer();
+
+ private EmptyCovariantReturnTypeAnnotationTransformerEventConsumer() {}
+
+ static EmptyCovariantReturnTypeAnnotationTransformerEventConsumer getInstance() {
+ return INSTANCE;
+ }
+
+ @Override
+ public void acceptCovariantReturnTypeBridgeMethod(ProgramMethod bridge, ProgramMethod target) {
+ // Intentionally empty.
+ }
+
+ @Override
+ public void finished(AppView<?> appView) {
+ // Intentionally empty.
+ }
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/profile/art/rewriting/ArtProfileRewritingCovariantReturnTypeAnnotationTransformerEventConsumer.java b/src/main/java/com/android/tools/r8/profile/art/rewriting/ArtProfileRewritingCovariantReturnTypeAnnotationTransformerEventConsumer.java
new file mode 100644
index 0000000..2188363
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/profile/art/rewriting/ArtProfileRewritingCovariantReturnTypeAnnotationTransformerEventConsumer.java
@@ -0,0 +1,45 @@
+// Copyright (c) 2023, 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.profile.art.rewriting;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.ir.desugar.CovariantReturnTypeAnnotationTransformerEventConsumer;
+
+public class ArtProfileRewritingCovariantReturnTypeAnnotationTransformerEventConsumer
+ implements CovariantReturnTypeAnnotationTransformerEventConsumer {
+
+ private final ConcreteArtProfileCollectionAdditions additionsCollection;
+ private final CovariantReturnTypeAnnotationTransformerEventConsumer parent;
+
+ private ArtProfileRewritingCovariantReturnTypeAnnotationTransformerEventConsumer(
+ ConcreteArtProfileCollectionAdditions additionsCollection,
+ CovariantReturnTypeAnnotationTransformerEventConsumer parent) {
+ this.additionsCollection = additionsCollection;
+ this.parent = parent;
+ }
+
+ public static CovariantReturnTypeAnnotationTransformerEventConsumer attach(
+ ArtProfileCollectionAdditions additionsCollection,
+ CovariantReturnTypeAnnotationTransformerEventConsumer eventConsumer) {
+ if (additionsCollection.isNop()) {
+ return eventConsumer;
+ }
+ return new ArtProfileRewritingCovariantReturnTypeAnnotationTransformerEventConsumer(
+ additionsCollection.asConcrete(), eventConsumer);
+ }
+
+ @Override
+ public void acceptCovariantReturnTypeBridgeMethod(ProgramMethod bridge, ProgramMethod target) {
+ additionsCollection.addMethodIfContextIsInProfile(bridge, target);
+ parent.acceptCovariantReturnTypeBridgeMethod(bridge, target);
+ }
+
+ @Override
+ public void finished(AppView<?> appView) {
+ additionsCollection.commit(appView);
+ parent.finished(appView);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/desugar/annotations/CovariantReturnTypeAnnotationTransformerTest.java b/src/test/java/com/android/tools/r8/ir/desugar/annotations/CovariantReturnTypeAnnotationTransformerTest.java
index b1899a3..8967763 100644
--- a/src/test/java/com/android/tools/r8/ir/desugar/annotations/CovariantReturnTypeAnnotationTransformerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/desugar/annotations/CovariantReturnTypeAnnotationTransformerTest.java
@@ -5,21 +5,30 @@
package com.android.tools.r8.ir.desugar.annotations;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import com.android.tools.r8.AsmTestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.nio.file.Path;
import java.util.Collections;
+import java.util.List;
import org.junit.Assert;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+@RunWith(Parameterized.class)
public class CovariantReturnTypeAnnotationTransformerTest extends AsmTestBase {
+
public static final String PACKAGE_NAME = "com/android/tools/r8/ir/desugar/annotations";
public static final String CRT_BINARY_NAME = "dalvik/annotation/codegen/CovariantReturnType";
public static final String CRTS_INNER_NAME = "CovariantReturnTypes";
@@ -28,10 +37,18 @@
public static final String CRT_TYPE_NAME = CRT_BINARY_NAME.replace('/', '.');
public static final String CRTS_TYPE_NAME = CRT_BINARY_NAME.replace('/', '.');
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDexRuntimesAndAllApiLevels().build();
+ }
+
@Test
public void testVersion1WithClient1And2() throws Exception {
- AndroidApp input =
- buildAndroidApp(
+ List<byte[]> input =
+ ImmutableList.of(
ToolHelper.getClassAsBytes(Client.class),
ToolHelper.getClassAsBytes(A.class),
ToolHelper.getClassAsBytes(B.class),
@@ -46,8 +63,8 @@
@Test
public void testVersion1WithClient3() throws Exception {
- AndroidApp input =
- buildAndroidApp(
+ List<byte[]> input =
+ ImmutableList.of(
com.android.tools.r8.ir.desugar.annotations.version3.ClientDump.dump(),
ToolHelper.getClassAsBytes(A.class),
ToolHelper.getClassAsBytes(B.class),
@@ -63,8 +80,8 @@
@Test
public void testVersion2WithClient1And2() throws Exception {
- AndroidApp input =
- buildAndroidApp(
+ List<byte[]> input =
+ ImmutableList.of(
ToolHelper.getClassAsBytes(Client.class),
ToolHelper.getClassAsBytes(A.class),
com.android.tools.r8.ir.desugar.annotations.version2.BDump.dump(),
@@ -79,8 +96,8 @@
@Test
public void testVersion2WithClient3() throws Exception {
- AndroidApp input =
- buildAndroidApp(
+ List<byte[]> input =
+ ImmutableList.of(
com.android.tools.r8.ir.desugar.annotations.version3.ClientDump.dump(),
ToolHelper.getClassAsBytes(A.class),
com.android.tools.r8.ir.desugar.annotations.version2.BDump.dump(),
@@ -100,8 +117,8 @@
@Test
public void testVersion3WithClient3() throws Exception {
- AndroidApp input =
- buildAndroidApp(
+ List<byte[]> input =
+ ImmutableList.of(
com.android.tools.r8.ir.desugar.annotations.version3.ClientDump.dump(),
ToolHelper.getClassAsBytes(A.class),
com.android.tools.r8.ir.desugar.annotations.version3.BDump.dump(),
@@ -116,8 +133,8 @@
@Test
public void testVersion3WithClient1And2() throws Exception {
- AndroidApp input =
- buildAndroidApp(
+ List<byte[]> input =
+ ImmutableList.of(
ToolHelper.getClassAsBytes(Client.class),
ToolHelper.getClassAsBytes(A.class),
com.android.tools.r8.ir.desugar.annotations.version3.BDump.dump(),
@@ -132,8 +149,8 @@
@Test
public void testRepeatedCompilation() throws Exception {
- AndroidApp input =
- buildAndroidApp(
+ List<byte[]> input =
+ ImmutableList.of(
ToolHelper.getClassAsBytes(Client.class),
ToolHelper.getClassAsBytes(A.class),
com.android.tools.r8.ir.desugar.annotations.version2.BDump.dump(),
@@ -142,51 +159,73 @@
// Version 2 contains annotations.
checkPresenceOfCovariantAnnotations(input, true);
- AndroidApp output =
- compileWithD8(input, options -> options.processCovariantReturnTypeAnnotations = true);
-
- // Compilation output does not contain annotations.
- checkPresenceOfCovariantAnnotations(output, false);
+ Path output =
+ testForD8(parameters.getBackend())
+ .addProgramClassFileData(input)
+ .addOptionsModification(options -> options.processCovariantReturnTypeAnnotations = true)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ // Compilation output does not contain annotations.
+ .inspect(inspector -> checkPresenceOfCovariantAnnotations(inspector, false))
+ .writeToZip();
// Compilation will fail with a compilation error the second time if the implementation does
// not remove the CovariantReturnType annotations properly during the first compilation.
- compileWithD8(output, options -> options.processCovariantReturnTypeAnnotations = true);
+ testForD8(parameters.getBackend())
+ .addProgramFiles(output)
+ .addOptionsModification(options -> options.processCovariantReturnTypeAnnotations = true)
+ .setMinApi(parameters.getApiLevel())
+ .compile();
}
private void succeedsWithOption(
- AndroidApp input, boolean option, boolean checkPresenceOfSyntheticMethods) throws Exception {
- AndroidApp output =
- compileWithD8(input, options -> options.processCovariantReturnTypeAnnotations = option);
- String stdout = runOnArt(output, Client.class.getCanonicalName());
- assertEquals(getExpectedOutput(), stdout);
- checkPresenceOfCovariantAnnotations(output, false);
- if (option && checkPresenceOfSyntheticMethods) {
- checkPresenceOfSyntheticMethods(output);
- }
- }
-
- private void failsWithOption(AndroidApp input, boolean option) throws Exception {
- AndroidApp output =
- compileWithD8(input, options -> options.processCovariantReturnTypeAnnotations = option);
- checkPresenceOfCovariantAnnotations(output, false);
- ToolHelper.ProcessResult result = runOnArtRaw(output, Client.class.getCanonicalName());
- assertThat(result.stderr, containsString("java.lang.NoSuchMethodError"));
- }
-
- private void succeedsIndependentOfFlag(AndroidApp input, boolean checkPresenceOfSyntheticMethods)
+ List<byte[]> input, boolean option, boolean checkPresenceOfSyntheticMethods)
throws Exception {
+ testForD8(parameters.getBackend())
+ .addProgramClassFileData(input)
+ .addOptionsModification(options -> options.processCovariantReturnTypeAnnotations = option)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ checkPresenceOfCovariantAnnotations(inspector, false);
+ if (option && checkPresenceOfSyntheticMethods) {
+ checkPresenceOfSyntheticMethods(inspector);
+ }
+ })
+ .run(parameters.getRuntime(), Client.class.getCanonicalName())
+ .assertSuccessWithOutput(getExpectedOutput());
+ }
+
+ private void failsWithOption(List<byte[]> input, boolean option) throws Exception {
+ testForD8(parameters.getBackend())
+ .addProgramClassFileData(input)
+ .addOptionsModification(options -> options.processCovariantReturnTypeAnnotations = option)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(inspector -> checkPresenceOfCovariantAnnotations(inspector, false))
+ .run(parameters.getRuntime(), Client.class.getCanonicalName())
+ .assertFailureWithErrorThatThrows(NoSuchMethodError.class);
+ }
+
+ private void succeedsIndependentOfFlag(
+ List<byte[]> input, boolean checkPresenceOfSyntheticMethods) throws Exception {
succeedsWithOption(input, true, checkPresenceOfSyntheticMethods);
succeedsWithOption(input, false, checkPresenceOfSyntheticMethods);
}
- private void failsIndependentOfFlag(AndroidApp input) throws Exception {
+ private void failsIndependentOfFlag(List<byte[]> input) throws Exception {
failsWithOption(input, true);
failsWithOption(input, false);
}
- private void checkPresenceOfCovariantAnnotations(AndroidApp app, boolean expected)
+ private void checkPresenceOfCovariantAnnotations(List<byte[]> input, boolean expected)
throws Exception {
- CodeInspector inspector = new CodeInspector(app);
+ CodeInspector inspector = new CodeInspector(buildAndroidApp(input));
+ checkPresenceOfCovariantAnnotations(inspector, expected);
+ }
+
+ private void checkPresenceOfCovariantAnnotations(CodeInspector inspector, boolean expected) {
assertEquals(
expected,
inspector.allClasses().stream()
@@ -196,9 +235,7 @@
.anyMatch(method -> method.annotation(CRTS_TYPE_NAME).isPresent())));
}
- private void checkPresenceOfSyntheticMethods(AndroidApp output) throws Exception {
- CodeInspector inspector = new CodeInspector(output);
-
+ private void checkPresenceOfSyntheticMethods(CodeInspector inspector) throws Exception {
// Get classes A, B, and C.
ClassSubject clazzA = inspector.clazz(A.class.getCanonicalName());
assertThat(clazzA, isPresent());
