Enum unboxing: Fix valueOf bug.
Bug: 147860220
Change-Id: I32f3e5905638bd1e481703b00db793211e5058ec
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 0e4a769..6a91188 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
@@ -126,7 +126,8 @@
for (Instruction instruction : block.getInstructions()) {
Value outValue = instruction.outValue();
if (outValue != null) {
- DexProgramClass enumClass = getEnumUnboxingCandidateOrNull(outValue.getType());
+ DexProgramClass enumClass =
+ getEnumUnboxingCandidateOrNull(outValue.getDynamicUpperBoundType(appView));
if (enumClass != null) {
Reason reason = validateEnumUsages(code, outValue, enumClass);
if (reason == Reason.ELIGIBLE) {
@@ -138,9 +139,9 @@
}
}
if (instruction.isConstClass()) {
- analyzeConstClass(instruction.asConstClass());
+ analyzeConstClass(instruction.asConstClass(), eligibleEnums);
} else if (instruction.isCheckCast()) {
- analyzeCheckCast(instruction.asCheckCast());
+ analyzeCheckCast(instruction.asCheckCast(), eligibleEnums);
} else if (instruction.isInvokeStatic()) {
// TODO(b/150370354): Since we temporary allow enum unboxing on enums with values and
// valueOf static methods only if such methods are unused, such methods cannot be
@@ -184,24 +185,30 @@
}
}
- private void analyzeCheckCast(CheckCast checkCast) {
+ private void analyzeCheckCast(CheckCast checkCast, Set<DexType> eligibleEnums) {
// We are doing a type check, which typically means the in-value is of an upper
// type and cannot be dealt with.
// If the cast is on a dynamically typed object, the checkCast can be simply removed.
// This allows enum array clone and valueOf to work correctly.
- TypeElement objectType = checkCast.object().getDynamicUpperBoundType(appView);
- if (objectType.equalUpToNullability(
- TypeElement.fromDexType(checkCast.getType(), definitelyNotNull(), appView))) {
- return;
- }
DexProgramClass enumClass =
getEnumUnboxingCandidateOrNull(checkCast.getType().toBaseType(factory));
- if (enumClass != null) {
- markEnumAsUnboxable(Reason.DOWN_CAST, enumClass);
+ if (enumClass == null) {
+ return;
}
+ if (allowCheckCast(checkCast)) {
+ eligibleEnums.add(enumClass.type);
+ return;
+ }
+ markEnumAsUnboxable(Reason.DOWN_CAST, enumClass);
}
- private void analyzeConstClass(ConstClass constClass) {
+ private boolean allowCheckCast(CheckCast checkCast) {
+ TypeElement objectType = checkCast.object().getDynamicUpperBoundType(appView);
+ return objectType.equalUpToNullability(
+ TypeElement.fromDexType(checkCast.getType(), definitelyNotNull(), appView));
+ }
+
+ private void analyzeConstClass(ConstClass constClass, Set<DexType> eligibleEnums) {
// We are using the ConstClass of an enum, which typically means the enum cannot be unboxed.
// We however allow unboxing if the ConstClass is only used as an argument to Enum#valueOf, to
// allow unboxing of: MyEnum a = Enum.valueOf(MyEnum.class, "A");.
@@ -209,6 +216,7 @@
return;
}
if (constClass.outValue() == null) {
+ eligibleEnums.add(constClass.getValue());
return;
}
if (constClass.outValue().hasPhiUsers()) {
@@ -224,6 +232,7 @@
return;
}
}
+ eligibleEnums.add(constClass.getValue());
}
private void addNullDependencies(Set<Instruction> uses, Set<DexType> eligibleEnums) {
@@ -472,6 +481,13 @@
return Reason.INVALID_IF_TYPES;
}
+ if (instruction.isCheckCast()) {
+ if (allowCheckCast(instruction.asCheckCast())) {
+ return Reason.ELIGIBLE;
+ }
+ return Reason.DOWN_CAST;
+ }
+
if (instruction.isArrayLength()) {
// MyEnum[] array = ...; array.length; is valid.
return Reason.ELIGIBLE;
@@ -624,6 +640,7 @@
FIELD_PUT_ON_ENUM,
TYPE_MISMATCH_FIELD_PUT,
INVALID_IF_TYPES,
+ DYNAMIC_TYPE,
ENUM_METHOD_CALLED_WITH_NULL_RECEIVER,
OTHER_UNSUPPORTED_INSTRUCTION;
}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/ValueOfEnumUnboxingFailureTest.java b/src/test/java/com/android/tools/r8/enumunboxing/ValueOfEnumUnboxingFailureTest.java
new file mode 100644
index 0000000..e2dcc4c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/enumunboxing/ValueOfEnumUnboxingFailureTest.java
@@ -0,0 +1,63 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.enumunboxing;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.TestParameters;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class ValueOfEnumUnboxingFailureTest extends EnumUnboxingTestBase {
+
+ private final TestParameters parameters;
+ private final boolean enumValueOptimization;
+ private final KeepRule enumKeepRules;
+
+ @Parameterized.Parameters(name = "{0} valueOpt: {1} keep: {2}")
+ public static List<Object[]> data() {
+ return enumUnboxingTestParameters();
+ }
+
+ public ValueOfEnumUnboxingFailureTest(
+ TestParameters parameters, boolean enumValueOptimization, KeepRule enumKeepRules) {
+ this.parameters = parameters;
+ this.enumValueOptimization = enumValueOptimization;
+ this.enumKeepRules = enumKeepRules;
+ }
+
+ @Test
+ public void testEnumUnboxing() throws Exception {
+ Class<?> success = Main.class;
+ testForR8(parameters.getBackend())
+ .addInnerClasses(ValueOfEnumUnboxingFailureTest.class)
+ .addKeepMainRule(success)
+ .enableNeverClassInliningAnnotations()
+ .addKeepRules(enumKeepRules.getKeepRule())
+ .addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
+ .allowDiagnosticInfoMessages()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspectDiagnosticMessages(
+ m -> assertEnumIsBoxed(success.getDeclaredClasses()[0], success.getSimpleName(), m))
+ .run(parameters.getRuntime(), success)
+ .assertSuccessWithOutput("VALUE1");
+ }
+
+ static class Main {
+
+ @NeverClassInline
+ enum Enum {
+ VALUE1,
+ VALUE2
+ }
+
+ public static void main(String[] args) {
+ System.out.print(java.lang.Enum.valueOf(Enum.class, "VALUE1"));
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/ValueOfEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/ValueOfEnumUnboxingTest.java
index b4e1ef6..37f31c9 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/ValueOfEnumUnboxingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/ValueOfEnumUnboxingTest.java
@@ -76,7 +76,7 @@
System.out.println(Enum.valueOf(EnumValueOf.MyEnum.class, "B").ordinal());
System.out.println(1);
try {
- Enum.valueOf(EnumValueOf.MyEnum.class, "C");
+ iae();
} catch (IllegalArgumentException argException) {
System.out.println(argException.getMessage());
System.out.println(
@@ -84,11 +84,20 @@
+ " com.android.tools.r8.enumunboxing.ValueOfEnumUnboxingTest.EnumValueOf.MyEnum.C");
}
try {
- Enum.valueOf(EnumValueOf.MyEnum.class, null);
+ npe();
} catch (NullPointerException npe) {
System.out.println(npe.getMessage());
System.out.println("Name is null");
}
}
+
+ @SuppressWarnings("ConstantConditions")
+ private static void npe() {
+ Enum.valueOf(MyEnum.class, null);
+ }
+
+ private static void iae() {
+ Enum.valueOf(MyEnum.class, "C");
+ }
}
}
