Rewrite synthetic methods to final signature in one step
Change-Id: I1220586d44b5d211c375ec723bf3a059a34bdafe
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLens.java b/src/main/java/com/android/tools/r8/graph/GraphLens.java
index c15db19..8f6530e 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLens.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLens.java
@@ -1033,7 +1033,7 @@
@Override
public DexField getRenamedFieldSignature(DexField originalField) {
DexField renamedField = getPrevious().getRenamedFieldSignature(originalField);
- return fieldMap.getOrDefault(renamedField, renamedField);
+ return internalGetNextFieldSignature(renamedField);
}
@Override
@@ -1133,6 +1133,10 @@
return prototypeChanges;
}
+ protected DexField internalGetNextFieldSignature(DexField field) {
+ return fieldMap.getOrDefault(field, field);
+ }
+
@Override
protected DexMethod internalGetPreviousMethodSignature(DexMethod method) {
return originalMethodSignatures.getRepresentativeValueOrDefault(method, method);
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 0204788..d610175 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.synthesis;
+import static com.google.common.base.Predicates.alwaysTrue;
+
import com.android.tools.r8.features.ClassToFeatureSplitMap;
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
@@ -29,7 +31,8 @@
import com.android.tools.r8.utils.collections.BidirectionalManyToManyRepresentativeMap;
import com.android.tools.r8.utils.collections.BidirectionalManyToOneRepresentativeHashMap;
import com.android.tools.r8.utils.collections.BidirectionalManyToOneRepresentativeMap;
-import com.android.tools.r8.utils.collections.BidirectionalOneToOneHashMap;
+import com.android.tools.r8.utils.collections.BidirectionalOneToManyRepresentativeHashMap;
+import com.android.tools.r8.utils.collections.MutableBidirectionalOneToManyRepresentativeMap;
import com.android.tools.r8.utils.structural.RepresentativeMap;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
@@ -43,7 +46,6 @@
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.Set;
import java.util.function.BiConsumer;
@@ -69,118 +71,60 @@
public static class SyntheticFinalizationGraphLens extends NestedGraphLens {
- private final Map<DexType, DexType> syntheticTypeMap;
- private final Map<DexMethod, DexMethod> syntheticMethodsMap;
-
private SyntheticFinalizationGraphLens(
GraphLens previous,
- Map<DexType, DexType> syntheticClassesMap,
- Map<DexMethod, DexMethod> syntheticMethodsMap,
Map<DexType, DexType> typeMap,
BidirectionalManyToOneRepresentativeMap<DexField, DexField> fieldMap,
Map<DexMethod, DexMethod> methodMap,
BidirectionalManyToManyRepresentativeMap<DexMethod, DexMethod> originalMethodSignatures,
DexItemFactory factory) {
super(typeMap, methodMap, fieldMap, originalMethodSignatures, previous, factory);
- this.syntheticTypeMap = syntheticClassesMap;
- this.syntheticMethodsMap = syntheticMethodsMap;
}
@Override
public boolean isSyntheticFinalizationGraphLens() {
return true;
}
-
- // The mapping is many to one, so the inverse is only defined up to equivalence groups.
- // Override the access to renamed signatures to first check for synthetic mappings before
- // using the original item mappings of the
-
- @Override
- public DexField getRenamedFieldSignature(DexField originalField) {
- if (syntheticTypeMap.containsKey(originalField.holder)) {
- DexField renamed = fieldMap.get(originalField);
- if (renamed != null) {
- return renamed;
- }
- }
- return super.getRenamedFieldSignature(originalField);
- }
-
- @Override
- public DexMethod getRenamedMethodSignature(DexMethod originalMethod, GraphLens applied) {
- if (syntheticTypeMap.containsKey(originalMethod.holder)) {
- DexMethod renamed = methodMap.get(originalMethod);
- if (renamed != null) {
- return renamed;
- }
- }
- DexMethod renamed = syntheticMethodsMap.get(originalMethod);
- return renamed != null ? renamed : super.getRenamedMethodSignature(originalMethod, applied);
- }
}
private static class Builder {
- // Forward mapping of internal to external synthetics.
- Map<DexType, DexType> syntheticClassesMap = new IdentityHashMap<>();
- Map<DexMethod, DexMethod> syntheticMethodsMap = new IdentityHashMap<>();
-
Map<DexType, DexType> typeMap = new IdentityHashMap<>();
BidirectionalManyToOneRepresentativeHashMap<DexField, DexField> fieldMap =
new BidirectionalManyToOneRepresentativeHashMap<>();
Map<DexMethod, DexMethod> methodMap = new IdentityHashMap<>();
- protected final BidirectionalOneToOneHashMap<DexMethod, DexMethod> originalMethodSignatures =
- new BidirectionalOneToOneHashMap<>();
-
- void moveSyntheticClass(DexType from, DexType to) {
- assert !syntheticClassesMap.containsKey(from);
- syntheticClassesMap.put(from, to);
- typeMap.put(from, to);
- }
-
- void moveSyntheticMethod(DexMethod from, DexMethod to) {
- assert !syntheticMethodsMap.containsKey(from);
- syntheticMethodsMap.put(from, to);
- methodMap.put(from, to);
- typeMap.put(from.getHolderType(), to.getHolderType());
- }
+ protected final MutableBidirectionalOneToManyRepresentativeMap<DexMethod, DexMethod>
+ originalMethodSignatures = new BidirectionalOneToManyRepresentativeHashMap<>();
void move(DexType from, DexType to) {
- typeMap.put(from, to);
+ DexType old = typeMap.put(from, to);
+ assert old == null || old == to;
}
void move(DexField from, DexField to) {
- fieldMap.put(from, to);
+ DexField old = fieldMap.put(from, to);
+ assert old == null || old == to;
}
void move(DexMethod from, DexMethod to) {
- methodMap.put(from, to);
+ DexMethod old = methodMap.put(from, to);
+ assert old == null || old == to;
originalMethodSignatures.put(to, from);
}
SyntheticFinalizationGraphLens build(GraphLens previous, DexItemFactory factory) {
- assert verifySubMap(syntheticClassesMap, typeMap);
if (typeMap.isEmpty() && fieldMap.isEmpty() && methodMap.isEmpty()) {
return null;
}
return new SyntheticFinalizationGraphLens(
previous,
- syntheticClassesMap,
- syntheticMethodsMap,
typeMap,
fieldMap,
methodMap,
originalMethodSignatures,
factory);
}
-
- private static <K, V> boolean verifySubMap(Map<K, V> sub, Map<K, V> sup) {
- for (Entry<K, V> entry : sub.entrySet()) {
- assert sup.get(entry.getKey()) == entry.getValue();
- }
- return true;
- }
}
public static class EquivalenceGroup<T extends SyntheticDefinition<?, T, ?>> {
@@ -279,24 +223,19 @@
ImmutableMap.builder();
ImmutableMap.Builder<DexType, SyntheticProgramClassReference> finalClassesBuilder =
ImmutableMap.builder();
- List<DexProgramClass> finalSyntheticProgramDefinitions = new ArrayList<>();
Set<DexType> derivedMainDexTypes = Sets.newIdentityHashSet();
{
Map<String, NumberGenerator> generators = new HashMap<>();
application =
buildLensAndProgram(
appView,
- computeEquivalences(appView, committed.getNonLegacyMethods().values(), generators),
- computeEquivalences(appView, committed.getNonLegacyClasses().values(), generators),
+ computeEquivalences(
+ appView, committed.getNonLegacyMethods().values(), generators, lensBuilder),
+ computeEquivalences(
+ appView, committed.getNonLegacyClasses().values(), generators, lensBuilder),
lensBuilder,
- (clazz, reference) -> {
- finalSyntheticProgramDefinitions.add(clazz);
- finalClassesBuilder.put(clazz.getType(), reference);
- },
- (clazz, reference) -> {
- finalSyntheticProgramDefinitions.add(clazz);
- finalMethodsBuilder.put(clazz.getType(), reference);
- },
+ (clazz, reference) -> finalClassesBuilder.put(clazz.getType(), reference),
+ (clazz, reference) -> finalMethodsBuilder.put(clazz.getType(), reference),
derivedMainDexTypes);
}
ImmutableMap<DexType, SyntheticMethodReference> finalMethods = finalMethodsBuilder.build();
@@ -332,7 +271,8 @@
Map<DexType, EquivalenceGroup<D>> computeEquivalences(
AppView<?> appView,
ImmutableCollection<R> references,
- Map<String, NumberGenerator> generators) {
+ Map<String, NumberGenerator> generators,
+ Builder lensBuilder) {
boolean intermediate = appView.options().intermediate;
Map<DexType, D> definitions = lookupDefinitions(appView, references);
ClassToFeatureSplitMap classToFeatureSplitMap =
@@ -348,7 +288,12 @@
classToFeatureSplitMap,
synthetics);
return computeActualEquivalences(
- potentialEquivalences, generators, appView, intermediate, classToFeatureSplitMap);
+ potentialEquivalences,
+ generators,
+ appView,
+ intermediate,
+ classToFeatureSplitMap,
+ lensBuilder);
}
private boolean isNotSyntheticType(DexType type) {
@@ -374,80 +319,15 @@
BiConsumer<DexProgramClass, SyntheticMethodReference> addFinalSyntheticMethod,
Set<DexType> derivedMainDexSynthetics) {
DexApplication application = appView.appInfo().app();
- DexItemFactory factory = appView.dexItemFactory();
MainDexInfo mainDexInfo = appView.appInfo().getMainDexInfo();
List<DexProgramClass> newProgramClasses = new ArrayList<>();
Set<DexType> pruned = Sets.newIdentityHashSet();
- syntheticMethodGroups.forEach(
- (syntheticType, syntheticGroup) -> {
- SyntheticMethodDefinition representative = syntheticGroup.getRepresentative();
- SynthesizingContext context = representative.getContext();
- context.registerPrefixRewriting(syntheticType, appView);
- DexProgramClass externalSyntheticClass =
- createExternalMethodClass(syntheticType, representative, factory);
- newProgramClasses.add(externalSyntheticClass);
- addSyntheticMarker(representative.getKind(), externalSyntheticClass, context, appView);
- assert externalSyntheticClass.getMethodCollection().size() == 1;
- DexEncodedMethod externalSyntheticMethod =
- externalSyntheticClass.methods().iterator().next();
- for (SyntheticMethodDefinition member : syntheticGroup.getMembers()) {
- DexMethod memberReference = member.getMethod().getReference();
- pruned.add(member.getHolder().getType());
- if (memberReference != externalSyntheticMethod.method) {
- lensBuilder.moveSyntheticMethod(memberReference, externalSyntheticMethod.method);
- }
- if (member.getContext().isDerivedFromMainDexList(mainDexInfo)) {
- derivedMainDexSynthetics.add(syntheticType);
- }
- }
- });
-
- List<DexProgramClass> deduplicatedClasses = new ArrayList<>();
- syntheticClassGroups.forEach(
- (syntheticType, syntheticGroup) -> {
- SyntheticProgramClassDefinition representative = syntheticGroup.getRepresentative();
- SynthesizingContext context = representative.getContext();
- context.registerPrefixRewriting(syntheticType, appView);
- DexProgramClass externalSyntheticClass = representative.getHolder();
- newProgramClasses.add(externalSyntheticClass);
- addSyntheticMarker(representative.getKind(), externalSyntheticClass, context, appView);
- for (SyntheticProgramClassDefinition member : syntheticGroup.getMembers()) {
- DexProgramClass memberClass = member.getHolder();
- DexType memberType = memberClass.getType();
- pruned.add(memberType);
- if (memberType != syntheticType) {
- lensBuilder.moveSyntheticClass(memberType, syntheticType);
- }
- // The aliasing of the non-representative members needs to be recorded manually.
- if (member != representative) {
- deduplicatedClasses.add(memberClass);
- }
- if (member.getContext().isDerivedFromMainDexList(mainDexInfo)) {
- derivedMainDexSynthetics.add(syntheticType);
- }
- }
- });
-
- for (DexProgramClass clazz : application.classes()) {
- if (!pruned.contains(clazz.type)) {
- newProgramClasses.add(clazz);
- }
- }
- application = application.builder().replaceProgramClasses(newProgramClasses).build();
-
- // We can only assert that the method container classes are in here as the classes need
- // to be rewritten by the tree-fixer.
- for (DexType key : syntheticMethodGroups.keySet()) {
- assert application.definitionFor(key) != null;
- }
-
- DexApplication.Builder<?> builder = application.builder();
TreeFixerBase treeFixer =
new TreeFixerBase(appView) {
@Override
public DexType mapClassType(DexType type) {
- return lensBuilder.syntheticClassesMap.getOrDefault(type, type);
+ return lensBuilder.typeMap.getOrDefault(type, type);
}
@Override
@@ -465,6 +345,59 @@
lensBuilder.move(from, to);
}
};
+
+ List<DexProgramClass> deduplicatedClasses = new ArrayList<>();
+ syntheticMethodGroups.forEach(
+ (syntheticType, syntheticGroup) -> {
+ SyntheticMethodDefinition representative = syntheticGroup.getRepresentative();
+ SynthesizingContext context = representative.getContext();
+ context.registerPrefixRewriting(syntheticType, appView);
+ DexProgramClass representativeClass = representative.getHolder();
+ addSyntheticMarker(representative.getKind(), representativeClass, context, appView);
+ assert representativeClass.getMethodCollection().size() == 1;
+ for (SyntheticMethodDefinition member : syntheticGroup.getMembers()) {
+ if (member != representative) {
+ pruned.add(member.getHolder().getType());
+ deduplicatedClasses.add(member.getHolder());
+ }
+ if (member.getContext().isDerivedFromMainDexList(mainDexInfo)) {
+ derivedMainDexSynthetics.add(syntheticType);
+ }
+ }
+ });
+
+ syntheticClassGroups.forEach(
+ (syntheticType, syntheticGroup) -> {
+ SyntheticProgramClassDefinition representative = syntheticGroup.getRepresentative();
+ SynthesizingContext context = representative.getContext();
+ context.registerPrefixRewriting(syntheticType, appView);
+ addSyntheticMarker(
+ representative.getKind(), representative.getHolder(), context, appView);
+ for (SyntheticProgramClassDefinition member : syntheticGroup.getMembers()) {
+ DexProgramClass memberClass = member.getHolder();
+ DexType memberType = memberClass.getType();
+ if (member != representative) {
+ pruned.add(memberType);
+ deduplicatedClasses.add(memberClass);
+ }
+ if (member.getContext().isDerivedFromMainDexList(mainDexInfo)) {
+ derivedMainDexSynthetics.add(syntheticType);
+ }
+ }
+ });
+
+ for (DexProgramClass clazz : application.classes()) {
+ if (!pruned.contains(clazz.type)) {
+ newProgramClasses.add(clazz);
+ }
+ }
+ application = application.builder().replaceProgramClasses(newProgramClasses).build();
+
+ // Assert that the non-representatives have been removed from the app.
+ assert verifyNonRepresentativesRemovedFromApplication(application, syntheticClassGroups);
+ assert verifyNonRepresentativesRemovedFromApplication(application, syntheticMethodGroups);
+
+ DexApplication.Builder<?> builder = application.builder();
treeFixer.fixupClasses(deduplicatedClasses);
builder.replaceProgramClasses(treeFixer.fixupClasses(application.classes()));
application = builder.build();
@@ -486,15 +419,16 @@
(syntheticType, syntheticGroup) -> {
DexProgramClass externalSyntheticClass = appForLookup.programDefinitionFor(syntheticType);
SyntheticMethodDefinition representative = syntheticGroup.getRepresentative();
+ assert externalSyntheticClass.getMethodCollection().size() == 1;
+ assert externalSyntheticClass.getMethodCollection().hasDirectMethods();
+ DexEncodedMethod syntheticMethodDefinition =
+ externalSyntheticClass.getMethodCollection().getDirectMethod(alwaysTrue());
addFinalSyntheticMethod.accept(
externalSyntheticClass,
new SyntheticMethodReference(
representative.getKind(),
representative.getContext(),
- representative
- .getMethod()
- .getReference()
- .withHolder(externalSyntheticClass.type, factory)));
+ syntheticMethodDefinition.getReference()));
});
for (DexType key : syntheticMethodGroups.keySet()) {
@@ -508,6 +442,18 @@
return application;
}
+ private static <T extends SyntheticDefinition<?, T, ?>>
+ boolean verifyNonRepresentativesRemovedFromApplication(
+ DexApplication application, Map<DexType, EquivalenceGroup<T>> syntheticGroups) {
+ for (EquivalenceGroup<?> syntheticGroup : syntheticGroups.values()) {
+ for (SyntheticDefinition<?, ?, ?> member : syntheticGroup.getMembers()) {
+ assert member == syntheticGroup.getRepresentative()
+ || application.definitionFor(member.getHolder().getType()) == null;
+ }
+ }
+ return true;
+ }
+
private static void addSyntheticMarker(
SyntheticKind kind,
DexProgramClass externalSyntheticClass,
@@ -519,25 +465,6 @@
}
}
- private static DexProgramClass createExternalMethodClass(
- DexType syntheticType, SyntheticMethodDefinition representative, DexItemFactory factory) {
- SyntheticProgramClassBuilder builder =
- new SyntheticProgramClassBuilder(syntheticType, representative.getContext(), factory);
- // TODO(b/158159959): Support grouping multiple methods per synthetic class.
- builder.addMethod(
- methodBuilder -> {
- DexEncodedMethod definition = representative.getMethod().getDefinition();
- methodBuilder
- .setName(SyntheticNaming.INTERNAL_SYNTHETIC_METHOD_PREFIX)
- .setAccessFlags(definition.accessFlags)
- .setProto(definition.getProto())
- .setClassFileVersion(
- definition.hasClassFileVersion() ? definition.getClassFileVersion() : null)
- .setCode(m -> definition.getCode());
- });
- return builder.build();
- }
-
private static boolean shouldAnnotateSynthetics(InternalOptions options) {
// Only intermediate builds have annotated synthetics to allow later sharing.
// This is currently also disabled on CF to CF desugaring to avoid missing class references to
@@ -552,7 +479,8 @@
Map<String, NumberGenerator> generators,
AppView<?> appView,
boolean intermediate,
- ClassToFeatureSplitMap classToFeatureSplitMap) {
+ ClassToFeatureSplitMap classToFeatureSplitMap,
+ Builder lensBuilder) {
Map<String, List<EquivalenceGroup<T>>> groupsPerPrefix = new HashMap<>();
potentialEquivalences.forEach(
members -> {
@@ -601,6 +529,9 @@
DexType representativeType =
createExternalType(kind, externalSyntheticTypePrefix, generators, appView);
equivalences.put(representativeType, group);
+ for (T member : group.getMembers()) {
+ lensBuilder.move(member.getHolder().getType(), representativeType);
+ }
}
});
return equivalences;
diff --git a/src/main/java/com/android/tools/r8/utils/collections/BidirectionalManyToOneHashMap.java b/src/main/java/com/android/tools/r8/utils/collections/BidirectionalManyToOneHashMap.java
index 8f29df9..b75d224 100644
--- a/src/main/java/com/android/tools/r8/utils/collections/BidirectionalManyToOneHashMap.java
+++ b/src/main/java/com/android/tools/r8/utils/collections/BidirectionalManyToOneHashMap.java
@@ -130,10 +130,11 @@
}
@Override
- public void put(K key, V value) {
- remove(key);
+ public V put(K key, V value) {
+ V old = remove(key);
backing.put(key, value);
inverse.computeIfAbsent(value, ignore -> new LinkedHashSet<>()).add(key);
+ return old;
}
@Override
diff --git a/src/main/java/com/android/tools/r8/utils/collections/BidirectionalOneToOneHashMap.java b/src/main/java/com/android/tools/r8/utils/collections/BidirectionalOneToOneHashMap.java
index 830138a..e23a565 100644
--- a/src/main/java/com/android/tools/r8/utils/collections/BidirectionalOneToOneHashMap.java
+++ b/src/main/java/com/android/tools/r8/utils/collections/BidirectionalOneToOneHashMap.java
@@ -77,6 +77,11 @@
}
@Override
+ public K getKey(V value) {
+ return backing.inverse().get(value);
+ }
+
+ @Override
public BiMap<K, V> getForwardMap() {
return backing;
}
@@ -88,12 +93,12 @@
@Override
public K getRepresentativeKey(V value) {
- return backing.inverse().get(value);
+ return getKey(value);
}
@Override
public V getRepresentativeValue(K key) {
- return backing.get(key);
+ return get(key);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/utils/collections/BidirectionalOneToOneMap.java b/src/main/java/com/android/tools/r8/utils/collections/BidirectionalOneToOneMap.java
index e5085c2..569c52e 100644
--- a/src/main/java/com/android/tools/r8/utils/collections/BidirectionalOneToOneMap.java
+++ b/src/main/java/com/android/tools/r8/utils/collections/BidirectionalOneToOneMap.java
@@ -15,6 +15,8 @@
public interface BidirectionalOneToOneMap<K, V>
extends BidirectionalManyToOneRepresentativeMap<K, V> {
+ K getKey(V value);
+
@Override
BiMap<K, V> getForwardMap();
diff --git a/src/main/java/com/android/tools/r8/utils/collections/EmptyBidirectionalOneToOneMap.java b/src/main/java/com/android/tools/r8/utils/collections/EmptyBidirectionalOneToOneMap.java
index f91f7df..62fba9a 100644
--- a/src/main/java/com/android/tools/r8/utils/collections/EmptyBidirectionalOneToOneMap.java
+++ b/src/main/java/com/android/tools/r8/utils/collections/EmptyBidirectionalOneToOneMap.java
@@ -57,6 +57,11 @@
}
@Override
+ public K getKey(V value) {
+ return null;
+ }
+
+ @Override
public BiMap<K, V> getForwardMap() {
return HashBiMap.create();
}
diff --git a/src/main/java/com/android/tools/r8/utils/collections/MutableBidirectionalManyToOneMap.java b/src/main/java/com/android/tools/r8/utils/collections/MutableBidirectionalManyToOneMap.java
index 7953e91..05bd5ea 100644
--- a/src/main/java/com/android/tools/r8/utils/collections/MutableBidirectionalManyToOneMap.java
+++ b/src/main/java/com/android/tools/r8/utils/collections/MutableBidirectionalManyToOneMap.java
@@ -11,7 +11,7 @@
void clear();
- void put(K key, V value);
+ V put(K key, V value);
void put(Iterable<K> key, V value);
diff --git a/src/main/java/com/android/tools/r8/utils/collections/MutableBidirectionalOneToOneMap.java b/src/main/java/com/android/tools/r8/utils/collections/MutableBidirectionalOneToOneMap.java
index 5e5f469..87a29fa 100644
--- a/src/main/java/com/android/tools/r8/utils/collections/MutableBidirectionalOneToOneMap.java
+++ b/src/main/java/com/android/tools/r8/utils/collections/MutableBidirectionalOneToOneMap.java
@@ -10,4 +10,6 @@
V put(K key, V value);
void putAll(BidirectionalManyToManyMap<K, V> map);
+
+ V remove(Object key);
}
