Remove context-dependent main-dex placement of synthetics.
Bug: 178127572
Bug: 181010111
Change-Id: Ic94a69ef0b0bc6d6d78887be7b0dfd6de8b4e1e0
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfo.java b/src/main/java/com/android/tools/r8/graph/AppInfo.java
index 679062f..5d62aae 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -131,7 +131,6 @@
assert checkIfObsolete();
assert context != null;
syntheticItems.addLegacySyntheticClass(clazz, context);
- mainDexInfo.addLegacySyntheticClass(clazz, context);
}
public void addSynthesizedClass(DexProgramClass clazz, Iterable<DexProgramClass> contexts) {
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
index cd90e16..c8d9834 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
@@ -100,14 +100,6 @@
commit, getClassToFeatureSplitMap(), getMainDexInfo(), getMissingClasses());
}
- public final AppInfoWithClassHierarchy rebuildWithClassHierarchy(MissingClasses missingClasses) {
- return new AppInfoWithClassHierarchy(
- getSyntheticItems().commit(app()),
- getClassToFeatureSplitMap(),
- getMainDexInfo(),
- missingClasses);
- }
-
public AppInfoWithClassHierarchy rebuildWithClassHierarchy(
Function<DexApplication, DexApplication> fn) {
assert checkIfObsolete();
@@ -123,7 +115,10 @@
assert getClass() == AppInfoWithClassHierarchy.class;
assert checkIfObsolete();
return new AppInfoWithClassHierarchy(
- getSyntheticItems().commit(app()), classToFeatureSplitMap, mainDexInfo, missingClasses);
+ getSyntheticItems().commit(app()),
+ getClassToFeatureSplitMap(),
+ mainDexInfo,
+ getMissingClasses());
}
@Override
diff --git a/src/main/java/com/android/tools/r8/shaking/MainDexInfo.java b/src/main/java/com/android/tools/r8/shaking/MainDexInfo.java
index 4bb2981..4e579f2 100644
--- a/src/main/java/com/android/tools/r8/shaking/MainDexInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/MainDexInfo.java
@@ -18,12 +18,9 @@
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.PrunedItems;
import com.android.tools.r8.utils.ConsumerUtils;
-import com.android.tools.r8.utils.SetUtils;
import com.google.common.collect.Sets;
import java.util.Collections;
-import java.util.Map;
import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
public class MainDexInfo {
@@ -34,7 +31,6 @@
Collections.emptySet(),
Collections.emptySet(),
Collections.emptySet(),
- Collections.emptySet(),
false);
public enum MainDexGroup {
@@ -46,7 +42,6 @@
// Specific set of classes specified to be in main-dex
private final Set<DexType> classList;
- private final Map<DexType, DexType> synthesizedClassesMap;
// Traced roots are traced main dex references.
private final Set<DexType> tracedRoots;
// Traced method roots are the methods visited from an initial main dex root set. The set is
@@ -64,7 +59,6 @@
Collections.emptySet(),
Collections.emptySet(),
Collections.emptySet(),
- /* synthesized classes - cannot be emptyset */ Sets.newIdentityHashSet(),
false);
}
@@ -73,17 +67,13 @@
Set<DexType> tracedRoots,
Set<DexMethod> tracedMethodRoots,
Set<DexType> tracedDependencies,
- Set<DexType> synthesizedClasses,
boolean tracedMethodRootsCleared) {
this.classList = classList;
this.tracedRoots = tracedRoots;
this.tracedMethodRoots = tracedMethodRoots;
this.tracedDependencies = tracedDependencies;
- this.synthesizedClassesMap = new ConcurrentHashMap<>();
this.tracedMethodRootsCleared = tracedMethodRootsCleared;
- synthesizedClasses.forEach(type -> synthesizedClassesMap.put(type, type));
assert tracedDependencies.stream().noneMatch(tracedRoots::contains);
- assert tracedRoots.containsAll(synthesizedClasses);
}
public boolean isNone() {
@@ -91,15 +81,6 @@
return this == NONE;
}
- public boolean isMainDexTypeThatShouldIncludeDependencies(DexType type) {
- // Dependencies of 'type' are only needed if 'type' is a direct/executed main-dex type.
- return classList.contains(type) || tracedRoots.contains(type);
- }
-
- public boolean isMainDex(ProgramDefinition definition) {
- return isFromList(definition) || isTracedRoot(definition) || isDependency(definition);
- }
-
public boolean isFromList(ProgramDefinition definition) {
return isFromList(definition.getContextType());
}
@@ -225,35 +206,6 @@
return NONE;
}
- // TODO(b/178127572): This mutates the MainDexClasses which otherwise should be immutable.
- public void addSyntheticClass(DexProgramClass clazz) {
- // TODO(b/178127572): This will add a synthesized type as long as the initial set is not empty.
- // A better approach would be to use the context for the synthetic with a containment check.
- assert !isNone();
- if (!classList.isEmpty()) {
- synthesizedClassesMap.computeIfAbsent(
- clazz.type,
- type -> {
- classList.add(type);
- return type;
- });
- }
- if (!tracedRoots.isEmpty()) {
- synthesizedClassesMap.computeIfAbsent(
- clazz.type,
- type -> {
- tracedRoots.add(type);
- return type;
- });
- }
- }
-
- public void addLegacySyntheticClass(DexProgramClass clazz, ProgramDefinition context) {
- if (isTracedRoot(context) || isFromList(context) || isDependency(context)) {
- addSyntheticClass(clazz);
- }
- }
-
public int size() {
return classList.size() + tracedRoots.size() + tracedDependencies.size();
}
@@ -279,11 +231,7 @@
}
Set<DexType> removedClasses = prunedItems.getRemovedClasses();
Set<DexType> modifiedClassList = Sets.newIdentityHashSet();
- Set<DexType> modifiedSynthesized = Sets.newIdentityHashSet();
classList.forEach(type -> ifNotRemoved(type, removedClasses, modifiedClassList::add));
- synthesizedClassesMap
- .keySet()
- .forEach(type -> ifNotRemoved(type, removedClasses, modifiedSynthesized::add));
MainDexInfo.Builder builder = builder();
tracedRoots.forEach(type -> ifNotRemoved(type, removedClasses, builder::addRoot));
// TODO(b/169927809): Methods could be pruned without the holder being pruned, however, one has
@@ -293,7 +241,7 @@
ifNotRemoved(
method.getHolderType(), removedClasses, ignored -> builder.addRoot(method)));
tracedDependencies.forEach(type -> ifNotRemoved(type, removedClasses, builder::addDependency));
- return builder.build(modifiedClassList, modifiedSynthesized);
+ return builder.build(modifiedClassList);
}
private void ifNotRemoved(
@@ -305,10 +253,8 @@
public MainDexInfo rewrittenWithLens(GraphLens lens) {
Set<DexType> modifiedClassList = Sets.newIdentityHashSet();
- Set<DexType> modifiedSynthesized = Sets.newIdentityHashSet();
classList.forEach(
type -> rewriteAndApplyIfNotPrimitiveType(lens, type, modifiedClassList::add));
- synthesizedClassesMap.keySet().forEach(type -> modifiedSynthesized.add(lens.lookupType(type)));
MainDexInfo.Builder builder = builder();
tracedRoots.forEach(type -> rewriteAndApplyIfNotPrimitiveType(lens, type, builder::addRoot));
tracedMethodRoots.forEach(method -> builder.addRoot(lens.getRenamedMethodSignature(method)));
@@ -322,7 +268,7 @@
rewriteAndApplyIfNotPrimitiveType(lens, type, builder::addDependency);
}
});
- return builder.build(modifiedClassList, modifiedSynthesized);
+ return builder.build(modifiedClassList);
}
public Builder builder() {
@@ -414,20 +360,14 @@
return new MainDexInfo(list);
}
- public MainDexInfo build(Set<DexType> classList, Set<DexType> synthesizedClasses) {
+ public MainDexInfo build(Set<DexType> classList) {
// Class can contain dependencies which we should not regard as roots.
assert list.isEmpty();
- return new MainDexInfo(
- classList,
- SetUtils.unionIdentityHashSet(roots, synthesizedClasses),
- methodRoots,
- Sets.difference(dependencies, synthesizedClasses),
- synthesizedClasses,
- tracedMethodRootsCleared);
+ return new MainDexInfo(classList, roots, methodRoots, dependencies, tracedMethodRootsCleared);
}
public MainDexInfo build(MainDexInfo previous) {
- return build(previous.classList, previous.synthesizedClassesMap.keySet());
+ return build(previous.classList);
}
}
}
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 68bca58..3e8f334 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SynthesizingContext.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SynthesizingContext.java
@@ -13,7 +13,6 @@
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 java.util.Comparator;
/**
@@ -126,19 +125,6 @@
appView.rewritePrefix.rewriteType(hygienicType, rewrittenType);
}
- // TODO(b/181010111): Remove this once main-dex is a computed property on synthetics.
- void addIfDerivedFromMainDexClass(
- DexProgramClass externalSyntheticClass, MainDexInfo mainDexInfo) {
- if (mainDexInfo.isMainDex(externalSyntheticClass)) {
- return;
- }
- // The input context type (not the annotated context) determines if the derived class is to be
- // in main dex, as it is the input context type that is traced as part of main-dex tracing.
- if (mainDexInfo.isMainDexTypeThatShouldIncludeDependencies(inputContextType)) {
- mainDexInfo.addSyntheticClass(externalSyntheticClass);
- }
- }
-
@Override
public String toString() {
return "SynthesizingContext{" + getSynthesizingContextType() + "}";
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 b2cf60e..916daac 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
@@ -22,7 +22,6 @@
import com.android.tools.r8.graph.TreeFixerBase;
import com.android.tools.r8.ir.code.NumberGenerator;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
-import com.android.tools.r8.shaking.MainDexInfo;
import com.android.tools.r8.synthesis.SyntheticNaming.SyntheticKind;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.SetUtils;
@@ -231,20 +230,30 @@
assert !appView.appInfo().hasLiveness();
Result result = appView.getSyntheticItems().computeFinalSynthetics(appView);
appView.setAppInfo(new AppInfo(result.commit, appView.appInfo().getMainDexInfo()));
- appView.pruneItems(result.prunedItems);
if (result.lens != null) {
+ appView.setAppInfo(
+ appView
+ .appInfo()
+ .rebuildWithMainDexInfo(
+ appView.appInfo().getMainDexInfo().rewrittenWithLens(result.lens)));
appView.setGraphLens(result.lens);
}
+ appView.pruneItems(result.prunedItems);
}
public static void finalizeWithClassHierarchy(AppView<AppInfoWithClassHierarchy> appView) {
assert !appView.appInfo().hasLiveness();
Result result = appView.getSyntheticItems().computeFinalSynthetics(appView);
appView.setAppInfo(appView.appInfo().rebuildWithClassHierarchy(result.commit));
- appView.pruneItems(result.prunedItems);
if (result.lens != null) {
appView.setGraphLens(result.lens);
+ appView.setAppInfo(
+ appView
+ .appInfo()
+ .rebuildWithMainDexInfo(
+ appView.appInfo().getMainDexInfo().rewrittenWithLens(result.lens)));
}
+ appView.pruneItems(result.prunedItems);
}
public static void finalizeWithLiveness(AppView<AppInfoWithLiveness> appView) {
@@ -452,10 +461,6 @@
representative.getKind(),
representative.getContext(),
externalSyntheticClass.type));
- for (SyntheticProgramClassDefinition member : syntheticGroup.getMembers()) {
- addMainDexAndSynthesizedFromForMember(
- member, externalSyntheticClass, appView.appInfo().getMainDexInfo());
- }
});
syntheticMethodGroups.forEach(
(syntheticType, syntheticGroup) -> {
@@ -470,10 +475,6 @@
.getMethod()
.getReference()
.withHolder(externalSyntheticClass.type, factory)));
- for (SyntheticMethodDefinition member : syntheticGroup.getMembers()) {
- addMainDexAndSynthesizedFromForMember(
- member, externalSyntheticClass, appView.appInfo().getMainDexInfo());
- }
});
for (DexType key : syntheticMethodGroups.keySet()) {
@@ -517,13 +518,6 @@
return builder.build();
}
- private static void addMainDexAndSynthesizedFromForMember(
- SyntheticDefinition<?, ?, ?> member,
- DexProgramClass externalSyntheticClass,
- MainDexInfo mainDexInfo) {
- member.getContext().addIfDerivedFromMainDexClass(externalSyntheticClass, mainDexInfo);
- }
-
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
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexListOutputTest.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexListOutputTest.java
index 8f62a1c..2dd90c1 100644
--- a/src/test/java/com/android/tools/r8/maindexlist/MainDexListOutputTest.java
+++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexListOutputTest.java
@@ -120,7 +120,7 @@
testForD8()
.setMinApi(AndroidApiLevel.K)
.addProgramClasses(ImmutableList.of(TestClass.class, MyConsumer.class))
- .addMainDexListClasses(TestClass.class)
+ .addMainDexKeepClassAndMemberRules(TestClass.class)
.setMainDexListConsumer(consumer)
.compile();
assertTrue(consumer.called);
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexWithSynthesizedClassesTest.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexWithSynthesizedClassesTest.java
index 5e88d7d..6c2ad2e 100644
--- a/src/test/java/com/android/tools/r8/maindexlist/MainDexWithSynthesizedClassesTest.java
+++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexWithSynthesizedClassesTest.java
@@ -26,7 +26,7 @@
static final AndroidApiLevel nativeMultiDexLevel = AndroidApiLevel.L;
- static final String EXPECTED = StringUtils.lines("AB");
+ static final String EXPECTED = StringUtils.lines("A");
private final TestParameters parameters;
@@ -53,7 +53,7 @@
D8TestCompileResult compileResult =
testForD8()
.addInnerClasses(MainDexWithSynthesizedClassesTest.class)
- .addMainDexListClasses(TestClass.class, A.class)
+ .addMainDexKeepClassAndMemberRules(TestClass.class)
.setMinApiThreshold(parameters.getApiLevel())
.compile();
checkCompilationResult(compileResult);
@@ -119,7 +119,7 @@
static class A {
Getter foo() {
- return () -> "A" + new B().foo().get();
+ return () -> "A";
}
}
