Add support for -if rule (without <n>th wildcards).
Fourth design (PS 21):
In RootSetBuilder, only -if rule itself is collected. A partial builder
that can share the same logic in RootSetBuilder is introduced to build
ConsequentRootSet, though.
In Enqueuer, using live items, both of -if conditional and consequent
parts are evaluated. Refer to 3rd design below for some other details.
------
Third design (PS15 ~ PS19):
In RootSetBuilder, only consequent -keep rule are evaluated to collect
which types/members will become live, depending on -if conditional rule.
In Enqueuer, which trace live items based on keep rules, -if conditional
rule is evaluated against newly-became-live items. Depending on the
modifiers of consequent rule, e.g., -keep*names, dependent items would
be enqueued, moved to no-optimization set or no-obfuscation set.
Abandoned to prepare back reference in mind.
------
Second design (PS13):
In RootSetBuilder, -if rule and consequent -keep rule are evaluated just
to collect which DexItem's, such as DexClass (will be hoisted to DexType
for minification), DexEncodedField and DexEncodedMethod, are bound.
Those bindings are maintained in a RootSet.
When Enqueuer records which type/field/method are live, the enablement
of involved -if rule is updated, and once enabled, dependent items are
either enqueued (-keep) or moved to no-obfuscation set (-keep*names).
Abandoned because member rules for any member (e.g., * or <fields>) are
not treated properly. They should be evaluated while tracing live items.
------
First design (PS1 ~ PS11):
In the first round of rule matching, root set builder will figure out
which types match with -if part. Then, in the second round, it will
iterate over subsequent part and bind matched types with precondition
types.
For -if/-keep*names that don't allow obfuscation, but allow shrinking,
dependentNoObfuscation, which mimics dependentNoShrinking, is added.
Abandoned because dependent items are *AND* logic, and both of
dependentNoShrinking and dependentNoObfuscation are bound to one item.
Bug: 73708139
Change-Id: I407ea9c6e20a7e49a36be66838696584c8968cfc
diff --git a/src/main/java/com/android/tools/r8/GenerateMainDexList.java b/src/main/java/com/android/tools/r8/GenerateMainDexList.java
index f525565..3db9699 100644
--- a/src/main/java/com/android/tools/r8/GenerateMainDexList.java
+++ b/src/main/java/com/android/tools/r8/GenerateMainDexList.java
@@ -40,9 +40,9 @@
new ApplicationReader(app, options, timing).read(executor).toDirect();
AppInfoWithSubtyping appInfo = new AppInfoWithSubtyping(application);
RootSet mainDexRootSet =
- new RootSetBuilder(application, appInfo, options.mainDexKeepRules, options).run(executor);
+ new RootSetBuilder(appInfo, application, options.mainDexKeepRules, options).run(executor);
Enqueuer enqueuer = new Enqueuer(appInfo, options, true);
- AppInfoWithLiveness mainDexAppInfo = enqueuer.traceMainDex(mainDexRootSet, timing);
+ AppInfoWithLiveness mainDexAppInfo = enqueuer.traceMainDex(mainDexRootSet, executor, timing);
// LiveTypes is the result.
Set<DexType> mainDexClasses =
new MainDexListBuilder(new HashSet<>(mainDexAppInfo.liveTypes), application).run();
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index d062d13..4f12ce1 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -282,13 +282,13 @@
ProguardConfiguration.builder(application.dexItemFactory, options.reporter);
rootSet = new RootSetBuilder(
- application, appInfo, options.proguardConfiguration.getRules(), options)
+ appInfo, application, options.proguardConfiguration.getRules(), options)
.run(executorService);
ProtoLiteExtension protoLiteExtension =
options.forceProguardCompatibility ? null : new ProtoLiteExtension(appInfo);
Enqueuer enqueuer = new Enqueuer(appInfo, options, options.forceProguardCompatibility,
compatibility, protoLiteExtension);
- appInfo = enqueuer.traceApplication(rootSet, timing);
+ appInfo = enqueuer.traceApplication(rootSet, executorService, timing);
if (options.proguardConfiguration.isPrintSeeds()) {
ByteArrayOutputStream bytes = new ByteArrayOutputStream();
PrintStream out = new PrintStream(bytes);
@@ -396,9 +396,10 @@
Enqueuer enqueuer = new Enqueuer(appInfo, options, true);
// Lets find classes which may have code executed before secondary dex files installation.
RootSet mainDexRootSet =
- new RootSetBuilder(application, appInfo, options.mainDexKeepRules, options)
+ new RootSetBuilder(appInfo, application, options.mainDexKeepRules, options)
.run(executorService);
- AppInfoWithLiveness mainDexAppInfo = enqueuer.traceMainDex(mainDexRootSet, timing);
+ AppInfoWithLiveness mainDexAppInfo =
+ enqueuer.traceMainDex(mainDexRootSet, executorService, timing);
// LiveTypes is the result.
Set<DexType> mainDexBaseClasses = new HashSet<>(mainDexAppInfo.liveTypes);
@@ -416,7 +417,7 @@
timing.begin("Post optimization code stripping");
try {
Enqueuer enqueuer = new Enqueuer(appInfo, options, options.forceProguardCompatibility);
- appInfo = enqueuer.traceApplication(rootSet, timing);
+ appInfo = enqueuer.traceApplication(rootSet, executorService, timing);
if (options.enableTreeShaking) {
TreePruner pruner = new TreePruner(application, appInfo.withLiveness(), options);
application = pruner.run();
diff --git a/src/main/java/com/android/tools/r8/shaking/DexStringCache.java b/src/main/java/com/android/tools/r8/shaking/DexStringCache.java
new file mode 100644
index 0000000..377dacd
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/DexStringCache.java
@@ -0,0 +1,15 @@
+// Copyright (c) 2018, 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;
+
+import com.android.tools.r8.graph.DexString;
+import java.util.concurrent.ConcurrentHashMap;
+
+final class DexStringCache {
+ private final ConcurrentHashMap<DexString, String> stringCache = new ConcurrentHashMap<>();
+
+ public String lookupString(DexString name) {
+ return stringCache.computeIfAbsent(name, DexString::toString);
+ }
+}
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 a726391..3a31203 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -41,6 +41,7 @@
import com.android.tools.r8.ir.code.Invoke.Type;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.logging.Log;
+import com.android.tools.r8.shaking.RootSetBuilder.ConsequentRootSet;
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
import com.android.tools.r8.shaking.protolite.ProtoLiteExtension;
import com.android.tools.r8.utils.InternalOptions;
@@ -70,6 +71,8 @@
import java.util.Queue;
import java.util.Set;
import java.util.SortedSet;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -128,7 +131,7 @@
.newIdentityHashMap();
/**
- * Set of types that are mentioned in the program. We at least need an empty abstract classitem
+ * Set of types that are mentioned in the program. We at least need an empty abstract class item
* for these.
*/
private final Set<DexType> liveTypes = Sets.newIdentityHashSet();
@@ -1027,30 +1030,36 @@
reachability, instantiatedTypes.getReasons());
}
- public AppInfoWithLiveness traceMainDex(RootSet rootSet, Timing timing) {
+ public AppInfoWithLiveness traceMainDex(
+ RootSet rootSet, ExecutorService executorService, Timing timing) throws ExecutionException {
this.tracingMainDex = true;
this.rootSet = rootSet;
// Translate the result of root-set computation into enqueuer actions.
enqueueRootItems(rootSet.noShrinking);
- AppInfoWithLiveness appInfo = trace(timing);
+ AppInfoWithLiveness appInfo = trace(executorService, timing);
options.reporter.failIfPendingErrors();
return appInfo;
}
- public AppInfoWithLiveness traceApplication(RootSet rootSet, Timing timing) {
+ public AppInfoWithLiveness traceApplication(
+ RootSet rootSet, ExecutorService executorService, Timing timing) throws ExecutionException {
this.rootSet = rootSet;
// Translate the result of root-set computation into enqueuer actions.
enqueueRootItems(rootSet.noShrinking);
appInfo.libraryClasses().forEach(this::markAllLibraryVirtualMethodsReachable);
- AppInfoWithLiveness result = trace(timing);
+ AppInfoWithLiveness result = trace(executorService, timing);
options.reporter.failIfPendingErrors();
return result;
}
- private AppInfoWithLiveness trace(Timing timing) {
+ private AppInfoWithLiveness trace(
+ ExecutorService executorService, Timing timing) throws ExecutionException {
timing.begin("Grow the tree.");
try {
while (true) {
+ long numOfLiveItems = (long) liveTypes.size();
+ numOfLiveItems += (long) liveMethods.items.size();
+ numOfLiveItems += (long) liveFields.items.size();
while (!workList.isEmpty()) {
Action action = workList.poll();
switch (action.kind) {
@@ -1083,6 +1092,24 @@
throw new IllegalArgumentException(action.kind.toString());
}
}
+
+ // Continue fix-point processing if -if rules are enabled by items that newly became live.
+ long numOfLiveItemsAfterProcessing = (long) liveTypes.size();
+ numOfLiveItemsAfterProcessing += (long) liveMethods.items.size();
+ numOfLiveItemsAfterProcessing += (long) liveFields.items.size();
+ if (numOfLiveItemsAfterProcessing > numOfLiveItems) {
+ RootSetBuilder consequentSetBuilder =
+ new RootSetBuilder(appInfo, rootSet.ifRules, options);
+ ConsequentRootSet consequentRootSet = consequentSetBuilder.runForIfRules(
+ executorService, liveTypes, liveMethods.getItems(), liveFields.getItems());
+ enqueueRootItems(consequentRootSet.noShrinking);
+ rootSet.noOptimization.addAll(consequentRootSet.noOptimization);
+ rootSet.noObfuscation.addAll(consequentRootSet.noObfuscation);
+ if (!workList.isEmpty()) {
+ continue;
+ }
+ }
+
// Continue fix-point processing while there are additional work items to ensure
// Proguard compatibility.
if (proguardCompatibilityWorkList.isEmpty()
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
index 35c8e47..c1af9f4 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -336,8 +336,6 @@
configurationBuilder.addRule(parseIdentifierNameStringRule());
} else if (acceptString("if")) {
configurationBuilder.addRule(parseIfRule(optionStart));
- // TODO(b/73708139): remove warning once we add support -if <class_spec>
- warnIgnoringOptions("if", optionStart);
} else {
String unknownOption = acceptString();
reporter.error(new StringDiagnostic(
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java b/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java
index 6facec1..9659954 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java
@@ -5,21 +5,19 @@
import java.util.Set;
-public class ProguardIfRule extends ProguardConfigurationRule {
+public class ProguardIfRule extends ProguardKeepRule {
final ProguardKeepRule subsequentRule;
- public static class Builder extends ProguardConfigurationRule.Builder {
+ public static class Builder extends ProguardKeepRule.Builder {
ProguardKeepRule subsequentRule = null;
- private Builder() {
- }
-
public void setSubsequentRule(ProguardKeepRule rule) {
subsequentRule = rule;
}
+ @Override
public ProguardIfRule build() {
assert subsequentRule != null : "Option -if without a subsequent rule.";
return new ProguardIfRule(classAnnotation, classAccessFlags,
@@ -37,7 +35,8 @@
Set<ProguardMemberRule> memberRules,
ProguardKeepRule subsequentRule) {
super(classAnnotation, classAccessFlags, negatedClassAccessFlags, classTypeNegated, classType,
- classNames, inheritanceAnnotation, inheritanceClassName, inheritanceIsExtends, memberRules);
+ classNames, inheritanceAnnotation, inheritanceClassName, inheritanceIsExtends, memberRules,
+ ProguardKeepRuleType.CONDITIONAL, ProguardKeepRuleModifiers.builder().build());
this.subsequentRule = subsequentRule;
}
@@ -46,6 +45,23 @@
}
@Override
+ public boolean equals(Object o) {
+ if (!(o instanceof ProguardIfRule)) {
+ return false;
+ }
+ ProguardIfRule other = (ProguardIfRule) o;
+ if (subsequentRule != other.subsequentRule) {
+ return false;
+ }
+ return super.equals(o);
+ }
+
+ @Override
+ public int hashCode() {
+ return super.hashCode() * 3 + subsequentRule.hashCode();
+ }
+
+ @Override
String typeString() {
return "if";
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardKeepRule.java b/src/main/java/com/android/tools/r8/shaking/ProguardKeepRule.java
index 9b1bc0b..d3d6705 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardKeepRule.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardKeepRule.java
@@ -14,7 +14,7 @@
private final ProguardKeepRuleModifiers.Builder modifiersBuilder
= ProguardKeepRuleModifiers.builder();
- private Builder() {}
+ protected Builder() {}
public void setType(ProguardKeepRuleType type) {
this.type = type;
@@ -34,7 +34,7 @@
private final ProguardKeepRuleType type;
private final ProguardKeepRuleModifiers modifiers;
- private ProguardKeepRule(
+ protected ProguardKeepRule(
ProguardTypeMatcher classAnnotation,
ProguardAccessFlags classAccessFlags,
ProguardAccessFlags negatedClassAccessFlags,
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleType.java b/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleType.java
index 4af299c..2018cbf 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleType.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleType.java
@@ -8,7 +8,8 @@
public enum ProguardKeepRuleType {
KEEP,
KEEP_CLASS_MEMBERS,
- KEEP_CLASSES_WITH_MEMBERS;
+ KEEP_CLASSES_WITH_MEMBERS,
+ CONDITIONAL;
@Override
public String toString() {
@@ -19,6 +20,8 @@
return "keepclassmembers";
case KEEP_CLASSES_WITH_MEMBERS:
return "keepclasseswithmembers";
+ case CONDITIONAL:
+ return "if";
default:
throw new Unreachable("Unknown ProguardKeepRuleType.");
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardMemberRule.java b/src/main/java/com/android/tools/r8/shaking/ProguardMemberRule.java
index db7dca8..5e1293f 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardMemberRule.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardMemberRule.java
@@ -156,7 +156,7 @@
return type;
}
- public boolean matches(DexEncodedField field, RootSetBuilder builder) {
+ public boolean matches(DexEncodedField field, DexStringCache stringCache) {
switch (getRuleType()) {
case ALL:
case ALL_FIELDS:
@@ -169,7 +169,7 @@
return RootSetBuilder.containsAnnotation(annotation, field.annotations);
case FIELD:
// Name check.
- String name = builder.lookupString(field.field.name);
+ String name = stringCache.lookupString(field.field.name);
if (!getName().matches(name)) {
break;
}
@@ -196,7 +196,7 @@
return false;
}
- public boolean matches(DexEncodedMethod method, RootSetBuilder builder) {
+ public boolean matches(DexEncodedMethod method, DexStringCache stringCache) {
switch (getRuleType()) {
case ALL_METHODS:
if (method.isClassInitializer()) {
@@ -220,7 +220,7 @@
case CONSTRUCTOR:
case INIT:
// Name check.
- String name = builder.lookupString(method.method.name);
+ String name = stringCache.lookupString(method.method.name);
if (!getName().matches(name)) {
break;
}
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 9c0dcdd..3912b1c 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -17,7 +17,6 @@
import com.android.tools.r8.graph.DexLibraryClass;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
-import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DirectMappedDexApplication;
import com.android.tools.r8.logging.Log;
@@ -40,17 +39,17 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
+import java.util.function.Function;
import java.util.stream.Collectors;
public class RootSetBuilder {
- private final DirectMappedDexApplication application;
private final AppInfo appInfo;
- private final List<ProguardConfigurationRule> rules;
+ private final DirectMappedDexApplication application;
+ private final Collection<ProguardConfigurationRule> rules;
private final Map<DexItem, ProguardKeepRule> noShrinking = new IdentityHashMap<>();
private final Set<DexItem> noOptimization = Sets.newIdentityHashSet();
private final Set<DexItem> noObfuscation = Sets.newIdentityHashSet();
@@ -67,18 +66,37 @@
private final Set<DexItem> identifierNameStrings = Sets.newIdentityHashSet();
private final InternalOptions options;
- public RootSetBuilder(DexApplication application, AppInfo appInfo,
- List<ProguardConfigurationRule> rules, InternalOptions options) {
- this.application = application.asDirect();
+ private final DexStringCache dexStringCache = new DexStringCache();
+ private final Set<ProguardIfRule> ifRules = Sets.newIdentityHashSet();
+
+ public RootSetBuilder(
+ AppInfo appInfo,
+ DexApplication application,
+ List<ProguardConfigurationRule> rules,
+ InternalOptions options) {
this.appInfo = appInfo;
- this.rules = rules;
+ this.application = application.asDirect();
+ this.rules = rules == null ? null : Collections.unmodifiableCollection(rules);
this.options = options;
}
- private boolean anySuperTypeMatches(DexType type, ProguardTypeMatcher name,
+ RootSetBuilder(
+ AppInfo appInfo,
+ Set<ProguardIfRule> ifRules,
+ InternalOptions options) {
+ this.appInfo = appInfo;
+ this.application = appInfo.app.asDirect();
+ this.rules = Collections.unmodifiableCollection(ifRules);
+ this.options = options;
+ }
+
+ private boolean anySuperTypeMatches(
+ DexType type,
+ Function<DexType, DexClass> definitionFor,
+ ProguardTypeMatcher name,
ProguardTypeMatcher annotation) {
while (type != null) {
- DexClass clazz = application.definitionFor(type);
+ DexClass clazz = definitionFor.apply(type);
if (clazz == null) {
// TODO(herhut): Warn about broken supertype chain?
return false;
@@ -91,36 +109,42 @@
return false;
}
- private boolean anyImplementedInterfaceMatches(DexClass clazz,
- ProguardTypeMatcher className, ProguardTypeMatcher annotation) {
+ private boolean anyImplementedInterfaceMatches(
+ DexClass clazz,
+ Function<DexType, DexClass> definitionFor,
+ ProguardTypeMatcher className,
+ ProguardTypeMatcher annotation) {
if (clazz == null) {
return false;
}
for (DexType iface : clazz.interfaces.values) {
- DexClass ifaceClass = application.definitionFor(iface);
+ DexClass ifaceClass = definitionFor.apply(iface);
if (ifaceClass == null) {
// TODO(herhut): Warn about broken supertype chain?
return false;
}
// TODO(herhut): Maybe it would be better to do this breadth first.
if ((className.matches(iface) && containsAnnotation(annotation, ifaceClass.annotations))
- || anyImplementedInterfaceMatches(ifaceClass, className, annotation)) {
+ || anyImplementedInterfaceMatches(ifaceClass, definitionFor, className, annotation)) {
return true;
}
}
if (clazz.superType == null) {
return false;
}
- DexClass superClass = application.definitionFor(clazz.superType);
+ DexClass superClass = definitionFor.apply(clazz.superType);
if (superClass == null) {
// TODO(herhut): Warn about broken supertype chain?
return false;
}
- return anyImplementedInterfaceMatches(superClass, className, annotation);
+ return anyImplementedInterfaceMatches(superClass, definitionFor, className, annotation);
}
// Process a class with the keep rule.
- private void process(DexClass clazz, ProguardConfigurationRule rule) {
+ private void process(
+ DexClass clazz,
+ ProguardConfigurationRule rule,
+ ProguardIfRule ifRule) {
if (rule.getClassType().matches(clazz) == rule.getClassTypeNegated()) {
return;
}
@@ -140,12 +164,18 @@
// TODO(herhut): One day make this do what it says.
if (rule.hasInheritanceClassName()) {
boolean extendsExpected =
- anySuperTypeMatches(clazz.superType, rule.getInheritanceClassName(),
+ anySuperTypeMatches(
+ clazz.superType,
+ application::definitionFor,
+ rule.getInheritanceClassName(),
rule.getInheritanceAnnotation());
boolean implementsExpected = false;
if (!extendsExpected) {
implementsExpected =
- anyImplementedInterfaceMatches(clazz, rule.getInheritanceClassName(),
+ anyImplementedInterfaceMatches(
+ clazz,
+ application::definitionFor,
+ rule.getInheritanceClassName(),
rule.getInheritanceAnnotation());
}
if (!extendsExpected && !implementsExpected) {
@@ -167,16 +197,15 @@
}
}
- if (rule instanceof ProguardIfRule) {
- // TODO(b/73708139): add support -if <class_spec>
- // Check if -if part matches or subsequent -keep part matches.
- } else if (rule.getClassNames().matches(clazz.type)) {
+ if (rule.getClassNames().matches(clazz.type)) {
Collection<ProguardMemberRule> memberKeepRules = rule.getMemberRules();
if (rule instanceof ProguardKeepRule) {
switch (((ProguardKeepRule) rule).getType()) {
case KEEP_CLASS_MEMBERS: {
- markMatchingVisibleMethods(clazz, memberKeepRules, rule, clazz.type);
- markMatchingFields(clazz, memberKeepRules, rule, clazz.type);
+ // If we're handling -if consequent part, that means precondition already met.
+ DexType precondition = ifRule != null ? null : clazz.type;
+ markMatchingVisibleMethods(clazz, memberKeepRules, rule, precondition);
+ markMatchingFields(clazz, memberKeepRules, rule, precondition);
break;
}
case KEEP_CLASSES_WITH_MEMBERS: {
@@ -191,6 +220,9 @@
markMatchingFields(clazz, memberKeepRules, rule, null);
break;
}
+ case CONDITIONAL:
+ assert rule instanceof ProguardIfRule;
+ throw new Unreachable("-if rule will be evaluated separately, not here.");
}
} else if (rule instanceof ProguardCheckDiscardRule) {
if (memberKeepRules.isEmpty()) {
@@ -220,6 +252,36 @@
}
}
+ private void runPerRule(
+ ExecutorService executorService,
+ List<Future<?>> futures,
+ ProguardConfigurationRule rule,
+ ProguardIfRule ifRule) {
+ List<DexType> specifics = rule.getClassNames().asSpecificDexTypes();
+ if (specifics != null) {
+ // This keep rule only lists specific type matches.
+ // This means there is no need to iterate over all classes.
+ for (DexType type : specifics) {
+ DexClass clazz = application.definitionFor(type);
+ // Ignore keep rule iff it does not reference a class in the app.
+ if (clazz != null) {
+ process(clazz, rule, ifRule);
+ }
+ }
+ } else {
+ futures.add(executorService.submit(() -> {
+ for (DexProgramClass clazz : application.classes()) {
+ process(clazz, rule, ifRule);
+ }
+ if (rule.applyToLibraryClasses()) {
+ for (DexLibraryClass clazz : application.libraryClasses()) {
+ process(clazz, rule, ifRule);
+ }
+ }
+ }));
+ }
+ }
+
public RootSet run(ExecutorService executorService) throws ExecutionException {
application.timing.begin("Build root set...");
try {
@@ -227,28 +289,11 @@
// Mark all the things explicitly listed in keep rules.
if (rules != null) {
for (ProguardConfigurationRule rule : rules) {
- List<DexType> specifics = rule.getClassNames().asSpecificDexTypes();
- if (specifics != null) {
- // This keep rule only lists specific type matches.
- // This means there is no need to iterate over all classes.
- for (DexType type : specifics) {
- DexClass clazz = application.definitionFor(type);
- // Ignore keep rule iff it does not reference a class in the app.
- if (clazz != null) {
- process(clazz, rule);
- }
- }
+ if (rule instanceof ProguardIfRule) {
+ ProguardIfRule ifRule = (ProguardIfRule) rule;
+ ifRules.add(ifRule);
} else {
- futures.add(executorService.submit(() -> {
- for (DexProgramClass clazz : application.classes()) {
- process(clazz, rule);
- }
- if (rule.applyToLibraryClasses()) {
- for (DexLibraryClass clazz : application.libraryClasses()) {
- process(clazz, rule);
- }
- }
- }));
+ runPerRule(executorService, futures, rule, null);
}
}
ThreadUtils.awaitFutures(futures);
@@ -267,33 +312,65 @@
noSideEffects,
assumedValues,
dependentNoShrinking,
- identifierNameStrings);
+ identifierNameStrings,
+ ifRules);
}
- private void markMatchingVisibleMethods(DexClass clazz,
- Collection<ProguardMemberRule> memberKeepRules, ProguardConfigurationRule rule,
+ ConsequentRootSet runForIfRules(
+ ExecutorService executorService,
+ Set<DexType> liveTypes,
+ Set<DexEncodedMethod> liveMethods,
+ Set<DexEncodedField> liveFields) throws ExecutionException {
+ application.timing.begin("Find consequent items for -if rules...");
+ try {
+ List<Future<?>> futures = new ArrayList<>();
+ if (rules != null) {
+ for (ProguardConfigurationRule rule : rules) {
+ assert rule instanceof ProguardIfRule;
+ ProguardIfRule ifRule = (ProguardIfRule) rule;
+ if (ruleSatisfiedWithLiveItems(
+ ifRule, appInfo::definitionFor, liveTypes, liveMethods, liveFields)) {
+ runPerRule(executorService, futures, ifRule.subsequentRule, ifRule);
+ }
+ }
+ ThreadUtils.awaitFutures(futures);
+ }
+ } finally {
+ application.timing.end();
+ }
+ return new ConsequentRootSet(noShrinking, noOptimization, noObfuscation);
+ }
+
+ private void markMatchingVisibleMethods(
+ DexClass clazz,
+ Collection<ProguardMemberRule> memberKeepRules,
+ ProguardConfigurationRule rule,
DexType onlyIfClassKept) {
Set<Wrapper<DexMethod>> methodsMarked = new HashSet<>();
Arrays.stream(clazz.directMethods()).forEach(method ->
- markMethod(method, memberKeepRules, rule, methodsMarked, onlyIfClassKept));
+ markMethod(method, memberKeepRules, methodsMarked, rule, onlyIfClassKept));
while (clazz != null) {
Arrays.stream(clazz.virtualMethods()).forEach(method ->
- markMethod(method, memberKeepRules, rule, methodsMarked, onlyIfClassKept));
+ markMethod(method, memberKeepRules, methodsMarked, rule, onlyIfClassKept));
clazz = clazz.superType == null ? null : application.definitionFor(clazz.superType);
}
}
- private void markMatchingMethods(DexClass clazz,
- Collection<ProguardMemberRule> memberKeepRules, ProguardConfigurationRule rule,
+ private void markMatchingMethods(
+ DexClass clazz,
+ Collection<ProguardMemberRule> memberKeepRules,
+ ProguardConfigurationRule rule,
DexType onlyIfClassKept) {
Arrays.stream(clazz.directMethods()).forEach(method ->
- markMethod(method, memberKeepRules, rule, null, onlyIfClassKept));
+ markMethod(method, memberKeepRules, null, rule, onlyIfClassKept));
Arrays.stream(clazz.virtualMethods()).forEach(method ->
- markMethod(method, memberKeepRules, rule, null, onlyIfClassKept));
+ markMethod(method, memberKeepRules, null, rule, onlyIfClassKept));
}
- private void markMatchingFields(DexClass clazz,
- Collection<ProguardMemberRule> memberKeepRules, ProguardConfigurationRule rule,
+ private void markMatchingFields(
+ DexClass clazz,
+ Collection<ProguardMemberRule> memberKeepRules,
+ ProguardConfigurationRule rule,
DexType onlyIfClassKept) {
clazz.forEachField(field -> markField(field, memberKeepRules, rule, onlyIfClassKept));
}
@@ -341,6 +418,79 @@
out.close();
}
+ private boolean satisfyInheritanceRule(
+ DexType type,
+ Function<DexType, DexClass> definitionFor,
+ ProguardConfigurationRule rule) {
+ DexClass clazz = definitionFor.apply(type);
+ if (clazz == null) {
+ return false;
+ }
+ return
+ anySuperTypeMatches(
+ clazz.superType,
+ definitionFor,
+ rule.getInheritanceClassName(),
+ rule.getInheritanceAnnotation())
+ || anyImplementedInterfaceMatches(
+ clazz,
+ definitionFor,
+ rule.getInheritanceClassName(),
+ rule.getInheritanceAnnotation());
+ }
+
+ private boolean ruleSatisfiedWithLiveItems(
+ ProguardIfRule ifRule,
+ Function<DexType, DexClass> definitionFor,
+ Set<DexType> liveTypes,
+ Set<DexEncodedMethod> liveMethods,
+ Set<DexEncodedField> liveFields) {
+ DexType matchedType = null;
+ Function<DexType, DexClass> definitionForWithLiveTypes = type -> {
+ DexClass clazz = definitionFor.apply(type);
+ if (clazz != null && liveTypes.contains(clazz.type)) {
+ return clazz;
+ }
+ return null;
+ };
+ for (DexType liveType : liveTypes) {
+ if (ifRule.hasInheritanceClassName()) {
+ if (!satisfyInheritanceRule(liveType, definitionForWithLiveTypes, ifRule)) {
+ // Try another live type since the current one doesn't satisfy the inheritance rule.
+ continue;
+ }
+ }
+ if (ifRule.getClassNames().matches(liveType)) {
+ matchedType = liveType;
+ final DexType filterType = matchedType;
+ Collection<ProguardMemberRule> memberKeepRules = ifRule.getMemberRules();
+ Set<DexEncodedMethod> filteredMethods =
+ liveMethods.stream().filter(method -> method.method.getHolder() == filterType)
+ .collect(Collectors.toSet());
+ Set<DexEncodedField> filteredFields =
+ liveFields.stream().filter(field -> field.field.getHolder() == filterType)
+ .collect(Collectors.toSet());
+ boolean notSatisfied = false;
+ for (ProguardMemberRule rule : memberKeepRules) {
+ if (!ruleSatisfiedByMethods(rule, filteredMethods)
+ && !ruleSatisfiedByFields(rule, filteredFields)) {
+ notSatisfied = true;
+ break;
+ }
+ }
+ // Try another live type if the current live type does not satisfy any member rule.
+ if (notSatisfied) {
+ continue;
+ }
+ // TODO(b/73800755): we may need to return the matched live type for back reference.
+ // Maybe, collect all matched live types.
+ return true;
+ }
+ }
+ // No live types satisfy the given -if rule.
+ return false;
+ }
+
private boolean allRulesSatisfied(Collection<ProguardMemberRule> memberKeepRules,
DexClass clazz) {
for (ProguardMemberRule rule : memberKeepRules) {
@@ -362,10 +512,36 @@
|| ruleSatisfiedByFields(rule, clazz.instanceFields());
}
+ private boolean ruleSatisfiedByMethods(
+ ProguardMemberRule rule,
+ Iterable<DexEncodedMethod> methods) {
+ if (rule.getRuleType().includesMethods()) {
+ for (DexEncodedMethod method : methods) {
+ if (rule.matches(method, dexStringCache)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
private boolean ruleSatisfiedByMethods(ProguardMemberRule rule, DexEncodedMethod[] methods) {
if (rule.getRuleType().includesMethods()) {
for (DexEncodedMethod method : methods) {
- if (rule.matches(method, this)) {
+ if (rule.matches(method, dexStringCache)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ private boolean ruleSatisfiedByFields(
+ ProguardMemberRule rule,
+ Iterable<DexEncodedField> fields) {
+ if (rule.getRuleType().includesFields()) {
+ for (DexEncodedField field : fields) {
+ if (rule.matches(field, dexStringCache)) {
return true;
}
}
@@ -376,7 +552,7 @@
private boolean ruleSatisfiedByFields(ProguardMemberRule rule, DexEncodedField[] fields) {
if (rule.getRuleType().includesFields()) {
for (DexEncodedField field : fields) {
- if (rule.matches(field, this)) {
+ if (rule.matches(field, dexStringCache)) {
return true;
}
}
@@ -400,21 +576,18 @@
return false;
}
- private final ConcurrentHashMap<DexString, String> stringCache = new ConcurrentHashMap<>();
-
- public String lookupString(DexString name) {
- return stringCache.computeIfAbsent(name, DexString::toString);
- }
-
- private void markMethod(DexEncodedMethod method, Collection<ProguardMemberRule> rules,
- ProguardConfigurationRule context, Set<Wrapper<DexMethod>> methodsMarked,
- DexType onlyIfClassKept) {
+ private void markMethod(
+ DexEncodedMethod method,
+ Collection<ProguardMemberRule> rules,
+ Set<Wrapper<DexMethod>> methodsMarked,
+ ProguardConfigurationRule context,
+ DexItem precondition) {
if ((methodsMarked != null)
&& methodsMarked.contains(MethodSignatureEquivalence.get().wrap(method.method))) {
return;
}
for (ProguardMemberRule rule : rules) {
- if (rule.matches(method, this)) {
+ if (rule.matches(method, dexStringCache)) {
if (Log.ENABLED) {
Log.verbose(getClass(), "Marking method `%s` due to `%s { %s }`.", method, context,
rule);
@@ -422,20 +595,23 @@
if (methodsMarked != null) {
methodsMarked.add(MethodSignatureEquivalence.get().wrap(method.method));
}
- addItemToSets(method, context, rule, onlyIfClassKept);
+ addItemToSets(method, context, rule, precondition);
}
}
}
- private void markField(DexEncodedField field, Collection<ProguardMemberRule> rules,
- ProguardConfigurationRule context, DexType onlyIfClassKept) {
+ private void markField(
+ DexEncodedField field,
+ Collection<ProguardMemberRule> rules,
+ ProguardConfigurationRule context,
+ DexItem precondition) {
for (ProguardMemberRule rule : rules) {
- if (rule.matches(field, this)) {
+ if (rule.matches(field, dexStringCache)) {
if (Log.ENABLED) {
Log.verbose(getClass(), "Marking field `%s` due to `%s { %s }`.", field, context,
rule);
}
- addItemToSets(field, context, rule, onlyIfClassKept);
+ addItemToSets(field, context, rule, precondition);
}
}
}
@@ -480,14 +656,17 @@
}
}
- private synchronized void addItemToSets(DexItem item, ProguardConfigurationRule context,
- ProguardMemberRule rule, DexType onlyIfClassKept) {
+ private synchronized void addItemToSets(
+ DexItem item,
+ ProguardConfigurationRule context,
+ ProguardMemberRule rule,
+ DexItem precondition) {
if (context instanceof ProguardKeepRule) {
ProguardKeepRule keepRule = (ProguardKeepRule) context;
ProguardKeepRuleModifiers modifiers = keepRule.getModifiers();
if (!modifiers.allowsShrinking) {
- if (onlyIfClassKept != null) {
- dependentNoShrinking.computeIfAbsent(onlyIfClassKept, x -> new IdentityHashMap<>())
+ if (precondition != null) {
+ dependentNoShrinking.computeIfAbsent(precondition, x -> new IdentityHashMap<>())
.put(item, keepRule);
} else {
noShrinking.put(item, keepRule);
@@ -540,6 +719,7 @@
public final Map<DexItem, ProguardMemberRule> assumedValues;
private final Map<DexItem, Map<DexItem, ProguardKeepRule>> dependentNoShrinking;
public final Set<DexItem> identifierNameStrings;
+ public final Set<ProguardIfRule> ifRules;
private boolean isTypeEncodedMethodOrEncodedField(DexItem item) {
assert item instanceof DexType
@@ -573,10 +753,11 @@
Map<DexItem, ProguardMemberRule> noSideEffects,
Map<DexItem, ProguardMemberRule> assumedValues,
Map<DexItem, Map<DexItem, ProguardKeepRule>> dependentNoShrinking,
- Set<DexItem> identifierNameStrings) {
+ Set<DexItem> identifierNameStrings,
+ Set<ProguardIfRule> ifRules) {
this.noShrinking = Collections.unmodifiableMap(noShrinking);
- this.noOptimization = Collections.unmodifiableSet(noOptimization);
- this.noObfuscation = Collections.unmodifiableSet(noObfuscation);
+ this.noOptimization = noOptimization;
+ this.noObfuscation = noObfuscation;
this.reasonAsked = Collections.unmodifiableSet(reasonAsked);
this.keepPackageName = Collections.unmodifiableSet(keepPackageName);
this.checkDiscarded = Collections.unmodifiableSet(checkDiscarded);
@@ -585,6 +766,7 @@
this.assumedValues = Collections.unmodifiableMap(assumedValues);
this.dependentNoShrinking = dependentNoShrinking;
this.identifierNameStrings = Collections.unmodifiableSet(identifierNameStrings);
+ this.ifRules = Collections.unmodifiableSet(ifRules);
assert legalNoObfuscationItems(noObfuscation);
assert legalDependentNoShrinkingItems(dependentNoShrinking);
}
@@ -629,6 +811,7 @@
builder.append("\nassumedValues: " + assumedValues.size());
builder.append("\ndependentNoShrinking: " + dependentNoShrinking.size());
builder.append("\nidentifierNameStrings: " + identifierNameStrings.size());
+ builder.append("\nifRules: " + ifRules.size());
builder.append("\n\nNo Shrinking:");
noShrinking.keySet().stream()
@@ -639,4 +822,20 @@
return builder.toString();
}
}
+
+ // A partial RootSet that becomes live due to the enabled -if rule.
+ static class ConsequentRootSet {
+ final Map<DexItem, ProguardKeepRule> noShrinking;
+ final Set<DexItem> noOptimization;
+ final Set<DexItem> noObfuscation;
+
+ private ConsequentRootSet(
+ Map<DexItem, ProguardKeepRule> noShrinking,
+ Set<DexItem> noOptimization,
+ Set<DexItem> noObfuscation) {
+ this.noShrinking = Collections.unmodifiableMap(noShrinking);
+ this.noOptimization = Collections.unmodifiableSet(noOptimization);
+ this.noObfuscation = Collections.unmodifiableSet(noObfuscation);
+ }
+ }
}
diff --git a/src/test/java/com/android/tools/r8/naming/NamingTestBase.java b/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
index 5b1e7e3..b60ede7 100644
--- a/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
+++ b/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
@@ -30,6 +30,7 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
import java.util.function.BiConsumer;
import org.junit.Before;
@@ -74,10 +75,11 @@
ClassAndMemberPublicizer.run(program, dexItemFactory);
}
- RootSet rootSet = new RootSetBuilder(program, appInfo, configuration.getRules(), options)
- .run(ThreadUtils.getExecutorService(options));
+ ExecutorService executor = ThreadUtils.getExecutorService(1);
+ RootSet rootSet = new RootSetBuilder(appInfo, program, configuration.getRules(), options)
+ .run(executor);
Enqueuer enqueuer = new Enqueuer(appInfo, options, options.forceProguardCompatibility);
- appInfo = enqueuer.traceApplication(rootSet, timing);
+ appInfo = enqueuer.traceApplication(rootSet, executor, timing);
return new Minifier(appInfo.withLiveness(), rootSet, options).run(timing);
}
diff --git a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
index c483267..1631a1f 100644
--- a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
@@ -42,11 +42,13 @@
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
import com.google.common.collect.ImmutableList;
import java.util.Collections;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.stream.Collectors;
import org.junit.Assert;
@@ -103,11 +105,11 @@
DexApplication application = new ApplicationReader(app, options, timing).read().toDirect();
AppInfoWithSubtyping appInfoWithSubtyping = new AppInfoWithSubtyping(application);
- RootSet rootSet = new RootSetBuilder(application, appInfoWithSubtyping,
- buildKeepRuleForClass(Main.class, application.dexItemFactory), options).run(
- Executors.newSingleThreadExecutor());
+ ExecutorService executor = Executors.newSingleThreadExecutor();
+ RootSet rootSet = new RootSetBuilder(appInfoWithSubtyping, application,
+ buildKeepRuleForClass(Main.class, application.dexItemFactory), options).run(executor);
appInfo = new Enqueuer(appInfoWithSubtyping, options, options.forceProguardCompatibility)
- .traceApplication(rootSet, timing);
+ .traceApplication(rootSet, executor, timing);
// We do not run the tree pruner to ensure that the hierarchy is as designed and not modified
// due to liveness.
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
index f9e0422..5dc4ec6 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -1163,10 +1163,7 @@
ProguardConfigurationParser parser =
new ProguardConfigurationParser(new DexItemFactory(), reporter);
parser.parse(proguardConfig);
- assertEquals(3, handler.warnings.size());
- for (int i = 0; i < 3; i++) {
- assertTrue(handler.warnings.get(i).getDiagnosticMessage().contains("Ignoring option: -if"));
- }
+ verifyParserEndsCleanly();
ProguardConfiguration config = parser.getConfig();
// Three -if rules and one independent -keepnames
assertEquals(4, config.getRules().size());
@@ -1196,8 +1193,7 @@
ProguardConfigurationParser parser =
new ProguardConfigurationParser(new DexItemFactory(), reporter);
parser.parse(proguardConfig);
- checkDiagnostic(handler.warnings, proguardConfig, 1, 1,
- "Ignoring", "-if");
+ verifyParserEndsCleanly();
ProguardConfiguration config = parser.getConfig();
assertEquals(1, config.getRules().size());
ProguardIfRule if0 = (ProguardIfRule) config.getRules().get(0);
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatabilityTestBase.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatabilityTestBase.java
index 004c31f..3976f89 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatabilityTestBase.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatabilityTestBase.java
@@ -1,9 +1,12 @@
// Copyright (c) 2018, 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.forceproguardcompatibility;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+
import com.android.tools.r8.CompatProguardCommandBuilder;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.R8Command;
@@ -12,6 +15,7 @@
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
import com.android.tools.r8.utils.FileUtils;
import com.google.common.collect.ImmutableList;
import java.io.File;
@@ -113,4 +117,20 @@
throws Exception {
return runProguard6AndD8(programClasses, keepMainProguardConfiguration(mainClass));
}
+
+ protected void verifyClassesPresent(
+ DexInspector dexInspector, Class<?>... classesOfInterest) {
+ for (Class klass : classesOfInterest) {
+ ClassSubject c = dexInspector.clazz(klass);
+ assertThat(c, isPresent());
+ }
+ }
+
+ protected void verifyClassesAbsent(
+ DexInspector dexInspector, Class<?>... classesOfInterest) {
+ for (Class klass : classesOfInterest) {
+ ClassSubject c = dexInspector.clazz(klass);
+ assertThat(c, not(isPresent()));
+ }
+ }
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAnnotationTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAnnotationTest.java
new file mode 100644
index 0000000..080df2a
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAnnotationTest.java
@@ -0,0 +1,91 @@
+// Copyright (c) 2018, 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.ifrule;
+
+import com.android.tools.r8.shaking.forceproguardcompatibility.ProguardCompatabilityTestBase;
+import com.android.tools.r8.utils.DexInspector;
+import com.google.common.collect.ImmutableList;
+import java.util.Collection;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class IfOnAnnotationTest extends ProguardCompatabilityTestBase {
+ private final static List<Class> CLASSES = ImmutableList.of(
+ UsedAnnotation.class, UnusedAnnotation.class,
+ UsedAnnotationDependent.class, UnusedAnnotationDependent.class,
+ AnnotationUser.class, MainUsesAnnotationUser.class);
+
+ private final Shrinker shrinker;
+
+ public IfOnAnnotationTest(Shrinker shrinker) {
+ this.shrinker = shrinker;
+ }
+
+ @Parameters(name = "shrinker: {0}")
+ public static Collection<Object> data() {
+ return ImmutableList.of(Shrinker.PROGUARD6, Shrinker.R8);
+ }
+
+ @Test
+ public void ifOnAnnotation_withoutNthWildcard() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-keepattributes *Annotation*",
+ "-keep class **.Main* {",
+ " public static void main(java.lang.String[]);",
+ "}",
+ // @UsedAnnotation <methods> -> UsedAnnotationDependent
+ "-if class **.*User {",
+ " @**.UsedAnnotation <methods>;",
+ "}",
+ "-keep class **.UsedAnnotation*",
+ // @UnusedAnnotation <methods> -> UnusedAnnotationDependent
+ "-if class **.*User {",
+ " @**.UnusedAnnotation <methods>;",
+ "}",
+ "-keep class **.UnusedAnnotation*"
+ );
+
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ verifyClassesAbsent(dexInspector,
+ UnusedAnnotation.class, UnusedAnnotationDependent.class);
+ verifyClassesPresent(dexInspector,
+ UsedAnnotation.class, UsedAnnotationDependent.class);
+ }
+
+ @Test
+ public void ifOnAnnotation_withNthWildcard() throws Exception {
+ // TODO(b/73800755): not implemented yet.
+ if (shrinker == Shrinker.R8) {
+ return;
+ }
+
+ List<String> config = ImmutableList.of(
+ "-keepattributes *Annotation*",
+ "-keep class **.Main* {",
+ " public static void main(java.lang.String[]);",
+ "}",
+ // @UsedAnnotation <methods> -> UsedAnnotationDependent
+ "-if class **.*User {",
+ " @<1>.Used<2> <methods>;",
+ "}",
+ "-keep class <1>.Used<2>*",
+ // @UnusedAnnotation <methods> -> UnusedAnnotationDependent
+ "-if class **.*User {",
+ " @<1>.Unused<2> <methods>;",
+ "}",
+ "-keep class <1>.Unused<2>*"
+ );
+
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ verifyClassesAbsent(dexInspector,
+ UnusedAnnotation.class, UnusedAnnotationDependent.class);
+ verifyClassesPresent(dexInspector,
+ UsedAnnotation.class, UsedAnnotationDependent.class);
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAnnotationTestClasses.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAnnotationTestClasses.java
new file mode 100644
index 0000000..56fb4c1
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAnnotationTestClasses.java
@@ -0,0 +1,39 @@
+// Copyright (c) 2018, 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.ifrule;
+
+@interface UsedAnnotation {
+ // Intentionally left empty.
+}
+
+@interface UnusedAnnotation {
+ // Intentionally left empty.
+}
+
+class UsedAnnotationDependent {
+ // Intentionally left empty.
+}
+
+class UnusedAnnotationDependent {
+ // Intentionally left empty.
+}
+
+class AnnotationUser {
+ private int intField;
+
+ @UsedAnnotation void live() {
+ System.out.println("live(" + intField++ + ")");
+ }
+
+ @UnusedAnnotation void dead() {
+ throw new AssertionError("Should be removed.");
+ }
+}
+
+class MainUsesAnnotationUser {
+ public static void main(String[] args) {
+ AnnotationUser user = new AnnotationUser();
+ user.live();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnClassTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnClassTest.java
index 5f3cae1..c26f43f 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnClassTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnClassTest.java
@@ -78,7 +78,44 @@
}
@Test
- public void ifThenKeep() throws Exception {
+ public void ifThenKeep_withoutNthWildcard() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-if class **.Precondition",
+ "-keep,allowobfuscation class **.*User",
+ "-if class **.DependentUser",
+ "-keep,allowobfuscation class **.Dependent"
+ );
+
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ if (!keepPrecondition) {
+ // TODO(b/73708139): Proguard6 kept Dependent (w/o any members), which is not necessary.
+ if (shrinker == Shrinker.PROGUARD6) {
+ return;
+ }
+ assertEquals(1, dexInspector.allClasses().size());
+ return;
+ }
+
+ ClassSubject clazz = dexInspector.clazz(DependentUser.class);
+ assertThat(clazz, isRenamed());
+ // Members of DependentUser are not used anywhere.
+ MethodSubject m = clazz.method("void", "callFoo", ImmutableList.of());
+ assertThat(m, not(isPresent()));
+ FieldSubject f = clazz.field("int", "canBeShrinked");
+ assertThat(f, not(isPresent()));
+
+ // Although DependentUser#callFoo is shrinked, Dependent is kept via -if.
+ clazz = dexInspector.clazz(Dependent.class);
+ assertThat(clazz, isRenamed());
+ // But, its members are gone.
+ m = clazz.method("java.lang.String", "foo", ImmutableList.of());
+ assertThat(m, not(isPresent()));
+ f = clazz.field("int", "intField");
+ assertThat(f, not(isPresent()));
+ }
+
+ @Test
+ public void ifThenKeep_withNthWildcard() throws Exception {
List<String> config = ImmutableList.of(
"-if class **.Precondition",
"-keep,allowobfuscation class **.*User",
@@ -96,7 +133,7 @@
return;
}
- // TODO(b/73708139): not implemented yet.
+ // TODO(b/73800755): not implemented yet.
if (shrinker == Shrinker.R8) {
return;
}
@@ -134,8 +171,43 @@
return;
}
- // TODO(b/73708139): not implemented yet.
- if (shrinker == Shrinker.R8) {
+ ClassSubject clazz = dexInspector.clazz(DependentUser.class);
+ assertThat(clazz, isRenamed());
+ MethodSubject m = clazz.method("void", "callFoo", ImmutableList.of());
+ assertThat(m, isRenamed());
+ FieldSubject f = clazz.field("int", "canBeShrinked");
+ assertThat(f, not(isPresent()));
+
+ // Dependent is kept due to DependentUser#callFoo, but renamed.
+ clazz = dexInspector.clazz(Dependent.class);
+ assertThat(clazz, isRenamed());
+ m = clazz.method("java.lang.String", "foo", ImmutableList.of());
+ assertThat(m, isRenamed());
+ f = clazz.field("int", "intField");
+ assertThat(f, isRenamed());
+ }
+
+ @Test
+ public void ifThenKeepClassMembers_withoutNthWildcard() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-if class **.Precondition",
+ "-keepclassmembers,allowobfuscation class **.*User {",
+ " static void callFoo(...);",
+ "}",
+ // If the member is kept, keep the enclosing class too.
+ "-if class **.*User {",
+ " static void callFoo(...);",
+ "}",
+ "-keep,allowobfuscation class **.*User"
+ );
+
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ if (!keepPrecondition) {
+ // TODO(b/73708139): Proguard6 kept DependentUser (w/o any members), which is not necessary.
+ if (shrinker == Shrinker.PROGUARD6) {
+ return;
+ }
+ assertEquals(1, dexInspector.allClasses().size());
return;
}
@@ -156,15 +228,15 @@
}
@Test
- public void ifThenKeepClassMembers() throws Exception {
+ public void ifThenKeepClassMembers_withNthWildcard() throws Exception {
List<String> config = ImmutableList.of(
"-if class **.Precondition",
"-keepclassmembers,allowobfuscation class **.*User {",
" static void callFoo(...);",
"}",
- // If members are kept, keep the enclosing class too.
+ // If the member is kept, keep the enclosing class too.
"-if class **.*User {",
- " <methods>;",
+ " static void callFoo(...);",
"}",
"-keep,allowobfuscation class <1>.<2>User"
);
@@ -179,7 +251,7 @@
return;
}
- // TODO(b/73708139): not implemented yet.
+ // TODO(b/73800755): not implemented yet.
if (shrinker == Shrinker.R8) {
return;
}
@@ -214,11 +286,6 @@
DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
- // TODO(b/73708139): not implemented yet.
- if (shrinker == Shrinker.R8) {
- return;
- }
-
ClassSubject clazz = dexInspector.clazz(Dependent.class);
// Only class name is not renamed, if triggered.
assertThat(clazz, keepPrecondition ? isNotRenamed() : isRenamed());
@@ -244,11 +311,6 @@
DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
- // TODO(b/73708139): not implemented yet.
- if (shrinker == Shrinker.R8) {
- return;
- }
-
ClassSubject clazz = dexInspector.clazz(Dependent.class);
// Class name is not renamed, if triggered.
assertThat(clazz, keepPrecondition ? isNotRenamed() : isRenamed());
@@ -275,11 +337,6 @@
DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
- // TODO(b/73708139): not implemented yet.
- if (shrinker == Shrinker.R8) {
- return;
- }
-
ClassSubject clazz = dexInspector.clazz(Dependent.class);
assertThat(clazz, isRenamed());
MethodSubject m = clazz.method("java.lang.String", "foo", ImmutableList.of());
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnFieldTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnFieldTest.java
index 07e94cd..2bf8a27 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnFieldTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnFieldTest.java
@@ -3,15 +3,11 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.shaking.ifrule;
-import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
-import static org.hamcrest.CoreMatchers.not;
-import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import com.android.tools.r8.shaking.forceproguardcompatibility.ProguardCompatabilityTestBase;
import com.android.tools.r8.utils.DexInspector;
-import com.android.tools.r8.utils.DexInspector.ClassSubject;
import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.Collection;
@@ -27,7 +23,8 @@
D.class, D1.class, D2.class,
R.class, R1.class, R2.class,
MainWithInner.InnerR.class, MainWithInner.InnerD.class,
- MainUsesR.class, MainWithIf.class, MainWithInner.class);
+ I.class, Impl.class,
+ MainUsesR.class, MainWithIf.class, MainWithInner.class, MainUsesImpl.class);
private final Shrinker shrinker;
@@ -59,24 +56,39 @@
return super.runProguard6(programClasses, adaptConfiguration(proguardConfig));
}
- private void verifyClassesPresent(
- DexInspector dexInspector, Class<?>... classesOfInterest) {
- for (Class klass : classesOfInterest) {
- ClassSubject c = dexInspector.clazz(klass);
- assertThat(c, isPresent());
- }
- }
+ @Test
+ public void ifOnField_withoutNthWildcard() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-keep class **.MainUsesR {",
+ " public static void main(java.lang.String[]);",
+ "}",
+ // R.id1 -> D1
+ "-if class **.R {",
+ " public static int id1;",
+ "}",
+ "-keep class **.D1",
+ // R.id2 -> D2
+ "-if class **.R {",
+ " public static int id2;",
+ "}",
+ "-keep class **.D2",
+ // R.id1 && R.id2 -> D
+ "-if class **.R {",
+ " public static int id1;",
+ " public static int id2;",
+ "}",
+ "-keep class **.D"
+ );
- private void verifyClassesAbsent(
- DexInspector dexInspector, Class<?>... classesOfInterest) {
- for (Class klass : classesOfInterest) {
- ClassSubject c = dexInspector.clazz(klass);
- assertThat(c, not(isPresent()));
- }
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ verifyClassesAbsent(dexInspector,
+ R1.class, R2.class, D.class, D2.class);
+ verifyClassesPresent(dexInspector,
+ R.class, D1.class);
}
@Test
- public void ifOnField() throws Exception {
+ public void ifOnField_withNthWildcard() throws Exception {
// TODO(b/73800755): not implemented yet.
if (shrinker == Shrinker.R8) {
return;
@@ -87,13 +99,9 @@
" public static void main(java.lang.String[]);",
"}",
"-if class **.R {",
- " public static int id1;",
+ " public static int id?;",
"}",
- "-keep class **.D1",
- "-if class **.R {",
- " public static int id2;",
- "}",
- "-keep class **.D2"
+ "-keep class **.D<2>"
);
DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
@@ -101,26 +109,33 @@
R1.class, R2.class, D.class, D2.class);
verifyClassesPresent(dexInspector,
R.class, D1.class);
-
- config = ImmutableList.of(
- "-keep class **.MainUsesR {",
- " public static void main(java.lang.String[]);",
- "}",
- "-if class **.R {",
- " public static int id?;",
- "}",
- "-keep class **.D<2>"
- );
-
- dexInspector = runShrinker(shrinker, CLASSES, config);
- verifyClassesAbsent(dexInspector,
- R1.class, R2.class, D.class, D2.class);
- verifyClassesPresent(dexInspector,
- R.class, D1.class);
}
@Test
- public void ifOnFieldWithCapture() throws Exception {
+ public void ifOnFieldWithCapture_withoutNthWildcard() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-keep class **.MainWithIf {",
+ " public static void main(java.lang.String[]);",
+ "}",
+ "-if class **.R1 {",
+ " public static int id*;",
+ "}",
+ "-keep class **.D1",
+ "-if class **.R2 {",
+ " public static int id*;",
+ "}",
+ "-keep class **.D2"
+ );
+
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ verifyClassesAbsent(dexInspector,
+ R.class, D.class, R1.class, D1.class);
+ verifyClassesPresent(dexInspector,
+ R2.class, D2.class);
+ }
+
+ @Test
+ public void ifOnFieldWithCapture_withNthWildcard() throws Exception {
// TODO(b/73800755): not implemented yet.
if (shrinker == Shrinker.R8) {
return;
@@ -131,8 +146,7 @@
" public static void main(java.lang.String[]);",
"}",
"-if class **.R* {",
- " public static int id1;",
- " public static int id2;",
+ " public static int id*;",
"}",
"-keep class **.D<2>"
);
@@ -145,7 +159,27 @@
}
@Test
- public void ifOnFieldWithInner() throws Exception {
+ public void ifOnFieldWithInner_withoutNthWildcard() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-keep class **.MainWithInner {",
+ " public static void main(java.lang.String[]);",
+ "}",
+ "-if class **$*R {",
+ " public static int id1;",
+ " public static int id2;",
+ "}",
+ "-keep class **$*D"
+ );
+
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ verifyClassesAbsent(dexInspector,
+ R.class, D.class, R1.class, D1.class, R2.class, D2.class);
+ verifyClassesPresent(dexInspector,
+ MainWithInner.InnerR.class, MainWithInner.InnerD.class);
+ }
+
+ @Test
+ public void ifOnFieldWithInner_withNthWildcard() throws Exception {
// TODO(b/73800755): not implemented yet.
if (shrinker == Shrinker.R8) {
return;
@@ -196,4 +230,53 @@
}
}
+ @Test
+ public void ifOnFieldInImplementer_withoutNthWildcard() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-keep class **.MainUsesImpl {",
+ " public static void main(java.lang.String[]);",
+ "}",
+ "-if class ** implements **.I {",
+ " private <fields>;",
+ "}",
+ "-keep class **.D1",
+ "-if class ** implements **.I {",
+ " public <fields>;",
+ "}",
+ "-keep class **.D2"
+ );
+
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ verifyClassesAbsent(dexInspector, D2.class);
+ verifyClassesPresent(dexInspector,
+ I.class, Impl.class, D1.class);
+ }
+
+ @Test
+ public void ifOnFieldInImplementer_withNthWildcard() throws Exception {
+ // TODO(b/73800755): not implemented yet.
+ if (shrinker == Shrinker.R8) {
+ return;
+ }
+
+ List<String> config = ImmutableList.of(
+ "-keep class **.MainUsesImpl {",
+ " public static void main(java.lang.String[]);",
+ "}",
+ "-if class ** implements **.I {",
+ " private <fields>;",
+ "}",
+ "-keep class <2>.D1",
+ "-if class ** implements **.I {",
+ " public <fields>;",
+ "}",
+ "-keep class <2>.D2"
+ );
+
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ verifyClassesAbsent(dexInspector, D2.class);
+ verifyClassesPresent(dexInspector,
+ I.class, Impl.class, D1.class);
+ }
+
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnFieldTestClasses.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnFieldTestClasses.java
index 892b63e..bd8e5c2 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnFieldTestClasses.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnFieldTestClasses.java
@@ -27,6 +27,21 @@
public static int id2 = 2;
}
+interface I {
+ void ack();
+}
+
+class Impl implements I {
+ private int usedPrivateIntField;
+ private int unusedPrivateIntField;
+ public int unusedPublicIntField;
+ public String unusedPublicStringField;
+
+ public void ack() {
+ System.out.println("ack(" + usedPrivateIntField++ + ")");
+ }
+}
+
class MainUsesR {
public static void main(String[] args) {
System.out.println(R.id1);
@@ -62,3 +77,9 @@
}
}
+class MainUsesImpl {
+ public static void main(String[] args) {
+ I instance = new Impl();
+ instance.ack();
+ }
+}
