Ignore _NONNULL_ attributes in keep rules
Bug: 186529489
Change-Id: I9c07ea2a74312865fe6ca4d2ea38cdfb6bc814ff
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
index 2871b74..ff71812 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -81,6 +81,8 @@
private static final List<String> IGNORED_CLASS_DESCRIPTOR_OPTIONS =
ImmutableList.of("isclassnamestring", "whyarenotsimple", "convertchecknotnull");
+ private static final List<String> IGNORED_RETURN_VALUE_ATTRIBUTES = ImmutableList.of("_NONNULL_");
+
private static final List<String> WARNED_SINGLE_ARG_OPTIONS = ImmutableList.of(
// TODO(b/37137994): -outjars should be reported as errors, not just as warnings!
"outjars");
@@ -588,6 +590,19 @@
|| parseOptimizationOption(optionStart);
}
+ private boolean parseIgnoredReturnValueAttribute() {
+ return Iterables.any(IGNORED_RETURN_VALUE_ATTRIBUTES, this::skipReturnValueAttribute);
+ }
+
+ private boolean parseIgnoredReturnValueAttributes() {
+ boolean anySkipped = false;
+ while (parseIgnoredReturnValueAttribute()) {
+ anySkipped = true;
+ skipWhitespace();
+ }
+ return anySkipped;
+ }
+
private void parseInclude() throws ProguardRuleParserException {
TextPosition start = getPosition();
Path included = parseFileName(false);
@@ -673,6 +688,10 @@
return false;
}
+ private boolean skipReturnValueAttribute(String name) {
+ return acceptString(name);
+ }
+
private boolean parseOptimizationOption(TextPosition optionStart)
throws ProguardRuleParserException {
if (!acceptString("optimizations")) {
@@ -1317,49 +1336,52 @@
// Parse "return ..." if present.
if (acceptString("return")) {
skipWhitespace();
- if (acceptString("true")) {
- ruleBuilder.setReturnValue(new ProguardMemberRuleReturnValue(true));
- } else if (acceptString("false")) {
- ruleBuilder.setReturnValue(new ProguardMemberRuleReturnValue(false));
- } else if (acceptString("null")) {
- ruleBuilder.setReturnValue(new ProguardMemberRuleReturnValue());
- } else {
- TextPosition fieldOrValueStart = getPosition();
- String qualifiedFieldNameOrInteger = acceptFieldNameOrIntegerForReturn();
- if (qualifiedFieldNameOrInteger != null) {
- if (isInteger(qualifiedFieldNameOrInteger)) {
- Integer min = Integer.parseInt(qualifiedFieldNameOrInteger);
- Integer max = min;
- skipWhitespace();
- if (acceptString("..")) {
+ boolean anyReturnValueAttributesSkipped = parseIgnoredReturnValueAttributes();
+ if (!anyReturnValueAttributesSkipped || !hasNextChar(';')) {
+ if (acceptString("true")) {
+ ruleBuilder.setReturnValue(new ProguardMemberRuleReturnValue(true));
+ } else if (acceptString("false")) {
+ ruleBuilder.setReturnValue(new ProguardMemberRuleReturnValue(false));
+ } else if (acceptString("null")) {
+ ruleBuilder.setReturnValue(new ProguardMemberRuleReturnValue());
+ } else {
+ TextPosition fieldOrValueStart = getPosition();
+ String qualifiedFieldNameOrInteger = acceptFieldNameOrIntegerForReturn();
+ if (qualifiedFieldNameOrInteger != null) {
+ if (isInteger(qualifiedFieldNameOrInteger)) {
+ Integer min = Integer.parseInt(qualifiedFieldNameOrInteger);
+ Integer max = min;
skipWhitespace();
- max = acceptInteger();
- if (max == null) {
- throw parseError("Expected integer value");
+ if (acceptString("..")) {
+ skipWhitespace();
+ max = acceptInteger();
+ if (max == null) {
+ throw parseError("Expected integer value");
+ }
}
- }
- if (!allowValueSpecification) {
- throw parseError("Unexpected value specification", fieldOrValueStart);
- }
- ruleBuilder.setReturnValue(
- new ProguardMemberRuleReturnValue(new LongInterval(min, max)));
- } else {
- if (ruleBuilder.getTypeMatcher() instanceof MatchSpecificType) {
- int lastDotIndex = qualifiedFieldNameOrInteger.lastIndexOf(".");
- DexType fieldType = ((MatchSpecificType) ruleBuilder
- .getTypeMatcher()).type;
- DexType fieldClass =
- dexItemFactory.createType(
- javaTypeToDescriptor(
- qualifiedFieldNameOrInteger.substring(0, lastDotIndex)));
- DexString fieldName =
- dexItemFactory.createString(
- qualifiedFieldNameOrInteger.substring(lastDotIndex + 1));
- DexField field = dexItemFactory
- .createField(fieldClass, fieldType, fieldName);
- ruleBuilder.setReturnValue(new ProguardMemberRuleReturnValue(field));
+ if (!allowValueSpecification) {
+ throw parseError("Unexpected value specification", fieldOrValueStart);
+ }
+ ruleBuilder.setReturnValue(
+ new ProguardMemberRuleReturnValue(new LongInterval(min, max)));
} else {
- throw parseError("Expected specific type", fieldOrValueStart);
+ if (ruleBuilder.getTypeMatcher() instanceof MatchSpecificType) {
+ int lastDotIndex = qualifiedFieldNameOrInteger.lastIndexOf(".");
+ DexType fieldType =
+ ((MatchSpecificType) ruleBuilder.getTypeMatcher()).type;
+ DexType fieldClass =
+ dexItemFactory.createType(
+ javaTypeToDescriptor(
+ qualifiedFieldNameOrInteger.substring(0, lastDotIndex)));
+ DexString fieldName =
+ dexItemFactory.createString(
+ qualifiedFieldNameOrInteger.substring(lastDotIndex + 1));
+ DexField field =
+ dexItemFactory.createField(fieldClass, fieldType, fieldName);
+ ruleBuilder.setReturnValue(new ProguardMemberRuleReturnValue(field));
+ } else {
+ throw parseError("Expected specific type", fieldOrValueStart);
+ }
}
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
index f49dc97..c93200c 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -442,8 +442,8 @@
assertEquals(1, assumeNoSideEffects.size());
int matches = 0;
for (ProguardMemberRule rule : assumeNoSideEffects.get(0).getMemberRules()) {
- assertTrue(rule.hasReturnValue());
if (rule.getName().matches("returnsTrue") || rule.getName().matches("returnsFalse")) {
+ assertTrue(rule.hasReturnValue());
assertTrue(rule.getReturnValue().isBoolean());
assertFalse(rule.getReturnValue().isValueRange());
assertFalse(rule.getReturnValue().isField());
@@ -451,6 +451,7 @@
assertEquals(rule.getName().matches("returnsTrue"), rule.getReturnValue().getBoolean());
matches |= 1 << 0;
} else if (rule.getName().matches("returns1")) {
+ assertTrue(rule.hasReturnValue());
assertFalse(rule.getReturnValue().isBoolean());
assertTrue(rule.getReturnValue().isValueRange());
assertFalse(rule.getReturnValue().isField());
@@ -461,6 +462,7 @@
assertEquals(1, rule.getReturnValue().getSingleValue());
matches |= 1 << 1;
} else if (rule.getName().matches("returns2To4")) {
+ assertTrue(rule.hasReturnValue());
assertFalse(rule.getReturnValue().isBoolean());
assertTrue(rule.getReturnValue().isValueRange());
assertFalse(rule.getReturnValue().isField());
@@ -470,6 +472,7 @@
assertEquals(4, rule.getReturnValue().getValueRange().getMax());
matches |= 1 << 2;
} else if (rule.getName().matches("returns234To567")) {
+ assertTrue(rule.hasReturnValue());
assertFalse(rule.getReturnValue().isBoolean());
assertTrue(rule.getReturnValue().isValueRange());
assertFalse(rule.getReturnValue().isField());
@@ -479,6 +482,7 @@
assertEquals(567, rule.getReturnValue().getValueRange().getMax());
matches |= 1 << 3;
} else if (rule.getName().matches("returnsField")) {
+ assertTrue(rule.hasReturnValue());
assertFalse(rule.getReturnValue().isBoolean());
assertFalse(rule.getReturnValue().isValueRange());
assertTrue(rule.getReturnValue().isField());
@@ -488,17 +492,31 @@
assertEquals("X", rule.getReturnValue().getField().name.toString());
matches |= 1 << 4;
} else if (rule.getName().matches("returnsNull")) {
+ assertTrue(rule.hasReturnValue());
assertFalse(rule.getReturnValue().isBoolean());
assertFalse(rule.getReturnValue().isValueRange());
assertFalse(rule.getReturnValue().isField());
assertTrue(rule.getReturnValue().isNull());
assertTrue(rule.getReturnValue().isSingleValue());
matches |= 1 << 5;
+ } else if (rule.getName().matches("returnsNonNull")) {
+ assertFalse(rule.hasReturnValue());
+ matches |= 1 << 6;
+ } else if (rule.getName().matches("returnsNonNullField")) {
+ assertTrue(rule.hasReturnValue());
+ assertFalse(rule.getReturnValue().isBoolean());
+ assertFalse(rule.getReturnValue().isValueRange());
+ assertTrue(rule.getReturnValue().isField());
+ assertFalse(rule.getReturnValue().isNull());
+ assertEquals("com.google.C", rule.getReturnValue().getField().holder.toString());
+ assertEquals("Object", rule.getReturnValue().getField().type.toString());
+ assertEquals("X", rule.getReturnValue().getField().name.toString());
+ matches |= 1 << 7;
} else {
fail("Unexpected");
}
}
- assertEquals((1 << 6) - 1, matches);
+ assertEquals((1 << 8) - 1, matches);
}
@Test
@@ -511,8 +529,8 @@
assertEquals(1, assumeValues.size());
int matches = 0;
for (ProguardMemberRule rule : assumeValues.get(0).getMemberRules()) {
- assertTrue(rule.hasReturnValue());
if (rule.getName().matches("isTrue") || rule.getName().matches("isFalse")) {
+ assertTrue(rule.hasReturnValue());
assertTrue(rule.getReturnValue().isBoolean());
assertFalse(rule.getReturnValue().isValueRange());
assertFalse(rule.getReturnValue().isField());
@@ -520,6 +538,7 @@
assertEquals(rule.getName().matches("isTrue"), rule.getReturnValue().getBoolean());
matches |= 1 << 0;
} else if (rule.getName().matches("is1")) {
+ assertTrue(rule.hasReturnValue());
assertFalse(rule.getReturnValue().isBoolean());
assertTrue(rule.getReturnValue().isValueRange());
assertFalse(rule.getReturnValue().isField());
@@ -530,6 +549,7 @@
assertEquals(1, rule.getReturnValue().getSingleValue());
matches |= 1 << 1;
} else if (rule.getName().matches("is2To4")) {
+ assertTrue(rule.hasReturnValue());
assertFalse(rule.getReturnValue().isBoolean());
assertTrue(rule.getReturnValue().isValueRange());
assertFalse(rule.getReturnValue().isField());
@@ -539,6 +559,7 @@
assertEquals(4, rule.getReturnValue().getValueRange().getMax());
matches |= 1 << 2;
} else if (rule.getName().matches("is234To567")) {
+ assertTrue(rule.hasReturnValue());
assertFalse(rule.getReturnValue().isBoolean());
assertTrue(rule.getReturnValue().isValueRange());
assertFalse(rule.getReturnValue().isField());
@@ -548,6 +569,7 @@
assertEquals(567, rule.getReturnValue().getValueRange().getMax());
matches |= 1 << 3;
} else if (rule.getName().matches("isField")) {
+ assertTrue(rule.hasReturnValue());
assertFalse(rule.getReturnValue().isBoolean());
assertFalse(rule.getReturnValue().isValueRange());
assertTrue(rule.getReturnValue().isField());
@@ -557,17 +579,31 @@
assertEquals("X", rule.getReturnValue().getField().name.toString());
matches |= 1 << 4;
} else if (rule.getName().matches("isNull")) {
+ assertTrue(rule.hasReturnValue());
assertFalse(rule.getReturnValue().isBoolean());
assertFalse(rule.getReturnValue().isValueRange());
assertFalse(rule.getReturnValue().isField());
assertTrue(rule.getReturnValue().isNull());
assertTrue(rule.getReturnValue().isSingleValue());
matches |= 1 << 5;
+ } else if (rule.getName().matches("returnsNonNull")) {
+ assertFalse(rule.hasReturnValue());
+ matches |= 1 << 6;
+ } else if (rule.getName().matches("returnsNonNullField")) {
+ assertTrue(rule.hasReturnValue());
+ assertFalse(rule.getReturnValue().isBoolean());
+ assertFalse(rule.getReturnValue().isValueRange());
+ assertTrue(rule.getReturnValue().isField());
+ assertFalse(rule.getReturnValue().isNull());
+ assertEquals("com.google.C", rule.getReturnValue().getField().holder.toString());
+ assertEquals("Object", rule.getReturnValue().getField().type.toString());
+ assertEquals("X", rule.getReturnValue().getField().name.toString());
+ matches |= 1 << 7;
} else {
fail("Unexpected");
}
}
- assertEquals((1 << 6) - 1, matches);
+ assertEquals((1 << 8) - 1, matches);
}
@Test
diff --git a/src/test/proguard/valid/assume-no-side-effects-with-return-value.flags b/src/test/proguard/valid/assume-no-side-effects-with-return-value.flags
index b233ea3..d63e2e1 100644
--- a/src/test/proguard/valid/assume-no-side-effects-with-return-value.flags
+++ b/src/test/proguard/valid/assume-no-side-effects-with-return-value.flags
@@ -10,4 +10,6 @@
public static int returns234To567() return 234..567;
public static int returnsField() return com.google.C.X;
public static Object returnsNull() return null;
+ public static Object returnsNonNull() return _NONNULL_;
+ public static Object returnsNonNullField() return _NONNULL_ com.google.C.X;
}
diff --git a/src/test/proguard/valid/assume-values-with-return-value.flags b/src/test/proguard/valid/assume-values-with-return-value.flags
index 90cefbe..fdb6a25 100644
--- a/src/test/proguard/valid/assume-values-with-return-value.flags
+++ b/src/test/proguard/valid/assume-values-with-return-value.flags
@@ -10,4 +10,6 @@
public static final int is234To567() return 234..567;
public static final int isField() return com.google.C.X;
public static final Object isNull() return null;
+ public static Object returnsNonNull() return _NONNULL_;
+ public static Object returnsNonNullField() return _NONNULL_ com.google.C.X;
}
\ No newline at end of file
