Fix enum unboxing and invalid equals
Bug: b/275522360
Change-Id: I9cccb56b206a577a8ad467041d91e0c35aced3e1
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
index a649af6..587beaf 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
@@ -91,6 +91,7 @@
import com.android.tools.r8.ir.optimize.enums.eligibility.Reason.MissingEnumStaticFieldValuesReason;
import com.android.tools.r8.ir.optimize.enums.eligibility.Reason.MissingInstanceFieldValueForEnumInstanceReason;
import com.android.tools.r8.ir.optimize.enums.eligibility.Reason.MissingObjectStateForEnumInstanceReason;
+import com.android.tools.r8.ir.optimize.enums.eligibility.Reason.UnboxedValueNonComparable;
import com.android.tools.r8.ir.optimize.enums.eligibility.Reason.UnsupportedInstanceFieldValueForEnumInstanceReason;
import com.android.tools.r8.ir.optimize.enums.eligibility.Reason.UnsupportedLibraryInvokeReason;
import com.android.tools.r8.ir.optimize.info.MutableFieldOptimizationInfo;
@@ -1296,6 +1297,24 @@
return reason;
}
+ private Reason comparableAsUnboxedValues(InvokeMethod invoke) {
+ assert invoke.inValues().size() == 2;
+ TypeElement type1 = invoke.getFirstArgument().getType();
+ TypeElement type2 = invoke.getLastArgument().getType();
+ DexProgramClass candidate1 = getEnumUnboxingCandidateOrNull(type1);
+ DexProgramClass candidate2 = getEnumUnboxingCandidateOrNull(type2);
+ assert candidate1 != null || candidate2 != null;
+ if (type1.isNullType() || type2.isNullType()) {
+ // Comparing an unboxed enum to null is always allowed.
+ return Reason.ELIGIBLE;
+ }
+ if (candidate1 == candidate2) {
+ // Comparing two unboxed enum values is valid only if they come from the same enum.
+ return Reason.ELIGIBLE;
+ }
+ return new UnboxedValueNonComparable(invoke.getInvokedMethod(), type1, type2);
+ }
+
private Reason analyzeLibraryInvoke(
InvokeMethod invoke,
IRCode code,
@@ -1309,13 +1328,9 @@
// TODO(b/147860220): EnumSet and EnumMap may be interesting to model.
if (singleTargetReference == factory.enumMembers.compareTo
|| singleTargetReference == factory.enumMembers.compareToWithObject) {
- DexProgramClass otherEnumClass =
- getEnumUnboxingCandidateOrNull(invoke.getLastArgument().getType());
- if (otherEnumClass == enumClass || invoke.getLastArgument().getType().isNullType()) {
- return Reason.ELIGIBLE;
- }
+ return comparableAsUnboxedValues(invoke);
} else if (singleTargetReference == factory.enumMembers.equals) {
- return Reason.ELIGIBLE;
+ return comparableAsUnboxedValues(invoke);
} else if (singleTargetReference == factory.enumMembers.nameMethod
|| singleTargetReference == factory.enumMembers.toString) {
assert invoke.asInvokeMethodWithReceiver().getReceiver() == enumValue;
@@ -1342,6 +1357,17 @@
// This is a hidden null check.
return Reason.ELIGIBLE;
}
+ if (singleTargetReference == factory.objectMembers.toString) {
+ assert invoke.asInvokeMethodWithReceiver().getReceiver() == enumValue;
+ addRequiredNameData(enumClass);
+ return Reason.ELIGIBLE;
+ }
+ if (singleTargetReference == factory.objectMembers.hashCode) {
+ return Reason.ELIGIBLE;
+ }
+ if (singleTargetReference == factory.objectMembers.equals) {
+ return comparableAsUnboxedValues(invoke);
+ }
return new UnsupportedLibraryInvokeReason(singleTargetReference);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/eligibility/Reason.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/eligibility/Reason.java
index 3e17a0a..6054468 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/eligibility/Reason.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/eligibility/Reason.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.google.common.collect.ImmutableList;
public abstract class Reason {
@@ -231,6 +232,43 @@
}
}
+ public static class UnboxedValueNonComparable extends Reason {
+
+ private final DexMethod invokedMethod;
+ private final TypeElement type1;
+ private final TypeElement type2;
+
+ public UnboxedValueNonComparable(
+ DexMethod invokedMethod, TypeElement type1, TypeElement type2) {
+ this.invokedMethod = invokedMethod;
+ this.type1 = type1;
+ this.type2 = type2;
+ }
+
+ @Override
+ public Object getKind() {
+ return ImmutableList.of(getClass(), invokedMethod);
+ }
+
+ private static String typeInformation(TypeElement type) {
+ if (type.isClassType()) {
+ return type.asClassType().getClassType().toSourceString();
+ }
+ return type.toString();
+ }
+
+ @Override
+ public String toString() {
+ return "NonComparableElements("
+ + invokedMethod.toSourceString()
+ + " - "
+ + typeInformation(type1)
+ + " vs "
+ + typeInformation(type2)
+ + ")";
+ }
+ }
+
public static class UnsupportedStaticFieldReason extends Reason {
private final DexField field;
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/CallToOtherEnumCompareToMethodNegativeUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/CallToOtherEnumCompareToMethodNegativeUnboxingTest.java
new file mode 100644
index 0000000..f190a34
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/enumunboxing/CallToOtherEnumCompareToMethodNegativeUnboxingTest.java
@@ -0,0 +1,73 @@
+// Copyright (c) 2023, 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.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.utils.codeinspector.EnumUnboxingInspector;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class CallToOtherEnumCompareToMethodNegativeUnboxingTest extends EnumUnboxingTestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ @Test
+ public void testRuntime() throws Exception {
+ testForRuntime(parameters)
+ .addInnerClasses(getClass())
+ .run(parameters.getRuntime(), Main.class)
+ .applyIf(
+ parameters.isCfRuntime()
+ || parameters.getDexRuntimeVersion().isNewerThanOrEqual(Version.V7_0_0),
+ runResult -> runResult.assertFailureWithErrorThatThrows(ClassCastException.class),
+ runResult -> runResult.assertSuccessWithOutputLines("0"));
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addKeepClassRules(Foo.class)
+ .addEnumUnboxingInspector(EnumUnboxingInspector::assertNoEnumsUnboxed)
+ .setMinApi(parameters)
+ .compile()
+ .run(parameters.getRuntime(), Main.class)
+ .applyIf(
+ parameters.isCfRuntime()
+ || parameters.getDexRuntimeVersion().isNewerThanOrEqual(Version.V7_0_0),
+ runResult -> runResult.assertFailureWithErrorThatThrows(ClassCastException.class),
+ runResult -> runResult.assertSuccessWithOutputLines("0"));
+ }
+
+ static class Main {
+
+ public static void main(String[] args) {
+ Enum<?> fooEnum = Foo.A;
+ Enum<Bar> fooEnumInDisguise = (Enum<Bar>) fooEnum;
+ System.out.println(fooEnumInDisguise.compareTo(Bar.B));
+ }
+ }
+
+ enum Foo {
+ A
+ }
+
+ enum Bar {
+ B
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/InvalidEqualsEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/InvalidEqualsEnumUnboxingTest.java
new file mode 100644
index 0000000..2ed2d40
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/enumunboxing/InvalidEqualsEnumUnboxingTest.java
@@ -0,0 +1,75 @@
+// Copyright (c) 2023, 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 com.android.tools.r8.utils.codeinspector.EnumUnboxingInspector;
+import java.util.List;
+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 InvalidEqualsEnumUnboxingTest 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 InvalidEqualsEnumUnboxingTest(
+ TestParameters parameters, boolean enumValueOptimization, EnumKeepRules enumKeepRules) {
+ this.parameters = parameters;
+ this.enumValueOptimization = enumValueOptimization;
+ this.enumKeepRules = enumKeepRules;
+ }
+
+ @Test
+ public void testEnumUnboxing() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(EnumEquals.class)
+ .addEnumUnboxingInspector(EnumUnboxingInspector::assertNoEnumsUnboxed)
+ .enableNeverClassInliningAnnotations()
+ .addKeepRules(enumKeepRules.getKeepRules())
+ .addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), EnumEquals.class)
+ .assertSuccessWithOutputLines("false", "false");
+ }
+
+ static class EnumEquals {
+
+ @NeverClassInline
+ enum MyEnumEquals {
+ A,
+ B
+ }
+
+ @NeverClassInline
+ enum MyEnumEquals1 {
+ A,
+ B
+ }
+
+ @NeverClassInline
+ enum MyEnumEquals2 {
+ A,
+ B
+ }
+
+ public static void main(String[] args) {
+ Object guineaPig = new Object();
+ System.out.println(MyEnumEquals.A.equals(guineaPig));
+ System.out.println(MyEnumEquals1.A.equals(MyEnumEquals2.A));
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/EnumUnboxingInspector.java b/src/test/java/com/android/tools/r8/utils/codeinspector/EnumUnboxingInspector.java
index c075406..3cbb42d 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/EnumUnboxingInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/EnumUnboxingInspector.java
@@ -22,6 +22,11 @@
this.unboxedEnums = unboxedEnums;
}
+ public EnumUnboxingInspector assertNoEnumsUnboxed() {
+ assertTrue(unboxedEnums.isEmpty());
+ return this;
+ }
+
public EnumUnboxingInspector assertUnboxed(String typeName) {
assertTrue(
unboxedEnums.isUnboxedEnum(
