[KeepAnno] Clean up AST after CLASS_AND_MEMBERS is a derived form
This also removed the 'any' item such that an item is now either
a class or a member, but cannot be both.
Bug: b/248408342
Change-Id: I9705191b94f0ff37216a854766b4c43b0f2eec14
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 5f7f156..d3fc1ba 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
@@ -34,7 +34,6 @@
import com.android.tools.r8.keepanno.ast.KeepFieldNamePattern;
import com.android.tools.r8.keepanno.ast.KeepFieldPattern;
import com.android.tools.r8.keepanno.ast.KeepFieldTypePattern;
-import com.android.tools.r8.keepanno.ast.KeepItemKind;
import com.android.tools.r8.keepanno.ast.KeepItemPattern;
import com.android.tools.r8.keepanno.ast.KeepItemReference;
import com.android.tools.r8.keepanno.ast.KeepMemberAccessPattern;
@@ -491,10 +490,8 @@
if (!descriptor.equals(itemDescriptor)) {
throw new KeepEdgeException("@KeepForApi must reference its class context " + className);
}
- if (itemPattern.getKind().equals(KeepItemKind.ONLY_MEMBERS)) {
- if (items.size() == 1) {
+ if (itemPattern.isMemberItemPattern() && items.size() == 1) {
throw new KeepEdgeException("@KeepForApi kind must include its class");
- }
}
if (!itemPattern.getExtendsPattern().isAny()) {
throw new KeepEdgeException("@KeepForApi cannot define an 'extends' pattern.");
@@ -668,10 +665,8 @@
throw new KeepEdgeException(
"@" + getAnnotationName() + " must reference its class context " + className);
}
- if (itemPattern.getKind().equals(KeepItemKind.ONLY_MEMBERS)) {
- if (items.size() == 1) {
- throw new KeepEdgeException("@" + getAnnotationName() + " kind must include its class");
- }
+ if (itemPattern.isMemberItemPattern() && items.size() == 1) {
+ throw new KeepEdgeException("@" + getAnnotationName() + " kind must include its class");
}
if (!itemPattern.getExtendsPattern().isAny()) {
throw new KeepEdgeException(
@@ -701,7 +696,7 @@
private final KeepEdgeMetaInfo.Builder metaInfoBuilder = KeepEdgeMetaInfo.builder();
private final UserBindingsHelper bindingsHelper = new UserBindingsHelper();
private final KeepConsequences.Builder consequences = KeepConsequences.builder();
- private KeepItemKind kind = KeepItemKind.ONLY_MEMBERS;
+ private String kind = Kind.ONLY_MEMBERS;
UsedByReflectionMemberVisitor(
String annotationDescriptor,
@@ -739,13 +734,9 @@
// The default value is obtained by not assigning a kind (e.g., null in the builder).
break;
case Kind.ONLY_CLASS:
- kind = KeepItemKind.ONLY_CLASS;
- break;
case Kind.ONLY_MEMBERS:
- kind = KeepItemKind.ONLY_MEMBERS;
- break;
case Kind.CLASS_AND_MEMBERS:
- kind = KeepItemKind.CLASS_AND_MEMBERS;
+ kind = value;
break;
default:
super.visitEnum(name, descriptor, value);
@@ -771,11 +762,11 @@
@Override
public void visitEnd() {
- if (kind.equals(KeepItemKind.ONLY_CLASS)) {
+ if (Kind.ONLY_CLASS.equals(kind)) {
throw new KeepEdgeException("@" + getAnnotationName() + " kind must include its member");
}
- assert context.getKind() == KeepItemKind.ONLY_MEMBERS;
- if (kind.equals(KeepItemKind.CLASS_AND_MEMBERS)) {
+ assert context.isMemberItemPattern();
+ if (Kind.CLASS_AND_MEMBERS.equals(kind)) {
consequences.addTarget(
KeepTarget.builder()
.setItemPattern(
@@ -1436,21 +1427,24 @@
assert !itemReference.isBindingReference();
KeepItemPattern itemPattern = itemReference.asItemPattern();
KeepItemPattern classPattern;
+ KeepItemPattern memberPattern;
if (itemPattern.isClassItemPattern()) {
classPattern = itemPattern;
+ memberPattern =
+ KeepItemPattern.builder()
+ .copyFrom(itemPattern)
+ .setMemberPattern(KeepMemberPattern.allMembers())
+ .build();
} else {
+ memberPattern = itemPattern;
classPattern =
KeepItemPattern.builder()
.copyFrom(itemPattern)
- .setKind(KeepItemKind.ONLY_CLASS)
.setMemberPattern(KeepMemberPattern.none())
.build();
}
- KeepItemPattern memberPattern =
- KeepItemPattern.builder()
- .copyFrom(itemPattern)
- .setKind(KeepItemKind.ONLY_MEMBERS)
- .build();
+ assert classPattern.isClassItemPattern();
+ assert memberPattern.isMemberItemPattern();
return ImmutableList.of(
KeepItemReference.fromItemPattern(classPattern),
KeepItemReference.fromItemPattern(memberPattern));
@@ -1470,27 +1464,31 @@
assert kind != null;
if (Kind.CLASS_AND_MEMBERS.equals(kind)) {
KeepItemPattern itemPattern = itemReference.asItemPattern();
+ KeepMemberPattern memberPattern;
KeepItemPattern classPattern;
if (itemPattern.isClassItemPattern()) {
+ memberPattern = KeepMemberPattern.allMembers();
classPattern = itemPattern;
} else {
+ memberPattern = itemPattern.getMemberPattern();
classPattern =
KeepItemPattern.builder()
.copyFrom(itemPattern)
- .setKind(KeepItemKind.ONLY_CLASS)
.setMemberPattern(KeepMemberPattern.none())
.build();
}
BindingSymbol symbol = getBindingsHelper().defineFreshBinding("CLASS", classPattern);
- KeepItemPattern memberPattern =
+ KeepItemPattern memberItemPattern =
KeepItemPattern.builder()
.copyFrom(itemPattern)
.setClassReference(KeepClassReference.fromBindingReference(symbol))
- .setKind(KeepItemKind.ONLY_MEMBERS)
+ .setMemberPattern(memberPattern)
.build();
+ assert classPattern.isClassItemPattern();
+ assert memberItemPattern.isMemberItemPattern();
return ImmutableList.of(
KeepItemReference.fromItemPattern(classPattern),
- KeepItemReference.fromItemPattern(memberPattern));
+ KeepItemReference.fromItemPattern(memberItemPattern));
} else {
return Collections.singletonList(itemReference);
}
diff --git a/src/keepanno/java/com/android/tools/r8/keepanno/asm/KeepEdgeWriter.java b/src/keepanno/java/com/android/tools/r8/keepanno/asm/KeepEdgeWriter.java
index dc8e7c9..ec3fb30 100644
--- a/src/keepanno/java/com/android/tools/r8/keepanno/asm/KeepEdgeWriter.java
+++ b/src/keepanno/java/com/android/tools/r8/keepanno/asm/KeepEdgeWriter.java
@@ -138,12 +138,6 @@
}
private void writeItem(AnnotationVisitor itemVisitor, KeepItemPattern item) {
- if (item.isAny(
- binding -> {
- throw new Unimplemented();
- })) {
- throw new Unimplemented();
- }
KeepClassReference classReference = item.getClassReference();
if (classReference.isBindingReference()) {
throw new Unimplemented();
diff --git a/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepBindings.java b/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepBindings.java
index 790fb9e..a3ba447 100644
--- a/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepBindings.java
+++ b/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepBindings.java
@@ -45,26 +45,6 @@
bindings.forEach((name, binding) -> fn.accept(name, binding.getItem()));
}
- public boolean isAny(KeepItemReference itemReference) {
- return itemReference.isBindingReference()
- ? isAny(get(itemReference.asBindingReference()).getItem())
- : isAny(itemReference.asItemPattern());
- }
-
- public boolean isAny(KeepItemPattern itemPattern) {
- return itemPattern.isAny(this::isAnyClassNamePattern);
- }
-
- // If the outer-most item has been judged to be "any" then we internally only need to check
- // that the class-name pattern itself is "any". The class-name could potentially reference names
- // of other item bindings so this is a recursive search.
- private boolean isAnyClassNamePattern(BindingSymbol bindingName) {
- KeepClassReference classReference = get(bindingName).getItem().getClassReference();
- return classReference.isBindingReference()
- ? isAnyClassNamePattern(classReference.asBindingReference())
- : classReference.asClassNamePattern().isAny();
- }
-
@Override
public String toString() {
return "{"
diff --git a/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepEdge.java b/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepEdge.java
index b2e541a..3b11a28 100644
--- a/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepEdge.java
+++ b/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepEdge.java
@@ -37,11 +37,7 @@
* ITEM_REFERENCE ::= BINDING_REFERENCE | ITEM_PATTERN
* CLASS_REFERENCE ::= BINDING_REFERENCE | QUALIFIED_CLASS_NAME_PATTERN
*
- * ITEM_PATTERN
- * ::= any
- * | ITEM_KIND class CLASS_REFERENCE extends EXTENDS_PATTERN { MEMBER_PATTERN }
- *
- * ITEM_KIND ::= ONLY_CLASS | ONLY_MEMBERS | CLASS_AND_MEMBERS
+ * ITEM_PATTERN ::= class CLASS_REFERENCE extends EXTENDS_PATTERN { MEMBER_PATTERN }
*
* TYPE_PATTERN ::= any | exact type-descriptor
* PACKAGE_PATTERN ::= any | exact package-name
diff --git a/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepItemKind.java b/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepItemKind.java
deleted file mode 100644
index 0d59769..0000000
--- a/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepItemKind.java
+++ /dev/null
@@ -1,10 +0,0 @@
-// 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.ast;
-
-public enum KeepItemKind {
- ONLY_CLASS,
- ONLY_MEMBERS,
- CLASS_AND_MEMBERS
-}
diff --git a/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepItemPattern.java b/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepItemPattern.java
index d81dde9..7634c68 100644
--- a/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepItemPattern.java
+++ b/src/keepanno/java/com/android/tools/r8/keepanno/ast/KeepItemPattern.java
@@ -6,7 +6,6 @@
import com.android.tools.r8.keepanno.ast.KeepBindings.BindingSymbol;
import java.util.Collection;
import java.util.Objects;
-import java.util.function.Predicate;
/**
* A pattern for matching items in the program.
@@ -20,29 +19,28 @@
*/
public class KeepItemPattern {
- public static KeepItemPattern any() {
- return builder().any().build();
+ public static KeepItemPattern anyClass() {
+ return builder().setMemberPattern(KeepMemberPattern.none()).build();
+ }
+
+ public static KeepItemPattern anyMember() {
+ return builder().setMemberPattern(KeepMemberPattern.allMembers()).build();
}
public static Builder builder() {
return new Builder();
}
- public boolean isClassAndMemberPattern() {
- return kind == KeepItemKind.CLASS_AND_MEMBERS;
- }
-
public boolean isClassItemPattern() {
- return kind == KeepItemKind.ONLY_CLASS;
+ return memberPattern.isNone();
}
public boolean isMemberItemPattern() {
- return kind == KeepItemKind.ONLY_MEMBERS;
+ return !isClassItemPattern();
}
public static class Builder {
- private KeepItemKind kind = null;
private KeepClassReference classReference =
KeepClassReference.fromClassNamePattern(KeepQualifiedClassNamePattern.any());
private KeepExtendsPattern extendsPattern = KeepExtendsPattern.any();
@@ -51,25 +49,11 @@
private Builder() {}
public Builder copyFrom(KeepItemPattern pattern) {
- return setKind(pattern.getKind())
- .setClassReference(pattern.getClassReference())
+ return setClassReference(pattern.getClassReference())
.setExtendsPattern(pattern.getExtendsPattern())
.setMemberPattern(pattern.getMemberPattern());
}
- public Builder any() {
- kind = KeepItemKind.CLASS_AND_MEMBERS;
- classReference = KeepClassReference.fromClassNamePattern(KeepQualifiedClassNamePattern.any());
- extendsPattern = KeepExtendsPattern.any();
- memberPattern = KeepMemberPattern.allMembers();
- return this;
- }
-
- public Builder setKind(KeepItemKind kind) {
- this.kind = kind;
- return this;
- }
-
public Builder setClassReference(KeepClassReference classReference) {
this.classReference = classReference;
return this;
@@ -90,56 +74,27 @@
}
public KeepItemPattern build() {
- if (kind == null) {
- kind = memberPattern.isNone() ? KeepItemKind.ONLY_CLASS : KeepItemKind.ONLY_MEMBERS;
- }
- if (kind == KeepItemKind.ONLY_CLASS && !memberPattern.isNone()) {
- throw new KeepEdgeException(
- "Invalid kind ONLY_CLASS for item with member pattern: " + memberPattern);
- }
- if (kind == KeepItemKind.ONLY_MEMBERS && memberPattern.isNone()) {
- throw new KeepEdgeException("Invalid kind ONLY_MEMBERS for item with no member pattern");
- }
- if (kind == KeepItemKind.CLASS_AND_MEMBERS && memberPattern.isNone()) {
- throw new KeepEdgeException(
- "Invalid kind CLASS_AND_MEMBERS for item with no member pattern");
- }
- return new KeepItemPattern(kind, classReference, extendsPattern, memberPattern);
+ return new KeepItemPattern(classReference, extendsPattern, memberPattern);
}
}
- private final KeepItemKind kind;
private final KeepClassReference classReference;
private final KeepExtendsPattern extendsPattern;
private final KeepMemberPattern memberPattern;
// TODO: class annotations
private KeepItemPattern(
- KeepItemKind kind,
KeepClassReference classReference,
KeepExtendsPattern extendsPattern,
KeepMemberPattern memberPattern) {
- assert kind != null;
assert classReference != null;
assert extendsPattern != null;
assert memberPattern != null;
- this.kind = kind;
this.classReference = classReference;
this.extendsPattern = extendsPattern;
this.memberPattern = memberPattern;
}
- public boolean isAny(Predicate<BindingSymbol> onReference) {
- return kind.equals(KeepItemKind.CLASS_AND_MEMBERS)
- && extendsPattern.isAny()
- && memberPattern.isAllMembers()
- && classReference.isAny(onReference);
- }
-
- public KeepItemKind getKind() {
- return kind;
- }
-
public KeepClassReference getClassReference() {
return classReference;
}
@@ -157,37 +112,40 @@
}
@Override
- @SuppressWarnings("EqualsGetClass")
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
- if (obj == null || getClass() != obj.getClass()) {
+ if (!(obj instanceof KeepItemPattern)) {
return false;
}
KeepItemPattern that = (KeepItemPattern) obj;
- return kind.equals(that.kind)
- && classReference.equals(that.classReference)
+ return classReference.equals(that.classReference)
&& extendsPattern.equals(that.extendsPattern)
&& memberPattern.equals(that.memberPattern);
}
@Override
public int hashCode() {
- return Objects.hash(kind, classReference, extendsPattern, memberPattern);
+ return Objects.hash(classReference, extendsPattern, memberPattern);
}
@Override
public String toString() {
- return "KeepClassPattern{"
- + "kind="
- + kind
- + ", classReference="
- + classReference
- + ", extendsPattern="
- + extendsPattern
- + ", memberPattern="
- + memberPattern
- + '}';
+ StringBuilder builder = new StringBuilder();
+ if (isClassItemPattern()) {
+ builder.append("KeepClassPattern");
+ } else {
+ assert isMemberItemPattern();
+ builder.append("KeepMemberPattern");
+ }
+ builder.append("{ class=").append(classReference);
+ if (!extendsPattern.isAny()) {
+ builder.append(", extends=").append(extendsPattern);
+ }
+ if (!memberPattern.isNone()) {
+ builder.append(", members=").append(memberPattern);
+ }
+ return builder.append('}').toString();
}
}
diff --git a/src/keepanno/java/com/android/tools/r8/keepanno/keeprules/KeepEdgeNormalizer.java b/src/keepanno/java/com/android/tools/r8/keepanno/keeprules/KeepEdgeNormalizer.java
index cf79e0f..9bd2c09 100644
--- a/src/keepanno/java/com/android/tools/r8/keepanno/keeprules/KeepEdgeNormalizer.java
+++ b/src/keepanno/java/com/android/tools/r8/keepanno/keeprules/KeepEdgeNormalizer.java
@@ -9,7 +9,6 @@
import com.android.tools.r8.keepanno.ast.KeepCondition;
import com.android.tools.r8.keepanno.ast.KeepConsequences;
import com.android.tools.r8.keepanno.ast.KeepEdge;
-import com.android.tools.r8.keepanno.ast.KeepItemKind;
import com.android.tools.r8.keepanno.ast.KeepItemPattern;
import com.android.tools.r8.keepanno.ast.KeepItemReference;
import com.android.tools.r8.keepanno.ast.KeepPreconditions;
@@ -119,10 +118,8 @@
private KeepItemPattern getMemberItemPattern(
KeepItemPattern fromPattern, KeepClassReference classReference) {
- assert fromPattern.getKind().equals(KeepItemKind.ONLY_MEMBERS)
- || fromPattern.getKind().equals(KeepItemKind.CLASS_AND_MEMBERS);
+ assert fromPattern.isMemberItemPattern();
return KeepItemPattern.builder()
- .setKind(fromPattern.getKind())
.setClassReference(classReference)
.setMemberPattern(fromPattern.getMemberPattern())
.build();
diff --git a/src/keepanno/java/com/android/tools/r8/keepanno/keeprules/KeepRuleExtractor.java b/src/keepanno/java/com/android/tools/r8/keepanno/keeprules/KeepRuleExtractor.java
index 3dffb77..5ad2ae3 100644
--- a/src/keepanno/java/com/android/tools/r8/keepanno/keeprules/KeepRuleExtractor.java
+++ b/src/keepanno/java/com/android/tools/r8/keepanno/keeprules/KeepRuleExtractor.java
@@ -297,15 +297,6 @@
List<BindingSymbol> targetMembers = new ArrayList<>();
for (BindingSymbol targetReference : targets) {
KeepItemPattern item = bindings.get(targetReference).getItem();
- if (bindings.isAny(item)) {
- // If the target is "any item" then it contains any other target pattern.
- memberPatterns.put(targetReference, item.getMemberPattern());
- callback.accept(
- memberPatterns,
- Collections.singletonList(targetReference),
- TargetKeepKind.CLASS_OR_MEMBERS);
- return;
- }
if (item.isClassItemPattern()) {
keepKind = TargetKeepKind.CLASS_AND_MEMBERS;
} else {
diff --git a/src/test/java/com/android/tools/r8/keepanno/ast/KeepEdgeAstTest.java b/src/test/java/com/android/tools/r8/keepanno/ast/KeepEdgeAstTest.java
index 64f9378..602ee63 100644
--- a/src/test/java/com/android/tools/r8/keepanno/ast/KeepEdgeAstTest.java
+++ b/src/test/java/com/android/tools/r8/keepanno/ast/KeepEdgeAstTest.java
@@ -45,22 +45,34 @@
KeepEdge.builder()
.setConsequences(
KeepConsequences.builder()
- .addTarget(KeepTarget.builder().setItemPattern(KeepItemPattern.any()).build())
+ .addTarget(
+ KeepTarget.builder().setItemPattern(KeepItemPattern.anyClass()).build())
+ .addTarget(
+ KeepTarget.builder().setItemPattern(KeepItemPattern.anyMember()).build())
.build())
.build();
- assertEquals(StringUtils.unixLines("-keep class * { *; }"), extract(edge));
+ assertEquals(
+ StringUtils.unixLines(
+ "-keep class * { void finalize(); }", "-keepclassmembers class * { *; }"),
+ extract(edge));
}
@Test
public void testSoftPinViaDisallow() {
+ KeepOptions disallowOptions = KeepOptions.disallow(KeepOption.OPTIMIZING);
KeepEdge edge =
KeepEdge.builder()
.setConsequences(
KeepConsequences.builder()
.addTarget(
KeepTarget.builder()
- .setItemPattern(KeepItemPattern.any())
- .setOptions(KeepOptions.disallow(KeepOption.OPTIMIZING))
+ .setItemPattern(KeepItemPattern.anyClass())
+ .setOptions(disallowOptions)
+ .build())
+ .addTarget(
+ KeepTarget.builder()
+ .setItemPattern(KeepItemPattern.anyMember())
+ .setOptions(disallowOptions)
.build())
.build())
.build();
@@ -70,26 +82,37 @@
String allows = String.join(",allow", options);
// The "any" item will be split in two rules, one for the targeted types and one for the
// targeted members.
- assertEquals(StringUtils.unixLines("-keep,allow" + allows + " class * { *; }"), extract(edge));
+ assertEquals(
+ StringUtils.unixLines(
+ "-keep,allow" + allows + " class * { void finalize(); }",
+ "-keepclassmembers,allow" + allows + " class * { *; }"),
+ extract(edge));
}
@Test
public void testSoftPinViaAllow() {
+ KeepOptions allowOptions = KeepOptions.allow(KeepOption.OBFUSCATING, KeepOption.SHRINKING);
KeepEdge edge =
KeepEdge.builder()
.setConsequences(
KeepConsequences.builder()
.addTarget(
KeepTarget.builder()
- .setItemPattern(KeepItemPattern.any())
- .setOptions(
- KeepOptions.allow(KeepOption.OBFUSCATING, KeepOption.SHRINKING))
+ .setItemPattern(KeepItemPattern.anyClass())
+ .setOptions(allowOptions)
+ .build())
+ .addTarget(
+ KeepTarget.builder()
+ .setItemPattern(KeepItemPattern.anyMember())
+ .setOptions(allowOptions)
.build())
.build())
.build();
// Allow is just the ordered list of options.
assertEquals(
- StringUtils.unixLines("-keep,allowshrinking,allowobfuscation class * { *; }"),
+ StringUtils.unixLines(
+ "-keep,allowshrinking,allowobfuscation class * { void finalize(); }",
+ "-keepclassmembers,allowshrinking,allowobfuscation class * { *; }"),
extract(edge));
}
