Reland "Use the type as the prefix for external synthetics."
This reverts commit df524d261735db40ca981db5a83bd5b4bb9bd4f6.
Bug: 179174151
Change-Id: Ic327e901f7723249976fef8c31ccb860bc5b4dbb
diff --git a/src/main/java/com/android/tools/r8/graph/DexField.java b/src/main/java/com/android/tools/r8/graph/DexField.java
index 5ebf3ae..ef99d01 100644
--- a/src/main/java/com/android/tools/r8/graph/DexField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexField.java
@@ -36,6 +36,19 @@
}
@Override
+ public int compareTo(DexReference other) {
+ if (other.isDexField()) {
+ return compareTo(other.asDexField());
+ }
+ if (other.isDexMethod()) {
+ int comparisonResult = getHolderType().compareTo(other.getContextType());
+ return comparisonResult != 0 ? comparisonResult : -1;
+ }
+ int comparisonResult = getHolderType().compareTo(other.asDexType());
+ return comparisonResult != 0 ? comparisonResult : 1;
+ }
+
+ @Override
public DexField self() {
return this;
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexMethod.java b/src/main/java/com/android/tools/r8/graph/DexMethod.java
index 29e9792..8484a94 100644
--- a/src/main/java/com/android/tools/r8/graph/DexMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexMethod.java
@@ -36,6 +36,15 @@
}
@Override
+ public int compareTo(DexReference other) {
+ if (other.isDexMethod()) {
+ return compareTo(other.asDexMethod());
+ }
+ int comparisonResult = getHolderType().compareTo(other.getContextType());
+ return comparisonResult != 0 ? comparisonResult : 1;
+ }
+
+ @Override
public StructuralMapping<DexMethod> getStructuralMapping() {
return DexMethod::specify;
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexReference.java b/src/main/java/com/android/tools/r8/graph/DexReference.java
index 77976f1..27264bb 100644
--- a/src/main/java/com/android/tools/r8/graph/DexReference.java
+++ b/src/main/java/com/android/tools/r8/graph/DexReference.java
@@ -29,6 +29,8 @@
public abstract void collectIndexedItems(IndexedItemCollection indexedItems);
+ public abstract int compareTo(DexReference other);
+
public abstract DexType getContextType();
public boolean isDexType() {
diff --git a/src/main/java/com/android/tools/r8/graph/DexType.java b/src/main/java/com/android/tools/r8/graph/DexType.java
index aaeb156..75b592b 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -53,6 +53,15 @@
}
@Override
+ public int compareTo(DexReference other) {
+ if (other.isDexType()) {
+ return compareTo(other.asDexType());
+ }
+ int comparisonResult = compareTo(other.getContextType());
+ return comparisonResult != 0 ? comparisonResult : -1;
+ }
+
+ @Override
public DexType self() {
return this;
}
diff --git a/src/main/java/com/android/tools/r8/synthesis/SynthesizingContext.java b/src/main/java/com/android/tools/r8/synthesis/SynthesizingContext.java
index 1067e93..82f3204 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SynthesizingContext.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SynthesizingContext.java
@@ -8,15 +8,12 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
-import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.GraphLens.NonIdentityGraphLens;
import com.android.tools.r8.graph.ProgramDefinition;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.MainDexInfo;
-import com.android.tools.r8.synthesis.SyntheticNaming.Phase;
-import com.android.tools.r8.synthesis.SyntheticNaming.SyntheticKind;
import java.util.Comparator;
import java.util.Set;
@@ -63,33 +60,6 @@
return new SynthesizingContext(synthesizingContextType, clazz.type, clazz.origin);
}
- static SynthesizingContext fromSyntheticContextChange(
- SyntheticKind kind,
- DexType syntheticType,
- SynthesizingContext oldContext,
- DexItemFactory factory) {
- String descriptor = syntheticType.toDescriptorString();
- DexType newContext;
- if (kind.isFixedSuffixSynthetic) {
- int i = descriptor.lastIndexOf(kind.descriptor);
- if (i < 0 || descriptor.length() != i + kind.descriptor.length() + 1) {
- assert false : "Unexpected fixed synthetic with invalid suffix: " + syntheticType;
- return null;
- }
- newContext = factory.createType(descriptor.substring(0, i) + ";");
- } else {
- int i = descriptor.indexOf(SyntheticNaming.getPhaseSeparator(Phase.INTERNAL));
- if (i <= 0) {
- assert false : "Unexpected synthetic without internal separator: " + syntheticType;
- return null;
- }
- newContext = factory.createType(descriptor.substring(0, i) + ";");
- }
- return newContext == oldContext.getSynthesizingContextType()
- ? oldContext
- : new SynthesizingContext(newContext, newContext, oldContext.inputContextOrigin);
- }
-
private SynthesizingContext(
DexType synthesizingContextType, DexType inputContextType, Origin inputContextOrigin) {
this.synthesizingContextType = synthesizingContextType;
@@ -127,10 +97,10 @@
}
void registerPrefixRewriting(DexType hygienicType, AppView<?> appView) {
- assert hygienicType.toSourceString().startsWith(synthesizingContextType.toSourceString());
if (!appView.options().isDesugaredLibraryCompilation()) {
return;
}
+ assert hygienicType.toSourceString().startsWith(synthesizingContextType.toSourceString());
DexType rewrittenContext =
appView
.options()
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticClassReference.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticClassReference.java
index 25142ee..69f87fe 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticClassReference.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticClassReference.java
@@ -30,4 +30,9 @@
DexType getHolder() {
return type;
}
+
+ @Override
+ DexType getReference() {
+ return type;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticClasspathClassReference.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticClasspathClassReference.java
index 5b9ba3c..72beb13 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticClasspathClassReference.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticClasspathClassReference.java
@@ -34,9 +34,12 @@
}
@Override
- SyntheticClasspathClassReference rewrite(NonIdentityGraphLens lens) {
+ SyntheticClasspathClassReference internalRewrite(
+ SynthesizingContext rewrittenContext, NonIdentityGraphLens lens) {
assert type == lens.lookupType(type)
: "Unexpected classpath rewrite of type " + type.toSourceString();
+ assert getContext() == rewrittenContext
+ : "Unexpected classpath rewrite of context type " + getContext();
return this;
}
}
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 239e7d0..9520d57 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java
@@ -59,12 +59,21 @@
return context;
}
+ final String getPrefixForExternalSyntheticType() {
+ return SyntheticNaming.getPrefixForExternalSyntheticType(getKind(), getHolder().getType());
+ }
+
public abstract C getHolder();
final HashCode computeHash(
RepresentativeMap map, boolean intermediate, ClassToFeatureSplitMap classToFeatureSplitMap) {
Hasher hasher = Hashing.murmur3_128().newHasher();
- if (intermediate || getKind().isFixedSuffixSynthetic) {
+ if (getKind().isFixedSuffixSynthetic) {
+ // Fixed synthetics are non-shareable. Its unique type is used as the hash key.
+ getHolder().getType().hash(hasher);
+ return hasher.hash();
+ }
+ if (intermediate) {
// If in intermediate mode, include the context type as sharing is restricted to within a
// single context.
getContext().getSynthesizingContextType().hashWithTypeEquivalence(hasher, map);
@@ -95,7 +104,13 @@
boolean includeContext,
GraphLens graphLens,
ClassToFeatureSplitMap classToFeatureSplitMap) {
- if (includeContext || getKind().isFixedSuffixSynthetic) {
+ DexType thisType = getHolder().getType();
+ DexType otherType = other.getHolder().getType();
+ if (getKind().isFixedSuffixSynthetic) {
+ // Fixed synthetics are non-shareable. Ordered by their unique type.
+ return thisType.compareTo(otherType);
+ }
+ if (includeContext) {
int order = getContext().compareTo(other.getContext());
if (order != 0) {
return order;
@@ -113,8 +128,6 @@
return order;
}
}
- DexType thisType = getHolder().getType();
- DexType otherType = other.getHolder().getType();
RepresentativeMap map = null;
// If the synthetics have been moved include the original types in the equivalence.
if (graphLens.isNonIdentityLens()) {
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 741831e..60dca8f 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
@@ -265,7 +265,7 @@
ImmutableMap.builder();
List<DexProgramClass> finalSyntheticProgramDefinitions = new ArrayList<>();
{
- Map<DexType, NumberGenerator> generators = new IdentityHashMap<>();
+ Map<String, NumberGenerator> generators = new HashMap<>();
application =
buildLensAndProgram(
appView,
@@ -317,7 +317,7 @@
Map<DexType, EquivalenceGroup<D>> computeEquivalences(
AppView<?> appView,
ImmutableCollection<R> references,
- Map<DexType, NumberGenerator> generators) {
+ Map<String, NumberGenerator> generators) {
boolean intermediate = appView.options().intermediate;
Map<DexType, D> definitions = lookupDefinitions(appView, references);
ClassToFeatureSplitMap classToFeatureSplitMap =
@@ -662,11 +662,11 @@
private <T extends SyntheticDefinition<?, T, ?>>
Map<DexType, EquivalenceGroup<T>> computeActualEquivalences(
Collection<List<T>> potentialEquivalences,
- Map<DexType, NumberGenerator> generators,
+ Map<String, NumberGenerator> generators,
AppView<?> appView,
boolean intermediate,
ClassToFeatureSplitMap classToFeatureSplitMap) {
- Map<DexType, List<EquivalenceGroup<T>>> groupsPerContext = new IdentityHashMap<>();
+ Map<String, List<EquivalenceGroup<T>>> groupsPerPrefix = new HashMap<>();
potentialEquivalences.forEach(
members -> {
List<List<T>> groups =
@@ -677,17 +677,16 @@
// The representative is required to be the first element of the group.
group.remove(representative);
group.add(0, representative);
- groupsPerContext
+ groupsPerPrefix
.computeIfAbsent(
- representative.getContext().getSynthesizingContextType(),
- k -> new ArrayList<>())
+ representative.getPrefixForExternalSyntheticType(), k -> new ArrayList<>())
.add(new EquivalenceGroup<>(representative, group));
}
});
Map<DexType, EquivalenceGroup<T>> equivalences = new IdentityHashMap<>();
- groupsPerContext.forEach(
- (context, groups) -> {
+ groupsPerPrefix.forEach(
+ (externalSyntheticTypePrefix, groups) -> {
// Sort the equivalence groups that go into 'context' including the context type of the
// representative which is equal to 'context' here (see assert below).
groups.sort(
@@ -695,14 +694,18 @@
a.compareToIncludingContext(b, appView.graphLens(), classToFeatureSplitMap));
for (int i = 0; i < groups.size(); i++) {
EquivalenceGroup<T> group = groups.get(i);
- assert group.getRepresentative().getContext().getSynthesizingContextType() == context;
+ assert group
+ .getRepresentative()
+ .getPrefixForExternalSyntheticType()
+ .equals(externalSyntheticTypePrefix);
// Two equivalence groups in same context type must be distinct otherwise the assignment
// of the synthetic name will be non-deterministic between the two.
assert i == 0
|| checkGroupsAreDistinct(
groups.get(i - 1), group, appView.graphLens(), classToFeatureSplitMap);
SyntheticKind kind = group.members.get(0).getKind();
- DexType representativeType = createExternalType(kind, context, generators, appView);
+ DexType representativeType =
+ createExternalType(kind, externalSyntheticTypePrefix, generators, appView);
equivalences.put(representativeType, group);
}
});
@@ -752,7 +755,7 @@
T smallest = members.get(0);
for (int i = 1; i < members.size(); i++) {
T next = members.get(i);
- if (next.compareTo(smallest, true, graphLens, classToFeatureSplitMap) < 0) {
+ if (next.toReference().getReference().compareTo(smallest.toReference().getReference()) < 0) {
smallest = next;
}
}
@@ -761,20 +764,20 @@
private DexType createExternalType(
SyntheticKind kind,
- DexType representativeContext,
- Map<DexType, NumberGenerator> generators,
+ String externalSyntheticTypePrefix,
+ Map<String, NumberGenerator> generators,
AppView<?> appView) {
DexItemFactory factory = appView.dexItemFactory();
if (kind.isFixedSuffixSynthetic) {
- return SyntheticNaming.createExternalType(kind, representativeContext, "", factory);
+ return SyntheticNaming.createExternalType(kind, externalSyntheticTypePrefix, "", factory);
}
NumberGenerator generator =
- generators.computeIfAbsent(representativeContext, k -> new NumberGenerator());
+ generators.computeIfAbsent(externalSyntheticTypePrefix, k -> new NumberGenerator());
DexType externalType;
do {
externalType =
SyntheticNaming.createExternalType(
- kind, representativeContext, Integer.toString(generator.next()), factory);
+ kind, externalSyntheticTypePrefix, Integer.toString(generator.next()), factory);
DexClass clazz = appView.appInfo().definitionForWithoutExistenceAssert(externalType);
if (clazz != null && isNotSyntheticType(clazz.type)) {
assert options.testing.allowConflictingSyntheticTypes
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java
index 930443a..fd63ea2 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java
@@ -34,6 +34,11 @@
}
@Override
+ DexMethod getReference() {
+ return method;
+ }
+
+ @Override
SyntheticMethodDefinition lookupDefinition(Function<DexType, DexClass> definitions) {
DexClass clazz = definitions.apply(method.holder);
if (clazz == null) {
@@ -47,7 +52,8 @@
}
@Override
- SyntheticMethodReference rewrite(NonIdentityGraphLens lens) {
+ SyntheticMethodReference internalRewrite(
+ SynthesizingContext rewrittenContext, NonIdentityGraphLens lens) {
DexMethod rewritten = lens.lookupMethod(method);
// If the reference has been non-trivially rewritten the compiler has changed it and it can no
// longer be considered a synthetic. The context may or may not have changed.
@@ -58,20 +64,10 @@
assert SyntheticNaming.verifyNotInternalSynthetic(rewritten.holder);
return null;
}
- SynthesizingContext context = getContext().rewrite(lens);
- if (context == getContext() && rewritten == method) {
+ if (rewrittenContext == getContext() && rewritten == method) {
return this;
}
- // Ensure that if a synthetic moves its context moves consistently.
- if (method != rewritten) {
- context =
- SynthesizingContext.fromSyntheticContextChange(
- getKind(), rewritten.holder, context, lens.dexItemFactory());
- if (context == null) {
- return null;
- }
- }
- return new SyntheticMethodReference(getKind(), context, rewritten);
+ return new SyntheticMethodReference(getKind(), rewrittenContext, rewritten);
}
@Override
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 63772a2..25e59ae 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticNaming.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticNaming.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.synthesis;
+import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.references.ClassReference;
@@ -62,24 +63,32 @@
}
}
+ private static final String SYNTHETIC_CLASS_SEPARATOR = "-$$";
/**
* The internal synthetic class separator is only used for representing synthetic items during
* compilation. In particular, this separator must never be used to write synthetic classes to the
* final compilation result.
*/
- private static final String INTERNAL_SYNTHETIC_CLASS_SEPARATOR = "-$$InternalSynthetic";
+ private static final String INTERNAL_SYNTHETIC_CLASS_SEPARATOR =
+ SYNTHETIC_CLASS_SEPARATOR + "InternalSynthetic";
/**
* The external synthetic class separator is used when writing classes. It may appear in types
* during compilation as the output of a compilation may be the input to another.
*/
- private static final String EXTERNAL_SYNTHETIC_CLASS_SEPARATOR = "-$$ExternalSynthetic";
+ private static final String EXTERNAL_SYNTHETIC_CLASS_SEPARATOR =
+ SYNTHETIC_CLASS_SEPARATOR + "ExternalSynthetic";
/** Method prefix when generating synthetic methods in a class. */
static final String INTERNAL_SYNTHETIC_METHOD_PREFIX = "m";
- // TODO(b/158159959): Remove usage of name-based identification.
- public static boolean isSyntheticName(String typeName) {
- return typeName.contains(INTERNAL_SYNTHETIC_CLASS_SEPARATOR)
- || typeName.contains(EXTERNAL_SYNTHETIC_CLASS_SEPARATOR);
+ static String getPrefixForExternalSyntheticType(SyntheticKind kind, DexType type) {
+ String binaryName = type.toBinaryName();
+ int index =
+ binaryName.lastIndexOf(
+ kind.isFixedSuffixSynthetic ? kind.descriptor : SYNTHETIC_CLASS_SEPARATOR);
+ if (index < 0) {
+ throw new Unreachable("Unexpected failure to compute an synthetic prefix");
+ }
+ return binaryName.substring(0, index);
}
public static DexType createFixedType(
@@ -100,12 +109,12 @@
}
static DexType createExternalType(
- SyntheticKind kind, DexType context, String id, DexItemFactory factory) {
+ SyntheticKind kind, String externalSyntheticTypePrefix, String id, DexItemFactory factory) {
assert kind.isFixedSuffixSynthetic == id.isEmpty();
return createType(
kind.isFixedSuffixSynthetic ? "" : EXTERNAL_SYNTHETIC_CLASS_SEPARATOR,
kind,
- context,
+ externalSyntheticTypePrefix,
id,
factory);
}
@@ -115,10 +124,19 @@
return factory.createType(createDescriptor(separator, kind, context.getInternalName(), id));
}
+ private static DexType createType(
+ String separator,
+ SyntheticKind kind,
+ String externalSyntheticTypePrefix,
+ String id,
+ DexItemFactory factory) {
+ return factory.createType(createDescriptor(separator, kind, externalSyntheticTypePrefix, id));
+ }
+
private static String createDescriptor(
- String separator, SyntheticKind kind, String context, String id) {
+ String separator, SyntheticKind kind, String externalSyntheticTypePrefix, String id) {
return DescriptorUtils.getDescriptorFromClassBinaryName(
- context + separator + kind.descriptor + id);
+ externalSyntheticTypePrefix + separator + kind.descriptor + id);
}
public static boolean verifyNotInternalSynthetic(DexType type) {
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticProgramClassReference.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticProgramClassReference.java
index 7e798e0..5a17b04 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticProgramClassReference.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticProgramClassReference.java
@@ -36,7 +36,8 @@
}
@Override
- SyntheticProgramClassReference rewrite(NonIdentityGraphLens lens) {
+ SyntheticProgramClassReference internalRewrite(
+ SynthesizingContext rewrittenContext, NonIdentityGraphLens lens) {
DexType rewritten = lens.lookupType(type);
// If the reference has been non-trivially rewritten the compiler has changed it and it can no
// longer be considered a synthetic. The context may or may not have changed.
@@ -46,20 +47,10 @@
assert SyntheticNaming.verifyNotInternalSynthetic(rewritten);
return null;
}
- SynthesizingContext context = getContext().rewrite(lens);
- if (context == getContext() && rewritten == type) {
+ if (rewrittenContext == getContext() && rewritten == type) {
return this;
}
- // Ensure that if a synthetic moves its context moves consistently.
- if (type != rewritten) {
- context =
- SynthesizingContext.fromSyntheticContextChange(
- getKind(), rewritten, context, lens.dexItemFactory());
- if (context == null) {
- return null;
- }
- }
- return new SyntheticProgramClassReference(getKind(), context, rewritten);
+ return new SyntheticProgramClassReference(getKind(), rewrittenContext, rewritten);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticReference.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticReference.java
index 1a2d2fb..ddb28f7 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticReference.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticReference.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.synthesis;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.GraphLens.NonIdentityGraphLens;
import com.android.tools.r8.synthesis.SyntheticNaming.SyntheticKind;
@@ -20,13 +21,13 @@
C extends DexClass> {
private final SyntheticKind kind;
- private final SynthesizingContext context;
+ private final SynthesizingContext rewrittenContext;
SyntheticReference(SyntheticKind kind, SynthesizingContext context) {
assert kind != null;
assert context != null;
this.kind = kind;
- this.context = context;
+ this.rewrittenContext = context;
}
abstract D lookupDefinition(Function<DexType, DexClass> definitions);
@@ -36,10 +37,17 @@
}
final SynthesizingContext getContext() {
- return context;
+ return rewrittenContext;
}
abstract DexType getHolder();
- abstract R rewrite(NonIdentityGraphLens lens);
+ abstract DexReference getReference();
+
+ final R rewrite(NonIdentityGraphLens lens) {
+ SynthesizingContext rewrittenContext = getContext().rewrite(lens);
+ return internalRewrite(rewrittenContext, lens);
+ }
+
+ abstract R internalRewrite(SynthesizingContext rewrittenContext, NonIdentityGraphLens lens);
}
