Fix joining of parameter types in instance initializer merger
Change-Id: I6f319b30689673001f06469cbda8cf2c406a1737
diff --git a/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java b/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java
index b6d9757..bff5528 100644
--- a/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java
+++ b/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java
@@ -4,86 +4,39 @@
package com.android.tools.r8.graph;
+import com.android.tools.r8.ir.analysis.type.ArrayTypeElement;
import com.android.tools.r8.ir.analysis.type.ClassTypeElement;
-import com.android.tools.r8.ir.analysis.type.InterfaceCollection;
-import com.android.tools.r8.utils.ObjectUtils;
-import java.util.Iterator;
+import com.android.tools.r8.ir.analysis.type.TypeElement;
+import com.google.common.collect.Iterables;
public class DexTypeUtils {
public static DexType computeLeastUpperBound(
AppView<? extends AppInfoWithClassHierarchy> appView, Iterable<DexType> types) {
- DexItemFactory dexItemFactory = appView.dexItemFactory();
-
- Iterator<DexType> iterator = types.iterator();
- assert iterator.hasNext();
-
- DexType result = iterator.next();
- while (iterator.hasNext()) {
- result = computeLeastUpperBound(appView, result, iterator.next());
- }
- return result;
+ TypeElement join =
+ TypeElement.join(Iterables.transform(types, type -> type.toTypeElement(appView)), appView);
+ return toDexType(appView, join);
}
- public static DexType computeLeastUpperBound(
- AppView<? extends AppInfoWithClassHierarchy> appView, DexType type, DexType other) {
- if (type == other) {
- return type;
- }
-
+ private static DexType toDexType(
+ AppView<? extends AppInfoWithClassHierarchy> appView, TypeElement type) {
DexItemFactory dexItemFactory = appView.dexItemFactory();
- if (type == dexItemFactory.objectType
- || other == dexItemFactory.objectType
- || type.isArrayType() != other.isArrayType()) {
- return dexItemFactory.objectType;
+ if (type.isPrimitiveType()) {
+ return type.asPrimitiveType().toDexType(dexItemFactory);
}
-
if (type.isArrayType()) {
- assert other.isArrayType();
- int arrayDimension = type.getNumberOfLeadingSquareBrackets();
- if (other.getNumberOfLeadingSquareBrackets() != arrayDimension) {
- return dexItemFactory.objectType;
- }
-
- DexType baseType = type.toBaseType(dexItemFactory);
- DexType otherBaseType = other.toBaseType(dexItemFactory);
- if (baseType.isPrimitiveType() || otherBaseType.isPrimitiveType()) {
- assert baseType != otherBaseType;
- return dexItemFactory.objectType;
- }
-
- return dexItemFactory.createArrayType(
- arrayDimension, computeLeastUpperBound(appView, baseType, otherBaseType));
+ ArrayTypeElement arrayType = type.asArrayType();
+ DexType baseType = toDexType(appView, arrayType.getBaseType());
+ return baseType.toArrayType(arrayType.getNesting(), dexItemFactory);
}
-
- assert !type.isArrayType();
- assert !other.isArrayType();
-
- boolean isInterface =
- type.isClassType()
- && ObjectUtils.getBooleanOrElse(
- appView.definitionFor(type), DexClass::isInterface, false);
- boolean otherIsInterface =
- other.isClassType()
- && ObjectUtils.getBooleanOrElse(
- appView.definitionFor(other), DexClass::isInterface, false);
- if (isInterface != otherIsInterface) {
- return dexItemFactory.objectType;
+ assert type.isClassType();
+ ClassTypeElement classType = type.asClassType();
+ if (classType.getClassType() != dexItemFactory.objectType) {
+ return classType.getClassType();
}
-
- if (isInterface) {
- assert otherIsInterface;
- InterfaceCollection interfaceCollection =
- ClassTypeElement.computeLeastUpperBoundOfInterfaces(
- appView, InterfaceCollection.singleton(type), InterfaceCollection.singleton(other));
- return interfaceCollection.hasSingleKnownInterface()
- ? interfaceCollection.getSingleKnownInterface()
- : dexItemFactory.objectType;
+ if (classType.getInterfaces().hasSingleKnownInterface()) {
+ return classType.getInterfaces().getSingleKnownInterface();
}
-
- assert !isInterface;
- assert !otherIsInterface;
-
- return ClassTypeElement.computeLeastUpperBoundOfClasses(appView.appInfo(), type, other);
+ return dexItemFactory.objectType;
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
index 4b79710..70d534e 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -279,6 +279,7 @@
classMergers.add(
new ClassMerger.Builder(appView, codeProvider, group, mode).build(lensBuilder));
}
+ appView.dexItemFactory().clearTypeElementsCache();
return classMergers;
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/IncompleteMergedInstanceInitializerCode.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/IncompleteMergedInstanceInitializerCode.java
index f60f332..329d43a 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/IncompleteMergedInstanceInitializerCode.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/IncompleteMergedInstanceInitializerCode.java
@@ -16,6 +16,7 @@
import com.android.tools.r8.cf.code.CfLoad;
import com.android.tools.r8.cf.code.CfPosition;
import com.android.tools.r8.cf.code.CfReturnVoid;
+import com.android.tools.r8.cf.code.CfSafeCheckCast;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.CfCode;
@@ -115,7 +116,13 @@
// Assign each field.
addCfInstructionsForInstanceFieldAssignments(
- instructionBuilder, instanceFieldAssignmentsPre, argumentToLocalIndex, maxStack, lens);
+ appView,
+ method,
+ instructionBuilder,
+ instanceFieldAssignmentsPre,
+ argumentToLocalIndex,
+ maxStack,
+ lens);
// Load receiver for parent constructor call.
int stackHeightForParentConstructorCall = 1;
@@ -155,7 +162,13 @@
// Assign each field.
addCfInstructionsForInstanceFieldAssignments(
- instructionBuilder, instanceFieldAssignmentsPost, argumentToLocalIndex, maxStack, lens);
+ appView,
+ method,
+ instructionBuilder,
+ instanceFieldAssignmentsPost,
+ argumentToLocalIndex,
+ maxStack,
+ lens);
// Return.
instructionBuilder.add(new CfReturnVoid());
@@ -174,6 +187,8 @@
}
private static void addCfInstructionsForInstanceFieldAssignments(
+ AppView<? extends AppInfoWithClassHierarchy> appView,
+ ProgramMethod method,
ImmutableList.Builder<CfInstruction> instructionBuilder,
Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignments,
int[] argumentToLocalIndex,
@@ -186,8 +201,28 @@
int stackSizeForInitializationInfo =
addCfInstructionsForInitializationInfo(
instructionBuilder, initializationInfo, argumentToLocalIndex, field.getType());
- instructionBuilder.add(
- new CfInstanceFieldWrite(lens.getRenamedFieldSignature(field, lens.getPrevious())));
+ DexField rewrittenField = lens.getRenamedFieldSignature(field, lens.getPrevious());
+
+ // Insert a check to ensure the program continues to type check according to Java type
+ // checking. Otherwise, instance initializer merging may cause open interfaces. If
+ // <init>(A) and <init>(B) both have the behavior `this.i = arg; this.j = arg` where the
+ // type of `i` is I and the type of `j` is J, and both A and B implements I and J, then
+ // the constructors are merged into a single constructor <init>(java.lang.Object), which
+ // is no longer strictly type checking. Note that no choice of parameter type would solve
+ // this.
+ if (initializationInfo.isArgumentInitializationInfo()) {
+ int argumentIndex =
+ initializationInfo.asArgumentInitializationInfo().getArgumentIndex();
+ if (argumentIndex > 0) {
+ DexType argumentType = method.getArgumentType(argumentIndex);
+ if (argumentType.isClassType()
+ && !appView.appInfo().isSubtype(argumentType, rewrittenField.getType())) {
+ instructionBuilder.add(new CfSafeCheckCast(rewrittenField.getType()));
+ }
+ }
+ }
+
+ instructionBuilder.add(new CfInstanceFieldWrite(rewrittenField));
maxStack.setMax(stackSizeForInitializationInfo + 1);
});
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
index 1c3443b..f669473 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
@@ -27,14 +27,15 @@
import com.android.tools.r8.utils.ArrayUtils;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.ListUtils;
+import com.android.tools.r8.utils.SetUtils;
import com.android.tools.r8.utils.structural.Ordered;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
import it.unimi.dsi.fastutil.ints.Int2ReferenceAVLTreeMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceSortedMap;
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
import java.util.ArrayList;
import java.util.List;
+import java.util.Set;
public class InstanceInitializerMerger {
@@ -101,10 +102,6 @@
return classFileVersion;
}
- private DexMethod getNewMethodReference(ProgramMethod representative) {
- return getNewMethodReference(representative, false);
- }
-
private DexMethod getNewMethodReference(ProgramMethod representative, boolean needsClassId) {
DexType[] oldParameters = representative.getParameters().values;
DexType[] newParameters =
@@ -112,12 +109,15 @@
System.arraycopy(oldParameters, 0, newParameters, 0, oldParameters.length);
for (int i = 0; i < oldParameters.length; i++) {
final int parameterIndex = i;
- newParameters[i] =
- DexTypeUtils.computeLeastUpperBound(
- appView,
- Iterables.transform(
- instanceInitializers,
- instanceInitializer -> instanceInitializer.getParameter(parameterIndex)));
+ Set<DexType> parameterTypes =
+ SetUtils.newIdentityHashSet(
+ builder ->
+ instanceInitializers.forEach(
+ instanceInitializer ->
+ builder.accept(instanceInitializer.getParameter(parameterIndex))));
+ if (parameterTypes.size() > 1) {
+ newParameters[i] = DexTypeUtils.computeLeastUpperBound(appView, parameterTypes);
+ }
}
if (needsClassId) {
assert ArrayUtils.last(newParameters) == null;
