Revert "Refactor AnnotationRemover to always use the same filtering"
This reverts commit 42a2db4103e7682180f1361a9c5c9ccd2221c163.
Reason for revert: dump bot fails, https://r8-review.git.corp.google.com/c/r8/+/91766 breaks bootstrap
Change-Id: I5fdd138186e2c54f2e86c87b65824494254e8a39
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 1cfa7f2..bf7c4f8 100644
--- a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
+++ b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
@@ -59,13 +59,10 @@
/** Used to filter annotations on classes, methods and fields. */
private boolean filterAnnotations(
- ProgramDefinition holder,
- DexAnnotation annotation,
- AnnotatedKind kind,
- KeepInfo<?, ?> keepInfo) {
+ ProgramDefinition holder, DexAnnotation annotation, AnnotatedKind kind) {
return annotationsToRetain.contains(annotation)
|| shouldKeepAnnotation(
- appView, holder, annotation, isAnnotationTypeLive(annotation), kind, mode, keepInfo);
+ appView, holder, annotation, isAnnotationTypeLive(annotation), kind, mode);
}
public static boolean shouldKeepAnnotation(
@@ -74,17 +71,13 @@
DexAnnotation annotation,
boolean isAnnotationTypeLive,
AnnotatedKind kind,
- Mode mode,
- KeepInfo<?, ?> keepInfo) {
+ Mode mode) {
// If we cannot run the AnnotationRemover we are keeping the annotation.
InternalOptions options = appView.options();
if (!options.isShrinking()) {
return true;
}
- boolean isAnnotationOnAnnotationClass =
- holder.isProgramClass() && holder.asProgramClass().isAnnotation();
-
ProguardKeepAttributes config =
options.getProguardConfiguration() != null
? options.getProguardConfiguration().getKeepAttributes()
@@ -116,17 +109,20 @@
&& DexAnnotation.isParameterNameAnnotation(annotation, dexItemFactory)) {
return true;
}
- if (isAnnotationOnAnnotationClass
- && DexAnnotation.isAnnotationDefaultAnnotation(annotation, dexItemFactory)
- && shouldRetainAnnotationDefaultAnnotationOnAnnotationClass(annotation)) {
+ if (DexAnnotation.isAnnotationDefaultAnnotation(annotation, dexItemFactory)) {
+ // These have to be kept if the corresponding annotation class is kept to retain default
+ // values.
return true;
}
return false;
case DexAnnotation.VISIBILITY_RUNTIME:
- if (isAnnotationOnAnnotationClass
- && DexAnnotation.isJavaLangRetentionAnnotation(annotation, dexItemFactory)
- && shouldRetainRetentionAnnotationOnAnnotationClass(annotation, dexItemFactory)) {
+ // We always keep the @java.lang.Retention annotation on annotation classes, since the
+ // removal of this annotation may change the annotation from being runtime visible to
+ // runtime invisible.
+ if (holder.isProgramClass()
+ && holder.asProgramClass().isAnnotation()
+ && DexAnnotation.isJavaLangRetentionAnnotation(annotation, dexItemFactory)) {
return true;
}
@@ -141,7 +137,7 @@
&& !options.isKeepRuntimeVisibleTypeAnnotationsEnabled()) {
return false;
}
- return !keepInfo.isAnnotationRemovalAllowed(options) && isAnnotationTypeLive;
+ return isAnnotationTypeLive;
case DexAnnotation.VISIBILITY_BUILD:
if (annotation
@@ -164,7 +160,7 @@
&& !options.isKeepRuntimeInvisibleTypeAnnotationsEnabled()) {
return false;
}
- return !keepInfo.isAnnotationRemovalAllowed(options) && isAnnotationTypeLive;
+ return isAnnotationTypeLive;
default:
throw new Unreachable("Unexpected annotation visibility.");
}
@@ -247,12 +243,9 @@
}
private DexAnnotation rewriteAnnotation(
- ProgramDefinition holder,
- DexAnnotation original,
- AnnotatedKind kind,
- KeepInfo<?, ?> keepInfo) {
+ ProgramDefinition holder, DexAnnotation original, AnnotatedKind kind) {
// Check if we should keep this annotation first.
- if (filterAnnotations(holder, original, kind, keepInfo)) {
+ if (filterAnnotations(holder, original, kind)) {
// Then, filter out values that refer to dead definitions.
return original.rewrite(this::rewriteEncodedAnnotation);
}
@@ -303,8 +296,36 @@
private void removeAnnotations(ProgramDefinition definition, KeepInfo<?, ?> keepInfo) {
assert mode.isInitialTreeShaking() || annotationsToRetain.isEmpty();
- definition.rewriteAllAnnotations(
- (annotation, kind) -> rewriteAnnotation(definition, annotation, kind, keepInfo));
+ boolean isAnnotation =
+ definition.isProgramClass() && definition.asProgramClass().isAnnotation();
+ if (keepInfo.isAnnotationRemovalAllowed(options)) {
+ if (isAnnotation || mode.isInitialTreeShaking()) {
+ definition.rewriteAllAnnotations(
+ (annotation, kind) ->
+ shouldRetainAnnotation(definition, annotation, kind) ? annotation : null);
+ } else {
+ definition.clearAllAnnotations();
+ }
+ } else {
+ definition.rewriteAllAnnotations(
+ (annotation, kind) -> rewriteAnnotation(definition, annotation, kind));
+ }
+ }
+
+ private boolean shouldRetainAnnotation(
+ ProgramDefinition definition, DexAnnotation annotation, AnnotatedKind kind) {
+ boolean isAnnotationOnAnnotationClass =
+ definition.isProgramClass() && definition.asProgramClass().isAnnotation();
+ if (isAnnotationOnAnnotationClass) {
+ if (DexAnnotation.isAnnotationDefaultAnnotation(annotation, appView.dexItemFactory())) {
+ return shouldRetainAnnotationDefaultAnnotationOnAnnotationClass(annotation);
+ }
+ if (DexAnnotation.isJavaLangRetentionAnnotation(annotation, appView.dexItemFactory())) {
+ return shouldRetainRetentionAnnotationOnAnnotationClass(annotation);
+ }
+ }
+ return annotationsToRetain.contains(annotation)
+ || isComposableAnnotationToRetain(appView, annotation, kind, mode, options);
}
private static boolean isComposableAnnotationToRetain(
@@ -321,30 +342,29 @@
.isIdenticalTo(appView.getComposeReferences().composableType);
}
- private static boolean shouldRetainAnnotationDefaultAnnotationOnAnnotationClass(
- DexAnnotation unusedAnnotation) {
+ @SuppressWarnings("UnusedVariable")
+ private boolean shouldRetainAnnotationDefaultAnnotationOnAnnotationClass(
+ DexAnnotation annotation) {
// We currently always retain the @AnnotationDefault annotations for annotation classes. In full
// mode we could consider only retaining @AnnotationDefault annotations for pinned annotations,
// as this is consistent with removing all annotations for non-kept items.
return true;
}
- private static boolean shouldRetainRetentionAnnotationOnAnnotationClass(
- DexAnnotation annotation, DexItemFactory dexItemFactory) {
+ @SuppressWarnings("ReferenceEquality")
+ private boolean shouldRetainRetentionAnnotationOnAnnotationClass(DexAnnotation annotation) {
// Retain @Retention annotations that are different from @Retention(RetentionPolicy.CLASS).
if (annotation.annotation.getNumberOfElements() != 1) {
return true;
}
DexAnnotationElement element = annotation.annotation.getElement(0);
- if (element.name.isNotIdenticalTo(dexItemFactory.valueString)) {
+ if (element.name != appView.dexItemFactory().valueString) {
return true;
}
DexValue value = element.getValue();
if (!value.isDexValueEnum()
- || value
- .asDexValueEnum()
- .getValue()
- .isNotIdenticalTo(dexItemFactory.javaLangAnnotationRetentionPolicyMembers.CLASS)) {
+ || value.asDexValueEnum().getValue()
+ != appView.dexItemFactory().javaLangAnnotationRetentionPolicyMembers.CLASS) {
return true;
}
return false;
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 8f79c52..6041570 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -2510,13 +2510,7 @@
return true;
}
return AnnotationRemover.shouldKeepAnnotation(
- appView,
- annotatedItem,
- annotation,
- isLive,
- annotatedKind,
- mode,
- keepInfo.getInfo(annotatedItem));
+ appView, annotatedItem, annotation, isLive, annotatedKind, mode);
}
private DexClass resolveBaseType(DexType type, ProgramDefinition context) {
diff --git a/src/main/java/com/android/tools/r8/shaking/TreePruner.java b/src/main/java/com/android/tools/r8/shaking/TreePruner.java
index 5055090..d13b3d0 100644
--- a/src/main/java/com/android/tools/r8/shaking/TreePruner.java
+++ b/src/main/java/com/android/tools/r8/shaking/TreePruner.java
@@ -24,7 +24,6 @@
import com.android.tools.r8.graph.InnerClassAttribute;
import com.android.tools.r8.graph.NestMemberClassAttribute;
import com.android.tools.r8.graph.PermittedSubclassAttribute;
-import com.android.tools.r8.graph.ProgramMember;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.PrunedItems;
import com.android.tools.r8.graph.RecordComponentInfo;
@@ -113,17 +112,12 @@
InternalOptions options = appView.options();
List<DexProgramClass> newClasses = new ArrayList<>();
for (DexProgramClass clazz : classes) {
- boolean isLiveClass = appInfo.isLiveProgramClass(clazz);
if (options.configurationDebugging) {
newClasses.add(clazz);
pruneMembersAndAttributes(clazz);
- if (!isLiveClass) {
- clazz.clearAllAnnotations();
- clazz.forEachProgramMember(ProgramMember::clearAllAnnotations);
- }
continue;
}
- if (isLiveClass) {
+ if (appInfo.isLiveProgramClass(clazz)) {
newClasses.add(clazz);
if (!appInfo.getObjectAllocationInfoCollection().isInstantiatedDirectly(clazz)
&& !options.forceProguardCompatibility) {
