Merge "Strengthen no duplicate methods check"
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 6c3e47e..011c026 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -616,15 +616,18 @@
new DiscardedChecker(rootSet, application, options).run();
}
- timing.begin("Minification");
- // If we did not have keep rules, everything will be marked as keep, so no minification
- // will happen. Just avoid the overhead.
- NamingLens namingLens =
- options.enableMinification
- ? new Minifier(appView.appInfo().withLiveness(), rootSet, desugaredCallSites, options)
- .run(timing)
- : NamingLens.getIdentityLens();
- timing.end();
+ // Perform minification.
+ NamingLens namingLens;
+ if (options.enableMinification) {
+ timing.begin("Minification");
+ namingLens =
+ new Minifier(appView.appInfo().withLiveness(), rootSet, desugaredCallSites, options)
+ .run(timing);
+ timing.end();
+ assert namingLens.verifyNoCollisions(application.classes(), options.itemFactory);
+ } else {
+ namingLens = NamingLens.getIdentityLens();
+ }
ProguardMapSupplier proguardMapSupplier;
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index 28aabea..5e65aa3 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -38,22 +38,17 @@
public DexTypeList interfaces;
public DexString sourceFile;
- /**
- * Access has to be synchronized during concurrent collection/writing phase.
- */
- protected DexEncodedField[] staticFields;
- /**
- * Access has to be synchronized during concurrent collection/writing phase.
- */
- protected DexEncodedField[] instanceFields;
- /**
- * Access has to be synchronized during concurrent collection/writing phase.
- */
- protected DexEncodedMethod[] directMethods;
- /**
- * Access has to be synchronized during concurrent collection/writing phase.
- */
- protected DexEncodedMethod[] virtualMethods;
+ /** Access has to be synchronized during concurrent collection/writing phase. */
+ protected DexEncodedField[] staticFields = NO_FIELDS;
+
+ /** Access has to be synchronized during concurrent collection/writing phase. */
+ protected DexEncodedField[] instanceFields = NO_FIELDS;
+
+ /** Access has to be synchronized during concurrent collection/writing phase. */
+ protected DexEncodedMethod[] directMethods = NO_METHODS;
+
+ /** Access has to be synchronized during concurrent collection/writing phase. */
+ protected DexEncodedMethod[] virtualMethods = NO_METHODS;
/** Enclosing context of this class if it is an inner class, null otherwise. */
private EnclosingMethodAttribute enclosingMethod;
@@ -146,7 +141,8 @@
System.arraycopy(directMethods, 0, newMethods, 0, directMethods.length);
newMethods[directMethods.length] = method;
directMethods = newMethods;
- assert verifyNoDuplicateMethods(directMethods);
+ assert verifyCorrectnessOfMethodHolder(method);
+ assert verifyNoDuplicateMethods();
}
public void appendDirectMethods(Collection<DexEncodedMethod> methods) {
@@ -158,7 +154,8 @@
i++;
}
directMethods = newMethods;
- assert verifyNoDuplicateMethods(directMethods);
+ assert verifyCorrectnessOfMethodHolders(methods);
+ assert verifyNoDuplicateMethods();
}
public void removeDirectMethod(int index) {
@@ -170,12 +167,14 @@
public void setDirectMethod(int index, DexEncodedMethod method) {
directMethods[index] = method;
- assert verifyNoDuplicateMethods(directMethods);
+ assert verifyCorrectnessOfMethodHolder(method);
+ assert verifyNoDuplicateMethods();
}
public void setDirectMethods(DexEncodedMethod[] values) {
directMethods = MoreObjects.firstNonNull(values, NO_METHODS);
- assert verifyNoDuplicateMethods(directMethods);
+ assert verifyCorrectnessOfMethodHolders(directMethods());
+ assert verifyNoDuplicateMethods();
}
public List<DexEncodedMethod> virtualMethods() {
@@ -191,7 +190,8 @@
System.arraycopy(virtualMethods, 0, newMethods, 0, virtualMethods.length);
newMethods[virtualMethods.length] = method;
virtualMethods = newMethods;
- assert verifyNoDuplicateMethods(virtualMethods);
+ assert verifyCorrectnessOfMethodHolder(method);
+ assert verifyNoDuplicateMethods();
}
public void appendVirtualMethods(Collection<DexEncodedMethod> methods) {
@@ -203,7 +203,8 @@
i++;
}
virtualMethods = newMethods;
- assert verifyNoDuplicateMethods(virtualMethods);
+ assert verifyCorrectnessOfMethodHolders(methods);
+ assert verifyNoDuplicateMethods();
}
public void removeVirtualMethod(int index) {
@@ -216,22 +217,39 @@
public void setVirtualMethod(int index, DexEncodedMethod method) {
virtualMethods[index] = method;
- assert verifyNoDuplicateMethods(virtualMethods);
+ assert verifyCorrectnessOfMethodHolder(method);
+ assert verifyNoDuplicateMethods();
}
public void setVirtualMethods(DexEncodedMethod[] values) {
virtualMethods = MoreObjects.firstNonNull(values, NO_METHODS);
- assert verifyNoDuplicateMethods(virtualMethods);
+ assert verifyCorrectnessOfMethodHolders(virtualMethods());
+ assert verifyNoDuplicateMethods();
}
- private boolean verifyNoDuplicateMethods(DexEncodedMethod[] methods) {
+ private boolean verifyCorrectnessOfMethodHolder(DexEncodedMethod method) {
+ assert method.method.holder == type
+ : "Expected method `"
+ + method.method.toSourceString()
+ + "` to have holder `"
+ + type.toSourceString()
+ + "`";
+ return true;
+ }
+
+ private boolean verifyCorrectnessOfMethodHolders(Iterable<DexEncodedMethod> methods) {
+ for (DexEncodedMethod method : methods) {
+ assert verifyCorrectnessOfMethodHolder(method);
+ }
+ return true;
+ }
+
+ private boolean verifyNoDuplicateMethods() {
Set<DexMethod> unique = Sets.newIdentityHashSet();
- Arrays.stream(methods)
- .forEach(
- method -> {
- boolean changed = unique.add(method.method);
- assert changed : "Duplicate method `" + method.method.toSourceString() + "`";
- });
+ for (DexEncodedMethod method : methods()) {
+ boolean changed = unique.add(method.method);
+ assert changed : "Duplicate method `" + method.method.toSourceString() + "`";
+ }
return true;
}
@@ -651,6 +669,8 @@
public boolean isValid() {
assert !isInterface()
|| Arrays.stream(virtualMethods).noneMatch(method -> method.accessFlags.isFinal());
+ assert verifyCorrectnessOfMethodHolders(methods());
+ assert verifyNoDuplicateMethods();
return true;
}
}
diff --git a/src/main/java/com/android/tools/r8/naming/NamingLens.java b/src/main/java/com/android/tools/r8/naming/NamingLens.java
index cee867e..aecae1e 100644
--- a/src/main/java/com/android/tools/r8/naming/NamingLens.java
+++ b/src/main/java/com/android/tools/r8/naming/NamingLens.java
@@ -7,10 +7,14 @@
import static com.android.tools.r8.utils.DescriptorUtils.descriptorToJavaType;
import com.android.tools.r8.graph.DexCallSite;
+import com.android.tools.r8.graph.DexEncodedField;
+import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItem;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
@@ -19,7 +23,10 @@
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Sets;
+import java.util.Arrays;
import java.util.Map;
+import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
@@ -70,6 +77,40 @@
return lookupName(reference.asDexField());
}
+ public final DexField lookupField(DexField field, DexItemFactory dexItemFactory) {
+ return dexItemFactory.createField(
+ lookupType(field.clazz, dexItemFactory),
+ lookupType(field.type, dexItemFactory),
+ lookupName(field));
+ }
+
+ public final DexMethod lookupMethod(DexMethod method, DexItemFactory dexItemFactory) {
+ return dexItemFactory.createMethod(
+ lookupType(method.holder, dexItemFactory),
+ lookupProto(method.proto, dexItemFactory),
+ lookupName(method));
+ }
+
+ private DexProto lookupProto(DexProto proto, DexItemFactory dexItemFactory) {
+ return dexItemFactory.createProto(
+ lookupType(proto.returnType, dexItemFactory),
+ Arrays.stream(proto.parameters.values)
+ .map(type -> lookupType(type, dexItemFactory))
+ .toArray(DexType[]::new));
+ }
+
+ public final DexType lookupType(DexType type, DexItemFactory dexItemFactory) {
+ if (type.isPrimitiveType() || type.isVoidType()) {
+ return type;
+ }
+ if (type.isArrayType()) {
+ DexType newBaseType = lookupType(type.toBaseType(dexItemFactory), dexItemFactory);
+ return type.replaceBaseType(newBaseType, dexItemFactory);
+ }
+ assert type.isClassType();
+ return dexItemFactory.createType(lookupDescriptor(type));
+ }
+
public static NamingLens getIdentityLens() {
return new IdentityLens();
}
@@ -97,6 +138,34 @@
*/
public abstract boolean checkTargetCanBeTranslated(DexMethod item);
+ public final boolean verifyNoCollisions(
+ Iterable<DexProgramClass> classes, DexItemFactory dexItemFactory) {
+ Set<DexReference> references = Sets.newIdentityHashSet();
+ for (DexProgramClass clazz : classes) {
+ {
+ DexType newType = lookupType(clazz.type, dexItemFactory);
+ boolean referencesChanged = references.add(newType);
+ assert referencesChanged
+ : "Duplicate definition of type `" + newType.toSourceString() + "`";
+ }
+
+ for (DexEncodedField field : clazz.fields()) {
+ DexField newField = lookupField(field.field, dexItemFactory);
+ boolean referencesChanged = references.add(newField);
+ assert referencesChanged
+ : "Duplicate definition of field `" + newField.toSourceString() + "`";
+ }
+
+ for (DexEncodedMethod method : clazz.methods()) {
+ DexMethod newMethod = lookupMethod(method.method, dexItemFactory);
+ boolean referencesChanged = references.add(newMethod);
+ assert referencesChanged
+ : "Duplicate definition of method `" + newMethod.toSourceString() + "`";
+ }
+ }
+ return true;
+ }
+
private static class IdentityLens extends NamingLens {
private IdentityLens() {
