Determine synthetic context from end of string
Bug: b/324270842
Change-Id: Ibd8ad7f15c1a72bd641b9b6b968c2834048107b7
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java
index dc0a4ac..00c6b10 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java
@@ -62,9 +62,8 @@
final String getPrefixForExternalSyntheticType(AppView<?> appView) {
if (!appView.options().intermediate && context.isSyntheticInputClass() && !kind.isGlobal()) {
// If the input class was a synthetic and the build is non-intermediate, unwind the synthetic
- // name back to the original context (if present in the textual type).
- return SyntheticNaming.getOuterContextFromExternalSyntheticType(
- getKind(), getHolder().getType());
+ // name back to the original context.
+ return context.getSynthesizingContextType().toBinaryName();
}
return SyntheticNaming.getPrefixForExternalSyntheticType(getKind(), getHolder().getType());
}
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
index abd4a13..bb72018 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
@@ -576,7 +576,9 @@
private static boolean shouldAnnotateSynthetics(InternalOptions options) {
// Only intermediate builds have annotated synthetics to allow later sharing.
// Also, CF builds are marked in the writer using an attribute.
- return options.intermediate && options.isGeneratingDex();
+ return options.intermediate
+ && options.isGeneratingDex()
+ && !options.testing.disableSyntheticMarkerAttributeWriting;
}
private <T extends SyntheticDefinition<?, T, ?>>
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticItems.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticItems.java
index 7f56b25..50c9f23 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticItems.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticItems.java
@@ -300,33 +300,19 @@
assert synthetics.committed.isEmpty();
assert synthetics.pending.isEmpty();
CommittedSyntheticsCollection.Builder builder = synthetics.committed.builder();
- // TODO(b/158159959): Consider populating the input synthetics when identified.
- for (DexProgramClass clazz : appView.appInfo().classes()) {
- SyntheticMarker marker = SyntheticMarker.stripMarkerFromClass(clazz, appView);
- if (!appView.options().intermediate && marker.getContext() != null) {
- DexClass contextClass =
- appView
- .appInfo()
- .definitionForWithoutExistenceAssert(
- marker.getContext().getSynthesizingContextType());
- if (contextClass == null || contextClass.isNotProgramClass()) {
- appView
- .reporter()
- .error(
- new StringDiagnostic(
- "Attempt at compiling intermediate artifact without its context",
- clazz.getOrigin()));
- }
- }
- if (marker.isSyntheticMethods()) {
- clazz.forEachProgramMethod(
- method ->
- builder.addMethod(
- new SyntheticMethodDefinition(marker.getKind(), marker.getContext(), method)));
- } else if (marker.isSyntheticClass()) {
- builder.addClass(
- new SyntheticProgramClassDefinition(marker.getKind(), marker.getContext(), clazz));
- }
+ if (appView.options().intermediate) {
+ appView
+ .appInfo()
+ .classes()
+ .forEach(
+ clazz -> {
+ SyntheticMarker marker = SyntheticMarker.stripMarkerFromClass(clazz, appView);
+ if (marker.isValidMarker()) {
+ addSyntheticInput(clazz, marker, marker.getContext(), builder);
+ }
+ });
+ } else {
+ collectSyntheticInputsTransitive(appView, builder);
}
CommittedSyntheticsCollection committed = builder.collectSyntheticInputs().build();
if (committed.isEmpty()) {
@@ -350,6 +336,77 @@
}
}
+ private static void addSyntheticInput(
+ DexProgramClass clazz,
+ SyntheticMarker marker,
+ SynthesizingContext context,
+ CommittedSyntheticsCollection.Builder builder) {
+ if (marker.isSyntheticMethods()) {
+ clazz.forEachProgramMethod(
+ method ->
+ builder.addMethod(new SyntheticMethodDefinition(marker.getKind(), context, method)));
+ } else if (marker.isSyntheticClass()) {
+ builder.addClass(new SyntheticProgramClassDefinition(marker.getKind(), context, clazz));
+ }
+ }
+
+ private static void collectSyntheticInputsTransitive(
+ AppView<?> appView, CommittedSyntheticsCollection.Builder builder) {
+ Map<DexType, SynthesizingContext> cache = new IdentityHashMap<>();
+ appView
+ .appInfo()
+ .classes()
+ .forEach(clazz -> collectSyntheticInputTransitive(clazz, cache, builder, appView));
+ }
+
+ private static SynthesizingContext collectSyntheticInputTransitive(
+ DexProgramClass clazz,
+ Map<DexType, SynthesizingContext> cache,
+ CommittedSyntheticsCollection.Builder builder,
+ AppView<?> appView) {
+ SyntheticMarker marker = SyntheticMarker.stripMarkerFromClass(clazz, appView);
+ if (!marker.isValidMarker()) {
+ return cache.get(clazz.getType());
+ }
+ return cache.computeIfAbsent(
+ clazz.getType(),
+ type -> {
+ SynthesizingContext context =
+ ensureSyntheticInputContext(clazz, marker, cache, builder, appView);
+ addSyntheticInput(clazz, marker, context, builder);
+ return context;
+ });
+ }
+
+ private static SynthesizingContext ensureSyntheticInputContext(
+ DexProgramClass clazz,
+ SyntheticMarker marker,
+ Map<DexType, SynthesizingContext> cache,
+ CommittedSyntheticsCollection.Builder builder,
+ AppView<?> appView) {
+ assert !appView.options().intermediate;
+ // The context should be flattened such that it is not itself a synthetic.
+ DexProgramClass contextClass =
+ DexProgramClass.asProgramClassOrNull(
+ appView
+ .appInfo()
+ .definitionForWithoutExistenceAssert(
+ marker.getContext().getSynthesizingContextType()));
+ if (contextClass == null) {
+ appView
+ .reporter()
+ .error(
+ new StringDiagnostic(
+ "Attempt at compiling intermediate artifact without its context",
+ clazz.getOrigin()));
+ return marker.getContext();
+ } else {
+ SynthesizingContext contextOfContext =
+ collectSyntheticInputTransitive(contextClass, cache, builder, appView);
+ return contextOfContext != null ? contextOfContext : marker.getContext();
+ }
+ }
+
// Predicates and accessors.
@Override
@@ -1165,6 +1222,9 @@
public void writeAttributeIfIntermediateSyntheticClass(
ClassWriter writer, DexProgramClass clazz, AppView<?> appView) {
+ if (appView.options().testing.disableSyntheticMarkerAttributeWriting) {
+ return;
+ }
if (!appView.options().intermediate || !appView.options().isGeneratingClassFiles()) {
return;
}
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticMarker.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticMarker.java
index 9e8ec26..7670696 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticMarker.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticMarker.java
@@ -205,6 +205,11 @@
this.context = context;
}
+ public boolean isValidMarker() {
+ assert getContext() != null || this == NO_MARKER;
+ return getContext() != null;
+ }
+
public boolean isSyntheticMethods() {
return kind != null && kind.isSingleSyntheticMethod();
}
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticNaming.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticNaming.java
index a9f9957..fad4880 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticNaming.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticNaming.java
@@ -424,8 +424,10 @@
assert !kind.isGlobal();
String binaryName = type.toBinaryName();
int index =
- binaryName.indexOf(
- kind.isFixedSuffixSynthetic() ? kind.descriptor : EXTERNAL_SYNTHETIC_CLASS_SEPARATOR);
+ binaryName.lastIndexOf(
+ kind.isFixedSuffixSynthetic()
+ ? kind.descriptor
+ : EXTERNAL_SYNTHETIC_CLASS_SEPARATOR + kind.getDescriptor());
if (index < 0) {
throw new Unreachable(
"Unexpected failure to determine the context of synthetic class: " + binaryName);
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 8a940af..e90aa6c 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -2404,6 +2404,8 @@
public boolean emitDebugLocalStartBeforeDefaultEvent = false;
+ public boolean disableSyntheticMarkerAttributeWriting = false;
+
// Option for testing outlining with interface array arguments, see b/132420510.
public boolean allowOutlinerInterfaceArrayArguments = false;
diff --git a/src/test/java/com/android/tools/r8/synthesis/RepeatedCompilationNestedSyntheticsAndStrippedMarkerTest.java b/src/test/java/com/android/tools/r8/synthesis/RepeatedCompilationNestedSyntheticsAndStrippedMarkerTest.java
new file mode 100644
index 0000000..6bd5b5a
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/synthesis/RepeatedCompilationNestedSyntheticsAndStrippedMarkerTest.java
@@ -0,0 +1,233 @@
+// Copyright (c) 2024, 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.synthesis;
+
+import static com.android.tools.r8.synthesis.SyntheticItemsTestUtils.syntheticBackportClass;
+import static com.android.tools.r8.synthesis.SyntheticItemsTestUtils.syntheticLambdaClass;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.ByteDataView;
+import com.android.tools.r8.ClassFileConsumer;
+import com.android.tools.r8.DesugarGraphConsumer;
+import com.android.tools.r8.DexFilePerClassFileConsumer;
+import com.android.tools.r8.DiagnosticsHandler;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.references.ClassReference;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.BooleanBox;
+import com.google.common.collect.ImmutableSet;
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+/** This test is a regression test for the issue identified in b/324270842#comment37 */
+@RunWith(Parameterized.class)
+public class RepeatedCompilationNestedSyntheticsAndStrippedMarkerTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public Backend intermediateBackend;
+
+ @Parameterized.Parameters(name = "{0}, intermediate: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withDefaultDexRuntime().withMinimumApiLevel().build(),
+ Backend.values());
+ }
+
+ @Test
+ public void test() throws Exception {
+ assertEquals(Backend.DEX, parameters.getBackend());
+
+ ClassReference syntheticLambdaClass = syntheticLambdaClass(UsesBackport.class, 0);
+ ImmutableSet<String> expectedClassOutputs =
+ ImmutableSet.of(descriptor(UsesBackport.class), syntheticLambdaClass.getDescriptor());
+
+ Map<String, byte[]> firstCompilation = new HashMap<>();
+ testForD8(Backend.CF)
+ // High API level such that only the lambda is desugared.
+ .setMinApi(AndroidApiLevel.S)
+ .setIntermediate(true)
+ .addClasspathClasses(I.class)
+ .addProgramClasses(UsesBackport.class)
+ .addOptionsModification(o -> o.testing.disableSyntheticMarkerAttributeWriting = true)
+ .setProgramConsumer(
+ new ClassFileConsumer() {
+ @Override
+ public void accept(ByteDataView data, String descriptor, DiagnosticsHandler handler) {
+ byte[] bytes = data.copyByteData();
+ assertEquals(Collections.emptyList(), SyntheticMarkerCfTest.readAttributes(bytes));
+ firstCompilation.put(descriptor, bytes);
+ }
+
+ @Override
+ public void finished(DiagnosticsHandler handler) {}
+ })
+ .compile();
+ assertEquals(expectedClassOutputs, firstCompilation.keySet());
+
+ Map<String, byte[]> secondCompilation = new HashMap<>();
+ ImmutableSet.Builder<String> allDescriptors = ImmutableSet.builder();
+ BooleanBox matched = new BooleanBox(false);
+ for (Entry<String, byte[]> entry : firstCompilation.entrySet()) {
+ byte[] bytes = entry.getValue();
+ Origin origin =
+ new Origin(Origin.root()) {
+ @Override
+ public String part() {
+ return entry.getKey();
+ }
+ };
+ testForD8(intermediateBackend)
+ .setMinApi(parameters)
+ .setIntermediate(true)
+ .addClasspathClasses(I.class)
+ .apply(b -> b.getBuilder().addClassProgramData(bytes, origin))
+ .apply(
+ b ->
+ b.getBuilder()
+ .setDesugarGraphConsumer(
+ new DesugarGraphConsumer() {
+
+ @Override
+ public void accept(Origin dependent, Origin dependency) {
+ assertThat(
+ dependency.toString(), containsString(binaryName(I.class)));
+ assertThat(
+ dependent.toString(),
+ containsString(syntheticLambdaClass.getBinaryName()));
+ matched.set(true);
+ }
+
+ @Override
+ public void finished() {}
+ }))
+ .applyIf(
+ intermediateBackend == Backend.CF,
+ b ->
+ b.setProgramConsumer(
+ new ClassFileConsumer() {
+ @Override
+ public void accept(
+ ByteDataView data, String descriptor, DiagnosticsHandler handler) {
+ secondCompilation.put(descriptor, data.copyByteData());
+ allDescriptors.add(descriptor);
+ }
+
+ @Override
+ public void finished(DiagnosticsHandler handler) {}
+ }),
+ b ->
+ b.setProgramConsumer(
+ new DexFilePerClassFileConsumer() {
+
+ @Override
+ public void accept(
+ String primaryClassDescriptor,
+ ByteDataView data,
+ Set<String> descriptors,
+ DiagnosticsHandler handler) {
+ secondCompilation.put(primaryClassDescriptor, data.copyByteData());
+ allDescriptors.addAll(descriptors);
+ }
+
+ @Override
+ public void finished(DiagnosticsHandler handler) {}
+ }))
+ .compile();
+ }
+ assertTrue(matched.get());
+ // The dex file per class file output should maintain the exact same set of primary descriptors.
+ if (intermediateBackend == Backend.DEX) {
+ assertEquals(expectedClassOutputs, secondCompilation.keySet());
+ }
+ // The total set of classes should also include the backport. The backport should be
+ // hygienically placed under the synthetic lambda (not the context of the lambda!).
+ assertEquals(
+ ImmutableSet.<String>builder()
+ .addAll(expectedClassOutputs)
+ .add(syntheticBackportClass(syntheticLambdaClass, 0).getDescriptor())
+ .build(),
+ allDescriptors.build());
+
+ List<byte[]> secondCompilationWitoutOuterContext =
+ secondCompilation.entrySet().stream()
+ .filter(e -> !e.getKey().equals(descriptor(UsesBackport.class)))
+ .map(Entry::getValue)
+ .collect(Collectors.toList());
+ Path out =
+ testForD8(Backend.DEX)
+ .setMinApi(parameters)
+ .addProgramClasses(I.class, TestClass.class)
+ .applyIf(
+ intermediateBackend == Backend.CF,
+ b -> b.addProgramClassFileData(secondCompilationWitoutOuterContext),
+ b -> b.addProgramDexFileData(secondCompilationWitoutOuterContext))
+ .compile()
+ .writeToZip();
+
+ byte[] secondCompilationOfOuterContext = secondCompilation.get(descriptor(UsesBackport.class));
+ testForD8(Backend.DEX)
+ .setMinApi(parameters)
+ .addProgramFiles(out)
+ .applyIf(
+ intermediateBackend == Backend.CF,
+ b -> b.addProgramClassFileData(secondCompilationOfOuterContext),
+ b -> b.addProgramDexFileData(secondCompilationOfOuterContext))
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("1")
+ .inspect(
+ inspector -> {
+ Set<String> descriptors =
+ inspector.allClasses().stream()
+ .map(c -> c.getFinalReference().getDescriptor())
+ .collect(Collectors.toSet());
+ // The initial lambda stays as the only item under UsesBackport.
+ ClassReference lambdaClass = syntheticLambdaClass(UsesBackport.class, 0);
+ // The nested backport has context in the lambda since the lambda was not marked.
+ ClassReference backportClass = syntheticBackportClass(lambdaClass, 0);
+ assertEquals(
+ ImmutableSet.of(
+ descriptor(I.class),
+ descriptor(TestClass.class),
+ descriptor(UsesBackport.class),
+ backportClass.getDescriptor(),
+ lambdaClass.getDescriptor()),
+ descriptors);
+ });
+ }
+
+ interface I {
+ int compare(boolean b1, boolean b2);
+ }
+
+ static class UsesBackport {
+ public static I foo() {
+ return Boolean::compare;
+ }
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(UsesBackport.foo().compare(true, false));
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/synthesis/SyntheticMarkerCfTest.java b/src/test/java/com/android/tools/r8/synthesis/SyntheticMarkerCfTest.java
index 697b2b5..74ba13b 100644
--- a/src/test/java/com/android/tools/r8/synthesis/SyntheticMarkerCfTest.java
+++ b/src/test/java/com/android/tools/r8/synthesis/SyntheticMarkerCfTest.java
@@ -137,7 +137,7 @@
}
}
- private static List<Attribute> readAttributes(byte[] bytes) {
+ public static List<Attribute> readAttributes(byte[] bytes) {
List<Attribute> attributes = new ArrayList<>();
ClassReader reader = new ClassReader(bytes);
reader.accept(
