Use keepclassmembers when applicable.
Bug: b/248408342
Change-Id: I4df4b973d3614e36a575f17f17271b0d631b1c25
diff --git a/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepPackagePattern.java b/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepPackagePattern.java
index c6acc4f..10db385 100644
--- a/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepPackagePattern.java
+++ b/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepPackagePattern.java
@@ -78,6 +78,16 @@
public boolean isExact() {
return false;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ return obj == this;
+ }
+
+ @Override
+ public int hashCode() {
+ return System.identityHashCode(this);
+ }
}
private static final class KeepPackageTopPattern extends KeepPackageExactPattern {
@@ -111,6 +121,7 @@
private final String fullPackage;
private KeepPackageExactPattern(String fullPackage) {
+ assert fullPackage != null;
this.fullPackage = fullPackage;
// TODO: Verify valid package identifiers.
}
@@ -138,6 +149,23 @@
public String getExactPackageAsString() {
return fullPackage;
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ KeepPackageExactPattern that = (KeepPackageExactPattern) o;
+ return fullPackage.equals(that.fullPackage);
+ }
+
+ @Override
+ public int hashCode() {
+ return fullPackage.hashCode();
+ }
}
public abstract boolean isAny();
diff --git a/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepQualifiedClassNamePattern.java b/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepQualifiedClassNamePattern.java
index 3f2433d..9485580 100644
--- a/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepQualifiedClassNamePattern.java
+++ b/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepQualifiedClassNamePattern.java
@@ -3,7 +3,9 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.experimental.keepanno.ast;
-public class KeepQualifiedClassNamePattern {
+import java.util.Objects;
+
+public final class KeepQualifiedClassNamePattern {
public static Builder builder() {
return new Builder();
@@ -62,6 +64,8 @@
public KeepQualifiedClassNamePattern(
KeepPackagePattern packagePattern, KeepUnqualfiedClassNamePattern namePattern) {
+ assert packagePattern != null;
+ assert namePattern != null;
this.packagePattern = packagePattern;
this.namePattern = namePattern;
}
@@ -77,4 +81,21 @@
public KeepUnqualfiedClassNamePattern getNamePattern() {
return namePattern;
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ KeepQualifiedClassNamePattern that = (KeepQualifiedClassNamePattern) o;
+ return packagePattern.equals(that.packagePattern) && namePattern.equals(that.namePattern);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(packagePattern.hashCode(), namePattern.hashCode());
+ }
}
diff --git a/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepUnqualfiedClassNamePattern.java b/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepUnqualfiedClassNamePattern.java
index 7c6b6ba..ab38ed2 100644
--- a/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepUnqualfiedClassNamePattern.java
+++ b/src/main/java/com/android/tools/r8/experimental/keepanno/ast/KeepUnqualfiedClassNamePattern.java
@@ -61,6 +61,16 @@
public boolean isExact() {
return false;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ return this == obj;
+ }
+
+ @Override
+ public int hashCode() {
+ return System.identityHashCode(this);
+ }
}
public static class KeepClassNameExactPattern extends KeepUnqualfiedClassNamePattern {
@@ -68,6 +78,7 @@
private final String className;
private KeepClassNameExactPattern(String className) {
+ assert className != null;
this.className = className;
}
@@ -89,6 +100,23 @@
public String getExactNameAsString() {
return className;
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ KeepClassNameExactPattern that = (KeepClassNameExactPattern) o;
+ return className.equals(that.className);
+ }
+
+ @Override
+ public int hashCode() {
+ return className.hashCode();
+ }
}
public abstract boolean isAny();
diff --git a/src/main/java/com/android/tools/r8/experimental/keepanno/keeprules/KeepRuleExtractor.java b/src/main/java/com/android/tools/r8/experimental/keepanno/keeprules/KeepRuleExtractor.java
index 4bda1fa..85c9f00 100644
--- a/src/main/java/com/android/tools/r8/experimental/keepanno/keeprules/KeepRuleExtractor.java
+++ b/src/main/java/com/android/tools/r8/experimental/keepanno/keeprules/KeepRuleExtractor.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.experimental.keepanno.ast.KeepPackagePattern;
import com.android.tools.r8.experimental.keepanno.ast.KeepPreconditions;
import com.android.tools.r8.experimental.keepanno.ast.KeepQualifiedClassNamePattern;
+import com.android.tools.r8.experimental.keepanno.ast.KeepTarget;
import com.android.tools.r8.experimental.keepanno.ast.KeepTypePattern;
import com.android.tools.r8.experimental.keepanno.ast.KeepUnqualfiedClassNamePattern;
import com.android.tools.r8.utils.StringUtils;
@@ -42,18 +43,7 @@
private List<ItemRule> getConsequentRules(KeepConsequences consequences) {
List<ItemRule> consequentItems = new ArrayList<>();
- consequences.forEachTarget(
- target ->
- target
- .getItem()
- .match(
- () -> consequentItems.add(new ItemRule("class * { *; }", target.getOptions())),
- clazzPattern -> {
- StringBuilder builder = new StringBuilder();
- printClassItem(builder, clazzPattern);
- consequentItems.add(new ItemRule(builder.toString(), target.getOptions()));
- return null;
- }));
+ consequences.forEachTarget(target -> consequentItems.add(new ItemRule(target)));
return consequentItems;
}
@@ -78,11 +68,16 @@
consequentItem -> {
// Since conjunctions are not supported in keep rules, we expand them into
// disjunctions so conservatively we keep the consequences if any one of
- // the
- // preconditions hold.
- StringBuilder builder = new StringBuilder("-if ");
- printClassItem(builder, conditionItem);
- builder.append(' ');
+ // the preconditions hold.
+ StringBuilder builder = new StringBuilder();
+ if (!consequentItem.isMemberConsequent()
+ || !conditionItem
+ .getClassNamePattern()
+ .equals(consequentItem.getHolderPattern())) {
+ builder.append("-if ");
+ printClassItem(builder, conditionItem);
+ builder.append(' ');
+ }
printConsequentRule(builder, consequentItem);
ruleConsumer.accept(builder.toString());
});
@@ -98,13 +93,17 @@
}
private static StringBuilder printConsequentRule(StringBuilder builder, ItemRule rule) {
- builder.append("-keep");
+ if (rule.isMemberConsequent()) {
+ builder.append("-keepclassmembers");
+ } else {
+ builder.append("-keep");
+ }
for (KeepOption option : KeepOption.values()) {
if (rule.options.allow(option)) {
builder.append(",allow").append(getOptionString(option));
}
}
- return builder.append(" ").append(rule.ruleLine);
+ return builder.append(" ").append(rule.getKeepRuleForItem());
}
private static StringBuilder printClassItem(
@@ -229,12 +228,35 @@
}
private static class ItemRule {
- private final String ruleLine;
+ private final KeepTarget target;
private final KeepOptions options;
+ private String ruleLine = null;
- public ItemRule(String ruleLine, KeepOptions options) {
- this.ruleLine = ruleLine;
- this.options = options;
+ public ItemRule(KeepTarget target) {
+ this.target = target;
+ this.options = target.getOptions();
+ }
+
+ public boolean isMemberConsequent() {
+ return target.getItem().match(() -> false, clazz -> !clazz.getMembersPattern().isNone());
+ }
+
+ public KeepQualifiedClassNamePattern getHolderPattern() {
+ return target
+ .getItem()
+ .match(KeepQualifiedClassNamePattern::any, KeepClassPattern::getClassNamePattern);
+ }
+
+ public String getKeepRuleForItem() {
+ if (ruleLine == null) {
+ ruleLine =
+ target
+ .getItem()
+ .match(
+ () -> "class * { *; }",
+ clazz -> printClassItem(new StringBuilder(), clazz).toString());
+ }
+ return ruleLine;
}
}
}
diff --git a/src/test/java/com/android/tools/r8/experimental/keepanno/ast/KeepEdgeApiTest.java b/src/test/java/com/android/tools/r8/experimental/keepanno/ast/KeepEdgeApiTest.java
index 2a5f4d6..3319a99 100644
--- a/src/test/java/com/android/tools/r8/experimental/keepanno/ast/KeepEdgeApiTest.java
+++ b/src/test/java/com/android/tools/r8/experimental/keepanno/ast/KeepEdgeApiTest.java
@@ -17,6 +17,8 @@
@RunWith(Parameterized.class)
public class KeepEdgeApiTest extends TestBase {
+ private static String CLASS = "com.example.Foo";
+
@Parameterized.Parameters(name = "{0}")
public static TestParametersCollection data() {
return getTestParameters().withNoneRuntime().build();
@@ -47,19 +49,14 @@
@Test
public void testKeepClass() throws Exception {
- KeepQualifiedClassNamePattern clazz = KeepQualifiedClassNamePattern.exact("com.example.Foo");
- KeepItemPattern item = KeepItemPattern.builder().setClassPattern(clazz).build();
- KeepTarget target = KeepTarget.builder().setItem(item).build();
+ KeepTarget target = target(classItem(CLASS));
KeepConsequences consequences = KeepConsequences.builder().addTarget(target).build();
KeepEdge edge = KeepEdge.builder().setConsequences(consequences).build();
- assertEquals(StringUtils.unixLines("-keep class com.example.Foo"), extract(edge));
+ assertEquals(StringUtils.unixLines("-keep class " + CLASS), extract(edge));
}
@Test
public void testKeepInitIfReferenced() throws Exception {
- // Equivalent of: -if class com.example.Foo -keep class com.example.Foo { void <init>(); }
- KeepQualifiedClassNamePattern classPattern =
- KeepQualifiedClassNamePattern.exact("com.example.Foo");
KeepEdge edge =
KeepEdge.builder()
.setPreconditions(
@@ -67,35 +64,90 @@
.addCondition(
KeepCondition.builder()
.setUsageKind(KeepUsageKind.symbolicReference())
- .setItem(
- KeepItemPattern.builder().setClassPattern(classPattern).build())
+ .setItem(classItem(CLASS))
.build())
.build())
.setConsequences(
KeepConsequences.builder()
.addTarget(
- KeepTarget.builder()
- .setItem(
- KeepItemPattern.builder()
- .setClassPattern(classPattern)
- .setMembersPattern(
- KeepMembersPattern.builder()
- .addMethodPattern(
- KeepMethodPattern.builder()
- .setNamePattern(
- KeepMethodNamePattern.initializer())
- .setParametersPattern(
- KeepMethodParametersPattern.none())
- .setReturnTypeVoid()
- .build())
- .build())
- .build())
+ target(
+ buildClassItem(CLASS)
+ .setMembersPattern(defaultInitializerPattern())
+ .build()))
+ .build())
+ .build();
+ assertEquals(
+ StringUtils.unixLines("-keepclassmembers class " + CLASS + " { void <init>(); }"),
+ extract(edge));
+ }
+
+ @Test
+ public void testKeepInstanceIfReferenced() throws Exception {
+ KeepEdge edge =
+ KeepEdge.builder()
+ .setPreconditions(
+ KeepPreconditions.builder()
+ .addCondition(
+ KeepCondition.builder()
+ .setUsageKind(KeepUsageKind.symbolicReference())
+ .setItem(classItem(CLASS))
.build())
.build())
+ .setConsequences(KeepConsequences.builder().addTarget(target(classItem(CLASS))).build())
+ .build();
+ assertEquals(
+ StringUtils.unixLines("-if class " + CLASS + " -keep class " + CLASS), extract(edge));
+ }
+
+ @Test
+ public void testKeepInstanceAndInitIfReferenced() throws Exception {
+ KeepEdge edge =
+ KeepEdge.builder()
+ .setPreconditions(
+ KeepPreconditions.builder()
+ .addCondition(
+ KeepCondition.builder()
+ .setUsageKind(KeepUsageKind.symbolicReference())
+ .setItem(classItem(CLASS))
+ .build())
+ .build())
+ .setConsequences(
+ KeepConsequences.builder()
+ .addTarget(target(classItem(CLASS)))
+ .addTarget(
+ target(
+ buildClassItem(CLASS)
+ .setMembersPattern(defaultInitializerPattern())
+ .build()))
+ .build())
.build();
assertEquals(
StringUtils.unixLines(
- "-if class com.example.Foo -keep class com.example.Foo { void <init>(); }"),
+ "-if class " + CLASS + " -keep class " + CLASS,
+ "-keepclassmembers class " + CLASS + " { void <init>(); }"),
extract(edge));
}
+
+ private KeepTarget target(KeepItemPattern item) {
+ return KeepTarget.builder().setItem(item).build();
+ }
+
+ private KeepItemPattern classItem(String typeName) {
+ return buildClassItem(typeName).build();
+ }
+
+ private KeepItemPattern.Builder buildClassItem(String typeName) {
+ return KeepItemPattern.builder().setClassPattern(KeepQualifiedClassNamePattern.exact(typeName));
+ }
+
+ private KeepMembersPattern defaultInitializerPattern() {
+ return KeepMembersPattern.builder()
+ .addMethodPattern(
+ KeepMethodPattern.builder()
+ .setNamePattern(KeepMethodNamePattern.initializer())
+ .setParametersPattern(KeepMethodParametersPattern.none())
+ .setReturnTypeVoid()
+ .build())
+ .build();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/KeepClassMembersDefaultCtorTest.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/KeepClassMembersDefaultCtorTest.java
index bdb4da5..2e726b0 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/KeepClassMembersDefaultCtorTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/KeepClassMembersDefaultCtorTest.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.shaking.forceproguardcompatibility.defaultctor;
import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.ProguardVersion;
@@ -43,20 +44,21 @@
@Test
public void testCompatR8() throws Exception {
- run(testForR8Compat(parameters.getBackend()));
+ run(testForR8Compat(parameters.getBackend()), false);
}
@Test
public void testR8() throws Exception {
- run(testForR8(parameters.getBackend()));
+ run(testForR8(parameters.getBackend()), true);
}
@Test
public void testPG() throws Exception {
- run(testForProguard(ProguardVersion.getLatest()).addDontWarn(getClass()));
+ run(testForProguard(ProguardVersion.getLatest()).addDontWarn(getClass()), false);
}
- private TestRunResult<?> run(TestShrinkerBuilder<?, ?, ?, ?, ?> builder) throws Exception {
+ private TestRunResult<?> run(TestShrinkerBuilder<?, ?, ?, ?, ?> builder, boolean fullMode)
+ throws Exception {
return builder
.addInnerClasses(KeepClassMembersDefaultCtorTest.class)
.addKeepRules("-keepclassmembers class * { <fields>; }")
@@ -65,7 +67,12 @@
.run(parameters.getRuntime(), TestClass.class)
.inspectFailure(
inspector -> {
+ assertThat(inspector.clazz(A.class), isPresent());
assertThat(inspector.clazz(A.class).init(), isAbsent());
+ assertThat(inspector.clazz(A.class).uniqueFieldWithName("y"), isPresent());
+ assertThat(
+ inspector.clazz(A.class).uniqueFieldWithName("x"),
+ fullMode ? isAbsent() : isPresent());
})
.assertFailureWithErrorThatThrows(NoSuchMethodException.class);
}
@@ -73,6 +80,7 @@
static class A {
public long x = System.nanoTime();
+ public static long y = System.nanoTime();
public A() {
System.out.println("A()");
@@ -95,6 +103,7 @@
if (args.length > 0) {
// Use the field so we are sure that the keep rule triggers.
A a = getA();
+ System.out.println(a.y);
System.out.println(a.x);
}
}
