Revert "Tests and fixes for missing classes reported from annotations" This reverts commit bf1060655e962e40e71c22ebab27d4602f6391b5. Reason for revert: Bot failures due to redundant tracing Change-Id: I985549ce6692fda8447889d65ea91430d13933ba
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 6ea4e55..9c672b1 100644 --- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java +++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -364,8 +364,7 @@ * A map from annotation classes to annotations that need to be processed should the classes ever * become live. */ - private final Map<DexType, Map<DexAnnotation, ProgramDefinition>> deferredAnnotations = - new IdentityHashMap<>(); + private final Map<DexType, Set<DexAnnotation>> deferredAnnotations = new IdentityHashMap<>(); /** Map of active if rules to speed up aapt2 generated keep rules. */ private Map<Wrapper<ProguardIfRule>, Set<ProguardIfRule>> activeIfRules; @@ -1831,17 +1830,14 @@ enqueueFirstNonSerializableClassInitializer(clazz, reason); } - processAnnotations(clazz); + processAnnotations(clazz, clazz); // If this type has deferred annotations, we have to process those now, too. if (clazz.isAnnotation()) { - Map<DexAnnotation, ProgramDefinition> annotations = - deferredAnnotations.remove(clazz.getType()); - if (annotations != null) { - assert annotations.keySet().stream() - .allMatch(a -> a.getAnnotationType() == clazz.getType()); - annotations.forEach( - (annotation, annotatedItem) -> processAnnotation(annotatedItem, annotation)); + Set<DexAnnotation> annotations = deferredAnnotations.remove(clazz.type); + if (annotations != null && !annotations.isEmpty()) { + assert annotations.stream().allMatch(a -> a.annotation.type == clazz.type); + annotations.forEach(annotation -> processAnnotation(clazz, clazz, annotation)); } } @@ -1934,32 +1930,35 @@ enqueueKeepRuleInstantiatedType(holder, reasons, instanceInitializer.getDefinition()); } - private void processAnnotations(ProgramDefinition annotatedItem) { - processAnnotations(annotatedItem, annotatedItem.getDefinition().annotations()); + private void processAnnotations(DexProgramClass holder, ProgramDefinition annotatedItem) { + processAnnotations(holder, annotatedItem, annotatedItem.getDefinition().annotations()); } - private void processAnnotations(ProgramDefinition annotatedItem, DexAnnotationSet annotations) { - processAnnotations(annotatedItem, annotations.annotations); + private void processAnnotations( + DexProgramClass holder, ProgramDefinition annotatedItem, DexAnnotationSet annotations) { + processAnnotations(holder, annotatedItem, annotations.annotations); } - private void processAnnotations(ProgramDefinition annotatedItem, DexAnnotation[] annotations) { + private void processAnnotations( + DexProgramClass holder, ProgramDefinition annotatedItem, DexAnnotation[] annotations) { for (DexAnnotation annotation : annotations) { - processAnnotation(annotatedItem, annotation); + processAnnotation(holder, annotatedItem, annotation); } } - private void processAnnotation(ProgramDefinition annotatedItem, DexAnnotation annotation) { + private void processAnnotation( + DexProgramClass holder, ProgramDefinition annotatedItem, DexAnnotation annotation) { + assert annotatedItem == holder || annotatedItem.asProgramMember().getHolder() == holder; + assert !holder.isDexClass() || holder.asDexClass().isProgramClass(); DexType type = annotation.getAnnotationType(); - DexClass clazz = definitionFor(type, annotatedItem); + recordTypeReference(type, annotatedItem); + DexClass clazz = appView.definitionFor(type); boolean annotationTypeIsLibraryClass = clazz == null || clazz.isNotProgramClass(); boolean isLive = annotationTypeIsLibraryClass || liveTypes.contains(clazz.asProgramClass()); if (!shouldKeepAnnotation(appView, annotatedItem.getDefinition(), annotation, isLive)) { // Remember this annotation for later. if (!annotationTypeIsLibraryClass) { - Map<DexAnnotation, ProgramDefinition> deferredAnnotationsForAnnotationType = - deferredAnnotations.computeIfAbsent(type, ignore -> new IdentityHashMap<>()); - assert !deferredAnnotationsForAnnotationType.containsKey(annotation); - deferredAnnotationsForAnnotationType.put(annotation, annotatedItem); + deferredAnnotations.computeIfAbsent(type, ignore -> new HashSet<>()).add(annotation); } return; } @@ -2272,14 +2271,15 @@ private void markMethodAsTargeted(ProgramMethod method, KeepReason reason) { DexEncodedMethod definition = method.getDefinition(); + DexProgramClass holder = method.getHolder(); if (!targetedMethods.add(definition, reason)) { // Already targeted. return; } markReferencedTypesAsLive(method); - processAnnotations(method); + processAnnotations(holder, method); definition.parameterAnnotationsList.forEachAnnotation( - annotation -> processAnnotation(method, annotation)); + annotation -> processAnnotation(holder, method, annotation)); if (Log.ENABLED) { Log.verbose(getClass(), "Method `%s` is targeted.", method); @@ -2287,7 +2287,7 @@ if (forceProguardCompatibility) { // Keep targeted default methods in compatibility mode. The tree pruner will otherwise make // these methods abstract, whereas Proguard does not (seem to) touch their code. - if (!definition.isAbstract() && method.getHolder().isInterface()) { + if (!definition.isAbstract() && holder.isInterface()) { markMethodAsLiveWithCompatRule(method); } } @@ -2680,7 +2680,7 @@ Log.verbose(getClass(), "Adding instance field `%s` to live set (static context).", field); } } - processAnnotations(field); + processAnnotations(field.getHolder(), field); liveFields.add(field, reason); // Add all dependent members to the workqueue. @@ -2695,7 +2695,11 @@ private void markInstanceFieldAsLive( ProgramField field, ProgramDefinition context, KeepReason reason) { markFieldAsTargeted(field); - processAnnotations(field); + + if (Log.ENABLED) { + Log.verbose(getClass(), "Adding instance field `%s` to live set.", field); + } + processAnnotations(field.getHolder(), field); liveFields.add(field, reason); // Add all dependent members to the workqueue. @@ -4003,6 +4007,7 @@ // Package protected due to entry point from worklist. void markMethodAsLive(ProgramMethod method, ProgramDefinition context) { + DexProgramClass holder = method.getHolder(); DexEncodedMethod definition = method.getDefinition(); assert liveMethods.contains(definition); @@ -4022,9 +4027,9 @@ } } markParameterAndReturnTypesAsLive(method); - processAnnotations(method); + processAnnotations(holder, method); definition.parameterAnnotationsList.forEachAnnotation( - annotation -> processAnnotation(method, annotation)); + annotation -> processAnnotation(holder, method, annotation)); traceNonDesugaredCode(method);
diff --git a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromClassAnnotationTest.java b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromClassAnnotationTest.java deleted file mode 100644 index 0af31a3..0000000 --- a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromClassAnnotationTest.java +++ /dev/null
@@ -1,62 +0,0 @@ -// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -package com.android.tools.r8.missingclasses; - -import com.android.tools.r8.CompilationFailedException; -import com.android.tools.r8.TestDiagnosticMessages; -import com.android.tools.r8.TestParameters; -import com.android.tools.r8.references.ClassReference; -import com.android.tools.r8.references.Reference; -import org.junit.Test; - -// TODO(b/179456539): This test should fail without -keepattributes RuntimeVisibleAnnotations, but -// we retain missing annotations even if there is no -keepattributes *Annotations*. -public class MissingClassReferencedFromClassAnnotationTest extends MissingClassesTestBase { - - private static final ClassReference referencedFrom = Reference.classFromClass(Main.class); - - public MissingClassReferencedFromClassAnnotationTest(TestParameters parameters) { - super(parameters); - } - - @Test(expected = CompilationFailedException.class) - public void testNoRules() throws Exception { - compileWithExpectedDiagnostics( - Main.class, diagnostics -> inspectDiagnosticsWithNoRules(diagnostics, referencedFrom)); - } - - @Test - public void testDontWarnMainClass() throws Exception { - compileWithExpectedDiagnostics( - Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(Main.class)); - } - - @Test - public void testDontWarnMissingClass() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - TestDiagnosticMessages::assertNoMessages, - addDontWarn(MissingRuntimeAnnotation.class)); - } - - @Test - public void testIgnoreWarnings() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - diagnostics -> inspectDiagnosticsWithIgnoreWarnings(diagnostics, referencedFrom), - addIgnoreWarnings()); - } - - @Override - ClassReference getMissingClassReference() { - return Reference.classFromClass(MissingRuntimeAnnotation.class); - } - - @MissingRuntimeAnnotation - static class Main { - - public static void main(String[] args) {} - } -}
diff --git a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromClassAnnotationWithDataTest.java b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromClassAnnotationWithDataTest.java deleted file mode 100644 index 0918d76..0000000 --- a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromClassAnnotationWithDataTest.java +++ /dev/null
@@ -1,76 +0,0 @@ -// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -package com.android.tools.r8.missingclasses; - -import com.android.tools.r8.CompilationFailedException; -import com.android.tools.r8.R8FullTestBuilder; -import com.android.tools.r8.TestDiagnosticMessages; -import com.android.tools.r8.TestParameters; -import com.android.tools.r8.ThrowableConsumer; -import com.android.tools.r8.references.ClassReference; -import com.android.tools.r8.references.Reference; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import org.junit.Test; - -public class MissingClassReferencedFromClassAnnotationWithDataTest extends MissingClassesTestBase { - - private static final ClassReference referencedFrom = Reference.classFromClass(Main.class); - - public MissingClassReferencedFromClassAnnotationWithDataTest(TestParameters parameters) { - super(parameters); - } - - @Test(expected = CompilationFailedException.class) - public void testNoRules() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - diagnostics -> inspectDiagnosticsWithNoRules(diagnostics, referencedFrom), - addRuntimeAnnotation()); - } - - @Test - public void testDontWarnMainClass() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - TestDiagnosticMessages::assertNoMessages, - addRuntimeAnnotation().andThen(addDontWarn(Main.class))); - } - - @Test - public void testDontWarnMissingClass() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - TestDiagnosticMessages::assertNoMessages, - addRuntimeAnnotation().andThen(addDontWarn(MissingClass.class))); - } - - @Test - public void testIgnoreWarnings() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - diagnostics -> inspectDiagnosticsWithIgnoreWarnings(diagnostics, referencedFrom), - addRuntimeAnnotation().andThen(addIgnoreWarnings())); - } - - private ThrowableConsumer<R8FullTestBuilder> addRuntimeAnnotation() { - return builder -> - builder - .addProgramClasses(RuntimeAnnotation.class) - .addKeepClassRules(RuntimeAnnotation.class) - .addKeepRuntimeVisibleAnnotations(); - } - - @RuntimeAnnotation(data = MissingClass.class) - static class Main { - - public static void main(String[] args) {} - } - - @Retention(RetentionPolicy.RUNTIME) - @interface RuntimeAnnotation { - Class<?> data(); - } -}
diff --git a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromFieldAnnotationTest.java b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromFieldAnnotationTest.java deleted file mode 100644 index 7788831..0000000 --- a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromFieldAnnotationTest.java +++ /dev/null
@@ -1,67 +0,0 @@ -// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -package com.android.tools.r8.missingclasses; - -import com.android.tools.r8.CompilationFailedException; -import com.android.tools.r8.TestDiagnosticMessages; -import com.android.tools.r8.TestParameters; -import com.android.tools.r8.references.ClassReference; -import com.android.tools.r8.references.FieldReference; -import com.android.tools.r8.references.Reference; -import org.junit.Test; - -// TODO(b/179456539): This test should fail without -keepattributes RuntimeVisibleAnnotations, but -// we retain missing annotations even if there is no -keepattributes *Annotations*. -public class MissingClassReferencedFromFieldAnnotationTest extends MissingClassesTestBase { - - private static final FieldReference referencedFrom = - Reference.field(Reference.classFromClass(Main.class), "FIELD", Reference.INT); - - public MissingClassReferencedFromFieldAnnotationTest(TestParameters parameters) { - super(parameters); - } - - @Test(expected = CompilationFailedException.class) - public void testNoRules() throws Exception { - compileWithExpectedDiagnostics( - Main.class, diagnostics -> inspectDiagnosticsWithNoRules(diagnostics, referencedFrom)); - } - - @Test - public void testDontWarnMainClass() throws Exception { - compileWithExpectedDiagnostics( - Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(Main.class)); - } - - @Test - public void testDontWarnMissingClass() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - TestDiagnosticMessages::assertNoMessages, - addDontWarn(MissingRuntimeAnnotation.class)); - } - - @Test - public void testIgnoreWarnings() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - diagnostics -> inspectDiagnosticsWithIgnoreWarnings(diagnostics, referencedFrom), - addIgnoreWarnings()); - } - - @Override - ClassReference getMissingClassReference() { - return Reference.classFromClass(MissingRuntimeAnnotation.class); - } - - static class Main { - - @MissingRuntimeAnnotation static int FIELD; - - public static void main(String[] args) { - int ignore = FIELD; - } - } -}
diff --git a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromMethodAnnotationTest.java b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromMethodAnnotationTest.java deleted file mode 100644 index 189b49e..0000000 --- a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromMethodAnnotationTest.java +++ /dev/null
@@ -1,65 +0,0 @@ -// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -package com.android.tools.r8.missingclasses; - -import com.android.tools.r8.CompilationFailedException; -import com.android.tools.r8.TestDiagnosticMessages; -import com.android.tools.r8.TestParameters; -import com.android.tools.r8.references.ClassReference; -import com.android.tools.r8.references.MethodReference; -import com.android.tools.r8.references.Reference; -import com.android.tools.r8.utils.MethodReferenceUtils; -import org.junit.Test; - -// TODO(b/179456539): This test should fail without -keepattributes RuntimeVisibleAnnotations, but -// we retain missing annotations even if there is no -keepattributes *Annotations*. -public class MissingClassReferencedFromMethodAnnotationTest extends MissingClassesTestBase { - - private static final MethodReference referencedFrom = - MethodReferenceUtils.mainMethod(Reference.classFromClass(Main.class)); - - public MissingClassReferencedFromMethodAnnotationTest(TestParameters parameters) { - super(parameters); - } - - @Test(expected = CompilationFailedException.class) - public void testNoRules() throws Exception { - compileWithExpectedDiagnostics( - Main.class, diagnostics -> inspectDiagnosticsWithNoRules(diagnostics, referencedFrom)); - } - - @Test - public void testDontWarnMainClass() throws Exception { - compileWithExpectedDiagnostics( - Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(Main.class)); - } - - @Test - public void testDontWarnMissingClass() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - TestDiagnosticMessages::assertNoMessages, - addDontWarn(MissingRuntimeAnnotation.class)); - } - - @Test - public void testIgnoreWarnings() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - diagnostics -> inspectDiagnosticsWithIgnoreWarnings(diagnostics, referencedFrom), - addIgnoreWarnings()); - } - - @Override - ClassReference getMissingClassReference() { - return Reference.classFromClass(MissingRuntimeAnnotation.class); - } - - static class Main { - - @MissingRuntimeAnnotation - public static void main(String[] args) {} - } -}
diff --git a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromParameterAnnotationTest.java b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromParameterAnnotationTest.java deleted file mode 100644 index 0017aa7..0000000 --- a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromParameterAnnotationTest.java +++ /dev/null
@@ -1,64 +0,0 @@ -// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -package com.android.tools.r8.missingclasses; - -import com.android.tools.r8.CompilationFailedException; -import com.android.tools.r8.TestDiagnosticMessages; -import com.android.tools.r8.TestParameters; -import com.android.tools.r8.references.ClassReference; -import com.android.tools.r8.references.MethodReference; -import com.android.tools.r8.references.Reference; -import com.android.tools.r8.utils.MethodReferenceUtils; -import org.junit.Test; - -// TODO(b/179456539): This test should fail without -keepattributes RuntimeVisibleAnnotations, but -// we retain missing annotations even if there is no -keepattributes *Annotations*. -public class MissingClassReferencedFromParameterAnnotationTest extends MissingClassesTestBase { - - private static final MethodReference referencedFrom = - MethodReferenceUtils.mainMethod(Reference.classFromClass(Main.class)); - - public MissingClassReferencedFromParameterAnnotationTest(TestParameters parameters) { - super(parameters); - } - - @Test(expected = CompilationFailedException.class) - public void testNoRules() throws Exception { - compileWithExpectedDiagnostics( - Main.class, diagnostics -> inspectDiagnosticsWithNoRules(diagnostics, referencedFrom)); - } - - @Test - public void testDontWarnMainClass() throws Exception { - compileWithExpectedDiagnostics( - Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(Main.class)); - } - - @Test - public void testDontWarnMissingClass() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - TestDiagnosticMessages::assertNoMessages, - addDontWarn(MissingRuntimeAnnotation.class)); - } - - @Test - public void testIgnoreWarnings() throws Exception { - compileWithExpectedDiagnostics( - Main.class, - diagnostics -> inspectDiagnosticsWithIgnoreWarnings(diagnostics, referencedFrom), - addIgnoreWarnings()); - } - - @Override - ClassReference getMissingClassReference() { - return Reference.classFromClass(MissingRuntimeAnnotation.class); - } - - static class Main { - - public static void main(@MissingRuntimeAnnotation String[] args) {} - } -}
diff --git a/src/test/java/com/android/tools/r8/missingclasses/MissingClassesTestBase.java b/src/test/java/com/android/tools/r8/missingclasses/MissingClassesTestBase.java index c08622c..dafb7b4 100644 --- a/src/test/java/com/android/tools/r8/missingclasses/MissingClassesTestBase.java +++ b/src/test/java/com/android/tools/r8/missingclasses/MissingClassesTestBase.java
@@ -23,8 +23,6 @@ import com.android.tools.r8.utils.InternalOptions.TestingOptions; import com.android.tools.r8.utils.MethodReferenceUtils; import com.google.common.collect.ImmutableSet; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; import java.util.List; import java.util.function.Function; import org.junit.runner.RunWith; @@ -41,9 +39,6 @@ int field; } - @Retention(RetentionPolicy.RUNTIME) - @interface MissingRuntimeAnnotation {} - interface MissingInterface {} private final TestParameters parameters;