Enum unboxing: support initializer signature changes
Bug: 160939354
Change-Id: I7fa00ae43db3447c3c47abb8c2c50f41c03f7cca
diff --git a/src/main/java/com/android/tools/r8/graph/RewrittenPrototypeDescription.java b/src/main/java/com/android/tools/r8/graph/RewrittenPrototypeDescription.java
index dd2cde4..2c50bf8 100644
--- a/src/main/java/com/android/tools/r8/graph/RewrittenPrototypeDescription.java
+++ b/src/main/java/com/android/tools/r8/graph/RewrittenPrototypeDescription.java
@@ -325,20 +325,20 @@
private static final RewrittenPrototypeDescription none = new RewrittenPrototypeDescription();
- private final boolean extraNullParameter;
+ private final int extraUnusedNullParameters;
private final ArgumentInfoCollection argumentInfoCollection;
private final RewrittenTypeInfo rewrittenReturnInfo;
private RewrittenPrototypeDescription() {
- this(false, null, ArgumentInfoCollection.empty());
+ this(0, null, ArgumentInfoCollection.empty());
}
private RewrittenPrototypeDescription(
- boolean extraNullParameter,
+ int extraUnusedNullParameters,
RewrittenTypeInfo rewrittenReturnInfo,
ArgumentInfoCollection argumentsInfo) {
assert argumentsInfo != null;
- this.extraNullParameter = extraNullParameter;
+ this.extraUnusedNullParameters = extraUnusedNullParameters;
this.rewrittenReturnInfo = rewrittenReturnInfo;
this.argumentInfoCollection = argumentsInfo;
}
@@ -350,12 +350,12 @@
DexType returnType = method.proto.returnType;
RewrittenTypeInfo returnInfo =
returnType.isAlwaysNull(appView) ? RewrittenTypeInfo.toVoid(returnType, appView) : null;
- return new RewrittenPrototypeDescription(false, returnInfo, removedArgumentsInfo);
+ return new RewrittenPrototypeDescription(0, returnInfo, removedArgumentsInfo);
}
public static RewrittenPrototypeDescription createForRewrittenTypes(
RewrittenTypeInfo returnInfo, ArgumentInfoCollection rewrittenArgumentsInfo) {
- return new RewrittenPrototypeDescription(false, returnInfo, rewrittenArgumentsInfo);
+ return new RewrittenPrototypeDescription(0, returnInfo, rewrittenArgumentsInfo);
}
public static RewrittenPrototypeDescription none() {
@@ -363,11 +363,13 @@
}
public boolean isEmpty() {
- return !extraNullParameter && rewrittenReturnInfo == null && argumentInfoCollection.isEmpty();
+ return extraUnusedNullParameters == 0
+ && rewrittenReturnInfo == null
+ && argumentInfoCollection.isEmpty();
}
- public boolean hasExtraNullParameter() {
- return extraNullParameter;
+ public int numberOfExtraUnusedNullParameters() {
+ return extraUnusedNullParameters;
}
public boolean hasBeenChangedToReturnVoid(AppView<?> appView) {
@@ -418,7 +420,7 @@
assert rewrittenReturnInfo == null;
return !hasBeenChangedToReturnVoid(appView)
? new RewrittenPrototypeDescription(
- extraNullParameter,
+ extraUnusedNullParameters,
RewrittenTypeInfo.toVoid(oldReturnType, appView),
argumentInfoCollection)
: this;
@@ -426,12 +428,21 @@
public RewrittenPrototypeDescription withRemovedArguments(ArgumentInfoCollection other) {
return new RewrittenPrototypeDescription(
- extraNullParameter, rewrittenReturnInfo, argumentInfoCollection.combine(other));
+ extraUnusedNullParameters, rewrittenReturnInfo, argumentInfoCollection.combine(other));
}
- public RewrittenPrototypeDescription withExtraNullParameter() {
- return !extraNullParameter
- ? new RewrittenPrototypeDescription(true, rewrittenReturnInfo, argumentInfoCollection)
- : this;
+ public RewrittenPrototypeDescription withExtraUnusedNullParameter() {
+ return withExtraUnusedNullParameters(1);
+ }
+
+ public RewrittenPrototypeDescription withExtraUnusedNullParameters(
+ int numberOfExtraUnusedNullParameters) {
+ if (numberOfExtraUnusedNullParameters == 0) {
+ return this;
+ }
+ return new RewrittenPrototypeDescription(
+ extraUnusedNullParameters + numberOfExtraUnusedNullParameters,
+ rewrittenReturnInfo,
+ argumentInfoCollection);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index 6b7b402..dfce72e 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -538,13 +538,14 @@
register++;
}
- int numberOfArguments =
+ int originalNumberOfArguments =
method.method.proto.parameters.values.length
+ argumentsInfo.numberOfRemovedArguments()
- + (method.isStatic() ? 0 : 1);
+ + (method.isStatic() ? 0 : 1)
+ - prototypeChanges.numberOfExtraUnusedNullParameters();
int usedArgumentIndex = 0;
- while (argumentIndex < numberOfArguments) {
+ while (argumentIndex < originalNumberOfArguments) {
TypeElement type;
ArgumentInfo argumentInfo = argumentsInfo.getArgumentInfo(argumentIndex);
if (argumentInfo.isRemovedArgumentInfo()) {
@@ -558,14 +559,14 @@
DexType argType;
if (argumentInfo.isRewrittenTypeInfo()) {
RewrittenTypeInfo argumentRewrittenTypeInfo = argumentInfo.asRewrittenTypeInfo();
- assert method.method.proto.parameters.values[usedArgumentIndex]
+ assert method.method.proto.getParameter(usedArgumentIndex)
== argumentRewrittenTypeInfo.getNewType();
// The old type is used to prevent that a changed value from reference to primitive
// type breaks IR building. Rewriting from the old to the new type will be done in the
// IRConverter (typically through the lensCodeRewriter).
argType = argumentRewrittenTypeInfo.getOldType();
} else {
- argType = method.method.proto.parameters.values[usedArgumentIndex];
+ argType = method.method.proto.getParameter(usedArgumentIndex);
}
usedArgumentIndex++;
writeCallback.accept(register, argType);
@@ -579,6 +580,16 @@
register += type.requiredRegisters();
argumentIndex++;
}
+
+ for (int i = 0; i < prototypeChanges.numberOfExtraUnusedNullParameters(); i++) {
+ DexType argType = method.method.proto.getParameter(usedArgumentIndex);
+ assert argType.isClassType();
+ TypeElement type = TypeElement.fromDexType(argType, Nullability.maybeNull(), appView);
+ register += type.requiredRegisters();
+ usedArgumentIndex++;
+ addExtraUnusedNullArgument(register);
+ }
+
flushArgumentInstructions();
}
@@ -954,6 +965,14 @@
value.markAsThis();
}
+ private void addExtraUnusedNullArgument(int register) {
+ // Extra unused null arguments should bypass the register check, they may use registers
+ // beyond the limit of what the method can use. They don't have debug information and are
+ // always null.
+ Value value = new Value(valueNumberGenerator.next(), TypeElement.getNull(), null);
+ addNonThisArgument(new Argument(value, currentBlock.size(), false));
+ }
+
public void addNonThisArgument(int register, TypeElement typeLattice) {
DebugLocalInfo local = getOutgoingLocal(register);
Value value = writeRegister(register, typeLattice, ThrowingInfo.NO_THROW, local);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index 6c9877c..3e59e47 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -31,6 +31,7 @@
import static com.android.tools.r8.ir.code.Opcodes.STATIC_PUT;
import static com.android.tools.r8.utils.ObjectUtils.getBooleanOrElse;
+import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
@@ -283,12 +284,21 @@
? null
: makeOutValue(invoke, code);
- if (prototypeChanges.hasExtraNullParameter()) {
+ if (prototypeChanges.numberOfExtraUnusedNullParameters() > 0) {
iterator.previous();
- Value extraNullValue =
+ Value nullInstruction =
iterator.insertConstNullInstruction(code, appView.options());
iterator.next();
- newInValues.add(extraNullValue);
+ for (int i = 0; i < prototypeChanges.numberOfExtraUnusedNullParameters(); i++) {
+ newInValues.add(nullInstruction);
+ }
+ // TODO(b/164901008): Fix when the number of arguments overflows.
+ if (newInValues.size() > 255) {
+ throw new CompilationError(
+ "The addition of extra unused null parameters in R8 led to the overflow of"
+ + " the number of arguments of the method "
+ + actualTarget);
+ }
}
assert newInValues.size()
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLens.java b/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLens.java
index cdca27a..2669724 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLens.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLens.java
@@ -100,7 +100,8 @@
if (isConstructorBridge(method)) {
// TODO (b/132767654): Try to write a test which breaks that assertion.
assert previousLens.lookupPrototypeChanges(method).isEmpty();
- return RewrittenPrototypeDescription.none().withExtraNullParameter();
+ // TODO(b/164901008): Fix when the number of arguments overflows.
+ return RewrittenPrototypeDescription.none().withExtraUnusedNullParameter();
} else {
return previousLens.lookupPrototypeChanges(method);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
index a74b4b6..12a23f1 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
@@ -58,7 +58,6 @@
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackDelayed;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.BooleanUtils;
-import com.android.tools.r8.utils.IntBox;
import com.android.tools.r8.utils.Reporter;
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
@@ -908,11 +907,9 @@
});
clazz.getMethodCollection().removeMethods(methodsToRemove);
} else {
- IntBox index = new IntBox(0);
clazz
.getMethodCollection()
- .replaceMethods(
- encodedMethod -> fixupEncodedMethod(encodedMethod, index.getAndIncrement()));
+ .replaceMethods(encodedMethod -> fixupEncodedMethod(encodedMethod));
fixupFields(clazz.staticFields(), clazz::setStaticField);
fixupFields(clazz.instanceFields(), clazz::setInstanceField);
}
@@ -962,22 +959,44 @@
return encodedMethod.toTypeSubstitutedMethod(newMethod);
}
- private DexEncodedMethod fixupEncodedMethod(DexEncodedMethod encodedMethod, int index) {
+ private DexEncodedMethod fixupEncodedMethod(DexEncodedMethod encodedMethod) {
DexProto newProto = fixupProto(encodedMethod.proto());
- if (newProto != encodedMethod.proto()) {
- DexString newMethodName =
- factory.createString(
- EnumUnboxingRewriter.ENUM_UNBOXING_UTILITY_METHOD_PREFIX
- + index
- + "$"
- + encodedMethod.getName().toString());
- DexMethod newMethod = factory.createMethod(encodedMethod.holder(), newProto, newMethodName);
- assert appView.definitionFor(encodedMethod.holder()).lookupMethod(newMethod) == null;
- boolean isStatic = encodedMethod.isStatic();
- lensBuilder.move(encodedMethod.method, isStatic, newMethod, isStatic);
- return encodedMethod.toTypeSubstitutedMethod(newMethod);
+ if (newProto == encodedMethod.proto()) {
+ return encodedMethod;
}
- return encodedMethod;
+ assert !encodedMethod.isClassInitializer();
+ DexMethod newMethod =
+ factory.createMethod(encodedMethod.holder(), newProto, encodedMethod.getName());
+ newMethod = ensureUniqueMethod(encodedMethod, newMethod);
+ int numberOfExtraNullParameters = newMethod.getArity() - encodedMethod.method.getArity();
+ boolean isStatic = encodedMethod.isStatic();
+ lensBuilder.move(
+ encodedMethod.method, isStatic, newMethod, isStatic, numberOfExtraNullParameters);
+ return encodedMethod.toTypeSubstitutedMethod(newMethod);
+ }
+
+ private DexMethod ensureUniqueMethod(DexEncodedMethod encodedMethod, DexMethod newMethod) {
+ DexClass holder = appView.definitionFor(encodedMethod.holder());
+ assert holder != null;
+ if (encodedMethod.isInstanceInitializer()) {
+ while (holder.lookupMethod(newMethod) != null) {
+ newMethod =
+ factory.createMethod(
+ newMethod.holder,
+ factory.appendTypeToProto(newMethod.proto, factory.enumUnboxingUtilityType),
+ newMethod.name);
+ }
+ } else {
+ int index = 0;
+ while (holder.lookupMethod(newMethod) != null) {
+ newMethod =
+ factory.createMethod(
+ newMethod.holder,
+ newMethod.proto,
+ encodedMethod.getName().toString() + "$enumunboxing$" + index++);
+ }
+ }
+ return newMethod;
}
private void fixupFields(List<DexEncodedField> fields, FieldSetter setter) {
@@ -1091,6 +1110,15 @@
new IdentityHashMap<>();
public void move(DexMethod from, boolean fromStatic, DexMethod to, boolean toStatic) {
+ move(from, fromStatic, to, toStatic, 0);
+ }
+
+ public void move(
+ DexMethod from,
+ boolean fromStatic,
+ DexMethod to,
+ boolean toStatic,
+ int numberOfExtraNullParameters) {
super.move(from, to);
int offsetDiff = 0;
int toOffset = BooleanUtils.intValue(!toStatic);
@@ -1114,7 +1142,9 @@
? null
: new RewrittenTypeInfo(from.proto.returnType, to.proto.returnType);
prototypeChanges.put(
- to, RewrittenPrototypeDescription.createForRewrittenTypes(returnInfo, builder.build()));
+ to,
+ RewrittenPrototypeDescription.createForRewrittenTypes(returnInfo, builder.build())
+ .withExtraUnusedNullParameters(numberOfExtraNullParameters));
}
public EnumUnboxingLens build(
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingClassStaticizerTest.java b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingClassStaticizerTest.java
index 758ed36..393ef89 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingClassStaticizerTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingClassStaticizerTest.java
@@ -75,7 +75,7 @@
codeInspector
.clazz(CompanionHost.class)
.uniqueMethodWithName(
- EnumUnboxingRewriter.ENUM_UNBOXING_UTILITY_METHOD_PREFIX + "1$method");
+ EnumUnboxingRewriter.ENUM_UNBOXING_UTILITY_METHOD_PREFIX + "0$method");
assertThat(method, isPresent());
assertEquals("int", method.getMethod().method.proto.parameters.toString());
}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/OverloadingEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/OverloadingEnumUnboxingTest.java
index 8e206b8..bf038fa 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/OverloadingEnumUnboxingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/OverloadingEnumUnboxingTest.java
@@ -72,11 +72,37 @@
C;
}
+ @NeverClassInline
+ enum MyEnum3 {
+ A,
+ B,
+ C;
+ }
+
static class TestClass {
public static void main(String[] args) {
virtualTest();
staticTest();
+ constructorTest();
+ }
+
+ @NeverInline
+ private static void constructorTest() {
+ new TestClass(42);
+ System.out.println("42");
+ new TestClass(MyEnum1.A);
+ System.out.println("0");
+ new TestClass(MyEnum1.B);
+ System.out.println("1");
+ new TestClass(MyEnum2.A);
+ System.out.println("0");
+ new TestClass(MyEnum2.B);
+ System.out.println("1");
+ new TestClass(MyEnum3.A);
+ System.out.println("0");
+ new TestClass(MyEnum3.B);
+ System.out.println("1");
}
@NeverInline
@@ -91,6 +117,10 @@
System.out.println("0");
staticMethod(MyEnum2.B);
System.out.println("1");
+ staticMethod(MyEnum3.A);
+ System.out.println("0");
+ staticMethod(MyEnum3.B);
+ System.out.println("1");
}
@NeverInline
@@ -106,6 +136,32 @@
System.out.println("0");
testClass.virtualMethod(MyEnum2.B);
System.out.println("1");
+ testClass.virtualMethod(MyEnum3.A);
+ System.out.println("0");
+ testClass.virtualMethod(MyEnum3.B);
+ System.out.println("1");
+ }
+
+ public TestClass() {}
+
+ @NeverInline
+ public TestClass(MyEnum1 e1) {
+ System.out.println(e1.ordinal());
+ }
+
+ @NeverInline
+ public TestClass(MyEnum2 e2) {
+ System.out.println(e2.ordinal());
+ }
+
+ @NeverInline
+ public TestClass(MyEnum3 e3) {
+ System.out.println(e3.ordinal());
+ }
+
+ @NeverInline
+ public TestClass(int i) {
+ System.out.println(i);
}
@NeverInline
@@ -114,8 +170,13 @@
}
@NeverInline
- public void virtualMethod(MyEnum2 e1) {
- System.out.println(e1.ordinal());
+ public void virtualMethod(MyEnum2 e2) {
+ System.out.println(e2.ordinal());
+ }
+
+ @NeverInline
+ public void virtualMethod(MyEnum3 e3) {
+ System.out.println(e3.ordinal());
}
@NeverInline
@@ -129,8 +190,13 @@
}
@NeverInline
- public static void staticMethod(MyEnum2 e1) {
- System.out.println(e1.ordinal());
+ public static void staticMethod(MyEnum2 e2) {
+ System.out.println(e2.ordinal());
+ }
+
+ @NeverInline
+ public static void staticMethod(MyEnum3 e3) {
+ System.out.println(e3.ordinal());
}
@NeverInline
