Fix signature collision after field type strengthening
Bug: b/231030461
Change-Id: I4ae6ccb9e5e683a7a43df86db95eb4332719a1a9
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorProgramOptimizer.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorProgramOptimizer.java
index 6b94973..1bc719c 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorProgramOptimizer.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorProgramOptimizer.java
@@ -55,6 +55,7 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
+import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -441,10 +442,24 @@
DexMethodSignatureSet interfaceDispatchOutsideProgram,
ArgumentPropagatorGraphLens.Builder partialGraphLensBuilder) {
BooleanBox affected = new BooleanBox();
+ Set<DexField> newFieldSignatures = Sets.newIdentityHashSet();
+ Map<DexField, DexType> newFieldTypes = new IdentityHashMap<>();
clazz.forEachProgramFieldMatching(
field -> field.getType().isClassType(),
field -> {
- DexField newFieldSignature = getNewFieldSignature(field);
+ DexType newFieldType = getNewFieldType(field);
+ if (newFieldType != field.getType()) {
+ newFieldTypes.put(field.getReference(), newFieldType);
+ } else {
+ // Reserve field signature.
+ newFieldSignatures.add(field.getReference());
+ }
+ });
+ clazz.forEachProgramFieldMatching(
+ field -> field.getType().isClassType(),
+ field -> {
+ DexField newFieldSignature =
+ getNewFieldSignature(field, newFieldSignatures, newFieldTypes);
if (newFieldSignature != field.getReference()) {
partialGraphLensBuilder.recordMove(field.getReference(), newFieldSignature);
affected.set();
@@ -469,10 +484,10 @@
return affected.get();
}
- private DexField getNewFieldSignature(ProgramField field) {
+ private DexType getNewFieldType(ProgramField field) {
DynamicType dynamicType = field.getOptimizationInfo().getDynamicType();
if (dynamicType.isUnknown()) {
- return field.getReference();
+ return field.getType();
}
KeepFieldInfo keepInfo = appView.getKeepInfo(field);
@@ -481,17 +496,17 @@
assert !keepInfo.isPinned(options);
if (!keepInfo.isFieldTypeStrengtheningAllowed(options)) {
- return field.getReference();
+ return field.getType();
}
if (dynamicType.isNullType()) {
// Don't optimize always null fields; these will be optimized anyway.
- return field.getReference();
+ return field.getType();
}
if (dynamicType.isNotNullType()) {
// We don't have a more specific type.
- return field.getReference();
+ return field.getType();
}
DynamicTypeWithUpperBound dynamicTypeWithUpperBound =
@@ -502,12 +517,12 @@
ClassTypeElement staticFieldType = field.getType().toTypeElement(appView).asClassType();
if (dynamicUpperBoundType.equalUpToNullability(staticFieldType)) {
// We don't have more precise type information.
- return field.getReference();
+ return field.getType();
}
if (!dynamicUpperBoundType.strictlyLessThan(staticFieldType, appView)) {
assert options.testing.allowTypeErrors;
- return field.getReference();
+ return field.getType();
}
DexType newStaticFieldType;
@@ -518,7 +533,7 @@
newStaticFieldType =
dynamicUpperBoundClassType.getInterfaces().getSingleKnownInterface();
} else {
- return field.getReference();
+ return field.getType();
}
} else {
newStaticFieldType = dynamicUpperBoundClassType.getClassType();
@@ -528,10 +543,24 @@
}
if (!AccessUtils.isAccessibleInSameContextsAs(newStaticFieldType, field.getType(), appView)) {
- return field.getReference();
+ return field.getType();
}
- return field.getReference().withType(newStaticFieldType, dexItemFactory);
+ return newStaticFieldType;
+ }
+
+ private DexField getNewFieldSignature(
+ ProgramField field,
+ Set<DexField> newFieldSignatures,
+ Map<DexField, DexType> newFieldTypes) {
+ DexType newFieldType = newFieldTypes.getOrDefault(field.getReference(), field.getType());
+ if (newFieldType == field.getType()) {
+ assert newFieldSignatures.contains(field.getReference());
+ return field.getReference();
+ }
+ // Find a new name for this field if the signature is already occupied.
+ return dexItemFactory.createFreshFieldNameWithoutHolder(
+ field.getHolderType(), newFieldType, field.getName().toString(), newFieldSignatures::add);
}
private DexMethod getNewMethodSignature(
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/fields/FieldTypeStrengtheningCollisionTest.java b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/fields/FieldTypeStrengtheningCollisionTest.java
index b79fb5c..9e5e433 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/fields/FieldTypeStrengtheningCollisionTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/fields/FieldTypeStrengtheningCollisionTest.java
@@ -4,12 +4,19 @@
package com.android.tools.r8.ir.optimize.membervaluepropagation.fields;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ir.optimize.membervaluepropagation.fields.FieldTypeStrengtheningTest.A;
+import com.android.tools.r8.ir.optimize.membervaluepropagation.fields.FieldTypeStrengtheningTest.Main;
import com.android.tools.r8.transformers.ClassFileTransformer.FieldPredicate;
-import com.android.tools.r8.utils.codeinspector.AssertUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.FieldSubject;
import java.io.IOException;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -30,21 +37,36 @@
@Test
public void test() throws Exception {
- AssertUtils.assertFailsCompilation(
- () ->
- testForR8(parameters.getBackend())
- .addProgramClasses(A.class)
- .addProgramClassFileData(getTransformedMain())
- .addKeepMainRule(Main.class)
- .addKeepRules(
- "-keep class "
- + Main.class.getTypeName()
- + " { "
- + A.class.getTypeName()
- + " f; }")
- .enableInliningAnnotations()
- .setMinApi(parameters.getApiLevel())
- .compile());
+ testForR8(parameters.getBackend())
+ .addProgramClasses(A.class)
+ .addProgramClassFileData(getTransformedMain())
+ .addKeepMainRule(Main.class)
+ .addKeepRules(
+ "-keep class " + Main.class.getTypeName() + " { " + A.class.getTypeName() + " f; }")
+ .enableInliningAnnotations()
+ .noMinification()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject mainClassSubject = inspector.clazz(Main.class);
+ assertThat(mainClassSubject, isPresent());
+
+ ClassSubject aClassSubject = inspector.clazz(A.class);
+ assertThat(aClassSubject, isPresent());
+
+ FieldSubject keptFieldSubject = mainClassSubject.uniqueFieldWithFinalName("f");
+ assertEquals(
+ aClassSubject.getFinalName(),
+ keptFieldSubject.getField().getType().getTypeName());
+
+ FieldSubject optimizedFieldSubject = mainClassSubject.uniqueFieldWithFinalName("f$1");
+ assertEquals(
+ aClassSubject.getFinalName(),
+ optimizedFieldSubject.getField().getType().getTypeName());
+ })
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("A");
}
private static byte[] getTransformedMain() throws IOException {
