Support for retaining annotations on items subject to optimization
Bug: 190469643
Change-Id: I6d98297a49366a32770e6df6e8b48810fd7bafa7
diff --git a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
index 6a4089d..fdf4ea0 100644
--- a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
+++ b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
@@ -265,15 +265,17 @@
private void removeAnnotations(ProgramDefinition definition, KeepInfo<?, ?> keepInfo) {
boolean isAnnotation =
definition.isProgramClass() && definition.asProgramClass().isAnnotation();
- if ((options.isForceProguardCompatibilityEnabled() || keepInfo.isPinned())) {
- definition.rewriteAllAnnotations(
- (annotation, kind) -> rewriteAnnotation(definition, annotation, kind));
- } else if (!isAnnotation) {
- definition.clearAllAnnotations();
+ if (keepInfo.isAnnotationRemovalAllowed(options)) {
+ if (isAnnotation) {
+ definition.rewriteAllAnnotations(
+ (annotation, isParameterAnnotation) ->
+ shouldRetainAnnotationOnAnnotationClass(annotation) ? annotation : null);
+ } else {
+ definition.clearAllAnnotations();
+ }
} else {
definition.rewriteAllAnnotations(
- (annotation, isParameterAnnotation) ->
- shouldRetainAnnotationOnAnnotationClass(annotation) ? annotation : null);
+ (annotation, kind) -> rewriteAnnotation(definition, annotation, kind));
}
}
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 1e2cab6..b1c0f56 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -3036,7 +3036,12 @@
// 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.
+ assert appView.options().isAnnotationRemovalEnabled()
+ || rootSet.noAnnotationRemoval.isEmpty();
assert appView.options().isMinificationEnabled() || rootSet.noObfuscation.isEmpty();
+ for (DexReference reference : rootSet.noAnnotationRemoval) {
+ keepInfo.evaluateRule(reference, appInfo, Joiner::disallowAnnotationRemoval);
+ }
for (DexReference reference : rootSet.noObfuscation) {
keepInfo.evaluateRule(reference, appInfo, Joiner::disallowMinification);
}
@@ -3114,12 +3119,15 @@
joiner.requireAccessModificationForRepackaging();
}
}
- if (!modifiers.allowsObfuscation) {
- joiner.disallowMinification();
- }
if (!modifiers.allowsAccessModification) {
joiner.disallowAccessModification();
}
+ if (!modifiers.allowsAnnotationRemoval) {
+ joiner.disallowAnnotationRemoval();
+ }
+ if (!modifiers.allowsObfuscation) {
+ joiner.disallowMinification();
+ }
}
}
@@ -3733,6 +3741,9 @@
// TODO(b/132600955): This modifies the root set. Should the consequent be persistent?
rootSet.addConsequentRootSet(consequentRootSet, addNoShrinking);
if (mode.isInitialTreeShaking()) {
+ for (DexReference reference : consequentRootSet.noAnnotationRemoval) {
+ keepInfo.evaluateRule(reference, appView, Joiner::disallowAnnotationRemoval);
+ }
for (DexReference reference : consequentRootSet.noObfuscation) {
keepInfo.evaluateRule(reference, appView, Joiner::disallowMinification);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/GlobalKeepInfoConfiguration.java b/src/main/java/com/android/tools/r8/shaking/GlobalKeepInfoConfiguration.java
index e509230..cd2df59 100644
--- a/src/main/java/com/android/tools/r8/shaking/GlobalKeepInfoConfiguration.java
+++ b/src/main/java/com/android/tools/r8/shaking/GlobalKeepInfoConfiguration.java
@@ -6,6 +6,8 @@
/** Globally controlled settings that affect the default values for kept items. */
public interface GlobalKeepInfoConfiguration {
+ boolean isAnnotationRemovalEnabled();
+
boolean isTreeShakingEnabled();
boolean isMinificationEnabled();
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 acf36aa..559362a 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
@@ -11,32 +11,55 @@
public abstract class KeepInfo<B extends Builder<B, K>, K extends KeepInfo<B, K>> {
private final boolean pinned;
- private final boolean allowMinification;
private final boolean allowAccessModification;
+ private final boolean allowAnnotationRemoval;
+ private final boolean allowMinification;
private final boolean requireAccessModificationForRepackaging;
private KeepInfo(
boolean pinned,
- boolean allowMinification,
boolean allowAccessModification,
+ boolean allowAnnotationRemoval,
+ boolean allowMinification,
boolean requireAccessModificationForRepackaging) {
this.pinned = pinned;
- this.allowMinification = allowMinification;
this.allowAccessModification = allowAccessModification;
+ this.allowAnnotationRemoval = allowAnnotationRemoval;
+ this.allowMinification = allowMinification;
this.requireAccessModificationForRepackaging = requireAccessModificationForRepackaging;
}
KeepInfo(B builder) {
this(
builder.isPinned(),
- builder.isMinificationAllowed(),
builder.isAccessModificationAllowed(),
+ builder.isAnnotationRemovalAllowed(),
+ builder.isMinificationAllowed(),
builder.isAccessModificationRequiredForRepackaging());
}
abstract B builder();
- /** True if an item must be present in the output. */
+ /**
+ * True if an item may have all of its annotations removed.
+ *
+ * <p>If this returns false, some annotations may still be removed if the configuration does not
+ * keep all annotation attributes.
+ */
+ public boolean isAnnotationRemovalAllowed(GlobalKeepInfoConfiguration configuration) {
+ return configuration.isAnnotationRemovalEnabled() && internalIsAnnotationRemovalAllowed();
+ }
+
+ boolean internalIsAnnotationRemovalAllowed() {
+ return allowAnnotationRemoval;
+ }
+
+ /**
+ * True if an item must be present in the output.
+ *
+ * @deprecated Prefer task dependent predicates.
+ */
+ @Deprecated
public boolean isPinned() {
return pinned;
}
@@ -151,8 +174,9 @@
private K original;
private boolean pinned;
- private boolean allowMinification;
private boolean allowAccessModification;
+ private boolean allowAnnotationRemoval;
+ private boolean allowMinification;
private boolean requireAccessModificationForRepackaging;
Builder() {
@@ -162,25 +186,28 @@
Builder(K original) {
this.original = original;
pinned = original.isPinned();
- allowMinification = original.internalIsMinificationAllowed();
allowAccessModification = original.internalIsAccessModificationAllowed();
+ allowAnnotationRemoval = original.internalIsAnnotationRemovalAllowed();
+ allowMinification = original.internalIsMinificationAllowed();
requireAccessModificationForRepackaging =
original.internalIsAccessModificationRequiredForRepackaging();
}
B makeTop() {
pin();
+ disallowAccessModification();
+ disallowAnnotationRemoval();
disallowMinification();
requireAccessModificationForRepackaging();
- disallowAccessModification();
return self();
}
B makeBottom() {
unpin();
+ allowAccessModification();
+ allowAnnotationRemoval();
allowMinification();
unsetRequireAccessModificationForRepackaging();
- allowAccessModification();
return self();
}
@@ -201,10 +228,11 @@
private boolean internalIsEqualTo(K other) {
return isPinned() == other.isPinned()
+ && isAccessModificationAllowed() == other.internalIsAccessModificationAllowed()
+ && isAnnotationRemovalAllowed() == other.internalIsAnnotationRemovalAllowed()
&& isMinificationAllowed() == other.internalIsMinificationAllowed()
&& isAccessModificationRequiredForRepackaging()
== other.internalIsAccessModificationRequiredForRepackaging()
- && isAccessModificationAllowed() == other.internalIsAccessModificationAllowed()
&& isEqualTo(other);
}
@@ -212,10 +240,6 @@
return pinned;
}
- public boolean isMinificationAllowed() {
- return allowMinification;
- }
-
public boolean isAccessModificationRequiredForRepackaging() {
return requireAccessModificationForRepackaging;
}
@@ -224,6 +248,14 @@
return allowAccessModification;
}
+ public boolean isAnnotationRemovalAllowed() {
+ return allowAnnotationRemoval;
+ }
+
+ public boolean isMinificationAllowed() {
+ return allowMinification;
+ }
+
public B setPinned(boolean pinned) {
this.pinned = pinned;
return self();
@@ -276,6 +308,19 @@
public B disallowAccessModification() {
return setAllowAccessModification(false);
}
+
+ public B setAllowAnnotationRemoval(boolean allowAnnotationRemoval) {
+ this.allowAnnotationRemoval = allowAnnotationRemoval;
+ return self();
+ }
+
+ public B allowAnnotationRemoval() {
+ return setAllowAnnotationRemoval(true);
+ }
+
+ public B disallowAnnotationRemoval() {
+ return setAllowAnnotationRemoval(false);
+ }
}
/** Joiner to construct monotonically increasing keep info object. */
@@ -304,13 +349,18 @@
return self();
}
- public J disallowMinification() {
- builder.disallowMinification();
+ public J disallowAccessModification() {
+ builder.disallowAccessModification();
return self();
}
- public J disallowAccessModification() {
- builder.disallowAccessModification();
+ public J disallowAnnotationRemoval() {
+ builder.disallowAnnotationRemoval();
+ return self();
+ }
+
+ public J disallowMinification() {
+ builder.disallowMinification();
return self();
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
index 49c4a1e..948caf2 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
@@ -388,6 +388,12 @@
// TODO(b/189807246): This should be removed.
modifiers.setAllowsOptimization(true);
modifiers.setAllowsObfuscation(isObfuscating());
+
+ // In non-compatibility mode, adding -dontoptimize does not cause all annotations
+ // to be retained.
+ if (!forceProguardCompatibility && isShrinking()) {
+ modifiers.setAllowsAnnotationRemoval(true);
+ }
});
addRule(rule);
this.keepAllRule = rule;
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 ff71812..3072667 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -1024,6 +1024,10 @@
builder.getModifiersBuilder().setAllowsObfuscation(true);
} else if (acceptString("accessmodification")) {
builder.getModifiersBuilder().setAllowsAccessModification(true);
+ } else if (allowTestOptions) {
+ if (acceptString("annotationremoval")) {
+ builder.getModifiersBuilder().setAllowsAnnotationRemoval(true);
+ }
}
} else if (acceptString("includedescriptorclasses")) {
builder.getModifiersBuilder().setIncludeDescriptorClasses(true);
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleModifiers.java b/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleModifiers.java
index a02117d..63c0787 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleModifiers.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardKeepRuleModifiers.java
@@ -7,6 +7,7 @@
public static class Builder {
private boolean allowsAccessModification = false;
+ private boolean allowsAnnotationRemoval = false;
private boolean allowsShrinking = false;
private boolean allowsOptimization = false;
private boolean allowsObfuscation = false;
@@ -19,6 +20,11 @@
return this;
}
+ public Builder setAllowsAnnotationRemoval(boolean allowsAnnotationRemoval) {
+ this.allowsAnnotationRemoval = allowsAnnotationRemoval;
+ return this;
+ }
+
public void setAllowsShrinking(boolean allowsShrinking) {
this.allowsShrinking = allowsShrinking;
}
@@ -39,6 +45,7 @@
ProguardKeepRuleModifiers build() {
return new ProguardKeepRuleModifiers(
allowsAccessModification,
+ allowsAnnotationRemoval,
allowsShrinking,
allowsOptimization,
allowsObfuscation,
@@ -47,6 +54,7 @@
}
public final boolean allowsAccessModification;
+ public final boolean allowsAnnotationRemoval;
public final boolean allowsShrinking;
public final boolean allowsOptimization;
public final boolean allowsObfuscation;
@@ -54,11 +62,13 @@
private ProguardKeepRuleModifiers(
boolean allowsAccessModification,
+ boolean allowsAnnotationRemoval,
boolean allowsShrinking,
boolean allowsOptimization,
boolean allowsObfuscation,
boolean includeDescriptorClasses) {
this.allowsAccessModification = allowsAccessModification;
+ this.allowsAnnotationRemoval = allowsAnnotationRemoval;
this.allowsShrinking = allowsShrinking;
this.allowsOptimization = allowsOptimization;
this.allowsObfuscation = allowsObfuscation;
@@ -79,6 +89,7 @@
}
ProguardKeepRuleModifiers that = (ProguardKeepRuleModifiers) o;
return allowsAccessModification == that.allowsAccessModification
+ && allowsAnnotationRemoval == that.allowsAnnotationRemoval
&& allowsShrinking == that.allowsShrinking
&& allowsOptimization == that.allowsOptimization
&& allowsObfuscation == that.allowsObfuscation
@@ -88,16 +99,18 @@
@Override
public int hashCode() {
return (allowsAccessModification ? 1 : 0)
- | (allowsShrinking ? 2 : 0)
- | (allowsOptimization ? 4 : 0)
- | (allowsObfuscation ? 8 : 0)
- | (includeDescriptorClasses ? 16 : 0);
+ | (allowsAnnotationRemoval ? 2 : 0)
+ | (allowsShrinking ? 4 : 0)
+ | (allowsOptimization ? 8 : 0)
+ | (allowsObfuscation ? 16 : 0)
+ | (includeDescriptorClasses ? 32 : 0);
}
@Override
public String toString() {
StringBuilder builder = new StringBuilder();
appendWithComma(builder, allowsAccessModification, "allowaccessmodification");
+ appendWithComma(builder, allowsAnnotationRemoval, "allowannotationremoval");
appendWithComma(builder, allowsObfuscation, "allowobfuscation");
appendWithComma(builder, allowsShrinking, "allowshrinking");
appendWithComma(builder, allowsOptimization, "allowoptimization");
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java b/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
index e4890a0..03ed0ad 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
@@ -91,6 +91,7 @@
private final Iterable<? extends ProguardConfigurationRule> rules;
private final MutableItemsWithRules noShrinking = new MutableItemsWithRules();
private final MutableItemsWithRules softPinned = new MutableItemsWithRules();
+ private final Set<DexReference> noAnnotationRemoval = Sets.newIdentityHashSet();
private final Set<DexReference> noObfuscation = Sets.newIdentityHashSet();
private final LinkedHashMap<DexReference, DexReference> reasonAsked = new LinkedHashMap<>();
private final LinkedHashMap<DexReference, DexReference> checkDiscarded = new LinkedHashMap<>();
@@ -346,10 +347,12 @@
assert Sets.intersection(neverInline, alwaysInline).isEmpty()
&& Sets.intersection(neverInline, forceInline).isEmpty()
: "A method cannot be marked as both -neverinline and -forceinline/-alwaysinline.";
+ assert appView.options().isAnnotationRemovalEnabled() || noAnnotationRemoval.isEmpty();
assert appView.options().isMinificationEnabled() || noObfuscation.isEmpty();
return new RootSet(
noShrinking,
softPinned,
+ noAnnotationRemoval,
noObfuscation,
ImmutableList.copyOf(reasonAsked.values()),
ImmutableList.copyOf(checkDiscarded.values()),
@@ -447,6 +450,7 @@
neverClassInline,
noShrinking,
softPinned,
+ noAnnotationRemoval,
noObfuscation,
dependentNoShrinking,
dependentSoftPinned,
@@ -1188,6 +1192,10 @@
context.markAsUsed();
}
+ if (appView.options().isAnnotationRemovalEnabled() && !modifiers.allowsAnnotationRemoval) {
+ noAnnotationRemoval.add(item.getReference());
+ context.markAsUsed();
+ }
if (appView.options().isMinificationEnabled() && !modifiers.allowsObfuscation) {
noObfuscation.add(item.getReference());
context.markAsUsed();
@@ -1410,6 +1418,7 @@
final Set<DexType> neverClassInline;
final MutableItemsWithRules noShrinking;
final MutableItemsWithRules softPinned;
+ final Set<DexReference> noAnnotationRemoval;
final Set<DexReference> noObfuscation;
final Map<DexReference, MutableItemsWithRules> dependentNoShrinking;
final Map<DexReference, MutableItemsWithRules> dependentSoftPinned;
@@ -1422,6 +1431,7 @@
Set<DexType> neverClassInline,
MutableItemsWithRules noShrinking,
MutableItemsWithRules softPinned,
+ Set<DexReference> noAnnotationRemoval,
Set<DexReference> noObfuscation,
Map<DexReference, MutableItemsWithRules> dependentNoShrinking,
Map<DexReference, MutableItemsWithRules> dependentSoftPinned,
@@ -1432,6 +1442,7 @@
this.neverClassInline = neverClassInline;
this.noShrinking = noShrinking;
this.softPinned = softPinned;
+ this.noAnnotationRemoval = noAnnotationRemoval;
this.noObfuscation = noObfuscation;
this.dependentNoShrinking = dependentNoShrinking;
this.dependentSoftPinned = dependentSoftPinned;
@@ -1835,6 +1846,7 @@
private RootSet(
MutableItemsWithRules noShrinking,
MutableItemsWithRules softPinned,
+ Set<DexReference> noAnnotationRemoval,
Set<DexReference> noObfuscation,
ImmutableList<DexReference> reasonAsked,
ImmutableList<DexReference> checkDiscarded,
@@ -1869,6 +1881,7 @@
neverClassInline,
noShrinking,
softPinned,
+ noAnnotationRemoval,
noObfuscation,
dependentNoShrinking,
dependentSoftPinned,
@@ -1916,6 +1929,7 @@
neverInline.addAll(consequentRootSet.neverInline);
neverInlineDueToSingleCaller.addAll(consequentRootSet.neverInlineDueToSingleCaller);
neverClassInline.addAll(consequentRootSet.neverClassInline);
+ noAnnotationRemoval.addAll(consequentRootSet.noAnnotationRemoval);
noObfuscation.addAll(consequentRootSet.noObfuscation);
if (addNoShrinking) {
noShrinking.addAll(consequentRootSet.noShrinking);
@@ -1945,6 +1959,9 @@
if (noShrinking.containsReference(original)) {
noShrinking.putReferenceWithRules(rewritten, noShrinking.getRulesForReference(original));
}
+ if (noAnnotationRemoval.contains(original)) {
+ noAnnotationRemoval.add(rewritten);
+ }
if (noObfuscation.contains(original)) {
noObfuscation.add(rewritten);
}
@@ -1962,6 +1979,7 @@
public void prune(DexReference reference) {
noShrinking.removeReference(reference);
+ noAnnotationRemoval.remove(reference);
noObfuscation.remove(reference);
noSideEffects.remove(reference);
assumedValues.remove(reference);
@@ -2154,6 +2172,7 @@
StringBuilder builder = new StringBuilder();
builder.append("RootSet");
builder.append("\nnoShrinking: " + noShrinking.size());
+ builder.append("\nnoAnnotationRemoval: " + noAnnotationRemoval.size());
builder.append("\nnoObfuscation: " + noObfuscation.size());
builder.append("\nreasonAsked: " + reasonAsked.size());
builder.append("\ncheckDiscarded: " + checkDiscarded.size());
@@ -2210,6 +2229,7 @@
Set<DexType> neverClassInline,
MutableItemsWithRules noShrinking,
MutableItemsWithRules softPinned,
+ Set<DexReference> noAnnotationRemoval,
Set<DexReference> noObfuscation,
Map<DexReference, MutableItemsWithRules> dependentNoShrinking,
Map<DexReference, MutableItemsWithRules> dependentSoftPinned,
@@ -2221,6 +2241,7 @@
neverClassInline,
noShrinking,
softPinned,
+ noAnnotationRemoval,
noObfuscation,
dependentNoShrinking,
dependentSoftPinned,
@@ -2277,6 +2298,7 @@
noShrinking,
new MutableItemsWithRules(),
Collections.emptySet(),
+ Collections.emptySet(),
reasonAsked,
checkDiscarded,
Collections.emptySet(),
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 9bb81bd..af2b4ad 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -587,6 +587,11 @@
}
@Override
+ public boolean isAnnotationRemovalEnabled() {
+ return !isForceProguardCompatibilityEnabled();
+ }
+
+ @Override
public boolean isTreeShakingEnabled() {
return isShrinking();
}
diff --git a/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepAllowShrinkingCompatibilityTest.java b/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepAllowShrinkingCompatibilityTest.java
index f198845..e6e8b69 100644
--- a/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepAllowShrinkingCompatibilityTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepAllowShrinkingCompatibilityTest.java
@@ -64,8 +64,6 @@
if (shrinker.isR8()) {
run(
testForR8(parameters.getBackend())
- // Allowing all of shrinking, optimization and obfuscation will amount to a nop rule.
- .allowUnusedProguardConfigurationRules(allowOptimization && allowObfuscation)
.enableNoHorizontalClassMergingAnnotations());
} else {
run(
diff --git a/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepClassFieldsAllowShrinkingCompatibilityTest.java b/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepClassFieldsAllowShrinkingCompatibilityTest.java
index 8b01419..314ea33 100644
--- a/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepClassFieldsAllowShrinkingCompatibilityTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepClassFieldsAllowShrinkingCompatibilityTest.java
@@ -63,10 +63,7 @@
@Test
public void test() throws Exception {
if (shrinker.isR8()) {
- run(
- testForR8(parameters.getBackend())
- // Allowing all of shrinking, optimization and obfuscation will amount to a nop rule.
- .allowUnusedProguardConfigurationRules(allowOptimization && allowObfuscation));
+ run(testForR8(parameters.getBackend()));
} else {
run(testForProguard(shrinker.getProguardVersion()).addDontWarn(getClass()));
}
diff --git a/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepClassMethodsAllowShrinkingCompatibilityTest.java b/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepClassMethodsAllowShrinkingCompatibilityTest.java
index 5a1d0f9..5305ced 100644
--- a/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepClassMethodsAllowShrinkingCompatibilityTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/allowshrinking/KeepClassMethodsAllowShrinkingCompatibilityTest.java
@@ -64,8 +64,6 @@
if (shrinker.isR8()) {
run(
testForR8(parameters.getBackend())
- // Allowing all of shrinking, optimization and obfuscation will amount to a nop rule.
- .allowUnusedProguardConfigurationRules(allowOptimization && allowObfuscation)
.enableNoHorizontalClassMergingAnnotations());
} else {
run(
diff --git a/src/test/java/com/android/tools/r8/shaking/annotations/KeepAllowAnnotationRemovalTest.java b/src/test/java/com/android/tools/r8/shaking/annotations/KeepAllowAnnotationRemovalTest.java
new file mode 100644
index 0000000..a47afde
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/annotations/KeepAllowAnnotationRemovalTest.java
@@ -0,0 +1,71 @@
+// Copyright (c) 2021, 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.annotations;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.onlyIf;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.Keep;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class KeepAllowAnnotationRemovalTest extends TestBase {
+
+ @Parameter(0)
+ public boolean enableCompatibilityMode;
+
+ @Parameter(1)
+ public TestParameters parameters;
+
+ @Parameters(name = "{1}, compat: {0}")
+ public static List<Object[]> parameters() {
+ return buildParameters(
+ BooleanUtils.values(), getTestParameters().withAllRuntimesAndApiLevels().build());
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8Compat(parameters.getBackend(), enableCompatibilityMode)
+ .addProgramClasses(Main.class, Keep.class)
+ .addKeepRules(
+ "-keep,allowannotationremoval @" + Keep.class.getTypeName() + " class * {",
+ " public static void main(java.lang.String[]);",
+ "}")
+ .addKeepRuntimeInvisibleAnnotations()
+ .enableProguardTestOptions()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject classSubject = inspector.clazz(Main.class);
+ assertThat(classSubject, isPresent());
+ assertThat(
+ classSubject.annotation(Keep.class),
+ onlyIf(enableCompatibilityMode, isPresent()));
+ })
+ .run(parameters.getRuntime(), Main.class)
+ .apply(
+ result ->
+ result.assertSuccessWithOutputLines(
+ result.inspector().clazz(Keep.class).getFinalName()));
+ }
+
+ @Keep
+ static class Main {
+ public static void main(String[] args) {
+ System.out.println(Keep.class.getName());
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/annotations/KeepDisallowAnnotationRemovalAllowOptimizationTest.java b/src/test/java/com/android/tools/r8/shaking/annotations/KeepDisallowAnnotationRemovalAllowOptimizationTest.java
new file mode 100644
index 0000000..8fa7bc1
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/annotations/KeepDisallowAnnotationRemovalAllowOptimizationTest.java
@@ -0,0 +1,97 @@
+// Copyright (c) 2021, 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.annotations;
+
+import static com.android.tools.r8.utils.codeinspector.CodeMatchers.containsThrow;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class KeepDisallowAnnotationRemovalAllowOptimizationTest extends TestBase {
+
+ @Parameter(0)
+ public boolean enableCompatibilityMode;
+
+ @Parameter(1)
+ public TestParameters parameters;
+
+ @Parameters(name = "{1}, compat: {0}")
+ public static List<Object[]> parameters() {
+ return buildParameters(
+ BooleanUtils.values(), getTestParameters().withAllRuntimesAndApiLevels().build());
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8Compat(parameters.getBackend(), enableCompatibilityMode)
+ .addProgramClasses(Main.class)
+ .addKeepMainRule(Main.class)
+ .addKeepClassAndMembersRules(NeverInline.class)
+ .addKeepRules(
+ "-keepclassmembers,allowobfuscation,allowoptimization,allowshrinking class * {",
+ " static java.lang.Object getNonNull();",
+ "}")
+ .addKeepRuntimeInvisibleAnnotations()
+ // In compatibility mode the rule above is a no-op.
+ .allowUnusedProguardConfigurationRules(enableCompatibilityMode)
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject classSubject = inspector.clazz(Main.class);
+ assertThat(classSubject, isPresent());
+
+ // The annotation on getNonNull() is kept meanwhile it is subject to other
+ // optimizations.
+ MethodSubject getNonNullSubject = classSubject.uniqueMethodWithName("getNonNull");
+ assertThat(getNonNullSubject, isPresentAndRenamed());
+ assertThat(getNonNullSubject.annotation(NeverInline.class), isPresent());
+
+ // Check that the code has been optimized using the fact that getNonNull() returns a
+ // non-null value.
+ assertThat(classSubject.uniqueMethodWithName("dead"), isAbsent());
+ assertThat(classSubject.mainMethod(), not(containsThrow()));
+ })
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("getNonNull()");
+ }
+
+ static class Main {
+ public static void main(String[] args) {
+ Object o = getNonNull();
+ if (o == null) {
+ dead();
+ }
+ }
+
+ @NeverInline
+ static Object getNonNull() {
+ System.out.println("getNonNull()");
+ return new Object();
+ }
+
+ @NeverInline
+ static void dead() {
+ throw new RuntimeException();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/annotations/SerializedNameAlternateTest.java b/src/test/java/com/android/tools/r8/shaking/annotations/SerializedNameAlternateTest.java
index 6913147..de259e5 100644
--- a/src/test/java/com/android/tools/r8/shaking/annotations/SerializedNameAlternateTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/annotations/SerializedNameAlternateTest.java
@@ -66,7 +66,7 @@
@Parameters(name = "{0}")
public static TestParametersCollection data() {
- return getTestParameters().withAllRuntimes().build();
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
}
public SerializedNameAlternateTest(TestParameters parameters) {
@@ -83,7 +83,14 @@
.addKeepMainRule(Main.class)
.addKeepClassRules(Foo.class)
.addKeepClassRules(SerializedName.class)
- .setMinApi(parameters.getRuntime())
+ .addKeepRules(
+ // Non-compat mode only retains annotations for items matched by a -keep rule.
+ "-keepclassmembers,allowobfuscation,allowshrinking class "
+ + Foo.class.getTypeName()
+ + " {",
+ " @" + SerializedName.class.getTypeName() + " <fields>;",
+ "}")
+ .setMinApi(parameters.getApiLevel())
.noMinification()
.run(parameters.getRuntime(), Main.class);
checkRunResult(result);
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/ClassOrMemberSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/ClassOrMemberSubject.java
index d86cc6c..bca691f 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/ClassOrMemberSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/ClassOrMemberSubject.java
@@ -5,9 +5,16 @@
package com.android.tools.r8.utils.codeinspector;
import com.android.tools.r8.graph.AccessFlags;
+import java.lang.annotation.Annotation;
public abstract class ClassOrMemberSubject extends Subject {
+ public abstract AnnotationSubject annotation(String name);
+
+ public final AnnotationSubject annotation(Class<? extends Annotation> clazz) {
+ return annotation(clazz.getTypeName());
+ }
+
public abstract AccessFlags<?> getAccessFlags();
public abstract String getOriginalName();
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java
index 452669a..4f8ecd3 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java
@@ -191,8 +191,6 @@
public abstract DexProgramClass getDexProgramClass();
- public abstract AnnotationSubject annotation(String name);
-
@Override
public abstract String getOriginalName();
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java
index 10a68c1..c5a8cde 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java
@@ -44,6 +44,27 @@
};
}
+ public static Matcher<MethodSubject> containsThrow() {
+ return new TypeSafeMatcher<MethodSubject>() {
+ @Override
+ protected boolean matchesSafely(MethodSubject subject) {
+ return subject.isPresent()
+ && subject.getMethod().hasCode()
+ && subject.streamInstructions().anyMatch(InstructionSubject::isThrow);
+ }
+
+ @Override
+ public void describeTo(Description description) {
+ description.appendText("contains throw");
+ }
+
+ @Override
+ public void describeMismatchSafely(final MethodSubject subject, Description description) {
+ description.appendText("method did not");
+ }
+ };
+ }
+
public static Matcher<MethodSubject> instantiatesClass(Class<?> clazz) {
return instantiatesClass(clazz.getTypeName());
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
index d8fd4b9..bd9081e 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
@@ -38,8 +38,6 @@
public abstract String getFinalSignatureAttribute();
- public abstract AnnotationSubject annotation(String name);
-
public FieldSubject asFieldSubject() {
return null;
}
