Add minification bit to keep info.
Bug: 157012327
Change-Id: I7d31564673fbaeb558f23aaabfacfb1096ab2ff8
diff --git a/src/main/java/com/android/tools/r8/graph/AppView.java b/src/main/java/com/android/tools/r8/graph/AppView.java
index 3adb81d..245a9ab 100644
--- a/src/main/java/com/android/tools/r8/graph/AppView.java
+++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -20,6 +20,7 @@
import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfoFactory;
import com.android.tools.r8.ir.optimize.library.LibraryMemberOptimizer;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.KeepInfoCollection;
import com.android.tools.r8.shaking.LibraryModeledPredicate;
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
import com.android.tools.r8.utils.InternalOptions;
@@ -47,6 +48,9 @@
private GraphLense graphLense;
private InitClassLens initClassLens;
private RootSet rootSet;
+ // This should perferably always be obtained via AppInfoWithLiveness.
+ // Currently however the liveness may be downgraded thus loosing the computed keep info.
+ private KeepInfoCollection keepInfo = null;
private final AbstractValueFactory abstractValueFactory = new AbstractValueFactory();
private final InstanceFieldInitializationInfoFactory instanceFieldInitializationInfoFactory =
new InstanceFieldInitializationInfoFactory();
@@ -176,6 +180,9 @@
if (appInfo != previous) {
previous.markObsolete();
}
+ if (appInfo.hasLiveness()) {
+ keepInfo = appInfo.withLiveness().getKeepInfo();
+ }
@SuppressWarnings("unchecked")
AppView<U> appViewWithSpecializedAppInfo = (AppView<U>) this;
return appViewWithSpecializedAppInfo;
@@ -377,6 +384,10 @@
this.rootSet = rootSet;
}
+ public KeepInfoCollection getKeepInfo() {
+ return keepInfo;
+ }
+
public MergedClassesCollection allMergedClasses() {
MergedClassesCollection collection = new MergedClassesCollection();
if (horizontallyMergedLambdaClasses != null) {
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 f617c31..04ac91a 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -919,9 +919,7 @@
public boolean isAccessModificationAllowed(DexReference reference) {
assert options().getProguardConfiguration().isAccessModificationAllowed();
- return keepInfo
- .getInfo(reference, this)
- .isAccessModificationAllowed(options().getProguardConfiguration());
+ return keepInfo.getInfo(reference, this).isAccessModificationAllowed(options());
}
public boolean isPinned(DexReference reference) {
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 c66940e..e033e78 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -92,6 +92,7 @@
import com.android.tools.r8.shaking.DelayedRootSetActionItem.InterfaceMethodSyntheticBridgeAction;
import com.android.tools.r8.shaking.EnqueuerWorklist.EnqueuerAction;
import com.android.tools.r8.shaking.GraphReporter.KeepReasonWitness;
+import com.android.tools.r8.shaking.KeepInfo.Joiner;
import com.android.tools.r8.shaking.KeepInfoCollection.MutableKeepInfoCollection;
import com.android.tools.r8.shaking.RootSetBuilder.ConsequentRootSet;
import com.android.tools.r8.shaking.RootSetBuilder.ItemsWithRules;
@@ -630,8 +631,13 @@
private void enqueueRootClass(
DexProgramClass clazz, Set<ProguardKeepRuleBase> rules, DexDefinition precondition) {
- KeepReasonWitness witness = graphReporter.reportKeepClass(precondition, rules, clazz);
keepClassWithRules(clazz, rules);
+ enqueueKeepRuleInstantiatedType(clazz, rules, precondition);
+ }
+
+ private void enqueueKeepRuleInstantiatedType(
+ DexProgramClass clazz, Set<ProguardKeepRuleBase> rules, DexDefinition precondition) {
+ KeepReasonWitness witness = graphReporter.reportKeepClass(precondition, rules, clazz);
if (clazz.isAnnotation()) {
workList.enqueueMarkAnnotationInstantiatedAction(clazz, witness);
} else if (clazz.isInterface()) {
@@ -743,11 +749,12 @@
}
private void compatEnqueueHolderIfDependentNonStaticMember(
- DexClass holder, Set<ProguardKeepRuleBase> compatRules) {
+ DexProgramClass holder, Set<ProguardKeepRuleBase> compatRules) {
if (!forceProguardCompatibility || compatRules == null) {
return;
}
- enqueueRootItem(holder, compatRules);
+ // TODO(b/120959039): This needs the set of instance member as preconditon.
+ enqueueKeepRuleInstantiatedType(holder, compatRules, null);
}
//
@@ -1583,7 +1590,7 @@
rootSet.forEachDependentInstanceConstructor(
holder, appView, this::enqueueHolderWithDependentInstanceConstructor);
- rootSet.forEachDependentStaticMember(holder, appView, this::enqueueDependentItem);
+ rootSet.forEachDependentStaticMember(holder, appView, this::enqueueDependentMember);
compatEnqueueHolderIfDependentNonStaticMember(
holder, rootSet.getDependentKeepClassCompatRule(holder.getType()));
@@ -1648,14 +1655,17 @@
}
}
- private void enqueueDependentItem(
- DexDefinition precondition, DexDefinition consequent, Set<ProguardKeepRuleBase> reasons) {
+ private void enqueueDependentMember(
+ DexDefinition precondition,
+ DexEncodedMember<?, ?> consequent,
+ Set<ProguardKeepRuleBase> reasons) {
internalEnqueueRootItem(consequent, reasons, precondition);
}
private void enqueueHolderWithDependentInstanceConstructor(
ProgramMethod instanceInitializer, Set<ProguardKeepRuleBase> reasons) {
- enqueueRootItem(instanceInitializer.getHolder(), reasons);
+ DexProgramClass holder = instanceInitializer.getHolder();
+ enqueueKeepRuleInstantiatedType(holder, reasons, instanceInitializer.getDefinition());
}
private void processAnnotations(DexProgramClass holder, DexDefinition annotatedItem) {
@@ -2246,7 +2256,7 @@
private void transitionDependentItemsForInstantiatedItem(DexProgramClass clazz) {
do {
// Handle keep rules that are dependent on the class being instantiated.
- rootSet.forEachDependentNonStaticMember(clazz, appView, this::enqueueDependentItem);
+ rootSet.forEachDependentNonStaticMember(clazz, appView, this::enqueueDependentMember);
// Visit the super type.
clazz =
@@ -2658,6 +2668,24 @@
new KotlinMetadataEnqueuerExtension(
appView, enqueuerDefinitionSupplier, initialPrunedTypes));
}
+ if (mode.isInitialTreeShaking()) {
+ // This is simulating the effect of the "root set" applied rules.
+ // This is done only in the initial pass, in subsequent passes the "rules" are reapplied
+ // by iterating the instances.
+ for (DexReference reference : rootSet.noObfuscation) {
+ keepInfo.evaluateRule(reference, appInfo, Joiner::disallowMinification);
+ }
+ } else if (appView.getKeepInfo() != null) {
+ appView
+ .getKeepInfo()
+ .getRuleInstances()
+ .forEach(
+ (reference, rules) -> {
+ for (Consumer<Joiner<?, ?, ?>> rule : rules) {
+ keepInfo.evaluateRule(reference, appInfo, rule);
+ }
+ });
+ }
if (appView.options().isShrinking() || appView.options().getProguardConfiguration() == null) {
enqueueRootItems(rootSet.noShrinking);
} else {
@@ -2688,43 +2716,35 @@
}
private void keepClassWithRules(DexProgramClass clazz, Set<ProguardKeepRuleBase> rules) {
- keepInfo.joinClass(
- clazz,
- info ->
- info.pin()
- .lazyDisallowAccessModification(() -> computeDisallowAccessModification(rules)));
+ keepInfo.joinClass(clazz, info -> applyKeepRules(rules, info));
}
private void keepMethodWithRules(
DexProgramClass holder, DexEncodedMethod method, Set<ProguardKeepRuleBase> rules) {
- keepInfo.joinMethod(
- holder,
- method,
- info ->
- info.pin()
- .lazyDisallowAccessModification(() -> computeDisallowAccessModification(rules)));
+ keepInfo.joinMethod(holder, method, info -> applyKeepRules(rules, info));
}
private void keepFieldWithRules(
DexProgramClass holder, DexEncodedField field, Set<ProguardKeepRuleBase> rules) {
- keepInfo.joinField(
- holder,
- field,
- info ->
- info.pin()
- .lazyDisallowAccessModification(() -> computeDisallowAccessModification(rules)));
+ keepInfo.joinField(holder, field, info -> applyKeepRules(rules, info));
}
- private boolean computeDisallowAccessModification(Set<ProguardKeepRuleBase> rules) {
+ private void applyKeepRules(Set<ProguardKeepRuleBase> rules, KeepInfo.Joiner<?, ?, ?> joiner) {
for (ProguardKeepRuleBase rule : rules) {
ProguardKeepRuleModifiers modifiers =
(rule.isProguardIfRule() ? rule.asProguardIfRule().getSubsequentRule() : rule)
.getModifiers();
+ if (!modifiers.allowsShrinking) {
+ // TODO(b/159589281): Evaluate this interpretation.
+ joiner.pin();
+ }
+ if (!modifiers.allowsObfuscation) {
+ joiner.disallowMinification();
+ }
if (!modifiers.allowsAccessModification) {
- return true;
+ joiner.disallowAccessModification();
}
}
- return false;
}
private static class SyntheticAdditions {
@@ -3302,11 +3322,11 @@
consequentRootSet.forEachDependentInstanceConstructor(
clazz, appView, this::enqueueHolderWithDependentInstanceConstructor);
consequentRootSet.forEachDependentStaticMember(
- clazz, appView, this::enqueueDependentItem);
+ clazz, appView, this::enqueueDependentMember);
if (objectAllocationInfoCollection.isInstantiatedDirectlyOrHasInstantiatedSubtype(
clazz)) {
consequentRootSet.forEachDependentNonStaticMember(
- clazz, appView, this::enqueueDependentItem);
+ clazz, appView, this::enqueueDependentMember);
}
compatEnqueueHolderIfDependentNonStaticMember(
clazz, consequentRootSet.getDependentKeepClassCompatRule(clazz.type));
@@ -3321,13 +3341,18 @@
});
// TODO(b/132600955): This modifies the root set. Should the consequent be persistent?
rootSet.addConsequentRootSet(consequentRootSet, addNoShrinking);
+ if (mode.isInitialTreeShaking()) {
+ for (DexReference reference : consequentRootSet.noObfuscation) {
+ keepInfo.evaluateRule(reference, appView, Joiner::disallowMinification);
+ }
+ }
enqueueRootItems(consequentRootSet.noShrinking);
// Check for compatibility rules indicating that the holder must be implicitly kept.
if (forceProguardCompatibility) {
consequentRootSet.dependentKeepClassCompatRule.forEach(
(precondition, compatRules) -> {
assert precondition.isDexType();
- DexClass preconditionHolder = appView.definitionFor(precondition.asDexType());
+ DexProgramClass preconditionHolder = getProgramClassOrNull(precondition.asDexType());
compatEnqueueHolderIfDependentNonStaticMember(preconditionHolder, compatRules);
});
}
diff --git a/src/main/java/com/android/tools/r8/shaking/GlobalKeepInfoConfiguration.java b/src/main/java/com/android/tools/r8/shaking/GlobalKeepInfoConfiguration.java
new file mode 100644
index 0000000..18ae09e
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/GlobalKeepInfoConfiguration.java
@@ -0,0 +1,14 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.shaking;
+
+/** Globally controlled settings that affect the default values for kept items. */
+public interface GlobalKeepInfoConfiguration {
+
+ boolean isTreeShakingEnabled();
+
+ boolean isMinificationEnabled();
+
+ boolean isAccessModificationEnabled();
+}
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 761547a..a1f8424 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
@@ -4,21 +4,23 @@
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<B extends Builder, K extends KeepInfo> {
private final boolean pinned;
+ private final boolean allowMinification;
private final boolean allowAccessModification;
- private KeepInfo(boolean pinned, boolean allowAccessModification) {
+ private KeepInfo(boolean pinned, boolean allowMinification, boolean allowAccessModification) {
this.pinned = pinned;
+ this.allowMinification = allowMinification;
this.allowAccessModification = allowAccessModification;
}
KeepInfo(B builder) {
- this(builder.isPinned(), builder.isAccessModificationAllowed());
+ this(
+ builder.isPinned(), builder.isMinificationAllowed(), builder.isAccessModificationAllowed());
}
/** True if an item must be present in the output. */
@@ -27,6 +29,20 @@
}
/**
+ * True if an item may have its name minified/changed.
+ *
+ * <p>This method requires knowledge of the global configuration as that can override the concrete
+ * value on a given item.
+ */
+ public boolean isMinificationAllowed(GlobalKeepInfoConfiguration configuration) {
+ return configuration.isMinificationEnabled() && internalIsMinificationAllowed();
+ }
+
+ boolean internalIsMinificationAllowed() {
+ return allowMinification;
+ }
+
+ /**
* 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
@@ -34,8 +50,8 @@
*
* @param configuration Global configuration object to determine access modification.
*/
- public boolean isAccessModificationAllowed(ProguardConfiguration configuration) {
- return configuration.isAccessModificationAllowed() && internalIsAccessModificationAllowed();
+ public boolean isAccessModificationAllowed(GlobalKeepInfoConfiguration configuration) {
+ return configuration.isAccessModificationEnabled() && internalIsAccessModificationAllowed();
}
// Internal accessor for the items access-modification bit.
@@ -69,6 +85,7 @@
private K original;
private boolean pinned;
+ private boolean allowMinification;
private boolean allowAccessModification;
Builder() {
@@ -78,17 +95,20 @@
Builder(K original) {
this.original = original;
pinned = original.isPinned();
+ allowMinification = original.internalIsMinificationAllowed();
allowAccessModification = original.internalIsAccessModificationAllowed();
}
B makeTop() {
pin();
+ disallowMinification();
disallowAccessModification();
return self();
}
B makeBottom() {
unpin();
+ allowMinification();
allowAccessModification();
return self();
}
@@ -110,6 +130,7 @@
private boolean internalIsEqualTo(K other) {
return isPinned() == other.isPinned()
+ && isMinificationAllowed() == other.internalIsMinificationAllowed()
&& isAccessModificationAllowed() == other.internalIsAccessModificationAllowed()
&& isEqualTo(other);
}
@@ -118,6 +139,10 @@
return pinned;
}
+ public boolean isMinificationAllowed() {
+ return allowMinification;
+ }
+
public boolean isAccessModificationAllowed() {
return allowAccessModification;
}
@@ -135,6 +160,19 @@
return setPinned(false);
}
+ public B setAllowMinification(boolean allowMinification) {
+ this.allowMinification = allowMinification;
+ return self();
+ }
+
+ public B allowMinification() {
+ return setAllowMinification(true);
+ }
+
+ public B disallowMinification() {
+ return setAllowMinification(false);
+ }
+
public B setAllowAccessModification(boolean allowAccessModification) {
this.allowAccessModification = allowAccessModification;
return self();
@@ -175,12 +213,13 @@
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();
- }
+ public J disallowMinification() {
+ builder.disallowMinification();
+ return self();
+ }
+
+ public J disallowAccessModification() {
+ builder.disallowAccessModification();
return self();
}
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 b85c71d..53af669 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java
@@ -20,8 +20,10 @@
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.ArrayList;
import java.util.Collection;
import java.util.IdentityHashMap;
+import java.util.List;
import java.util.Map;
import java.util.function.Consumer;
@@ -43,6 +45,8 @@
return KeepFieldInfo.bottom();
}
+ abstract Map<DexReference, List<Consumer<KeepInfo.Joiner<?, ?, ?>>>> getRuleInstances();
+
/**
* Base accessor for keep info on a class.
*
@@ -127,6 +131,14 @@
return getFieldInfo(field, definitions).isPinned();
}
+ public final boolean isMinificationAllowed(
+ DexReference reference,
+ DexDefinitionSupplier definitions,
+ GlobalKeepInfoConfiguration configuration) {
+ return configuration.isMinificationEnabled()
+ && getInfo(reference, definitions).isMinificationAllowed(configuration);
+ }
+
public final boolean verifyNoneArePinned(Collection<DexType> types, AppInfo appInfo) {
for (DexType type : types) {
DexProgramClass clazz =
@@ -161,17 +173,26 @@
private final Map<DexMethod, KeepMethodInfo> keepMethodInfo;
private final Map<DexField, KeepFieldInfo> keepFieldInfo;
+ // Map of applied rules for which keys may need to be mutated.
+ private final Map<DexReference, List<Consumer<KeepInfo.Joiner<?, ?, ?>>>> ruleInstances;
+
MutableKeepInfoCollection() {
- this(new IdentityHashMap<>(), new IdentityHashMap<>(), new IdentityHashMap<>());
+ this(
+ new IdentityHashMap<>(),
+ new IdentityHashMap<>(),
+ new IdentityHashMap<>(),
+ new IdentityHashMap<>());
}
private MutableKeepInfoCollection(
Map<DexType, KeepClassInfo> keepClassInfo,
Map<DexMethod, KeepMethodInfo> keepMethodInfo,
- Map<DexField, KeepFieldInfo> keepFieldInfo) {
+ Map<DexField, KeepFieldInfo> keepFieldInfo,
+ Map<DexReference, List<Consumer<KeepInfo.Joiner<?, ?, ?>>>> ruleInstances) {
this.keepClassInfo = keepClassInfo;
this.keepMethodInfo = keepMethodInfo;
this.keepFieldInfo = keepFieldInfo;
+ this.ruleInstances = ruleInstances;
}
@Override
@@ -197,7 +218,43 @@
assert !info.isPinned() || field == newField;
newFieldInfo.put(newField, info);
});
- return new MutableKeepInfoCollection(newClassInfo, newMethodInfo, newFieldInfo);
+ Map<DexReference, List<Consumer<KeepInfo.Joiner<?, ?, ?>>>> newRuleInstances =
+ new IdentityHashMap<>(ruleInstances.size());
+ ruleInstances.forEach(
+ (reference, consumers) -> {
+ DexReference newReference;
+ if (reference.isDexType()) {
+ DexType newType = lens.lookupType(reference.asDexType());
+ if (!newType.isClassType()) {
+ assert newType.isIntType() : "Expected only enum unboxing type changes.";
+ return;
+ }
+ newReference = newType;
+ } else if (reference.isDexMethod()) {
+ newReference = lens.getRenamedMethodSignature(reference.asDexMethod());
+ } else {
+ assert reference.isDexField();
+ newReference = lens.getRenamedFieldSignature(reference.asDexField());
+ }
+ newRuleInstances.put(newReference, consumers);
+ });
+ return new MutableKeepInfoCollection(
+ newClassInfo, newMethodInfo, newFieldInfo, newRuleInstances);
+ }
+
+ @Override
+ Map<DexReference, List<Consumer<KeepInfo.Joiner<?, ?, ?>>>> getRuleInstances() {
+ return ruleInstances;
+ }
+
+ void evaluateRule(
+ DexReference reference,
+ DexDefinitionSupplier definitions,
+ Consumer<KeepInfo.Joiner<?, ?, ?>> fn) {
+ joinInfo(reference, definitions, fn);
+ if (!getInfo(reference, definitions).isBottom()) {
+ ruleInstances.computeIfAbsent(reference, k -> new ArrayList<>()).add(fn);
+ }
}
@Override
@@ -230,6 +287,34 @@
}
}
+ public void joinInfo(
+ DexReference reference,
+ DexDefinitionSupplier definitions,
+ Consumer<KeepInfo.Joiner<?, ?, ?>> fn) {
+ if (reference.isDexType()) {
+ DexType type = reference.asDexType();
+ DexProgramClass clazz = asProgramClassOrNull(definitions.definitionFor(type));
+ if (clazz != null) {
+ joinClass(clazz, fn::accept);
+ }
+ } else if (reference.isDexMethod()) {
+ DexMethod method = reference.asDexMethod();
+ DexProgramClass clazz = asProgramClassOrNull(definitions.definitionFor(method.holder));
+ DexEncodedMethod definition = method.lookupOnClass(clazz);
+ if (definition != null) {
+ joinMethod(clazz, definition, fn::accept);
+ }
+ } else {
+ assert reference.isDexField();
+ DexField field = reference.asDexField();
+ DexProgramClass clazz = asProgramClassOrNull(definitions.definitionFor(field.holder));
+ DexEncodedField definition = field.lookupOnClass(clazz);
+ if (definition != null) {
+ joinField(clazz, definition, fn::accept);
+ }
+ }
+ }
+
public void keepClass(DexProgramClass clazz) {
joinClass(clazz, KeepInfo.Joiner::top);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
index b38d077..09a6587 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -29,10 +29,12 @@
import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DirectMappedDexApplication;
+import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult;
import com.android.tools.r8.graph.SubtypingInfo;
import com.android.tools.r8.ir.analysis.proto.GeneratedMessageLiteBuilderShrinker;
+import com.android.tools.r8.ir.optimize.enums.EnumUnboxingRewriter;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.shaking.AnnotationMatchResult.AnnotationsIgnoredMatchResult;
import com.android.tools.r8.shaking.AnnotationMatchResult.ConcreteAnnotationMatchResult;
@@ -1368,7 +1370,7 @@
public void forEachDependentMember(
DexDefinition item,
AppView<?> appView,
- Consumer3<DexDefinition, DexDefinition, Set<ProguardKeepRuleBase>> fn) {
+ Consumer3<DexDefinition, DexEncodedMember<?, ?>, Set<ProguardKeepRuleBase>> fn) {
getDependentItems(item)
.forEachMember(
(reference, reasons) -> {
@@ -1386,7 +1388,7 @@
public void forEachDependentNonStaticMember(
DexDefinition item,
AppView<?> appView,
- Consumer3<DexDefinition, DexDefinition, Set<ProguardKeepRuleBase>> fn) {
+ Consumer3<DexDefinition, DexEncodedMember<?, ?>, Set<ProguardKeepRuleBase>> fn) {
forEachDependentMember(
item,
appView,
@@ -1400,7 +1402,7 @@
public void forEachDependentStaticMember(
DexDefinition item,
AppView<?> appView,
- Consumer3<DexDefinition, DexDefinition, Set<ProguardKeepRuleBase>> fn) {
+ Consumer3<DexDefinition, DexEncodedMember<?, ?>, Set<ProguardKeepRuleBase>> fn) {
forEachDependentMember(
item,
appView,
@@ -1904,21 +1906,50 @@
}
public boolean mayBeMinified(DexReference reference, AppView<?> appView) {
- return !mayNotBeMinified(reference, appView);
+ boolean minificationAllowed =
+ appView.getKeepInfo().isMinificationAllowed(reference, appView, appView.options());
+ // TODO: Remove this once consistency is established.
+ // Assert consistency with existing root set info.
+ // Split in to checks to make the direction clean in errors.
+ // minify -> not-in-set
+ assert !minificationAllowed
+ || !noObfuscation.contains(getOriginal(reference, appView))
+ // Compiler synthesized methods can be renamed regardless of keep rules.
+ || isCompilerSynthesizedMethod(reference, appView);
+ // !minify -> in-set
+ assert minificationAllowed
+ || noObfuscation.contains(getOriginal(reference, appView))
+ || !appView.options().isMinificationEnabled();
+ return minificationAllowed;
}
public boolean mayNotBeMinified(DexReference reference, AppView<?> appView) {
- if (reference.isDexType()) {
- return noObfuscation.contains(
- appView.graphLense().getOriginalType(reference.asDexType()));
- } else if (reference.isDexMethod()) {
- return noObfuscation.contains(
- appView.graphLense().getOriginalMethodSignature(reference.asDexMethod()));
- } else {
- assert reference.isDexField();
- return noObfuscation.contains(
- appView.graphLense().getOriginalFieldSignature(reference.asDexField()));
+ return !mayBeMinified(reference, appView);
+ }
+
+ private boolean isCompilerSynthesizedMethod(DexReference reference, AppView<?> appView) {
+ DexMethod method = reference.asDexMethod();
+ if (method == null) {
+ return false;
}
+ DexEncodedMethod definition = method.lookupOnClass(appView.definitionForHolder(method));
+ return definition != null
+ && (definition.isD8R8Synthesized()
+ || definition
+ .qualifiedName()
+ .contains(EnumUnboxingRewriter.ENUM_UNBOXING_UTILITY_CLASS_NAME));
+ }
+
+ private DexReference getOriginal(DexReference reference, AppView<?> appView) {
+ GraphLense lens = appView.graphLense();
+ if (reference.isDexType()) {
+ return lens.getOriginalType(reference.asDexType());
+ }
+ if (reference.isDexMethod()) {
+ return lens.getOriginalMethodSignature(reference.asDexMethod());
+ }
+ assert reference.isDexField();
+ return lens.getOriginalFieldSignature(reference.asDexField());
}
public boolean verifyKeptFieldsAreAccessedAndLive(AppInfoWithLiveness appInfo) {
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 35e4614..f7353d9 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -45,6 +45,7 @@
import com.android.tools.r8.references.Reference;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.Enqueuer;
+import com.android.tools.r8.shaking.GlobalKeepInfoConfiguration;
import com.android.tools.r8.shaking.ProguardConfiguration;
import com.android.tools.r8.shaking.ProguardConfigurationRule;
import com.android.tools.r8.utils.IROrdering.IdentityIROrdering;
@@ -77,7 +78,7 @@
import java.util.function.Predicate;
import org.objectweb.asm.Opcodes;
-public class InternalOptions {
+public class InternalOptions implements GlobalKeepInfoConfiguration {
// Set to true to run compilation in a single thread and without randomly shuffling the input.
// This makes life easier when running R8 in a debugger.
@@ -481,13 +482,33 @@
private final boolean enableMinification;
public boolean isShrinking() {
+ assert proguardConfiguration == null
+ || enableTreeShaking == proguardConfiguration.isShrinking();
return enableTreeShaking;
}
public boolean isMinifying() {
+ assert proguardConfiguration == null
+ || enableMinification == proguardConfiguration.isObfuscating();
return enableMinification;
}
+ @Override
+ public boolean isTreeShakingEnabled() {
+ return isShrinking();
+ }
+
+ @Override
+ public boolean isMinificationEnabled() {
+ return isMinifying();
+ }
+
+ @Override
+ public boolean isAccessModificationEnabled() {
+ return getProguardConfiguration() != null
+ && getProguardConfiguration().isAccessModificationAllowed();
+ }
+
public boolean keepInnerClassStructure() {
return getProguardConfiguration().getKeepAttributes().signature
|| getProguardConfiguration().getKeepAttributes().innerClasses;
diff --git a/src/test/java/com/android/tools/r8/TestDiagnosticMessages.java b/src/test/java/com/android/tools/r8/TestDiagnosticMessages.java
index ed0e8f1..72a22e6 100644
--- a/src/test/java/com/android/tools/r8/TestDiagnosticMessages.java
+++ b/src/test/java/com/android/tools/r8/TestDiagnosticMessages.java
@@ -34,6 +34,18 @@
TestDiagnosticMessages assertErrorsCount(int count);
+ default TestDiagnosticMessages assertNoInfos() {
+ return assertInfosCount(0);
+ }
+
+ default TestDiagnosticMessages assertNoWarnings() {
+ return assertWarningsCount(0);
+ }
+
+ default TestDiagnosticMessages assertNoErrors() {
+ return assertErrorsCount(0);
+ }
+
// Match exact.
default TestDiagnosticMessages assertDiagnosticsMatch(Matcher<Diagnostic> matcher) {
diff --git a/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardedTest.java b/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardedTest.java
index b0c7ecf..df38cb2 100644
--- a/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardedTest.java
+++ b/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardedTest.java
@@ -3,25 +3,27 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.checkdiscarded;
-import static org.hamcrest.MatcherAssert.assertThat;
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
+import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.core.StringContains.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import com.android.tools.r8.CompilationFailedException;
-import com.android.tools.r8.Diagnostic;
import com.android.tools.r8.R8FullTestBuilder;
import com.android.tools.r8.R8TestCompileResult;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestDiagnosticMessages;
import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.TestRuntime.NoneRuntime;
import com.android.tools.r8.checkdiscarded.testclasses.Main;
import com.android.tools.r8.checkdiscarded.testclasses.UnusedClass;
import com.android.tools.r8.checkdiscarded.testclasses.UsedClass;
import com.android.tools.r8.checkdiscarded.testclasses.WillBeGone;
import com.android.tools.r8.checkdiscarded.testclasses.WillStay;
+import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.InternalOptions;
+import com.google.common.collect.ImmutableList;
import java.util.List;
import java.util.function.Consumer;
import org.junit.Test;
@@ -32,19 +34,19 @@
@RunWith(Parameterized.class)
public class CheckDiscardedTest extends TestBase {
- @Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withNoneRuntime().build();
+ @Parameters(name = "{0}, minify:{1}")
+ public static List<Object[]> data() {
+ return buildParameters(getTestParameters().withNoneRuntime().build(), BooleanUtils.values());
}
- private final TestParameters parameters;
+ public final boolean minify;
- public CheckDiscardedTest(TestParameters parameters) {
- this.parameters = parameters;
+ public CheckDiscardedTest(TestParameters parameters, boolean minify) {
+ assertEquals(NoneRuntime.getInstance(), parameters.getRuntime());
+ this.minify = minify;
}
private void compile(
- boolean obfuscate,
Class annotation,
boolean checkMembers,
Consumer<TestDiagnosticMessages> onCompilationFailure) {
@@ -56,7 +58,7 @@
.addProgramClasses(UnusedClass.class, UsedClass.class, Main.class)
.addKeepMainRule(Main.class)
.addKeepRules(checkDiscardRule(checkMembers, annotation))
- .minification(obfuscate)
+ .minification(minify)
.addOptionsModification(this::noInlining)
.compile();
assertNull(onCompilationFailure);
@@ -80,45 +82,44 @@
@Test
public void classesAreGone() {
- compile(false, WillBeGone.class, false, null);
- compile(true, WillBeGone.class, false, null);
+ compile(WillBeGone.class, false, null);
}
@Test
public void classesAreNotGone() {
Consumer<TestDiagnosticMessages> check =
- diagnostics -> {
- List<Diagnostic> infos = diagnostics.getInfos();
- assertEquals(2, infos.size());
- String messageUsedClass = infos.get(1).getDiagnosticMessage();
- assertThat(messageUsedClass, containsString("UsedClass was not discarded"));
- assertThat(messageUsedClass, containsString("is instantiated in"));
- String messageMain = infos.get(0).getDiagnosticMessage();
- assertThat(messageMain, containsString("Main was not discarded"));
- assertThat(messageMain, containsString("is referenced in keep rule"));
- };
- compile(false, WillStay.class, false, check);
- compile(true, WillStay.class, false, check);
+ diagnostics ->
+ diagnostics
+ .assertNoWarnings()
+ .assertErrorsMatch(diagnosticMessage(containsString("Discard checks failed")))
+ .assertInfosMatch(
+ ImmutableList.of(
+ allOf(
+ diagnosticMessage(containsString("UsedClass was not discarded")),
+ diagnosticMessage(containsString("is instantiated in"))),
+ allOf(
+ diagnosticMessage(containsString("Main was not discarded")),
+ diagnosticMessage(containsString("is referenced in keep rule")))));
+ compile(WillStay.class, false, check);
}
@Test
public void membersAreGone() {
- compile(false, WillBeGone.class, true, null);
- compile(true, WillBeGone.class, true, null);
+ compile(WillBeGone.class, true, null);
}
@Test
public void membersAreNotGone() {
Consumer<TestDiagnosticMessages> check =
- diagnostics -> {
- List<Diagnostic> infos = diagnostics.getInfos();
- assertEquals(1, infos.size());
- String message = infos.get(0).getDiagnosticMessage();
- assertThat(message, containsString("was not discarded"));
- assertThat(message, containsString("is invoked from"));
- };
- compile(false, WillStay.class, true, check);
- compile(true, WillStay.class, true, check);
+ diagnostics ->
+ diagnostics
+ .assertNoWarnings()
+ .assertErrorsMatch(diagnosticMessage(containsString("Discard checks failed")))
+ .assertInfosMatch(
+ allOf(
+ diagnosticMessage(containsString("was not discarded")),
+ diagnosticMessage(containsString("is invoked from"))));
+ compile(WillStay.class, true, check);
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/keepclassmembers/KeepInterfaceMethodTest.java b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/KeepInterfaceMethodTest.java
index 550957e..e6baeae 100644
--- a/src/test/java/com/android/tools/r8/shaking/keepclassmembers/KeepInterfaceMethodTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/KeepInterfaceMethodTest.java
@@ -40,6 +40,7 @@
@Test
public void testIProguard() throws CompilationFailedException, IOException, ExecutionException {
testForProguard()
+ // TODO(b/159694276): Run the resulting code on runtime.
.addProgramClasses(I.class)
.addKeepRules(
"-keepclassmembers class " + I.class.getTypeName() + " { void foo(); }",
@@ -51,6 +52,7 @@
@Test
public void testIR8() throws CompilationFailedException, IOException, ExecutionException {
+ // TODO(b/159694276): Add compat variant of this.
testForR8(parameters.getBackend())
.addProgramClasses(I.class)
.addKeepRules("-keepclassmembers class " + I.class.getTypeName() + " { void foo(); }")
@@ -75,6 +77,7 @@
@Test
public void testAR8() throws CompilationFailedException, IOException, ExecutionException {
+ // TODO(b/159694276): Add non-compat variant of this.
testForR8Compat(parameters.getBackend())
.addProgramClasses(I.class, A.class, B.class, Main.class)
.addKeepRules("-keepclassmembers class " + I.class.getTypeName() + " { void foo(); }")
