Revert "Enum unboxing: unbox null checks"
This reverts commit 5bf9d446e3812a7dfa051354e36a60a30bf80600.
Reason for revert: Breaking the bots.
Change-Id: I10f54d7baeca689de2736363acb4926ac8a0969b
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 3d7d397..30773a4 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
@@ -872,20 +872,9 @@
assert dexClass.isLibraryClass();
if (dexClass.type != factory.enumType) {
// System.identityHashCode(Object) is supported for proto enums.
- // Object#getClass without outValue and Objects.requireNonNull are supported since R8
- // rewrites explicit null checks to such instructions.
if (singleTarget == factory.javaLangSystemMethods.identityHashCode) {
return Reason.ELIGIBLE;
}
- if (singleTarget == factory.objectMembers.getClass
- && (!invokeMethod.hasOutValue() || !invokeMethod.outValue().hasAnyUsers())) {
- // This is a hidden null check.
- return Reason.ELIGIBLE;
- }
- if (singleTarget == factory.objectsMethods.requireNonNull
- || singleTarget == factory.objectsMethods.requireNonNullWithMessage) {
- return Reason.ELIGIBLE;
- }
return Reason.UNSUPPORTED_LIBRARY_CALL;
}
// TODO(b/147860220): EnumSet and EnumMap may be interesting to model.
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCfMethods.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCfMethods.java
index aa7ec4c..daf1ad4 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCfMethods.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCfMethods.java
@@ -24,7 +24,6 @@
import com.android.tools.r8.cf.code.CfNew;
import com.android.tools.r8.cf.code.CfNewArray;
import com.android.tools.r8.cf.code.CfReturn;
-import com.android.tools.r8.cf.code.CfReturnVoid;
import com.android.tools.r8.cf.code.CfStackInstruction;
import com.android.tools.r8.cf.code.CfStore;
import com.android.tools.r8.cf.code.CfThrow;
@@ -261,69 +260,4 @@
ImmutableList.of(),
ImmutableList.of());
}
-
- public static CfCode EnumUnboxingMethods_zeroCheck(InternalOptions options, DexMethod method) {
- CfLabel label0 = new CfLabel();
- CfLabel label1 = new CfLabel();
- CfLabel label2 = new CfLabel();
- CfLabel label3 = new CfLabel();
- return new CfCode(
- method.holder,
- 2,
- 1,
- ImmutableList.of(
- label0,
- new CfLoad(ValueType.INT, 0),
- new CfIf(If.Type.NE, ValueType.INT, label2),
- label1,
- new CfNew(options.itemFactory.createType("Ljava/lang/NullPointerException;")),
- new CfStackInstruction(CfStackInstruction.Opcode.Dup),
- new CfInvoke(
- 183,
- options.itemFactory.createMethod(
- options.itemFactory.createType("Ljava/lang/NullPointerException;"),
- options.itemFactory.createProto(options.itemFactory.voidType),
- options.itemFactory.createString("<init>")),
- false),
- new CfThrow(),
- label2,
- new CfReturnVoid(),
- label3),
- ImmutableList.of(),
- ImmutableList.of());
- }
-
- public static CfCode EnumUnboxingMethods_zeroCheckMessage(
- InternalOptions options, DexMethod method) {
- CfLabel label0 = new CfLabel();
- CfLabel label1 = new CfLabel();
- CfLabel label2 = new CfLabel();
- CfLabel label3 = new CfLabel();
- return new CfCode(
- method.holder,
- 3,
- 2,
- ImmutableList.of(
- label0,
- new CfLoad(ValueType.INT, 0),
- new CfIf(If.Type.NE, ValueType.INT, label2),
- label1,
- new CfNew(options.itemFactory.createType("Ljava/lang/NullPointerException;")),
- new CfStackInstruction(CfStackInstruction.Opcode.Dup),
- new CfLoad(ValueType.OBJECT, 1),
- new CfInvoke(
- 183,
- options.itemFactory.createMethod(
- options.itemFactory.createType("Ljava/lang/NullPointerException;"),
- options.itemFactory.createProto(
- options.itemFactory.voidType, options.itemFactory.stringType),
- options.itemFactory.createString("<init>")),
- false),
- new CfThrow(),
- label2,
- new CfReturnVoid(),
- label3),
- ImmutableList.of(),
- ImmutableList.of());
- }
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java
index ca106ab..54d9aaf 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java
@@ -77,8 +77,6 @@
private final DexMethod equalsUtilityMethod;
private final DexMethod compareToUtilityMethod;
private final DexMethod valuesUtilityMethod;
- private final DexMethod zeroCheckMethod;
- private final DexMethod zeroCheckMessageMethod;
EnumUnboxingRewriter(
AppView<AppInfoWithLiveness> appView,
@@ -116,17 +114,6 @@
factory.enumUnboxingUtilityType,
factory.createProto(factory.intArrayType, factory.intType),
ENUM_UNBOXING_UTILITY_METHOD_PREFIX + "values");
- // Custom methods for Object#getClass without outValue and Objects.requireNonNull.
- this.zeroCheckMethod =
- factory.createMethod(
- factory.enumUnboxingUtilityType,
- factory.createProto(factory.voidType, factory.intType),
- ENUM_UNBOXING_UTILITY_METHOD_PREFIX + "zeroCheck");
- this.zeroCheckMessageMethod =
- factory.createMethod(
- factory.enumUnboxingUtilityType,
- factory.createProto(factory.voidType, factory.intType, factory.stringType),
- ENUM_UNBOXING_UTILITY_METHOD_PREFIX + "zeroCheckMessage");
}
public EnumValueInfoMapCollection getEnumsToUnbox() {
@@ -173,10 +160,6 @@
new InvokeStatic(
toStringMethod, invokeMethod.outValue(), invokeMethod.arguments()));
continue;
- } else if (invokedMethod == factory.objectMembers.getClass) {
- assert !invokeMethod.hasOutValue() || !invokeMethod.outValue().hasAnyUsers();
- replaceEnumInvoke(
- iterator, invokeMethod, zeroCheckMethod, m -> synthesizeZeroCheckMethod());
}
}
// TODO(b/147860220): rewrite also other enum methods.
@@ -212,25 +195,6 @@
invokeStatic.outValue().replaceUsers(argument);
iterator.removeOrReplaceByDebugLocalRead();
}
- } else if (invokedMethod == factory.objectsMethods.requireNonNull) {
- assert invokeStatic.inValues().size() == 1;
- Value argument = invokeStatic.getArgument(0);
- DexType enumType = getEnumTypeOrNull(argument, convertedEnums);
- if (enumType != null) {
- replaceEnumInvoke(
- iterator, invokeStatic, zeroCheckMethod, m -> synthesizeZeroCheckMethod());
- }
- } else if (invokedMethod == factory.objectsMethods.requireNonNullWithMessage) {
- assert invokeStatic.inValues().size() == 2;
- Value argument = invokeStatic.getArgument(0);
- DexType enumType = getEnumTypeOrNull(argument, convertedEnums);
- if (enumType != null) {
- replaceEnumInvoke(
- iterator,
- invokeStatic,
- zeroCheckMessageMethod,
- m -> synthesizeZeroCheckMessageMethod());
- }
}
}
if (instruction.isStaticGet()) {
@@ -533,19 +497,6 @@
return false;
}
- private DexEncodedMethod synthesizeZeroCheckMethod() {
- CfCode cfCode =
- EnumUnboxingCfMethods.EnumUnboxingMethods_zeroCheck(appView.options(), zeroCheckMethod);
- return synthesizeUtilityMethod(cfCode, zeroCheckMethod, false);
- }
-
- private DexEncodedMethod synthesizeZeroCheckMessageMethod() {
- CfCode cfCode =
- EnumUnboxingCfMethods.EnumUnboxingMethods_zeroCheckMessage(
- appView.options(), zeroCheckMessageMethod);
- return synthesizeUtilityMethod(cfCode, zeroCheckMessageMethod, false);
- }
-
private DexEncodedMethod synthesizeOrdinalMethod() {
CfCode cfCode =
EnumUnboxingCfMethods.EnumUnboxingMethods_ordinal(appView.options(), ordinalUtilityMethod);
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingMethods.java b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingMethods.java
index cc2aa0c..e1d51cc 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingMethods.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingMethods.java
@@ -51,17 +51,4 @@
}
return unboxedEnum1 == unboxedEnum2;
}
-
- // Methods zeroCheck and zeroCheckMessage are used to replace null checks on unboxed enums.
- public static void zeroCheck(int unboxedEnum) {
- if (unboxedEnum == 0) {
- throw new NullPointerException();
- }
- }
-
- public static void zeroCheckMessage(int unboxedEnum, String message) {
- if (unboxedEnum == 0) {
- throw new NullPointerException(message);
- }
- }
}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/NullCheckEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/NullCheckEnumUnboxingTest.java
deleted file mode 100644
index 25a3434..0000000
--- a/src/test/java/com/android/tools/r8/enumunboxing/NullCheckEnumUnboxingTest.java
+++ /dev/null
@@ -1,197 +0,0 @@
-// 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.NeverInline;
-import com.android.tools.r8.R8TestRunResult;
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.utils.AndroidApiLevel;
-import java.util.List;
-import java.util.Objects;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameters;
-
-@RunWith(Parameterized.class)
-public class NullCheckEnumUnboxingTest extends EnumUnboxingTestBase {
-
- private final TestParameters parameters;
- private final boolean enumValueOptimization;
- private final EnumKeepRules enumKeepRules;
-
- @Parameters(name = "{0} valueOpt: {1} keep: {2}")
- public static List<Object[]> data() {
- return enumUnboxingTestParameters();
- }
-
- public NullCheckEnumUnboxingTest(
- TestParameters parameters, boolean enumValueOptimization, EnumKeepRules enumKeepRules) {
- this.parameters = parameters;
- this.enumValueOptimization = enumValueOptimization;
- this.enumKeepRules = enumKeepRules;
- }
-
- @Test
- public void testEnumUnboxing() throws Exception {
- R8TestRunResult run =
- testForR8(parameters.getBackend())
- .addInnerClasses(NullCheckEnumUnboxingTest.class)
- .addKeepMainRule(MainNullTest.class)
- .addKeepRules(enumKeepRules.getKeepRules())
- .enableNeverClassInliningAnnotations()
- .enableInliningAnnotations()
- .addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
- .allowDiagnosticMessages()
- .setMinApi(parameters.getApiLevel())
- .compile()
- .inspectDiagnosticMessages(
- m -> {
- assertEnumIsUnboxed(MyEnum.class, MyEnum.class.getSimpleName(), m);
- // MyEnum19 is unboxed only if minAPI > 19 because Objects#requiredNonNull is then
- // present.
- if (parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.K)) {
- assertEnumIsUnboxed(MyEnum19.class, MyEnum19.class.getSimpleName(), m);
- } else {
- assertEnumIsBoxed(MyEnum19.class, MyEnum19.class.getSimpleName(), m);
- }
- })
- .run(parameters.getRuntime(), MainNullTest.class)
- .assertSuccess();
- assertLines2By2Correct(run.getStdOut());
- }
-
- @NeverClassInline
- enum MyEnum {
- A,
- B,
- C
- }
-
- @NeverClassInline
- enum MyEnum19 {
- A,
- B,
- C
- }
-
- static class MainNullTest {
-
- public static void main(String[] args) {
- nullCheckTests();
- nullCheckMessageTests();
- }
-
- private static void nullCheckTests() {
- nullCheck0Test(MyEnum.A, false);
- nullCheck0Test(MyEnum.B, false);
- nullCheck0Test(null, true);
-
- nullCheck1Test(MyEnum.A, false);
- nullCheck1Test(MyEnum.B, false);
- nullCheck1Test(null, true);
-
- nullCheck2Test(MyEnum19.A, false);
- nullCheck2Test(MyEnum19.B, false);
- nullCheck2Test(null, true);
- }
-
- private static void nullCheckMessageTests() {
- nullCheckMessage0Test(MyEnum.A, "myMessageA", false);
- nullCheckMessage0Test(MyEnum.B, "myMessageB", false);
- nullCheckMessage0Test(null, "myMessageN", true);
-
- nullCheckMessage1Test(MyEnum19.A, "myMessageA", false);
- nullCheckMessage1Test(MyEnum19.B, "myMessageB", false);
- nullCheckMessage1Test(null, "myMessageN", true);
- }
-
- private static void nullCheck0Test(MyEnum input, boolean isNull) {
- String result = "pass";
- try {
- nullCheck0(input);
- } catch (NullPointerException ex8) {
- result = "fail";
- }
- System.out.println(result);
- System.out.println(isNull ? "fail" : "pass");
- }
-
- private static void nullCheck1Test(MyEnum input, boolean isNull) {
- String result = "pass";
- try {
- nullCheck1(input);
- } catch (NullPointerException ex8) {
- result = "fail";
- }
- System.out.println(result);
- System.out.println(isNull ? "fail" : "pass");
- }
-
- private static void nullCheck2Test(MyEnum19 input, boolean isNull) {
- String result = "pass";
- try {
- nullCheck2(input);
- } catch (NullPointerException ex) {
- result = "fail";
- }
- System.out.println(result);
- System.out.println(isNull ? "fail" : "pass");
- }
-
- private static void nullCheckMessage0Test(MyEnum input, String message, boolean isNull) {
- String result = "pass";
- try {
- nullCheckMessage0(input, message);
- } catch (NullPointerException ex) {
- result = ex.getMessage();
- }
- System.out.println(result);
- System.out.println(isNull ? message : "pass");
- }
-
- private static void nullCheckMessage1Test(MyEnum19 input, String message, boolean isNull) {
- String result = "pass";
- try {
- nullCheckMessage1(input, message);
- } catch (NullPointerException ex) {
- result = ex.getMessage();
- }
- System.out.println(result);
- System.out.println(isNull ? message : "pass");
- }
-
- @NeverInline
- private static void nullCheck0(MyEnum e) {
- if (e == null) {
- throw new NullPointerException();
- }
- }
-
- @SuppressWarnings("ResultOfMethodCallIgnored")
- @NeverInline
- private static void nullCheck1(MyEnum e) {
- e.getClass();
- }
-
- @NeverInline
- private static void nullCheck2(MyEnum19 e) {
- Objects.requireNonNull(e);
- }
-
- @NeverInline
- private static void nullCheckMessage0(MyEnum e, String message) {
- if (e == null) {
- throw new NullPointerException(message);
- }
- }
-
- @NeverInline
- private static void nullCheckMessage1(MyEnum19 e, String message) {
- Objects.requireNonNull(e, message);
- }
- }
-}
