Reland "Do not remove runtime-visible-annotations if kept and not resolved"
This reverts commit 42c10ead8fa1c1bb41a8e1850c811f57accf3b1a.
Reason for revert: Revert has been cherry-picked to 1.7.24-dev and this can reland.
Change-Id: I14843e9b8450bc2fa7e5d138ad8e4792cedbdfd3
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 693058b..701904c 100644
--- a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
+++ b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
@@ -108,11 +108,6 @@
private boolean isAnnotationTypeLive(DexAnnotation annotation) {
DexType annotationType = annotation.annotation.type.toBaseType(appView.dexItemFactory());
- DexClass definition = appView.definitionFor(annotationType);
- // TODO(b/73102187): How to handle annotations without definition.
- if (appView.options().isShrinking() && definition == null) {
- return false;
- }
return appView.appInfo().isNonProgramTypeOrLiveProgramType(annotationType);
}
@@ -276,7 +271,8 @@
private DexAnnotationElement rewriteAnnotationElement(
DexType annotationType, DexAnnotationElement original) {
DexClass definition = appView.definitionFor(annotationType);
- // TODO(b/73102187): How to handle annotations without definition.
+ // We cannot strip annotations where we cannot look up the definition, because this will break
+ // apps that rely on the annotation to exist. See b/134766810 for more information.
if (definition == null) {
return original;
}
diff --git a/src/test/java/com/android/tools/r8/shaking/annotations/LibraryAndMissingAnnotationsTest.java b/src/test/java/com/android/tools/r8/shaking/annotations/LibraryAndMissingAnnotationsTest.java
new file mode 100644
index 0000000..4e6adad
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/annotations/LibraryAndMissingAnnotationsTest.java
@@ -0,0 +1,181 @@
+// Copyright (c) 2019, 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 org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.BaseCompilerCommand;
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestCompileResult;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestRunResult;
+import com.android.tools.r8.TestShrinkerBuilder;
+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.io.IOException;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.function.Function;
+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 LibraryAndMissingAnnotationsTest extends TestBase {
+
+ private final TestParameters parameters;
+ private final boolean includeOnLibraryPath;
+
+ @Parameters(name = "{0}, includeOnLibraryPath: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimesAndApiLevels().build(), BooleanUtils.values());
+ }
+
+ private static Function<TestParameters, Path> compilationResults =
+ memoizeFunction(LibraryAndMissingAnnotationsTest::compileLibraryAnnotationToRuntime);
+
+ private static Path compileLibraryAnnotationToRuntime(TestParameters parameters)
+ throws CompilationFailedException, IOException {
+ return testForR8(getStaticTemp(), parameters.getBackend())
+ .addProgramClasses(LibraryAnnotation.class)
+ .addKeepClassAndMembersRulesWithAllowObfuscation(LibraryAnnotation.class)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .writeToZip();
+ }
+
+ public LibraryAndMissingAnnotationsTest(TestParameters parameters, boolean includeOnLibraryPath) {
+ this.parameters = parameters;
+ this.includeOnLibraryPath = includeOnLibraryPath;
+ }
+
+ @Test
+ public void testMainWithUseR8() throws Exception {
+ runTest(testForR8(parameters.getBackend()), MainWithUse.class);
+ }
+
+ @Test
+ public void testMainWithUseR8Compat() throws Exception {
+ runTest(testForR8Compat(parameters.getBackend()), MainWithUse.class);
+ }
+
+ @Test
+ public void testMainWithUseProguard() throws Exception {
+ assumeTrue(parameters.isCfRuntime());
+ runTest(testForProguard(), MainWithUse.class);
+ }
+
+ @Test
+ public void testMainWithoutUseR8() throws Exception {
+ runTest(testForR8(parameters.getBackend()), MainWithoutUse.class);
+ }
+
+ @Test
+ public void testMainWithoutUseR8Compat() throws Exception {
+ runTest(testForR8Compat(parameters.getBackend()), MainWithoutUse.class);
+ }
+
+ @Test
+ public void testMainWithoutUseProguard() throws Exception {
+ assumeTrue(parameters.isCfRuntime());
+ runTest(testForProguard(), MainWithoutUse.class);
+ }
+
+ private <
+ C extends BaseCompilerCommand,
+ B extends BaseCompilerCommand.Builder<C, B>,
+ CR extends TestCompileResult<CR, RR>,
+ RR extends TestRunResult<RR>,
+ T extends TestShrinkerBuilder<C, B, CR, RR, T>>
+ void runTest(TestShrinkerBuilder<C, B, CR, RR, T> builder, Class<?> mainClass)
+ throws Exception {
+ T t =
+ builder
+ .addProgramClasses(Foo.class, mainClass)
+ .addKeepAttributes("*Annotation*")
+ .addLibraryFiles(runtimeJar(parameters))
+ .addKeepClassAndMembersRules(Foo.class)
+ .addKeepRules("-dontwarn " + LibraryAndMissingAnnotationsTest.class.getTypeName())
+ .addKeepMainRule(mainClass)
+ .setMinApi(parameters.getApiLevel());
+ if (includeOnLibraryPath) {
+ t.addLibraryClasses(LibraryAnnotation.class);
+ } else {
+ t.addKeepRules("-dontwarn " + LibraryAnnotation.class.getTypeName());
+ }
+ t.compile()
+ .addRunClasspathFiles(compilationResults.apply(parameters))
+ .run(parameters.getRuntime(), mainClass)
+ .inspect(
+ inspector -> {
+ ClassSubject clazz = inspector.clazz(Foo.class);
+ assertThat(clazz, isPresent());
+ assertThat(clazz.annotation(LibraryAnnotation.class.getTypeName()), isPresent());
+ MethodSubject foo = clazz.uniqueMethodWithName("foo");
+ assertThat(foo, isPresent());
+ assertThat(foo.annotation(LibraryAnnotation.class.getTypeName()), isPresent());
+ assertFalse(foo.getMethod().parameterAnnotationsList.isEmpty());
+ assertEquals(
+ LibraryAnnotation.class.getTypeName(),
+ foo.getMethod()
+ .parameterAnnotationsList
+ .get(0)
+ .annotations[0]
+ .getAnnotationType()
+ .toSourceString());
+ assertThat(
+ clazz
+ .uniqueFieldWithName("bar")
+ .annotation(LibraryAnnotation.class.getTypeName()),
+ isPresent());
+ })
+ .assertSuccessWithOutputLines("Hello World!");
+ }
+
+ @Test
+ public void testMainWithoutUse() {}
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @interface LibraryAnnotation {}
+
+ @LibraryAnnotation
+ public static class Foo {
+
+ @LibraryAnnotation public String bar = "Hello World!";
+
+ @LibraryAnnotation
+ public void foo(@LibraryAnnotation String arg) {
+ System.out.println(arg);
+ }
+ }
+
+ public static class MainWithoutUse {
+
+ public static void main(String[] args) {
+ Foo foo = new Foo();
+ foo.foo(foo.bar);
+ }
+ }
+
+ public static class MainWithUse {
+ public static void main(String[] args) {
+ if (args.length > 0 && args[0].equals(LibraryAnnotation.class.getTypeName())) {
+ System.out.print("This will never be printed");
+ }
+ Foo foo = new Foo();
+ foo.foo(foo.bar);
+ }
+ }
+}
