Ensure fresh names when merging instance fields
Bug: b/263934503
Change-Id: I56c1297bc6a89f5f4a6473254efd5c0426477f23
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
index 25933dd..195a8fc 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexEncodedField;
+import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DexTypeUtils;
@@ -14,6 +15,7 @@
import com.android.tools.r8.horizontalclassmerging.policies.SameInstanceFields.InstanceFieldInfo;
import com.android.tools.r8.utils.IterableUtils;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashMap;
@@ -22,6 +24,7 @@
import java.util.Map;
import java.util.Set;
import java.util.function.BiConsumer;
+import java.util.function.Predicate;
public interface ClassInstanceFieldsMerger {
@@ -135,6 +138,8 @@
private DexEncodedField classIdField;
+ private final Set<DexField> committedFields = Sets.newIdentityHashSet();
+
private ClassInstanceFieldsMergerImpl(
AppView<? extends AppInfoWithClassHierarchy> appView,
HorizontalClassMergerGraphLens.Builder lensBuilder,
@@ -155,12 +160,17 @@
List<DexEncodedField> newFields = new ArrayList<>();
if (classIdField != null) {
newFields.add(classIdField);
+ committedFields.add(classIdField.getReference());
}
group
.getInstanceFieldMap()
.forEachManyToOneMapping(
- (sourceFields, targetField) ->
- newFields.add(mergeSourceFieldsToTargetField(targetField, sourceFields)));
+ (sourceFields, targetField) -> {
+ DexEncodedField newField =
+ mergeSourceFieldsToTargetField(targetField, sourceFields);
+ newFields.add(newField);
+ committedFields.add(newField.getReference());
+ });
return newFields.toArray(DexEncodedField.EMPTY_ARRAY);
}
@@ -184,6 +194,19 @@
newField = targetField;
}
+ if (committedFields.contains(newField.getReference())) {
+ newField =
+ targetField.toTypeSubstitutedField(
+ appView,
+ appView
+ .dexItemFactory()
+ .createFreshFieldNameWithoutHolder(
+ newField.getHolderType(),
+ newField.getType(),
+ newField.getName().toString(),
+ Predicate.not(committedFields::contains)));
+ }
+
lensBuilder.recordNewFieldSignature(
Iterables.transform(
IterableUtils.append(sourceFields, targetField), DexEncodedField::getReference),
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/RelaxedInstanceFieldCollisionTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/RelaxedInstanceFieldCollisionTest.java
index fa784ca..d35ce82 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/RelaxedInstanceFieldCollisionTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/RelaxedInstanceFieldCollisionTest.java
@@ -4,10 +4,6 @@
package com.android.tools.r8.classmerging.horizontal;
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.junit.Assert.assertThrows;
-
-import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
@@ -48,22 +44,16 @@
@Test
public void testR8() throws Exception {
- assertThrows(
- CompilationFailedException.class,
- () ->
- testForR8(parameters.getBackend())
- .addProgramClasses(
- UnrelatedA.class, UnrelatedB.class, UnrelatedC.class, UnrelatedD.class)
- .addProgramClassFileData(getTransformedClasses())
- .setMinApi(parameters.getApiLevel())
- .addKeepMainRule(Main.class)
- .addKeepClassRules(
- UnrelatedA.class, UnrelatedB.class, UnrelatedC.class, UnrelatedD.class)
- // TODO(b/263934503): We should not fail with a duplicate field message.
- .compileWithExpectedDiagnostics(
- diagnostics ->
- diagnostics.assertErrorMessageThatMatches(
- containsString("Duplicate field"))));
+ testForR8(parameters.getBackend())
+ .addProgramClasses(UnrelatedA.class, UnrelatedB.class, UnrelatedC.class, UnrelatedD.class)
+ .addProgramClassFileData(getTransformedClasses())
+ .setMinApi(parameters.getApiLevel())
+ .addKeepMainRule(Main.class)
+ .addKeepClassRules(UnrelatedA.class, UnrelatedB.class, UnrelatedC.class, UnrelatedD.class)
+ .addHorizontallyMergedClassesInspector(
+ inspector -> inspector.assertClassesMerged(A.class, B.class))
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
}
private Collection<byte[]> getTransformedClasses() throws Exception {
