Fix is-fresh-field-name check in tree fixer
Change-Id: I88fc1fa769fc7f7e8154922fe56d6450ef967e88
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index b751475..40b630c 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -375,8 +375,10 @@
assert verifyNoDuplicateFields();
}
- public void clearStaticFields() {
+ public DexEncodedField[] clearStaticFields() {
+ DexEncodedField[] previousFields = staticFields;
setStaticFields(DexEncodedField.EMPTY_ARRAY);
+ return previousFields;
}
public void removeStaticField(int index) {
@@ -465,8 +467,10 @@
assert verifyNoDuplicateFields();
}
- public void clearInstanceFields() {
+ public DexEncodedField[] clearInstanceFields() {
+ DexEncodedField[] previousFields = instanceFields;
instanceFields = DexEncodedField.EMPTY_ARRAY;
+ return previousFields;
}
private boolean verifyCorrectnessOfFieldHolder(DexEncodedField field) {
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
index fc4e3e9..db0178c 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
@@ -312,7 +312,7 @@
void mergeStaticFields() {
group.forEachSource(classStaticFieldsMerger::addFields);
- classStaticFieldsMerger.merge(group.getTarget());
+ classStaticFieldsMerger.merge();
group.forEachSource(DexClass::clearStaticFields);
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassStaticFieldsMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassStaticFieldsMerger.java
index 6183391..f94d407 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassStaticFieldsMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassStaticFieldsMerger.java
@@ -14,27 +14,31 @@
import java.util.Map;
public class ClassStaticFieldsMerger {
- private final Builder lensBuilder;
- private final MergeGroup group;
- private final Map<DexField, DexEncodedField> targetFields = new LinkedHashMap<>();
+
private final DexItemFactory dexItemFactory;
+ private final MergeGroup group;
+ private final Builder lensBuilder;
+
+ private final Map<DexField, DexEncodedField> targetFields = new LinkedHashMap<>();
public ClassStaticFieldsMerger(
AppView<?> appView, HorizontalClassMergerGraphLens.Builder lensBuilder, MergeGroup group) {
- this.lensBuilder = lensBuilder;
-
- this.group = group;
- // Add mappings for all target fields.
- group
- .getTarget()
- .staticFields()
- .forEach(field -> targetFields.put(field.getReference(), field));
-
this.dexItemFactory = appView.dexItemFactory();
+ this.group = group;
+ this.lensBuilder = lensBuilder;
}
private boolean isFresh(DexField fieldReference) {
- return !targetFields.containsKey(fieldReference);
+ if (group.getTarget().lookupField(fieldReference) != null) {
+ // The target class has an instance or static field with the given reference.
+ return false;
+ }
+ if (targetFields.containsKey(fieldReference)) {
+ // We have already committed another static field from a source class in the merge group to
+ // the given field reference (but the field is not yet added to the target class).
+ return false;
+ }
+ return true;
}
private void addField(DexEncodedField field) {
@@ -53,10 +57,10 @@
}
public void addFields(DexProgramClass toMerge) {
- toMerge.staticFields().forEach(this::addField);
+ toMerge.forEachStaticField(this::addField);
}
- public void merge(DexProgramClass clazz) {
- clazz.setStaticFields(targetFields.values().toArray(DexEncodedField.EMPTY_ARRAY));
+ public void merge() {
+ group.getTarget().appendStaticFields(targetFields.values());
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
index 20eb5a3..2b12d6d 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
@@ -5,7 +5,9 @@
package com.android.tools.r8.horizontalclassmerging;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexEncodedMember;
import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexMember;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.NestedGraphLens;
@@ -15,6 +17,7 @@
import com.android.tools.r8.utils.collections.BidirectionalManyToOneHashMap;
import com.android.tools.r8.utils.collections.BidirectionalManyToOneRepresentativeHashMap;
import com.android.tools.r8.utils.collections.BidirectionalManyToOneRepresentativeMap;
+import com.android.tools.r8.utils.collections.MutableBidirectionalManyToOneMap;
import com.android.tools.r8.utils.collections.MutableBidirectionalManyToOneRepresentativeMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
@@ -83,18 +86,21 @@
public static class Builder {
- private final MutableBidirectionalManyToOneRepresentativeMap<DexField, DexField> fieldMap =
- BidirectionalManyToOneRepresentativeHashMap.newIdentityHashMap();
- private final BidirectionalManyToOneHashMap<DexMethod, DexMethod> methodMap =
+ private final MutableBidirectionalManyToOneRepresentativeMap<DexField, DexField>
+ newFieldSignatures = BidirectionalManyToOneRepresentativeHashMap.newIdentityHashMap();
+ private final MutableBidirectionalManyToOneMap<DexMethod, DexMethod> methodMap =
BidirectionalManyToOneHashMap.newIdentityHashMap();
- private final BidirectionalManyToOneRepresentativeHashMap<DexMethod, DexMethod>
+ private final MutableBidirectionalManyToOneRepresentativeMap<DexMethod, DexMethod>
newMethodSignatures = BidirectionalManyToOneRepresentativeHashMap.newIdentityHashMap();
private final Map<DexMethod, List<ExtraParameter>> methodExtraParameters =
new IdentityHashMap<>();
- private final BidirectionalManyToOneHashMap<DexMethod, DexMethod> pendingMethodMapUpdates =
+ private final MutableBidirectionalManyToOneMap<DexMethod, DexMethod> pendingMethodMapUpdates =
BidirectionalManyToOneHashMap.newIdentityHashMap();
- private final BidirectionalManyToOneRepresentativeHashMap<DexMethod, DexMethod>
+ private final MutableBidirectionalManyToOneRepresentativeMap<DexField, DexField>
+ pendingNewFieldSignatureUpdates =
+ BidirectionalManyToOneRepresentativeHashMap.newIdentityHashMap();
+ private final MutableBidirectionalManyToOneRepresentativeMap<DexMethod, DexMethod>
pendingNewMethodSignatureUpdates =
BidirectionalManyToOneRepresentativeHashMap.newIdentityHashMap();
@@ -103,6 +109,7 @@
HorizontalClassMergerGraphLens build(
AppView<?> appView, HorizontallyMergedClasses mergedClasses) {
assert pendingMethodMapUpdates.isEmpty();
+ assert pendingNewFieldSignatureUpdates.isEmpty();
assert pendingNewMethodSignatureUpdates.isEmpty();
assert newMethodSignatures.values().stream()
.allMatch(
@@ -115,7 +122,7 @@
appView,
mergedClasses,
methodExtraParameters,
- fieldMap,
+ newFieldSignatures,
methodMap.getForwardMap(),
newMethodSignatures);
}
@@ -126,7 +133,7 @@
}
void recordNewFieldSignature(DexField oldFieldSignature, DexField newFieldSignature) {
- fieldMap.put(oldFieldSignature, newFieldSignature);
+ newFieldSignatures.put(oldFieldSignature, newFieldSignature);
}
void recordNewFieldSignature(
@@ -135,26 +142,20 @@
DexField representative) {
assert Streams.stream(oldFieldSignatures)
.anyMatch(oldFieldSignature -> oldFieldSignature != newFieldSignature);
- assert Streams.stream(oldFieldSignatures).noneMatch(fieldMap::containsValue);
+ assert Streams.stream(oldFieldSignatures).noneMatch(newFieldSignatures::containsValue);
assert Iterables.contains(oldFieldSignatures, representative);
for (DexField oldFieldSignature : oldFieldSignatures) {
recordNewFieldSignature(oldFieldSignature, newFieldSignature);
}
- fieldMap.setRepresentative(newFieldSignature, representative);
+ newFieldSignatures.setRepresentative(newFieldSignature, representative);
}
void fixupField(DexField oldFieldSignature, DexField newFieldSignature) {
- DexField representative = fieldMap.removeRepresentativeFor(oldFieldSignature);
- Set<DexField> originalFieldSignatures = fieldMap.removeValue(oldFieldSignature);
- if (originalFieldSignatures.isEmpty()) {
- fieldMap.put(oldFieldSignature, newFieldSignature);
- } else if (originalFieldSignatures.size() == 1) {
- fieldMap.put(originalFieldSignatures, newFieldSignature);
- } else {
- assert representative != null;
- fieldMap.put(originalFieldSignatures, newFieldSignature);
- fieldMap.setRepresentative(newFieldSignature, representative);
- }
+ fixupOriginalMemberSignatures(
+ oldFieldSignature,
+ newFieldSignature,
+ newFieldSignatures,
+ pendingNewFieldSignatureUpdates);
}
void mapMethod(DexMethod oldMethodSignature, DexMethod newMethodSignature) {
@@ -195,7 +196,11 @@
void fixupMethod(DexMethod oldMethodSignature, DexMethod newMethodSignature) {
fixupMethodMap(oldMethodSignature, newMethodSignature);
- fixupOriginalMethodSignatures(oldMethodSignature, newMethodSignature);
+ fixupOriginalMemberSignatures(
+ oldMethodSignature,
+ newMethodSignature,
+ newMethodSignatures,
+ pendingNewMethodSignatureUpdates);
}
private void fixupMethodMap(DexMethod oldMethodSignature, DexMethod newMethodSignature) {
@@ -209,18 +214,22 @@
}
}
- private void fixupOriginalMethodSignatures(
- DexMethod oldMethodSignature, DexMethod newMethodSignature) {
- Set<DexMethod> oldMethodSignatures = newMethodSignatures.getKeys(oldMethodSignature);
- if (oldMethodSignatures.isEmpty()) {
- pendingNewMethodSignatureUpdates.put(oldMethodSignature, newMethodSignature);
+ private <D extends DexEncodedMember<D, R>, R extends DexMember<D, R>>
+ void fixupOriginalMemberSignatures(
+ R oldMemberSignature,
+ R newMemberSignature,
+ MutableBidirectionalManyToOneRepresentativeMap<R, R> newMemberSignatures,
+ MutableBidirectionalManyToOneRepresentativeMap<R, R> pendingNewMemberSignatureUpdates) {
+ Set<R> oldMemberSignatures = newMemberSignatures.getKeys(oldMemberSignature);
+ if (oldMemberSignatures.isEmpty()) {
+ pendingNewMemberSignatureUpdates.put(oldMemberSignature, newMemberSignature);
} else {
- for (DexMethod originalMethodSignature : oldMethodSignatures) {
- pendingNewMethodSignatureUpdates.put(originalMethodSignature, newMethodSignature);
+ for (R originalMethodSignature : oldMemberSignatures) {
+ pendingNewMemberSignatureUpdates.put(originalMethodSignature, newMemberSignature);
}
- DexMethod representative = newMethodSignatures.getRepresentativeKey(oldMethodSignature);
+ R representative = newMemberSignatures.getRepresentativeKey(oldMemberSignature);
if (representative != null) {
- pendingNewMethodSignatureUpdates.setRepresentative(newMethodSignature, representative);
+ pendingNewMemberSignatureUpdates.setRepresentative(newMemberSignature, representative);
}
}
}
@@ -231,16 +240,24 @@
pendingMethodMapUpdates.forEachManyToOneMapping(methodMap::put);
pendingMethodMapUpdates.clear();
- // Commit pending original method signatures updates.
- newMethodSignatures.removeAll(pendingNewMethodSignatureUpdates.keySet());
- pendingNewMethodSignatureUpdates.forEachManyToOneMapping(
+ // Commit pending original field and method signatures updates.
+ commitPendingNewMemberSignatureUpdates(newFieldSignatures, pendingNewFieldSignatureUpdates);
+ commitPendingNewMemberSignatureUpdates(newMethodSignatures, pendingNewMethodSignatureUpdates);
+ }
+
+ private <D extends DexEncodedMember<D, R>, R extends DexMember<D, R>>
+ void commitPendingNewMemberSignatureUpdates(
+ MutableBidirectionalManyToOneRepresentativeMap<R, R> newMemberSignatures,
+ MutableBidirectionalManyToOneRepresentativeMap<R, R> pendingNewMemberSignatureUpdates) {
+ newMemberSignatures.removeAll(pendingNewMemberSignatureUpdates.keySet());
+ pendingNewMemberSignatureUpdates.forEachManyToOneMapping(
(keys, value, representative) -> {
- newMethodSignatures.put(keys, value);
+ newMemberSignatures.put(keys, value);
if (keys.size() > 1) {
- newMethodSignatures.setRepresentative(value, representative);
+ newMemberSignatures.setRepresentative(value, representative);
}
});
- pendingNewMethodSignatureUpdates.clear();
+ pendingNewMemberSignatureUpdates.clear();
}
/**
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/TreeFixer.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/TreeFixer.java
index 442489d..b85eab3 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/TreeFixer.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/TreeFixer.java
@@ -7,7 +7,6 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.DexClass.FieldSetter;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
@@ -22,6 +21,7 @@
import com.android.tools.r8.graph.TreeFixerBase;
import com.android.tools.r8.ir.conversion.ExtraUnusedNullParameter;
import com.android.tools.r8.shaking.AnnotationFixer;
+import com.android.tools.r8.utils.ArrayUtils;
import com.android.tools.r8.utils.OptionalBool;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
@@ -168,8 +168,11 @@
.getMethodCollection()
.replaceAllDirectMethods(method -> fixupDirectMethod(newMethodReferences, method));
- fixupFields(clazz.staticFields(), clazz::setStaticField);
- fixupFields(clazz.instanceFields(), clazz::setInstanceField);
+ Set<DexField> newFieldReferences = Sets.newIdentityHashSet();
+ DexEncodedField[] instanceFields = clazz.clearInstanceFields();
+ DexEncodedField[] staticFields = clazz.clearStaticFields();
+ clazz.setInstanceFields(fixupFields(instanceFields, newFieldReferences));
+ clazz.setStaticFields(fixupFields(staticFields, newFieldReferences));
lensBuilder.commitPendingUpdates();
@@ -219,8 +222,13 @@
.getMethodCollection()
.replaceDirectMethods(method -> fixupDirectMethod(newDirectMethods, method));
iface.getMethodCollection().replaceVirtualMethods(this::fixupVirtualInterfaceMethod);
- fixupFields(iface.staticFields(), iface::setStaticField);
- fixupFields(iface.instanceFields(), iface::setInstanceField);
+
+ assert !iface.hasInstanceFields();
+
+ Set<DexField> newFieldReferences = Sets.newIdentityHashSet();
+ DexEncodedField[] staticFields = iface.clearStaticFields();
+ iface.setStaticFields(fixupFields(staticFields, newFieldReferences));
+
lensBuilder.commitPendingUpdates();
}
@@ -371,35 +379,40 @@
return fixupProgramMethod(newMethodReference, method);
}
- private void fixupFields(List<DexEncodedField> fields, FieldSetter setter) {
- if (fields == null) {
- return;
+ private DexEncodedField[] fixupFields(
+ DexEncodedField[] fields, Set<DexField> newFieldReferences) {
+ if (fields == null || ArrayUtils.isEmpty(fields)) {
+ return DexEncodedField.EMPTY_ARRAY;
}
- Set<DexField> existingFields = Sets.newIdentityHashSet();
- for (int i = 0; i < fields.size(); i++) {
- DexEncodedField oldField = fields.get(i);
+ DexEncodedField[] newFields = new DexEncodedField[fields.length];
+ for (int i = 0; i < fields.length; i++) {
+ DexEncodedField oldField = fields[i];
DexField oldFieldReference = oldField.getReference();
DexField newFieldReference = fixupFieldReference(oldFieldReference);
// Rename the field if it already exists.
- if (!existingFields.add(newFieldReference)) {
+ if (!newFieldReferences.add(newFieldReference)) {
DexField template = newFieldReference;
newFieldReference =
dexItemFactory.createFreshMember(
tryName ->
Optional.of(template.withName(tryName, dexItemFactory))
- .filter(tryMethod -> !existingFields.contains(tryMethod)),
+ .filter(tryMethod -> !newFieldReferences.contains(tryMethod)),
newFieldReference.name.toSourceString());
- boolean added = existingFields.add(newFieldReference);
+ boolean added = newFieldReferences.add(newFieldReference);
assert added;
}
if (newFieldReference != oldFieldReference) {
lensBuilder.fixupField(oldFieldReference, newFieldReference);
- setter.setField(i, oldField.toTypeSubstitutedField(newFieldReference));
+ newFields[i] = oldField.toTypeSubstitutedField(newFieldReference);
+ } else {
+ newFields[i] = oldField;
}
}
+
+ return newFields;
}
@Override
