Add access modification bit to keep info.
Bug: 157012327
Change-Id: I8030aafc43d58e2287c29d09a06570cf2fdcce2d
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
index 3c55b1a..9c5e2e4 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
@@ -381,8 +381,8 @@
for (DexType type : enumsToUnbox) {
DexProgramClass clazz = asProgramClassOrNull(appView.definitionFor(type));
assert !keepInfo.getClassInfo(clazz).isPinned();
- clazz.forEachProgramMethod(keepInfo::unpinMethod);
- clazz.forEachField(field -> keepInfo.unpinField(clazz, field));
+ clazz.forEachProgramMethod(keepInfo::unsafeUnpinMethod);
+ clazz.forEachField(field -> keepInfo.unsafeUnpinField(clazz, field));
}
});
}
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index 4d3b6aa..df5a09c 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -133,12 +133,6 @@
public final Set<DexCallSite> callSites;
/** Collection of keep requirements for the program. */
private final KeepInfoCollection keepInfo;
- /**
- * Set of kept items that are allowed to be publicized.
- *
- * <p>TODO(b/156715504): merge into keep info.
- */
- private final Set<DexReference> allowAccessModification;
/** All items with assumemayhavesideeffects rule. */
public final Map<DexReference, ProguardMemberRule> mayHaveSideEffects;
/** All items with assumenosideeffects rule. */
@@ -217,7 +211,6 @@
SortedMap<DexMethod, ProgramMethodSet> staticInvokes,
Set<DexCallSite> callSites,
KeepInfoCollection keepInfo,
- Set<DexReference> allowAccessModification,
Map<DexReference, ProguardMemberRule> mayHaveSideEffects,
Map<DexReference, ProguardMemberRule> noSideEffects,
Map<DexReference, ProguardMemberRule> assumedValues,
@@ -253,7 +246,6 @@
this.fieldAccessInfoCollection = fieldAccessInfoCollection;
this.objectAllocationInfoCollection = objectAllocationInfoCollection;
this.keepInfo = keepInfo;
- this.allowAccessModification = allowAccessModification;
this.mayHaveSideEffects = mayHaveSideEffects;
this.noSideEffects = noSideEffects;
this.assumedValues = assumedValues;
@@ -304,7 +296,6 @@
SortedMap<DexMethod, ProgramMethodSet> staticInvokes,
Set<DexCallSite> callSites,
KeepInfoCollection keepInfo,
- Set<DexReference> allowAccessModification,
Map<DexReference, ProguardMemberRule> mayHaveSideEffects,
Map<DexReference, ProguardMemberRule> noSideEffects,
Map<DexReference, ProguardMemberRule> assumedValues,
@@ -340,7 +331,6 @@
this.fieldAccessInfoCollection = fieldAccessInfoCollection;
this.objectAllocationInfoCollection = objectAllocationInfoCollection;
this.keepInfo = keepInfo;
- this.allowAccessModification = allowAccessModification;
this.mayHaveSideEffects = mayHaveSideEffects;
this.noSideEffects = noSideEffects;
this.assumedValues = assumedValues;
@@ -392,7 +382,6 @@
previous.staticInvokes,
previous.callSites,
previous.keepInfo,
- previous.allowAccessModification,
previous.mayHaveSideEffects,
previous.noSideEffects,
previous.assumedValues,
@@ -443,7 +432,6 @@
previous.staticInvokes,
previous.callSites,
extendPinnedItems(previous, additionalPinnedItems),
- previous.allowAccessModification,
previous.mayHaveSideEffects,
previous.noSideEffects,
previous.assumedValues,
@@ -526,7 +514,6 @@
this.fieldAccessInfoCollection = previous.fieldAccessInfoCollection;
this.objectAllocationInfoCollection = previous.objectAllocationInfoCollection;
this.keepInfo = previous.keepInfo;
- this.allowAccessModification = previous.allowAccessModification;
this.mayHaveSideEffects = previous.mayHaveSideEffects;
this.noSideEffects = previous.noSideEffects;
this.assumedValues = previous.assumedValues;
@@ -930,7 +917,9 @@
public boolean isAccessModificationAllowed(DexReference reference) {
assert options().getProguardConfiguration().isAccessModificationAllowed();
- return allowAccessModification.contains(reference) || !isPinned(reference);
+ return keepInfo
+ .getInfo(reference, this)
+ .isAccessModificationAllowed(options().getProguardConfiguration());
}
public boolean isPinned(DexReference reference) {
@@ -1025,7 +1014,6 @@
// after second tree shaking.
callSites,
keepInfo.rewrite(lens),
- lens.rewriteReferences(allowAccessModification),
lens.rewriteReferenceKeys(mayHaveSideEffects),
lens.rewriteReferenceKeys(noSideEffects),
lens.rewriteReferenceKeys(assumedValues),
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 01032ab..9938f36 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -310,13 +310,6 @@
private final MutableKeepInfoCollection keepInfo = new MutableKeepInfoCollection();
/**
- * A set of references that we are keeping due to keep rules, which we are allowed to publicize.
- */
- // TODO(b/156715504): This should be maintained in a structure that describes what we are allowed
- // and not allowed to do with program items that are referenced from keep rules.
- private final Map<DexReference, OptionalBool> allowAccessModification = new IdentityHashMap<>();
-
- /**
* A set of seen const-class references that both serve as an initial lock-candidate set and will
* prevent statically merging the classes referenced.
*/
@@ -643,7 +636,7 @@
if (item.isDexClass()) {
DexProgramClass clazz = item.asDexClass().asProgramClass();
KeepReasonWitness witness = graphReporter.reportKeepClass(precondition, rules, clazz);
- keepInfo.pinClass(clazz);
+ keepClassWithRules(clazz, rules);
if (clazz.isAnnotation()) {
workList.enqueueMarkAnnotationInstantiatedAction(clazz, witness);
} else if (clazz.isInterface()) {
@@ -666,7 +659,7 @@
DexEncodedField field = item.asDexEncodedField();
DexProgramClass holder = getProgramClassOrNull(field.holder());
if (holder != null) {
- keepInfo.pinField(holder, field);
+ keepFieldWithRules(holder, field, rules);
workList.enqueueMarkFieldKeptAction(
new ProgramField(holder, field),
graphReporter.reportKeepField(precondition, rules, field));
@@ -675,7 +668,7 @@
DexEncodedMethod encodedMethod = item.asDexEncodedMethod();
DexProgramClass holder = getProgramClassOrNull(encodedMethod.holder());
if (holder != null) {
- keepInfo.pinMethod(holder, encodedMethod);
+ keepMethodWithRules(holder, encodedMethod, rules);
workList.enqueueMarkMethodKeptAction(
new ProgramMethod(holder, encodedMethod),
graphReporter.reportKeepMethod(precondition, rules, encodedMethod));
@@ -683,7 +676,6 @@
} else {
throw new IllegalArgumentException(item.toString());
}
- setAllowAccessModification(item.toReference(), rules);
}
private void enqueueFirstNonSerializableClassInitializer(
@@ -1814,13 +1806,11 @@
(dexType, ignored) -> {
if (holder.isProgramClass()) {
DexProgramClass holderClass = holder.asProgramClass();
- keepInfo.pinClass(holderClass);
- setAllowAccessModification(holderClass.type);
+ keepInfo.keepClass(holderClass);
rootSet.shouldNotBeMinified(holder.toReference());
for (DexEncodedMember<?, ?> member : holder.members()) {
- keepInfo.pinMember(holderClass, member);
+ keepInfo.keepMember(holderClass, member);
DexMember<?, ?> memberReference = member.toReference();
- setAllowAccessModification(memberReference);
rootSet.shouldNotBeMinified(memberReference);
}
}
@@ -2535,8 +2525,7 @@
// TODO(sgjesse): Does this have to be enqueued as a root item? Right now it is done as the
// marking for not renaming it is in the root set.
workList.enqueueMarkMethodKeptAction(new ProgramMethod(clazz, valuesMethod), reason);
- keepInfo.pinMethod(clazz, valuesMethod);
- setAllowAccessModification(valuesMethod.toReference());
+ keepInfo.keepMethod(clazz, valuesMethod);
rootSet.shouldNotBeMinified(valuesMethod.toReference());
}
}
@@ -2640,27 +2629,44 @@
return appInfoWithLiveness;
}
- private void setAllowAccessModification(DexReference reference) {
- allowAccessModification.put(reference, OptionalBool.unknown());
+ private void keepClassWithRules(DexProgramClass clazz, Set<ProguardKeepRuleBase> rules) {
+ keepInfo.joinClass(
+ clazz,
+ info ->
+ info.pin()
+ .lazyDisallowAccessModification(() -> computeDisallowAccessModification(rules)));
}
- private void setAllowAccessModification(DexReference reference, Set<ProguardKeepRuleBase> rules) {
- assert rules != null;
- assert !rules.isEmpty();
- OptionalBool allowAccessModificationOfReference =
- allowAccessModification.getOrDefault(reference, OptionalBool.TRUE);
- if (allowAccessModificationOfReference.isTrue()) {
- for (ProguardKeepRuleBase rule : rules) {
- ProguardKeepRuleModifiers modifiers =
- (rule.isProguardIfRule() ? rule.asProguardIfRule().getSubsequentRule() : rule)
- .getModifiers();
- if (!modifiers.allowsAccessModification) {
- allowAccessModificationOfReference = OptionalBool.FALSE;
- break;
- }
+ private void keepMethodWithRules(
+ DexProgramClass holder, DexEncodedMethod method, Set<ProguardKeepRuleBase> rules) {
+ keepInfo.joinMethod(
+ holder,
+ method,
+ info ->
+ info.pin()
+ .lazyDisallowAccessModification(() -> computeDisallowAccessModification(rules)));
+ }
+
+ private void keepFieldWithRules(
+ DexProgramClass holder, DexEncodedField field, Set<ProguardKeepRuleBase> rules) {
+ keepInfo.joinField(
+ holder,
+ field,
+ info ->
+ info.pin()
+ .lazyDisallowAccessModification(() -> computeDisallowAccessModification(rules)));
+ }
+
+ private boolean computeDisallowAccessModification(Set<ProguardKeepRuleBase> rules) {
+ for (ProguardKeepRuleBase rule : rules) {
+ ProguardKeepRuleModifiers modifiers =
+ (rule.isProguardIfRule() ? rule.asProguardIfRule().getSubsequentRule() : rule)
+ .getModifiers();
+ if (!modifiers.allowsAccessModification) {
+ return true;
}
- allowAccessModification.put(reference, allowAccessModificationOfReference);
}
+ return false;
}
private static class SyntheticAdditions {
@@ -2672,15 +2678,16 @@
Map<DexType, DexClasspathClass> syntheticClasspathClasses = new IdentityHashMap<>();
- // Subset of live methods that need to be pinned.
- Set<ProgramMethod> pinnedMethods = Sets.newIdentityHashSet();
+ // Subset of live methods that need have keep requirements.
+ List<Pair<ProgramMethod, Consumer<KeepMethodInfo.Joiner>>> liveMethodsWithKeepActions =
+ new ArrayList<>();
// Subset of synthesized classes that need to be added to the main-dex file.
Set<DexType> mainDexTypes = Sets.newIdentityHashSet();
boolean isEmpty() {
boolean empty = syntheticInstantiations.isEmpty() && liveMethods.isEmpty();
- assert !empty || (pinnedMethods.isEmpty() && mainDexTypes.isEmpty());
+ assert !empty || (liveMethodsWithKeepActions.isEmpty() && mainDexTypes.isEmpty());
return empty;
}
@@ -2704,9 +2711,10 @@
liveMethods.put(signature, method);
}
- void addLiveAndPinnedMethod(ProgramMethod method) {
+ void addLiveMethodWithKeepAction(
+ ProgramMethod method, Consumer<KeepMethodInfo.Joiner> keepAction) {
addLiveMethod(method);
- pinnedMethods.add(method);
+ liveMethodsWithKeepActions.add(new Pair<>(method, keepAction));
}
void amendApplication(Builder appBuilder) {
@@ -2725,11 +2733,8 @@
// All synthetic additions are initial tree shaking only. No need to track keep reasons.
KeepReasonWitness fakeReason = enqueuer.graphReporter.fakeReportShouldNotBeUsed();
- pinnedMethods.forEach(
- method -> {
- enqueuer.keepInfo.pinMethod(method);
- enqueuer.setAllowAccessModification(method.getReference());
- });
+ liveMethodsWithKeepActions.forEach(
+ item -> enqueuer.keepInfo.joinMethod(item.getFirst(), item.getSecond()));
for (Pair<DexProgramClass, ProgramMethod> clazzAndContext :
syntheticInstantiations.values()) {
enqueuer.workList.enqueueMarkInstantiatedAction(
@@ -2779,7 +2784,7 @@
DexProgramClass holder = bridge.getHolder();
DexEncodedMethod method = bridge.getDefinition();
holder.addVirtualMethod(method);
- additions.addLiveAndPinnedMethod(bridge);
+ additions.addLiveMethodWithKeepAction(bridge, KeepMethodInfo.Joiner::pin);
}
syntheticInterfaceMethodBridges.clear();
}
@@ -2844,9 +2849,6 @@
// to a static method will invalidate the reachable method sets for tracing methods.
ensureLambdaAccessibility();
- // Remove the items from `allowAccessModification` that we are not allowed to publicize.
- allowAccessModification.entrySet().removeIf(entry -> !entry.getValue().isTrue());
-
// Compute the set of dead proto types.
deadProtoTypeCandidates.removeIf(this::isTypeLive);
Set<DexType> deadProtoTypes =
@@ -2921,7 +2923,6 @@
toImmutableSortedMap(staticInvokes, PresortedComparable::slowCompare),
callSites,
keepInfo,
- allowAccessModification.keySet(),
rootSet.mayHaveSideEffects,
rootSet.noSideEffects,
rootSet.assumedValues,
@@ -3329,7 +3330,7 @@
assert desugaredLambdaImplementationMethods.isEmpty()
|| options.desugarState == DesugarState.ON;
for (DexMethod method : desugaredLambdaImplementationMethods) {
- keepInfo.unpinMethod(method);
+ keepInfo.unsafeUnpinMethod(method);
rootSet.prune(method);
}
desugaredLambdaImplementationMethods.clear();
@@ -3618,7 +3619,6 @@
}
if (!keepInfo.getFieldInfo(encodedField, clazz).isPinned()) {
keepInfo.pinField(clazz, encodedField);
- setAllowAccessModification(encodedField.field);
markFieldAsKept(new ProgramField(clazz, encodedField), KeepReason.reflectiveUseIn(method));
}
} else {
@@ -3806,7 +3806,6 @@
// interface into its unique subtype, if any.
// TODO(b/145344105): This should be superseded by the unknown interface hierarchy.
keepInfo.pinClass(clazz);
- setAllowAccessModification(clazz.type);
KeepReason reason = KeepReason.reflectiveUseIn(method);
markInterfaceAsInstantiated(clazz, graphReporter.registerClass(clazz, reason));
@@ -3814,7 +3813,6 @@
// illegal rewritings of invoke-interface instructions into invoke-virtual instructions.
for (DexEncodedMethod virtualMethod : clazz.virtualMethods()) {
keepInfo.pinMethod(clazz, virtualMethod);
- setAllowAccessModification(virtualMethod.method);
markVirtualMethodAsReachable(virtualMethod.method, true, null, reason);
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java
index a8cf2df..58367a9 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java
@@ -4,13 +4,13 @@
package com.android.tools.r8.shaking;
/** Immutable keep requirements for a class. */
-public final class KeepClassInfo extends KeepInfo {
+public final class KeepClassInfo extends KeepInfo<KeepClassInfo.Builder, KeepClassInfo> {
// Requires all aspects of a class to be kept.
- private static final KeepClassInfo TOP = new KeepClassInfo(true);
+ private static final KeepClassInfo TOP = new Builder().makeTop().build();
// Requires no aspects of a class to be kept.
- private static final KeepClassInfo BOTTOM = new KeepClassInfo(false);
+ private static final KeepClassInfo BOTTOM = new Builder().makeBottom().build();
public static KeepClassInfo top() {
return TOP;
@@ -20,27 +20,46 @@
return BOTTOM;
}
- private KeepClassInfo(boolean pinned) {
- super(pinned);
+ private KeepClassInfo(Builder builder) {
+ super(builder);
}
- public Builder builder() {
+ private Builder builder() {
return new Builder(this);
}
+ public Joiner joiner() {
+ assert !isTop();
+ return new Joiner(this);
+ }
+
+ @Override
+ public boolean isTop() {
+ return this.equals(top());
+ }
+
+ @Override
+ public boolean isBottom() {
+ return this.equals(bottom());
+ }
+
public static class Builder extends KeepInfo.Builder<Builder, KeepClassInfo> {
+ private Builder() {
+ super();
+ }
+
private Builder(KeepClassInfo original) {
super(original);
}
@Override
- public KeepClassInfo top() {
+ public KeepClassInfo getTopInfo() {
return TOP;
}
@Override
- public KeepClassInfo bottom() {
+ public KeepClassInfo getBottomInfo() {
return BOTTOM;
}
@@ -56,7 +75,19 @@
@Override
public KeepClassInfo doBuild() {
- return new KeepClassInfo(isPinned());
+ return new KeepClassInfo(this);
+ }
+ }
+
+ public static class Joiner extends KeepInfo.Joiner<Joiner, Builder, KeepClassInfo> {
+
+ public Joiner(KeepClassInfo info) {
+ super(info.builder());
+ }
+
+ @Override
+ Joiner self() {
+ return this;
}
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepFieldInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepFieldInfo.java
index 7416d8f..6f22db4 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepFieldInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepFieldInfo.java
@@ -4,13 +4,13 @@
package com.android.tools.r8.shaking;
/** Immutable keep requirements for a field. */
-public final class KeepFieldInfo extends KeepInfo {
+public final class KeepFieldInfo extends KeepInfo<KeepFieldInfo.Builder, KeepFieldInfo> {
// Requires all aspects of a field to be kept.
- private static final KeepFieldInfo TOP = new KeepFieldInfo(true);
+ private static final KeepFieldInfo TOP = new Builder().makeTop().build();
// Requires no aspects of a field to be kept.
- private static final KeepFieldInfo BOTTOM = new KeepFieldInfo(false);
+ private static final KeepFieldInfo BOTTOM = new Builder().makeBottom().build();
public static KeepFieldInfo top() {
return TOP;
@@ -20,27 +20,48 @@
return BOTTOM;
}
- private KeepFieldInfo(boolean pinned) {
- super(pinned);
+ private KeepFieldInfo(Builder builder) {
+ super(builder);
}
- public Builder builder() {
+ // This builder is not private as there are known instances where it is safe to modify keep info
+ // in a non-upwards direction.
+ Builder builder() {
return new Builder(this);
}
+ public Joiner joiner() {
+ assert !isTop();
+ return new Joiner(this);
+ }
+
+ @Override
+ public boolean isTop() {
+ return this.equals(top());
+ }
+
+ @Override
+ public boolean isBottom() {
+ return this.equals(bottom());
+ }
+
public static class Builder extends KeepInfo.Builder<Builder, KeepFieldInfo> {
+ private Builder() {
+ super();
+ }
+
private Builder(KeepFieldInfo original) {
super(original);
}
@Override
- public KeepFieldInfo top() {
+ public KeepFieldInfo getTopInfo() {
return TOP;
}
@Override
- public KeepFieldInfo bottom() {
+ public KeepFieldInfo getBottomInfo() {
return BOTTOM;
}
@@ -56,7 +77,19 @@
@Override
public KeepFieldInfo doBuild() {
- return new KeepFieldInfo(isPinned());
+ return new KeepFieldInfo(this);
+ }
+ }
+
+ public static class Joiner extends KeepInfo.Joiner<Joiner, Builder, KeepFieldInfo> {
+
+ public Joiner(KeepFieldInfo info) {
+ super(info.builder());
+ }
+
+ @Override
+ Joiner self() {
+ return this;
}
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
index 246eeb7..761547a 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
@@ -3,60 +3,125 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.shaking;
+import com.android.tools.r8.shaking.KeepInfo.Builder;
+import java.util.function.Supplier;
+
/** Keep information that can be associated with any item, i.e., class, method or field. */
-public abstract class KeepInfo {
+public abstract class KeepInfo<B extends Builder, K extends KeepInfo> {
private final boolean pinned;
+ private final boolean allowAccessModification;
- public KeepInfo(boolean pinned) {
+ private KeepInfo(boolean pinned, boolean allowAccessModification) {
this.pinned = pinned;
+ this.allowAccessModification = allowAccessModification;
}
+ KeepInfo(B builder) {
+ this(builder.isPinned(), builder.isAccessModificationAllowed());
+ }
+
+ /** True if an item must be present in the output. */
public boolean isPinned() {
return pinned;
}
+ /**
+ * True if an item may have its access flags modified.
+ *
+ * <p>This method requires knowledge of the global access modification as that will override the
+ * concrete value on a given item.
+ *
+ * @param configuration Global configuration object to determine access modification.
+ */
+ public boolean isAccessModificationAllowed(ProguardConfiguration configuration) {
+ return configuration.isAccessModificationAllowed() && internalIsAccessModificationAllowed();
+ }
+
+ // Internal accessor for the items access-modification bit.
+ boolean internalIsAccessModificationAllowed() {
+ return allowAccessModification;
+ }
+
+ public abstract boolean isTop();
+
+ public abstract boolean isBottom();
+
+ public boolean isLessThanOrEquals(K other) {
+ // An item is less, aka, lower in the lattice, if each of its attributes is at least as
+ // permissive of that on other.
+ return (!pinned || other.isPinned())
+ && (allowAccessModification || !other.internalIsAccessModificationAllowed());
+ }
+
+ /** Builder to construct an arbitrary keep info object. */
public abstract static class Builder<B extends Builder, K extends KeepInfo> {
- public abstract B self();
+ abstract B self();
- public abstract K doBuild();
+ abstract K doBuild();
- public abstract K top();
+ abstract K getTopInfo();
- public abstract K bottom();
+ abstract K getBottomInfo();
- public abstract boolean isEqualTo(K other);
+ abstract boolean isEqualTo(K other);
private K original;
private boolean pinned;
+ private boolean allowAccessModification;
- public Builder(K original) {
+ Builder() {
+ // Default initialized. Use should be followed by makeTop/makeBottom.
+ }
+
+ Builder(K original) {
this.original = original;
pinned = original.isPinned();
+ allowAccessModification = original.internalIsAccessModificationAllowed();
+ }
+
+ B makeTop() {
+ pin();
+ disallowAccessModification();
+ return self();
+ }
+
+ B makeBottom() {
+ unpin();
+ allowAccessModification();
+ return self();
}
public K build() {
- if (internalIsEqualTo(original)) {
- return original;
- }
- if (internalIsEqualTo(top())) {
- return top();
- }
- if (internalIsEqualTo(bottom())) {
- return bottom();
+ if (original != null) {
+ if (internalIsEqualTo(original)) {
+ return original;
+ }
+ if (internalIsEqualTo(getTopInfo())) {
+ return getTopInfo();
+ }
+ if (internalIsEqualTo(getBottomInfo())) {
+ return getBottomInfo();
+ }
}
return doBuild();
}
private boolean internalIsEqualTo(K other) {
- return isPinned() == other.isPinned() && isEqualTo(other);
+ return isPinned() == other.isPinned()
+ && isAccessModificationAllowed() == other.internalIsAccessModificationAllowed()
+ && isEqualTo(other);
}
public boolean isPinned() {
return pinned;
}
+ public boolean isAccessModificationAllowed() {
+ return allowAccessModification;
+ }
+
public B setPinned(boolean pinned) {
this.pinned = pinned;
return self();
@@ -69,5 +134,61 @@
public B unpin() {
return setPinned(false);
}
+
+ public B setAllowAccessModification(boolean allowAccessModification) {
+ this.allowAccessModification = allowAccessModification;
+ return self();
+ }
+
+ public B allowAccessModification() {
+ return setAllowAccessModification(true);
+ }
+
+ public B disallowAccessModification() {
+ return setAllowAccessModification(false);
+ }
+ }
+
+ /** Joiner to construct monotonically increasing keep info object. */
+ public abstract static class Joiner<
+ J extends Joiner, B extends Builder, K extends KeepInfo<B, K>> {
+
+ abstract J self();
+
+ private final Builder<B, K> builder;
+
+ Joiner(Builder<B, K> builder) {
+ this.builder = builder;
+ }
+
+ public boolean isTop() {
+ return builder.isEqualTo(builder.getTopInfo());
+ }
+
+ public J top() {
+ builder.makeTop();
+ return self();
+ }
+
+ public J pin() {
+ builder.pin();
+ return self();
+ }
+
+ // Lazy modification of access modification.
+ // Only forced if access modification is still allowed.
+ public J lazyDisallowAccessModification(Supplier<Boolean> lazyShouldDisallow) {
+ if (builder.isAccessModificationAllowed() && lazyShouldDisallow.get()) {
+ builder.disallowAccessModification();
+ }
+ return self();
+ }
+
+ public K join() {
+ K joined = builder.build();
+ K original = builder.original;
+ assert original.isLessThanOrEquals(joined);
+ return joined;
+ }
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java b/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java
index 57c1f32..97a7fb2 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.graph.GraphLense.NestedGraphLense;
import com.android.tools.r8.graph.ProgramField;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.shaking.KeepFieldInfo.Joiner;
import java.util.Collection;
import java.util.IdentityHashMap;
import java.util.Map;
@@ -216,59 +217,110 @@
return keepFieldInfo.getOrDefault(field.field, KeepFieldInfo.bottom());
}
- public void pinClass(DexProgramClass clazz) {
+ public void joinClass(DexProgramClass clazz, Consumer<KeepClassInfo.Joiner> fn) {
KeepClassInfo info = getClassInfo(clazz);
- if (!info.isPinned()) {
- keepClassInfo.put(clazz.type, info.builder().pin().build());
+ if (info.isTop()) {
+ return;
+ }
+ KeepClassInfo.Joiner joiner = info.joiner();
+ fn.accept(joiner);
+ KeepClassInfo joined = joiner.join();
+ if (!info.equals(joined)) {
+ keepClassInfo.put(clazz.type, joined);
}
}
- public void pinMember(DexProgramClass holder, DexEncodedMember<?, ?> member) {
- if (member.isDexEncodedMethod()) {
- pinMethod(holder, member.asDexEncodedMethod());
- } else {
- assert member.isDexEncodedField();
- pinField(holder, member.asDexEncodedField());
+ public void keepClass(DexProgramClass clazz) {
+ joinClass(clazz, KeepInfo.Joiner::top);
+ }
+
+ public void pinClass(DexProgramClass clazz) {
+ joinClass(clazz, KeepInfo.Joiner::pin);
+ }
+
+ public void joinMethod(
+ DexProgramClass holder, DexEncodedMethod method, Consumer<KeepMethodInfo.Joiner> fn) {
+ KeepMethodInfo info = getMethodInfo(method, holder);
+ if (info == KeepMethodInfo.top()) {
+ return;
+ }
+ KeepMethodInfo.Joiner joiner = info.joiner();
+ fn.accept(joiner);
+ KeepMethodInfo joined = joiner.join();
+ if (!info.equals(joined)) {
+ keepMethodInfo.put(method.method, joined);
}
}
- public void pinMethod(ProgramMethod programMethod) {
- pinMethod(programMethod.getHolder(), programMethod.getDefinition());
+ public void joinMethod(ProgramMethod programMethod, Consumer<KeepMethodInfo.Joiner> fn) {
+ joinMethod(programMethod.getHolder(), programMethod.getDefinition(), fn);
+ }
+
+ public void keepMethod(ProgramMethod programMethod) {
+ keepMethod(programMethod.getHolder(), programMethod.getDefinition());
+ }
+
+ public void keepMethod(DexProgramClass holder, DexEncodedMethod method) {
+ joinMethod(holder, method, KeepInfo.Joiner::top);
}
public void pinMethod(DexProgramClass holder, DexEncodedMethod method) {
- KeepMethodInfo info = getMethodInfo(method, holder);
- if (!info.isPinned()) {
- keepMethodInfo.put(method.method, info.builder().pin().build());
- }
+ joinMethod(holder, method, KeepInfo.Joiner::pin);
}
- public void unpinMethod(ProgramMethod method) {
+ // Unpinning a method represents a non-monotonic change to the keep info of that item.
+ // This is generally unsound as it requires additional analysis to determine that a method that
+ // was pinned no longer is. A known sound example is the enum analysis that will identify
+ // non-escaping enums on enum types that are not pinned, thus their methods do not need to be
+ // retained even if a rule has marked them as conditionally pinned.
+ public void unsafeUnpinMethod(ProgramMethod method) {
+ // This asserts that the holder is not pinned as some analysis must have established that the
+ // type is not "present" and thus the method need not be pinned.
assert !getClassInfo(method.getHolder()).isPinned();
- unpinMethod(method.getReference());
+ unsafeUnpinMethod(method.getReference());
}
- // TODO(b/156715504): We should never need to unpin items. Rather avoid pinning to begin with.
+ // TODO(b/156715504): Avoid pinning/unpinning references.
@Deprecated
- public void unpinMethod(DexMethod method) {
+ public void unsafeUnpinMethod(DexMethod method) {
KeepMethodInfo info = keepMethodInfo.get(method);
if (info != null && info.isPinned()) {
keepMethodInfo.put(method, info.builder().unpin().build());
}
}
- public void pinField(ProgramField programField) {
- pinField(programField.getHolder(), programField.getDefinition());
- }
-
- public void pinField(DexProgramClass holder, DexEncodedField field) {
+ public void joinField(
+ DexProgramClass holder, DexEncodedField field, Consumer<KeepFieldInfo.Joiner> fn) {
KeepFieldInfo info = getFieldInfo(field, holder);
- if (!info.isPinned()) {
- keepFieldInfo.put(field.field, info.builder().pin().build());
+ if (info.isTop()) {
+ return;
+ }
+ Joiner joiner = info.joiner();
+ fn.accept(joiner);
+ KeepFieldInfo joined = joiner.join();
+ if (!info.equals(joined)) {
+ keepFieldInfo.put(field.field, joined);
}
}
- public void unpinField(DexProgramClass holder, DexEncodedField field) {
+ public void keepField(DexProgramClass holder, DexEncodedField field) {
+ joinField(holder, field, KeepInfo.Joiner::top);
+ }
+
+ public void pinField(DexProgramClass holder, DexEncodedField field) {
+ joinField(holder, field, KeepInfo.Joiner::pin);
+ }
+
+ public void keepMember(DexProgramClass holder, DexEncodedMember<?, ?> member) {
+ if (member.isDexEncodedMethod()) {
+ keepMethod(holder, member.asDexEncodedMethod());
+ } else {
+ assert member.isDexEncodedField();
+ keepField(holder, member.asDexEncodedField());
+ }
+ }
+
+ public void unsafeUnpinField(DexProgramClass holder, DexEncodedField field) {
assert holder.type == field.holder();
assert !getClassInfo(holder).isPinned();
KeepFieldInfo info = this.keepFieldInfo.get(field.toReference());
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java
index f3bb073..98eae1b 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java
@@ -4,13 +4,13 @@
package com.android.tools.r8.shaking;
/** Immutable keep requirements for a method. */
-public final class KeepMethodInfo extends KeepInfo {
+public final class KeepMethodInfo extends KeepInfo<KeepMethodInfo.Builder, KeepMethodInfo> {
// Requires all aspects of a method to be kept.
- private static final KeepMethodInfo TOP = new KeepMethodInfo(true);
+ private static final KeepMethodInfo TOP = new Builder().makeTop().build();
// Requires no aspects of a method to be kept.
- private static final KeepMethodInfo BOTTOM = new KeepMethodInfo(false);
+ private static final KeepMethodInfo BOTTOM = new Builder().makeBottom().build();
public static KeepMethodInfo top() {
return TOP;
@@ -20,16 +20,37 @@
return BOTTOM;
}
- private KeepMethodInfo(boolean pinned) {
- super(pinned);
+ private KeepMethodInfo(Builder builder) {
+ super(builder);
}
- public Builder builder() {
+ // This builder is not private as there are known instances where it is safe to modify keep info
+ // in a non-upwards direction.
+ Builder builder() {
return new Builder(this);
}
+ public Joiner joiner() {
+ assert !isTop();
+ return new Joiner(this);
+ }
+
+ @Override
+ public boolean isTop() {
+ return this.equals(top());
+ }
+
+ @Override
+ public boolean isBottom() {
+ return this.equals(bottom());
+ }
+
public static class Builder extends KeepInfo.Builder<Builder, KeepMethodInfo> {
+ private Builder() {
+ super();
+ }
+
private Builder(KeepMethodInfo original) {
super(original);
}
@@ -40,12 +61,12 @@
}
@Override
- public KeepMethodInfo top() {
+ public KeepMethodInfo getTopInfo() {
return TOP;
}
@Override
- public KeepMethodInfo bottom() {
+ public KeepMethodInfo getBottomInfo() {
return BOTTOM;
}
@@ -56,7 +77,19 @@
@Override
public KeepMethodInfo doBuild() {
- return new KeepMethodInfo(isPinned());
+ return new KeepMethodInfo(this);
+ }
+ }
+
+ public static class Joiner extends KeepInfo.Joiner<Joiner, Builder, KeepMethodInfo> {
+
+ public Joiner(KeepMethodInfo info) {
+ super(info.builder());
+ }
+
+ @Override
+ Joiner self() {
+ return this;
}
}
}
