[KeepAnno] Remove KeepForApi default member targets on empty classes
Bug: b/248408342
Fixes: b/310545869
Change-Id: I63924732f525e79a890529857b1d2ecdf00e25a2
diff --git a/src/keepanno/java/com/android/tools/r8/keepanno/asm/KeepEdgeReader.java b/src/keepanno/java/com/android/tools/r8/keepanno/asm/KeepEdgeReader.java
index bc466c5..cf705ed 100644
--- a/src/keepanno/java/com/android/tools/r8/keepanno/asm/KeepEdgeReader.java
+++ b/src/keepanno/java/com/android/tools/r8/keepanno/asm/KeepEdgeReader.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.keepanno.asm;
+import com.android.tools.r8.keepanno.annotations.KeepItemKind;
import com.android.tools.r8.keepanno.ast.AccessVisibility;
import com.android.tools.r8.keepanno.ast.AnnotationConstants;
import com.android.tools.r8.keepanno.ast.AnnotationConstants.Binding;
@@ -91,6 +92,9 @@
private static class KeepEdgeClassVisitor extends ClassVisitor {
private final Parent<KeepDeclaration> parent;
private String className;
+ private List<AnnotationVisitorBase> annotationVisitors = new ArrayList<>();
+ private boolean done = false;
+ private boolean hasMembers = false;
KeepEdgeClassVisitor(Parent<KeepDeclaration> parent) {
super(ASM_VERSION);
@@ -101,6 +105,17 @@
return binaryName.replace('/', '.');
}
+ public boolean classHasMembers() {
+ assert done;
+ return hasMembers;
+ }
+
+ @Override
+ public void visitEnd() {
+ done = true;
+ annotationVisitors.forEach(a -> a.onClassVisitEnd(this));
+ }
+
@Override
public void visit(
int version,
@@ -119,20 +134,28 @@
if (visible) {
return null;
}
+ AnnotationVisitorBase annotationVisitor = determineAnnotationVisitor(descriptor);
+ if (annotationVisitor != null) {
+ annotationVisitors.add(annotationVisitor);
+ }
+ return annotationVisitor;
+ }
+
+ private AnnotationVisitorBase determineAnnotationVisitor(String descriptor) {
if (descriptor.equals(Edge.DESCRIPTOR)) {
return new KeepEdgeVisitor(parent::accept, this::setContext);
}
- if (descriptor.equals(AnnotationConstants.UsesReflection.DESCRIPTOR)) {
+ if (descriptor.equals(UsesReflection.DESCRIPTOR)) {
KeepClassItemPattern classItem =
KeepClassItemPattern.builder()
.setClassNamePattern(KeepQualifiedClassNamePattern.exact(className))
.build();
return new UsesReflectionVisitor(parent::accept, this::setContext, classItem);
}
- if (descriptor.equals(AnnotationConstants.ForApi.DESCRIPTOR)) {
+ if (descriptor.equals(ForApi.DESCRIPTOR)) {
return new ForApiClassVisitor(parent::accept, this::setContext, className);
}
- if (descriptor.equals(AnnotationConstants.UsedByReflection.DESCRIPTOR)
+ if (descriptor.equals(UsedByReflection.DESCRIPTOR)
|| descriptor.equals(AnnotationConstants.UsedByNative.DESCRIPTOR)) {
return new UsedByReflectionClassVisitor(
descriptor, parent::accept, this::setContext, className);
@@ -155,12 +178,14 @@
@Override
public MethodVisitor visitMethod(
int access, String name, String descriptor, String signature, String[] exceptions) {
+ hasMembers = true;
return new KeepEdgeMethodVisitor(parent::accept, className, name, descriptor);
}
@Override
public FieldVisitor visitField(
int access, String name, String descriptor, String signature, Object value) {
+ hasMembers = true;
return new KeepEdgeFieldVisitor(parent::accept, className, name, descriptor);
}
}
@@ -320,6 +345,8 @@
super(ASM_VERSION);
}
+ public void onClassVisitEnd(KeepEdgeClassVisitor visitor) {}
+
public abstract String getAnnotationName();
private String errorMessagePrefix() {
@@ -441,8 +468,6 @@
addContext.accept(metaInfoBuilder);
// The class context/holder is the annotated class.
visit(Item.className, className);
- // The default kind is to target the class and its members.
- visitEnum(null, Kind.DESCRIPTOR, Kind.CLASS_AND_MEMBERS);
}
@Override
@@ -478,9 +503,16 @@
}
@Override
- public void visitEnd() {
+ public void onClassVisitEnd(KeepEdgeClassVisitor visitor) {
+ if (getKind() == null) {
+ // The default kind is to target the class and its members if any members exist.
+ KeepItemKind defaultKind =
+ visitor.classHasMembers() ? KeepItemKind.CLASS_AND_MEMBERS : KeepItemKind.ONLY_CLASS;
+ visitEnum(null, Kind.DESCRIPTOR, defaultKind.name());
+ }
if (!getKind().equals(Kind.ONLY_CLASS) && isDefaultMemberDeclaration()) {
// If no member declarations have been made, set public & protected as the default.
+ // Only do so if the class actually has any members.
AnnotationVisitor v = visitArray(Item.memberAccess);
v.visitEnum(null, MemberAccess.DESCRIPTOR, MemberAccess.PUBLIC);
v.visitEnum(null, MemberAccess.DESCRIPTOR, MemberAccess.PROTECTED);
diff --git a/src/main/java/com/android/tools/r8/profile/art/ArtProfileClassRuleInfo.java b/src/main/java/com/android/tools/r8/profile/art/ArtProfileClassRuleInfo.java
index 6dcde1d..29d745b 100644
--- a/src/main/java/com/android/tools/r8/profile/art/ArtProfileClassRuleInfo.java
+++ b/src/main/java/com/android/tools/r8/profile/art/ArtProfileClassRuleInfo.java
@@ -5,7 +5,6 @@
package com.android.tools.r8.profile.art;
import com.android.tools.r8.keepanno.annotations.KeepForApi;
-import com.android.tools.r8.keepanno.annotations.KeepItemKind;
-@KeepForApi(kind = KeepItemKind.ONLY_CLASS)
+@KeepForApi
public interface ArtProfileClassRuleInfo {}
diff --git a/src/test/java/com/android/tools/r8/keepanno/KeepForApiWithoutMembersTest.java b/src/test/java/com/android/tools/r8/keepanno/KeepForApiWithoutMembersTest.java
new file mode 100644
index 0000000..e715fff
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/keepanno/KeepForApiWithoutMembersTest.java
@@ -0,0 +1,70 @@
+// Copyright (c) 2023, 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.keepanno;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestDiagnosticMessages;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.keepanno.annotations.KeepForApi;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class KeepForApiWithoutMembersTest extends TestBase {
+
+ static final String EXPECTED = StringUtils.lines("true");
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDefaultRuntimes().withApiLevel(AndroidApiLevel.B).build();
+ }
+
+ public KeepForApiWithoutMembersTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testReference() throws Exception {
+ testForRuntime(parameters)
+ .addProgramClasses(getInputClasses())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test
+ public void testWithRuleExtraction() throws Exception {
+ testForR8(parameters.getBackend())
+ .enableExperimentalKeepAnnotations()
+ .addProgramClasses(getInputClasses())
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters)
+ .compileWithExpectedDiagnostics(TestDiagnosticMessages::assertNoInfos)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ public List<Class<?>> getInputClasses() {
+ return ImmutableList.of(TestClass.class, EmptyInterface.class, A.class);
+ }
+
+ @KeepForApi
+ interface EmptyInterface {}
+
+ static class A implements EmptyInterface {}
+
+ static class TestClass {
+
+ public static void main(String[] args) throws Exception {
+ System.out.println(new A() instanceof EmptyInterface);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/keepanno/UsedByReflectionWithoutMembersTest.java b/src/test/java/com/android/tools/r8/keepanno/UsedByReflectionWithoutMembersTest.java
new file mode 100644
index 0000000..7fa44ba
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/keepanno/UsedByReflectionWithoutMembersTest.java
@@ -0,0 +1,70 @@
+// Copyright (c) 2023, 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.keepanno;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestDiagnosticMessages;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.keepanno.annotations.UsedByReflection;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class UsedByReflectionWithoutMembersTest extends TestBase {
+
+ static final String EXPECTED = StringUtils.lines("true");
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDefaultRuntimes().withApiLevel(AndroidApiLevel.B).build();
+ }
+
+ public UsedByReflectionWithoutMembersTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testReference() throws Exception {
+ testForRuntime(parameters)
+ .addProgramClasses(getInputClasses())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test
+ public void testWithRuleExtraction() throws Exception {
+ testForR8(parameters.getBackend())
+ .enableExperimentalKeepAnnotations()
+ .addProgramClasses(getInputClasses())
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters)
+ .compileWithExpectedDiagnostics(TestDiagnosticMessages::assertNoInfos)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ public List<Class<?>> getInputClasses() {
+ return ImmutableList.of(TestClass.class, EmptyInterface.class, A.class);
+ }
+
+ @UsedByReflection
+ interface EmptyInterface {}
+
+ static class A implements EmptyInterface {}
+
+ static class TestClass {
+
+ public static void main(String[] args) throws Exception {
+ System.out.println(new A() instanceof EmptyInterface);
+ }
+ }
+}
