Reapply "Refactor AnnotationRemover to always use the same filtering"
This reverts commit 9a3c378cb7e50fde819232ec04b624dc82f31ce0.
Change-Id: I9fffcfea13507d0386e38383cfd8e446d9c79c9d
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 bf7c4f8..1cfa7f2 100644
--- a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
+++ b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
@@ -59,10 +59,13 @@
/** Used to filter annotations on classes, methods and fields. */
private boolean filterAnnotations(
- ProgramDefinition holder, DexAnnotation annotation, AnnotatedKind kind) {
+ ProgramDefinition holder,
+ DexAnnotation annotation,
+ AnnotatedKind kind,
+ KeepInfo<?, ?> keepInfo) {
return annotationsToRetain.contains(annotation)
|| shouldKeepAnnotation(
- appView, holder, annotation, isAnnotationTypeLive(annotation), kind, mode);
+ appView, holder, annotation, isAnnotationTypeLive(annotation), kind, mode, keepInfo);
}
public static boolean shouldKeepAnnotation(
@@ -71,13 +74,17 @@
DexAnnotation annotation,
boolean isAnnotationTypeLive,
AnnotatedKind kind,
- Mode mode) {
+ Mode mode,
+ KeepInfo<?, ?> keepInfo) {
// 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()
@@ -109,20 +116,17 @@
&& DexAnnotation.isParameterNameAnnotation(annotation, dexItemFactory)) {
return true;
}
- if (DexAnnotation.isAnnotationDefaultAnnotation(annotation, dexItemFactory)) {
- // These have to be kept if the corresponding annotation class is kept to retain default
- // values.
+ if (isAnnotationOnAnnotationClass
+ && DexAnnotation.isAnnotationDefaultAnnotation(annotation, dexItemFactory)
+ && shouldRetainAnnotationDefaultAnnotationOnAnnotationClass(annotation)) {
return true;
}
return false;
case DexAnnotation.VISIBILITY_RUNTIME:
- // 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)) {
+ if (isAnnotationOnAnnotationClass
+ && DexAnnotation.isJavaLangRetentionAnnotation(annotation, dexItemFactory)
+ && shouldRetainRetentionAnnotationOnAnnotationClass(annotation, dexItemFactory)) {
return true;
}
@@ -137,7 +141,7 @@
&& !options.isKeepRuntimeVisibleTypeAnnotationsEnabled()) {
return false;
}
- return isAnnotationTypeLive;
+ return !keepInfo.isAnnotationRemovalAllowed(options) && isAnnotationTypeLive;
case DexAnnotation.VISIBILITY_BUILD:
if (annotation
@@ -160,7 +164,7 @@
&& !options.isKeepRuntimeInvisibleTypeAnnotationsEnabled()) {
return false;
}
- return isAnnotationTypeLive;
+ return !keepInfo.isAnnotationRemovalAllowed(options) && isAnnotationTypeLive;
default:
throw new Unreachable("Unexpected annotation visibility.");
}
@@ -243,9 +247,12 @@
}
private DexAnnotation rewriteAnnotation(
- ProgramDefinition holder, DexAnnotation original, AnnotatedKind kind) {
+ ProgramDefinition holder,
+ DexAnnotation original,
+ AnnotatedKind kind,
+ KeepInfo<?, ?> keepInfo) {
// Check if we should keep this annotation first.
- if (filterAnnotations(holder, original, kind)) {
+ if (filterAnnotations(holder, original, kind, keepInfo)) {
// Then, filter out values that refer to dead definitions.
return original.rewrite(this::rewriteEncodedAnnotation);
}
@@ -296,36 +303,8 @@
private void removeAnnotations(ProgramDefinition definition, KeepInfo<?, ?> keepInfo) {
assert mode.isInitialTreeShaking() || annotationsToRetain.isEmpty();
- 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);
+ definition.rewriteAllAnnotations(
+ (annotation, kind) -> rewriteAnnotation(definition, annotation, kind, keepInfo));
}
private static boolean isComposableAnnotationToRetain(
@@ -342,29 +321,30 @@
.isIdenticalTo(appView.getComposeReferences().composableType);
}
- @SuppressWarnings("UnusedVariable")
- private boolean shouldRetainAnnotationDefaultAnnotationOnAnnotationClass(
- DexAnnotation annotation) {
+ private static boolean shouldRetainAnnotationDefaultAnnotationOnAnnotationClass(
+ DexAnnotation unusedAnnotation) {
// 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;
}
- @SuppressWarnings("ReferenceEquality")
- private boolean shouldRetainRetentionAnnotationOnAnnotationClass(DexAnnotation annotation) {
+ private static boolean shouldRetainRetentionAnnotationOnAnnotationClass(
+ DexAnnotation annotation, DexItemFactory dexItemFactory) {
// 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 != appView.dexItemFactory().valueString) {
+ if (element.name.isNotIdenticalTo(dexItemFactory.valueString)) {
return true;
}
DexValue value = element.getValue();
if (!value.isDexValueEnum()
- || value.asDexValueEnum().getValue()
- != appView.dexItemFactory().javaLangAnnotationRetentionPolicyMembers.CLASS) {
+ || value
+ .asDexValueEnum()
+ .getValue()
+ .isNotIdenticalTo(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 6041570..c1978ca 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -2473,18 +2473,20 @@
ProgramDefinition annotatedItem, DexAnnotation annotation, AnnotatedKind kind) {
DexType type = annotation.getAnnotationType();
DexClass clazz = definitionFor(type, annotatedItem);
- boolean annotationTypeIsLibraryClass = clazz == null || clazz.isNotProgramClass();
- boolean isLive = annotationTypeIsLibraryClass || liveTypes.contains(clazz.asProgramClass());
+ boolean annotationTypeIsNotProgramClass = clazz == null || clazz.isNotProgramClass();
+ boolean isLive = annotationTypeIsNotProgramClass || liveTypes.contains(clazz.asProgramClass());
if (!shouldKeepAnnotation(annotatedItem, annotation, kind, isLive)) {
// Remember this annotation for later.
- if (!annotationTypeIsLibraryClass) {
- Map<DexType, Map<DexAnnotation, List<ProgramDefinition>>> deferredAnnotations =
- kind.isParameter() ? deferredParameterAnnotations : this.deferredAnnotations;
- Map<DexAnnotation, List<ProgramDefinition>> deferredAnnotationsForAnnotationType =
- deferredAnnotations.computeIfAbsent(type, ignore -> new IdentityHashMap<>());
- deferredAnnotationsForAnnotationType
- .computeIfAbsent(annotation, ignore -> new ArrayList<>())
- .add(annotatedItem);
+ Map<DexType, Map<DexAnnotation, List<ProgramDefinition>>> deferredAnnotations =
+ kind.isParameter() ? deferredParameterAnnotations : this.deferredAnnotations;
+ Map<DexAnnotation, List<ProgramDefinition>> deferredAnnotationsForAnnotationType =
+ deferredAnnotations.computeIfAbsent(type, ignore -> new IdentityHashMap<>());
+ deferredAnnotationsForAnnotationType
+ .computeIfAbsent(annotation, ignore -> new ArrayList<>())
+ .add(annotatedItem);
+ // Also, non-program annotations should be considered live w.r.t the deferred processing.
+ if (annotationTypeIsNotProgramClass) {
+ liveAnnotations.add(type);
}
return;
}
@@ -2510,7 +2512,13 @@
return true;
}
return AnnotationRemover.shouldKeepAnnotation(
- appView, annotatedItem, annotation, isLive, annotatedKind, mode);
+ appView,
+ annotatedItem,
+ annotation,
+ isLive,
+ annotatedKind,
+ mode,
+ keepInfo.getInfo(annotatedItem));
}
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 d13b3d0..5055090 100644
--- a/src/main/java/com/android/tools/r8/shaking/TreePruner.java
+++ b/src/main/java/com/android/tools/r8/shaking/TreePruner.java
@@ -24,6 +24,7 @@
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;
@@ -112,12 +113,17 @@
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 (appInfo.isLiveProgramClass(clazz)) {
+ if (isLiveClass) {
newClasses.add(clazz);
if (!appInfo.getObjectAllocationInfoCollection().isInstantiatedDirectly(clazz)
&& !options.forceProguardCompatibility) {
