Merge "Apply gradle build-scan plugin."
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java b/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
index cebcc8c..08d0cae 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
@@ -354,7 +354,8 @@
// avoid a bug where the index variable could end up being uninitialized.
if (code.options.canHaveBoundsCheckEliminationBug()
&& move.from.getValue().isConstNumber()
- && move.type == MoveType.SINGLE) {
+ && move.type == MoveType.SINGLE
+ && allocator.unadjustedRealRegisterFromAllocated(move.to.getRegister()) < 256) {
scheduler.addMove(
new RegisterMove(move.to.getRegister(), move.type, move.from.getValue().definition));
} else {
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 c4276dc..1de3f17 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -873,6 +873,7 @@
ruleBuilder.setName(IdentifierPatternWithWildcards.withoutWildcards("<init>"));
ruleBuilder.setArguments(parseArgumentList());
} else {
+ TextPosition firstStart = getPosition();
IdentifierPatternWithWildcards first =
acceptIdentifierWithBackreference(IdentifierType.ANY);
if (first != null) {
@@ -885,6 +886,7 @@
ruleBuilder.setName(first);
ruleBuilder.setArguments(parseArgumentList());
} else {
+ TextPosition secondStart = getPosition();
IdentifierPatternWithWildcards second =
acceptIdentifierWithBackreference(IdentifierType.ANY);
if (second != null) {
@@ -897,6 +899,12 @@
ProguardTypeMatcher.create(first, ClassOrType.TYPE, dexItemFactory));
ruleBuilder.setArguments(parseArgumentList());
} else {
+ if (first.hasUnusualCharacters()) {
+ warnUnusualCharacters("type", first.pattern, "field", firstStart);
+ }
+ if (second.hasUnusualCharacters()) {
+ warnUnusualCharacters("field name", second.pattern, "field", secondStart);
+ }
ruleBuilder.setRuleType(ProguardMemberType.FIELD);
ruleBuilder.setName(second);
ruleBuilder
@@ -1490,6 +1498,16 @@
"Option -" + optionName + " overrides -" + victim, origin, getPosition(start)));
}
+ private void warnUnusualCharacters(
+ String kind, String pattern, String ruleType, TextPosition start) {
+ reporter.warning(new StringDiagnostic(
+ "The " + kind + " \"" + pattern + "\" is used in a " + ruleType + " rule. The "
+ + "characters in this " + kind + " are legal for the JVM, "
+ + "but unlikely to originate from a source language. "
+ + "Maybe this is not the rule you are looking for.",
+ origin, getPosition(start)));
+ }
+
private void failPartiallyImplementedOption(String optionName, TextPosition start) {
throw reporter.fatalError(new StringDiagnostic(
"Option " + optionName + " currently not supported", origin, getPosition(start)));
@@ -1528,5 +1546,25 @@
boolean isMatchAllNames() {
return pattern.equals("*");
}
+
+ boolean hasUnusualCharacters() {
+ if (pattern.contains("<") || pattern.contains(">")) {
+ int angleStartCount = 0;
+ int angleEndCount = 0;
+ for (int i = 0; i < pattern.length(); i++) {
+ char c = pattern.charAt(i);
+ if (c == '<') {
+ angleStartCount++;
+ }
+ if (c == '>') {
+ angleEndCount++;
+ }
+ }
+ // Check that start/end angles are matched, and *only* used for well-formed wildcard
+ // backreferences (e.g. '<1>', but not '<<1>>', '<<*>>' or '>1<').
+ return !(angleStartCount == angleEndCount && angleStartCount == wildcards.size());
+ }
+ return false;
+ }
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 6f935ee..ccc1ccf 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -37,7 +37,6 @@
import com.google.common.base.Equivalence;
import com.google.common.base.Equivalence.Wrapper;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterators;
import it.unimi.dsi.fastutil.ints.Int2IntMap;
import it.unimi.dsi.fastutil.ints.Int2IntOpenHashMap;
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
@@ -55,7 +54,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.function.BiFunction;
import java.util.function.Predicate;
import java.util.stream.Collectors;
@@ -697,34 +695,36 @@
deferredRenamings.map(virtualMethod.method, shadowedBy.method);
}
- Collection<DexEncodedMethod> mergedDirectMethods =
- mergeItems(
- directMethods.values().iterator(),
- target.directMethods(),
- MethodSignatureEquivalence.get(),
- this::resolveMethodConflict);
- Collection<DexEncodedMethod> mergedVirtualMethods =
- mergeItems(
- virtualMethods.values().iterator(),
- target.virtualMethods(),
- MethodSignatureEquivalence.get(),
- this::resolveMethodConflict);
if (abortMerge) {
return false;
}
+
+ DexEncodedMethod[] mergedDirectMethods =
+ mergeMethods(directMethods.values(), target.directMethods());
+ DexEncodedMethod[] mergedVirtualMethods =
+ mergeMethods(virtualMethods.values(), target.virtualMethods());
+
// Step 2: Merge fields
- Collection<DexEncodedField> mergedStaticFields =
- mergeItems(
- Iterators.forArray(source.staticFields()),
- target.staticFields(),
- FieldSignatureEquivalence.get(),
- this::resolveFieldConflict);
- Collection<DexEncodedField> mergedInstanceFields =
- mergeItems(
- Iterators.forArray(source.instanceFields()),
+ Set<Wrapper<DexField>> existingFieldSignatures = new HashSet<>();
+ addAll(existingFieldSignatures, target.fields(), FieldSignatureEquivalence.get());
+
+ Predicate<DexField> availableFieldSignatures =
+ field -> !existingFieldSignatures.contains(FieldSignatureEquivalence.get().wrap(field));
+
+ DexEncodedField[] mergedInstanceFields =
+ mergeFields(
+ source.instanceFields(),
target.instanceFields(),
- FieldSignatureEquivalence.get(),
- this::resolveFieldConflict);
+ availableFieldSignatures,
+ existingFieldSignatures);
+
+ DexEncodedField[] mergedStaticFields =
+ mergeFields(
+ source.staticFields(),
+ target.staticFields(),
+ availableFieldSignatures,
+ existingFieldSignatures);
+
// Step 3: Merge interfaces
Set<DexType> interfaces = mergeArrays(target.interfaces.values, source.interfaces.values);
// Now destructively update the class.
@@ -739,14 +739,10 @@
? DexTypeList.empty()
: new DexTypeList(interfaces.toArray(new DexType[interfaces.size()]));
// Step 2: replace fields and methods.
- target.setDirectMethods(mergedDirectMethods
- .toArray(new DexEncodedMethod[mergedDirectMethods.size()]));
- target.setVirtualMethods(mergedVirtualMethods
- .toArray(new DexEncodedMethod[mergedVirtualMethods.size()]));
- target.setStaticFields(mergedStaticFields
- .toArray(new DexEncodedField[mergedStaticFields.size()]));
- target.setInstanceFields(mergedInstanceFields
- .toArray(new DexEncodedField[mergedInstanceFields.size()]));
+ target.setDirectMethods(mergedDirectMethods);
+ target.setVirtualMethods(mergedVirtualMethods);
+ target.setInstanceFields(mergedInstanceFields);
+ target.setStaticFields(mergedStaticFields);
// Step 3: Unlink old class to ease tree shaking.
source.superType = application.dexItemFactory.objectType;
source.setDirectMethods(null);
@@ -889,41 +885,38 @@
return merged;
}
- private <T extends PresortedComparable<T>, S extends KeyedDexItem<T>> Collection<S> mergeItems(
- Iterator<S> fromItems,
- S[] toItems,
- Equivalence<T> equivalence,
- BiFunction<S, Predicate<T>, S> onConflict) {
- Map<Wrapper<T>, S> map = new HashMap<>();
- // First add everything from the target class. These items are not preprocessed.
- for (S item : toItems) {
- map.put(equivalence.wrap(item.getKey()), item);
+ private DexEncodedField[] mergeFields(
+ DexEncodedField[] sourceFields,
+ DexEncodedField[] targetFields,
+ Predicate<DexField> availableFieldSignatures,
+ Set<Wrapper<DexField>> existingFieldSignatures) {
+ DexEncodedField[] result = new DexEncodedField[sourceFields.length + targetFields.length];
+ // Add fields from source
+ int i = 0;
+ for (DexEncodedField field : sourceFields) {
+ DexEncodedField resultingField = renameFieldIfNeeded(field, availableFieldSignatures);
+ existingFieldSignatures.add(FieldSignatureEquivalence.get().wrap(resultingField.field));
+ deferredRenamings.map(field.field, resultingField.field);
+ result[i] = resultingField;
+ i++;
}
- // Now add the new items, resolving shadowing.
- addNonShadowed(fromItems, map, equivalence, onConflict);
- return map.values();
+ // Add fields from target.
+ System.arraycopy(targetFields, 0, result, i, targetFields.length);
+ return result;
}
- private <T extends PresortedComparable<T>, S extends KeyedDexItem<T>> void addNonShadowed(
- Iterator<S> items,
- Map<Wrapper<T>, S> map,
- Equivalence<T> equivalence,
- BiFunction<S, Predicate<T>, S> onConflict) {
- Predicate<T> availableSignatures = key -> !map.containsKey(equivalence.wrap(key));
- while (items.hasNext()) {
- S item = items.next();
- assert item != null;
- Wrapper<T> wrapped = equivalence.wrap(item.getKey());
- if (map.containsKey(wrapped)) {
- S resolved = onConflict.apply(item, availableSignatures);
- assert availableSignatures.test(resolved.getKey());
- wrapped = equivalence.wrap(resolved.getKey());
- map.put(wrapped, resolved);
- assert !availableSignatures.test(resolved.getKey());
- } else {
- map.put(wrapped, item);
- }
+ private DexEncodedMethod[] mergeMethods(
+ Collection<DexEncodedMethod> sourceMethods, DexEncodedMethod[] targetMethods) {
+ DexEncodedMethod[] result = new DexEncodedMethod[sourceMethods.size() + targetMethods.length];
+ // Add methods from source.
+ int i = 0;
+ for (DexEncodedMethod method : sourceMethods) {
+ result[i] = method;
+ i++;
}
+ // Add methods from target.
+ System.arraycopy(targetMethods, 0, result, i, targetMethods.length);
+ return result;
}
// Note that names returned by this function are not necessarily unique. Clients should
@@ -936,13 +929,6 @@
return application.dexItemFactory.createString(freshName);
}
- private DexEncodedMethod resolveMethodConflict(
- DexEncodedMethod method, Predicate<DexMethod> availableMethodSignatures) {
- assert false;
- abortMerge = true;
- return method;
- }
-
private DexEncodedMethod renameConstructor(
DexEncodedMethod method, Predicate<DexMethod> availableMethodSignatures) {
assert method.isInstanceInitializer();
@@ -1008,23 +994,24 @@
return method.toTypeSubstitutedMethod(newSignature);
}
- private DexEncodedField resolveFieldConflict(
+ private DexEncodedField renameFieldIfNeeded(
DexEncodedField field, Predicate<DexField> availableFieldSignatures) {
DexString oldName = field.field.name;
DexType oldHolder = field.field.clazz;
- DexField newSignature;
- int count = 1;
- do {
- DexString newName = getFreshName(oldName.toSourceString(), count, oldHolder);
- newSignature =
- application.dexItemFactory.createField(target.type, field.field.type, newName);
- count++;
- } while (!availableFieldSignatures.test(newSignature));
+ DexField newSignature =
+ application.dexItemFactory.createField(target.type, field.field.type, oldName);
+ if (!availableFieldSignatures.test(newSignature)) {
+ int count = 1;
+ do {
+ DexString newName = getFreshName(oldName.toSourceString(), count, oldHolder);
+ newSignature =
+ application.dexItemFactory.createField(target.type, field.field.type, newName);
+ count++;
+ } while (!availableFieldSignatures.test(newSignature));
+ }
- DexEncodedField result = field.toTypeSubstitutedField(newSignature);
- deferredRenamings.map(field.field, result.field);
- return result;
+ return field.toTypeSubstitutedField(newSignature);
}
}
diff --git a/src/test/java/com/android/tools/r8/DiagnosticsChecker.java b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
index 2f45ae2..d79aaf0 100644
--- a/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
+++ b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
@@ -116,9 +116,14 @@
messageParts);
}
+ public static Diagnostic checkDiagnostics(List<Diagnostic> diagnostics, int index, Path path,
+ int lineStart, int columnStart, String... messageParts) {
+ return checkDiagnostic(diagnostics.get(index), path, lineStart, columnStart, messageParts);
+ }
+
public static Diagnostic checkDiagnostics(List<Diagnostic> diagnostics, Path path,
int lineStart, int columnStart, String... messageParts) {
assertEquals(1, diagnostics.size());
- return checkDiagnostic(diagnostics.get(0), path, lineStart, columnStart, messageParts);
+ return checkDiagnostics(diagnostics, 0, path, lineStart, columnStart, messageParts);
}
}
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 f96dc45..f484bc8 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -1485,7 +1485,7 @@
ProguardConfigurationParser parser =
new ProguardConfigurationParser(new DexItemFactory(), reporter);
parser.parse(proguardConfig);
- verifyParserEndsCleanly();
+ checkDiagnostics(handler.warnings, proguardConfig, 3, 7, "The field name \"id<<*>>\" is");
verifyWithProguard6(proguardConfig);
}
@@ -1731,7 +1731,7 @@
ProguardConfigurationParser parser =
new ProguardConfigurationParser(new DexItemFactory(), reporter);
parser.parse(proguardConfig);
- verifyParserEndsCleanly();
+ checkDiagnostics(handler.warnings, proguardConfig, 2, 5, "The field name \"<fields>\" is");
verifyWithProguard(proguardConfig);
}
@@ -1753,6 +1753,26 @@
verifyWithProguard(proguardConfig);
}
+ @Test
+ public void parse_regress110021323() throws Exception {
+ Path proguardConfig = writeTextToTempFile(
+ "-keepclassmembernames class A {",
+ " <public methods>;",
+ " <public fields>;",
+ "}"
+ );
+ ProguardConfigurationParser parser =
+ new ProguardConfigurationParser(new DexItemFactory(), reporter);
+ parser.parse(proguardConfig);
+ assertEquals(4, handler.warnings.size());
+ checkDiagnostics(handler.warnings, 0, proguardConfig, 2, 3, "The type \"<public\" is");
+ checkDiagnostics(handler.warnings, 1, proguardConfig, 2, 11, "The field name \"methods>\" is");
+ checkDiagnostics(handler.warnings, 2, proguardConfig, 3, 3, "The type \"<public\" is");
+ checkDiagnostics(handler.warnings, 3, proguardConfig, 3, 11, "The field name \"fields>\" is");
+
+ verifyWithProguard(proguardConfig);
+ }
+
public void testNotSupported(String option) {
try {
reset();
diff --git a/src/test/java/com/android/tools/r8/shaking/defaultmethods/DefaultMethodsTest.java b/src/test/java/com/android/tools/r8/shaking/defaultmethods/DefaultMethodsTest.java
index 5845dd3..e543d46 100644
--- a/src/test/java/com/android/tools/r8/shaking/defaultmethods/DefaultMethodsTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/defaultmethods/DefaultMethodsTest.java
@@ -86,18 +86,24 @@
" public int method();",
"}"
), this::defaultMethodKept);
- runTest(ImmutableList.of(
- "-keep class " + ClassImplementingInterface.class.getCanonicalName() + "{",
- " <methods>;",
- "}"
- ), this::defaultMethodNotKept);
- runTest(ImmutableList.of(
- "-keep class " + ClassImplementingInterface.class.getCanonicalName() + "{",
- " <methods>;",
- "}",
- "-keep class " + TestClass.class.getCanonicalName() + "{",
- " public void useInterfaceMethod();",
- "}"
- ), this::defaultMethodAbstract);
+ runTest(
+ ImmutableList.of(
+ "-keep class " + ClassImplementingInterface.class.getCanonicalName() + "{",
+ " <methods>;",
+ "}",
+ // Prevent InterfaceWithDefaultMethods from being merged into ClassImplementingInterface
+ "-keep class " + InterfaceWithDefaultMethods.class.getCanonicalName()),
+ this::defaultMethodNotKept);
+ runTest(
+ ImmutableList.of(
+ "-keep class " + ClassImplementingInterface.class.getCanonicalName() + "{",
+ " <methods>;",
+ "}",
+ "-keep class " + TestClass.class.getCanonicalName() + "{",
+ " public void useInterfaceMethod();",
+ "}",
+ // Prevent InterfaceWithDefaultMethods from being merged into ClassImplementingInterface
+ "-keep class " + InterfaceWithDefaultMethods.class.getCanonicalName()),
+ this::defaultMethodAbstract);
}
}
