Don't include source and class retention annotation classes in maindex
They are not annotating any classes in the DEX output.
The is based on https://r8-review.googlesource.com/c/r8/+/59608.
Bug: 186090713
Bug: 202173862
Change-Id: I508cf52b8262775b0ed49ac8a94340367fed96fc
diff --git a/src/main/java/com/android/tools/r8/D8Command.java b/src/main/java/com/android/tools/r8/D8Command.java
index 2013fb9..242064b 100644
--- a/src/main/java/com/android/tools/r8/D8Command.java
+++ b/src/main/java/com/android/tools/r8/D8Command.java
@@ -467,7 +467,7 @@
internal.enableMainDexListCheck = enableMainDexListCheck;
internal.minApiLevel = AndroidApiLevel.getAndroidApiLevel(getMinApiLevel());
internal.intermediate = intermediate;
- internal.readCompileTimeAnnotations = intermediate;
+ internal.retainCompileTimeAnnotations = intermediate;
internal.desugarGraphConsumer = desugarGraphConsumer;
internal.mainDexKeepRules = mainDexKeepRules;
internal.lineNumberOptimization = LineNumberOptimization.OFF;
diff --git a/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java b/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java
index 2512782..337fad7 100644
--- a/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java
+++ b/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java
@@ -224,6 +224,8 @@
internal.minimalMainDex = internal.debug;
internal.enableEnumValueOptimization = false;
internal.inlinerOptions().enableInlining = false;
+ assert internal.retainCompileTimeAnnotations;
+ internal.retainCompileTimeAnnotations = false;
return internal;
}
}
diff --git a/src/main/java/com/android/tools/r8/L8Command.java b/src/main/java/com/android/tools/r8/L8Command.java
index 133ca25..c0fb797 100644
--- a/src/main/java/com/android/tools/r8/L8Command.java
+++ b/src/main/java/com/android/tools/r8/L8Command.java
@@ -166,7 +166,7 @@
assert !internal.minimalMainDex;
internal.minApiLevel = AndroidApiLevel.getAndroidApiLevel(getMinApiLevel());
assert !internal.intermediate;
- assert internal.readCompileTimeAnnotations;
+ assert internal.retainCompileTimeAnnotations;
internal.programConsumer = getProgramConsumer();
assert internal.programConsumer instanceof ClassFileConsumer;
diff --git a/src/main/java/com/android/tools/r8/graph/DexAnnotation.java b/src/main/java/com/android/tools/r8/graph/DexAnnotation.java
index 1141602..6952bf7 100644
--- a/src/main/java/com/android/tools/r8/graph/DexAnnotation.java
+++ b/src/main/java/com/android/tools/r8/graph/DexAnnotation.java
@@ -111,7 +111,7 @@
}
public static boolean retainCompileTimeAnnotation(DexType annotation, InternalOptions options) {
- if (options.readCompileTimeAnnotations) {
+ if (options.retainCompileTimeAnnotations) {
return true;
}
if (annotation == options.itemFactory.dalvikFastNativeAnnotation
diff --git a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
index 0c78a72..b59c44f 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -175,9 +175,8 @@
private static boolean retainCompileTimeAnnotation(
String desc, JarApplicationReader application) {
- return application.options.readCompileTimeAnnotations
- || DexAnnotation.retainCompileTimeAnnotation(
- application.getTypeFromDescriptor(desc), application.options);
+ return DexAnnotation.retainCompileTimeAnnotation(
+ application.getTypeFromDescriptor(desc), application.options);
}
private static DexEncodedAnnotation createEncodedAnnotation(String desc,
diff --git a/src/main/java/com/android/tools/r8/shaking/MainDexListBuilder.java b/src/main/java/com/android/tools/r8/shaking/MainDexListBuilder.java
index 23e9e26..4952830 100644
--- a/src/main/java/com/android/tools/r8/shaking/MainDexListBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/MainDexListBuilder.java
@@ -76,7 +76,9 @@
}
DexType dexType = clazz.type;
if (isAnnotation(dexType) && isAnnotationWithEnum(dexType)) {
- addAnnotationsWithEnum(clazz);
+ if (isVisibleAnnotation(clazz)) {
+ addAnnotationsWithEnum(clazz);
+ }
continue;
}
// Classes with annotations must be in the same dex file as the annotation. As all
@@ -93,6 +95,23 @@
}
}
+ private boolean isVisibleAnnotation(DexProgramClass clazz) {
+ if (retainCompileTimeAnnotation(clazz.type)) {
+ return true;
+ }
+ DexAnnotation retentionAnnotation =
+ clazz.annotations().getFirstMatching(appView.dexItemFactory().retentionType);
+ // Default is CLASS retention
+ if (retentionAnnotation == null) {
+ return false;
+ }
+ return retentionAnnotation.annotation.toString().contains("RUNTIME");
+ }
+
+ private boolean retainCompileTimeAnnotation(DexType type) {
+ return DexAnnotation.retainCompileTimeAnnotation(type, appView.options());
+ }
+
private boolean isAnnotationWithEnum(DexType dexType) {
Boolean value = annotationTypeContainEnum.get(dexType);
if (value == null) {
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 78aab7d..1899f46 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -518,7 +518,7 @@
// Skipping min_api check and compiling an intermediate result intended for later merging.
// Intermediate builds also emits or update synthesized classes mapping.
public boolean intermediate = false;
- public boolean readCompileTimeAnnotations = true;
+ public boolean retainCompileTimeAnnotations = true;
public List<String> logArgumentsFilter = ImmutableList.of();
// Flag to turn on/offLoad/store optimization in the Cf back-end.
diff --git a/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java b/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java
index 634cfe8..853c057 100644
--- a/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java
+++ b/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java
@@ -32,6 +32,12 @@
});
}
+ public final GenerateMainDexListRunResult inspectMainDexClasses(
+ Consumer<List<ClassReference>> consumer) {
+ consumer.accept(getMainDexList());
+ return self();
+ }
+
public GenerateMainDexListRunResult inspectDiagnosticMessages(
Consumer<TestDiagnosticMessages> consumer) {
consumer.accept(state.getDiagnosticsMessages());
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 0d02a13..8f3b1e4 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -1342,6 +1342,14 @@
D8.runForTesting(command.getInputApp(), internalOptions);
}
+ public static List<String> runGenerateMainDexList(
+ GenerateMainDexListCommand command, Consumer<InternalOptions> optionsConsumer)
+ throws CompilationFailedException {
+ InternalOptions internalOptions = command.getInternalOptions();
+ optionsConsumer.accept(internalOptions);
+ return GenerateMainDexList.runForTesting(command.getInputApp(), internalOptions);
+ }
+
public static AndroidApp runDexer(String fileName, String outDir, String... extraArgs)
throws IOException {
List<String> args = new ArrayList<>();
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexSourceAndClassRetentionTest.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexSourceAndClassRetentionTest.java
index 74cc91d..4bcb143 100644
--- a/src/test/java/com/android/tools/r8/maindexlist/MainDexSourceAndClassRetentionTest.java
+++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexSourceAndClassRetentionTest.java
@@ -4,99 +4,101 @@
package com.android.tools.r8.maindexlist;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.references.ClassReference;
import com.android.tools.r8.references.Reference;
-import com.android.tools.r8.utils.AndroidApiLevel;
import com.google.common.collect.ImmutableSet;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.HashSet;
-import java.util.List;
import java.util.Set;
+import java.util.stream.Collectors;
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 MainDexSourceAndClassRetentionTest extends TestBase {
- private final TestParameters parameters;
+ private static final Set<Class<?>> MAINDEX_CLASSES = ImmutableSet.of(Main.class);
+
+ @Parameter(0)
+ public TestParameters parameters;
@Parameters(name = "{0}")
- public static TestParametersCollection data() {
+ public static TestParametersCollection parameters() {
return getTestParameters()
.withDexRuntimes()
.withApiLevelsEndingAtExcluding(apiLevelWithNativeMultiDexSupport())
.build();
}
- public MainDexSourceAndClassRetentionTest(TestParameters parameters) {
- this.parameters = parameters;
- }
-
@Test
public void testMainDex() throws Exception {
- List<ClassReference> mainDexList =
- testForMainDexListGenerator(temp)
- .addInnerClasses(getClass())
- .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.B))
- .addMainDexRules(
- "-keep class " + Main.class.getTypeName() + " {",
- " public static void main(java.lang.String[]);",
- "}")
- .run()
- .getMainDexList();
- // TODO(b/186090713): {Foo, BAR} and {Source,Class}RetentionAnnotation should not be included.
- assertEquals(
- ImmutableSet.of(
- Reference.classFromClass(Foo.class),
- Reference.classFromClass(Bar.class),
- Reference.classFromClass(Main.class),
- Reference.classFromClass(ClassRetentionAnnotation.class),
- Reference.classFromClass(SourceRetentionAnnotation.class)),
- new HashSet<>(mainDexList));
+ testForMainDexListGenerator(temp)
+ .addInnerClasses(getClass())
+ .addLibraryFiles(ToolHelper.getMostRecentAndroidJar())
+ .addMainDexRules(
+ "-keep class " + Main.class.getTypeName() + " {",
+ " public static void main(java.lang.String[]);",
+ "}")
+ .run()
+ .inspectMainDexClasses(
+ mainDexList -> {
+ assertEquals(
+ MAINDEX_CLASSES.stream()
+ .map(Reference::classFromClass)
+ .collect(Collectors.toSet()),
+ new HashSet<>(mainDexList));
+ });
}
@Test
public void testD8() throws Exception {
- Set<String> mainDexClasses =
- testForD8(temp)
- .addInnerClasses(getClass())
- .addLibraryFiles(ToolHelper.getMostRecentAndroidJar())
- .setMinApi(parameters.getApiLevel())
- .collectMainDexClasses()
- .addMainDexRules(
- "-keep class " + Main.class.getTypeName() + " {",
- " public static void main(java.lang.String[]);",
- "}")
- .compile()
- .getMainDexClasses();
- // TODO(b/186090713): {Foo, BAR} and {Source,Class}RetentionAnnotation should not be included.
- assertEquals(
- ImmutableSet.of(
- typeName(Foo.class),
- typeName(Bar.class),
- typeName(Main.class),
- typeName(ClassRetentionAnnotation.class),
- typeName(SourceRetentionAnnotation.class)),
- mainDexClasses);
+ testForD8(temp)
+ .addInnerClasses(getClass())
+ .addLibraryFiles(ToolHelper.getMostRecentAndroidJar())
+ .setMinApi(parameters.getApiLevel())
+ .collectMainDexClasses()
+ .addMainDexRules(
+ "-keep class " + Main.class.getTypeName() + " {",
+ " public static void main(java.lang.String[]);",
+ "}")
+ .allowStdoutMessages()
+ .compile()
+ .inspect(
+ inspector -> {
+ // Source and class retention annotation classes are still in the output, but does not
+ // annotate anything.
+ assertThat(inspector.clazz(SourceRetentionAnnotation.class), isPresent());
+ assertThat(inspector.clazz(ClassRetentionAnnotation.class), isPresent());
+ assertEquals(0, inspector.clazz(Main.class).annotations().size());
+ assertEquals(0, inspector.clazz(A.class).annotations().size());
+ })
+ .inspectMainDexClasses(
+ mainDexClasses -> {
+ assertEquals(
+ MAINDEX_CLASSES.stream().map(TestBase::typeName).collect(Collectors.toSet()),
+ new HashSet<>(mainDexClasses));
+ });
}
public enum Foo {
- TEST;
+ TEST
}
public enum Bar {
- TEST;
+ TEST
}
@Retention(RetentionPolicy.SOURCE)
@@ -115,6 +117,13 @@
@SourceRetentionAnnotation(Foo.TEST)
@ClassRetentionAnnotation(Bar.TEST)
+ public static class A {
+
+ public static void main(String[] args) {}
+ }
+
+ @SourceRetentionAnnotation(Foo.TEST)
+ @ClassRetentionAnnotation(Bar.TEST)
public static class Main {
public static void main(String[] args) {}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java
index 4ea9a82..2d74926 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java
@@ -110,6 +110,11 @@
}
@Override
+ public List<AnnotationSubject> annotations() {
+ throw new Unreachable("Cannot determine if an absent class has annotations");
+ }
+
+ @Override
public AnnotationSubject annotation(String name) {
return new AbsentAnnotationSubject();
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
index 6d7067a..8d91f3c 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.graph.DexValue;
import com.android.tools.r8.naming.MemberNaming.Signature;
import com.android.tools.r8.references.FieldReference;
+import java.util.List;
public class AbsentFieldSubject extends FieldSubject {
@@ -59,6 +60,11 @@
}
@Override
+ public List<AnnotationSubject> annotations() {
+ throw new Unreachable("Cannot determine if an absent field has annotations");
+ }
+
+ @Override
public AnnotationSubject annotation(String name) {
return new AbsentAnnotationSubject();
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
index 9299e7f..4ffcdb4 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.naming.MemberNaming.Signature;
+import java.util.List;
public class AbsentMethodSubject extends MethodSubject {
@@ -116,6 +117,11 @@
}
@Override
+ public List<AnnotationSubject> annotations() {
+ throw new Unreachable("Cannot determine if an absent method has annotations");
+ }
+
+ @Override
public AnnotationSubject annotation(String name) {
return new AbsentAnnotationSubject();
}
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 2dc0774..754b470 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
@@ -6,9 +6,12 @@
import com.android.tools.r8.graph.AccessFlags;
import java.lang.annotation.Annotation;
+import java.util.List;
public abstract class ClassOrMemberSubject extends Subject {
+ public abstract List<AnnotationSubject> annotations();
+
public abstract AnnotationSubject annotation(String name);
public final AnnotationSubject annotation(Class<? extends Annotation> clazz) {
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java
index 318f0d9..a0839d8 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java
@@ -349,6 +349,15 @@
}
@Override
+ public List<AnnotationSubject> annotations() {
+ List<AnnotationSubject> result = new ArrayList<>();
+ for (DexAnnotation annotation : dexClass.annotations().annotations) {
+ result.add(new FoundAnnotationSubject(annotation));
+ }
+ return result;
+ }
+
+ @Override
public AnnotationSubject annotation(String name) {
// Ensure we don't check for annotations represented as attributes.
assert !name.endsWith("EnclosingClass")
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
index 023c088..2ea0107 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.utils.codeinspector;
+import com.android.tools.r8.errors.Unimplemented;
import com.android.tools.r8.graph.AccessFlags;
import com.android.tools.r8.graph.DexAnnotation;
import com.android.tools.r8.graph.DexEncodedField;
@@ -15,6 +16,7 @@
import com.android.tools.r8.naming.signature.GenericSignatureParser;
import com.android.tools.r8.references.FieldReference;
import com.android.tools.r8.references.Reference;
+import java.util.List;
public class FoundFieldSubject extends FieldSubject {
@@ -112,6 +114,11 @@
}
@Override
+ public List<AnnotationSubject> annotations() {
+ throw new Unimplemented();
+ }
+
+ @Override
public AnnotationSubject annotation(String name) {
DexAnnotation annotation = codeInspector.findAnnotation(name, dexField.annotations());
return annotation == null
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
index d781ef5..7c882b4 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
@@ -325,6 +325,11 @@
}
@Override
+ public List<AnnotationSubject> annotations() {
+ throw new Unimplemented();
+ }
+
+ @Override
public AnnotationSubject annotation(String name) {
DexAnnotation annotation = codeInspector.findAnnotation(name, dexMethod.annotations());
return annotation == null