Enum unboxing: support simple interfaces
- Also fix FailingEnumUnboxingTest which was not testing
anything due to noTreeShaking().
Change-Id: I34e8792b2d24710a817fa677d501b57a9ffb750d
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 29a4256..3c55b1a 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
@@ -437,6 +437,7 @@
DexMethod singleTarget = encodedSingleTarget.method;
DexClass dexClass = appView.definitionFor(singleTarget.holder);
if (dexClass == null) {
+ assert false;
return Reason.INVALID_INVOKE;
}
if (dexClass.isProgramClass()) {
@@ -448,14 +449,22 @@
return Reason.INVALID_INIT;
}
}
+ // Check that the enum-value only flows into parameters whose type exactly matches the
+ // enum's type.
int offset = BooleanUtils.intValue(!encodedSingleTarget.isStatic());
for (int i = 0; i < singleTarget.proto.parameters.size(); i++) {
- if (invokeMethod.inValues().get(offset + i) == enumValue) {
+ if (invokeMethod.getArgument(offset + i) == enumValue) {
if (singleTarget.proto.parameters.values[i].toBaseType(factory) != enumClass.type) {
return Reason.GENERIC_INVOKE;
}
}
}
+ if (invokeMethod.isInvokeMethodWithReceiver()) {
+ Value receiver = invokeMethod.asInvokeMethodWithReceiver().getReceiver();
+ if (receiver == enumValue && dexClass.isInterface()) {
+ return Reason.DEFAULT_METHOD_INVOKE;
+ }
+ }
return Reason.ELIGIBLE;
}
if (dexClass.isClasspathClass()) {
@@ -666,6 +675,7 @@
INTERFACE,
INSTANCE_FIELD,
GENERIC_INVOKE,
+ DEFAULT_METHOD_INVOKE,
UNEXPECTED_STATIC_FIELD,
UNRESOLVABLE_FIELD,
CONST_CLASS,
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateAnalysis.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateAnalysis.java
index 4e42286..08ba8b4 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateAnalysis.java
@@ -55,12 +55,6 @@
enumUnboxer.reportFailure(clazz.type, Reason.SUBTYPES);
return false;
}
- // TODO(b/147860220): interfaces without default methods should be acceptable if the build setup
- // is correct (all abstract methods are implemented).
- if (!clazz.interfaces.isEmpty()) {
- enumUnboxer.reportFailure(clazz.type, Reason.INTERFACE);
- return false;
- }
if (!clazz.instanceFields().isEmpty()) {
enumUnboxer.reportFailure(clazz.type, Reason.INSTANCE_FIELD);
return false;
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/FailingEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/FailingEnumUnboxingTest.java
index 2cfe861..f828413 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/FailingEnumUnboxingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/FailingEnumUnboxingTest.java
@@ -4,21 +4,13 @@
package com.android.tools.r8.enumunboxing;
-import static junit.framework.TestCase.assertTrue;
-import static org.junit.Assert.assertEquals;
-
import com.android.tools.r8.NeverClassInline;
-import com.android.tools.r8.NeverInline;
import com.android.tools.r8.R8FullTestBuilder;
import com.android.tools.r8.R8TestCompileResult;
import com.android.tools.r8.R8TestRunResult;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.enumunboxing.FailingEnumUnboxingTest.EnumInstanceFieldMain.EnumInstanceField;
-import com.android.tools.r8.enumunboxing.FailingEnumUnboxingTest.EnumInterfaceMain.EnumInterface;
import com.android.tools.r8.enumunboxing.FailingEnumUnboxingTest.EnumStaticFieldMain.EnumStaticField;
-import com.android.tools.r8.enumunboxing.FailingEnumUnboxingTest.EnumStaticMethodMain.EnumStaticMethod;
-import com.android.tools.r8.enumunboxing.FailingEnumUnboxingTest.EnumVirtualMethodMain.EnumVirtualMethod;
-import com.android.tools.r8.utils.codeinspector.CodeInspector;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -29,11 +21,8 @@
public class FailingEnumUnboxingTest extends EnumUnboxingTestBase {
private static final Class<?>[] FAILURES = {
- EnumInterface.class,
EnumStaticField.class,
EnumInstanceField.class,
- EnumStaticMethod.class,
- EnumVirtualMethod.class
};
private final TestParameters parameters;
@@ -61,15 +50,12 @@
}
R8TestCompileResult compile =
r8FullTestBuilder
- .noTreeShaking() // Disabled to avoid merging Itf into EnumInterface.
- .enableInliningAnnotations()
.enableNeverClassInliningAnnotations()
.addKeepRules(enumKeepRules.getKeepRule())
.addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
.allowDiagnosticInfoMessages()
.setMinApi(parameters.getApiLevel())
- .compile()
- .inspect(this::assertEnumsAsExpected);
+ .compile();
for (Class<?> failure : FAILURES) {
R8TestRunResult run =
compile
@@ -81,44 +67,6 @@
}
}
- private void assertEnumsAsExpected(CodeInspector inspector) {
- assertEquals(1, inspector.clazz(EnumInterface.class).getDexProgramClass().interfaces.size());
-
- assertTrue(inspector.clazz(EnumStaticField.class).uniqueFieldWithName("X").isPresent());
- assertTrue(inspector.clazz(EnumInstanceField.class).uniqueFieldWithName("a").isPresent());
-
- assertEquals(
- 5,
- inspector
- .clazz(EnumStaticMethod.class)
- .getDexProgramClass()
- .getMethodCollection()
- .numberOfDirectMethods());
- assertEquals(1, inspector.clazz(EnumVirtualMethod.class).virtualMethods().size());
- }
-
- static class EnumInterfaceMain {
-
- public static void main(String[] args) {
- System.out.println(EnumInterface.A.ordinal());
- System.out.println(0);
- }
-
- @NeverClassInline
- enum EnumInterface implements Itf {
- A,
- B,
- C
- }
-
- interface Itf {
-
- default int ordinal() {
- return -1;
- }
- }
- }
-
static class EnumStaticFieldMain {
public static void main(String[] args) {
@@ -158,53 +106,4 @@
System.out.println(10);
}
}
-
- static class EnumStaticMethodMain {
-
- @NeverClassInline
- enum EnumStaticMethod {
- A,
- B,
- C;
-
- // Enum cannot be unboxed if it has a static method, we do not inline so the method is
- // present.
- @NeverInline
- static int foo() {
- return Math.addExact(-1, 0);
- }
- }
-
- public static void main(String[] args) {
- System.out.println(EnumStaticMethod.A.ordinal());
- System.out.println(0);
- System.out.println(EnumStaticMethod.foo());
- System.out.println(-1);
- }
- }
-
- static class EnumVirtualMethodMain {
-
- public static void main(String[] args) {
- EnumVirtualMethod e1 = EnumVirtualMethod.A;
- System.out.println(e1.ordinal());
- System.out.println(0);
- System.out.println(e1.valueOf());
- System.out.println(-1);
- }
-
- @NeverClassInline
- enum EnumVirtualMethod {
- A,
- B,
- C;
-
- // Enum cannot be unboxed if it has a virtual method, we do not inline so the method is
- // present.
- @NeverInline
- int valueOf() {
- return Math.addExact(-1, 0);
- }
- }
- }
}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/InterfaceEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/InterfaceEnumUnboxingTest.java
new file mode 100644
index 0000000..5dcf75d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/enumunboxing/InterfaceEnumUnboxingTest.java
@@ -0,0 +1,270 @@
+// 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.NeverMerge;
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.R8TestRunResult;
+import com.android.tools.r8.TestParameters;
+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 InterfaceEnumUnboxingTest extends EnumUnboxingTestBase {
+
+ private static final Class<?>[] FAILURES = {
+ FailureDefaultMethodUsed.class, FailureUsedAsInterface.class,
+ };
+
+ private static final Class<?>[] SUCCESSES = {
+ SuccessAbstractMethod.class,
+ SuccessEmptyInterface.class,
+ SuccessUnusedDefaultMethod.class,
+ SuccessUnusedDefaultMethodOverride.class,
+ SuccessUnusedDefaultMethodOverrideEnum.class
+ };
+
+ private final TestParameters parameters;
+ private final boolean enumValueOptimization;
+ private final KeepRule enumKeepRules;
+
+ @Parameters(name = "{0} valueOpt: {1} keep: {2}")
+ public static List<Object[]> data() {
+ return enumUnboxingTestParameters();
+ }
+
+ public InterfaceEnumUnboxingTest(
+ TestParameters parameters, boolean enumValueOptimization, KeepRule enumKeepRules) {
+ this.parameters = parameters;
+ this.enumValueOptimization = enumValueOptimization;
+ this.enumKeepRules = enumKeepRules;
+ }
+
+ @Test
+ public void testEnumUnboxingFailure() throws Exception {
+ R8TestCompileResult compile =
+ testForR8(parameters.getBackend())
+ .addInnerClasses(InterfaceEnumUnboxingTest.class)
+ .addKeepMainRules(SUCCESSES)
+ .addKeepMainRules(FAILURES)
+ .noMinification()
+ .enableMergeAnnotations()
+ .enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .addKeepRules(enumKeepRules.getKeepRule())
+ .addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
+ .allowDiagnosticInfoMessages()
+ .setMinApi(parameters.getApiLevel())
+ .compile();
+ for (Class<?> failure : FAILURES) {
+ testClass(compile, failure, true);
+ }
+ for (Class<?> success : SUCCESSES) {
+ testClass(compile, success, false);
+ }
+ }
+
+ private void testClass(R8TestCompileResult compile, Class<?> testClass, boolean failure)
+ throws Exception {
+ R8TestRunResult run =
+ compile
+ .inspectDiagnosticMessages(
+ m -> {
+ for (Class<?> declaredClass : testClass.getDeclaredClasses()) {
+ if (declaredClass.isEnum()) {
+ if (failure) {
+ assertEnumIsBoxed(declaredClass, testClass.getSimpleName(), m);
+ } else {
+ assertEnumIsUnboxed(declaredClass, testClass.getSimpleName(), m);
+ }
+ }
+ }
+ })
+ .run(parameters.getRuntime(), testClass)
+ .assertSuccess();
+ assertLines2By2Correct(run.getStdOut());
+ }
+
+ static class SuccessEmptyInterface {
+
+ public static void main(String[] args) {
+ System.out.println(EnumInterface.A.ordinal());
+ System.out.println(0);
+ }
+
+ @NeverClassInline
+ enum EnumInterface implements Itf {
+ A,
+ B,
+ C
+ }
+
+ @NeverMerge
+ interface Itf {}
+ }
+
+ static class SuccessUnusedDefaultMethodOverrideEnum {
+
+ public static void main(String[] args) {
+ System.out.println(EnumInterface.A.ordinal());
+ System.out.println(0);
+ }
+
+ @NeverClassInline
+ enum EnumInterface implements Itf {
+ A,
+ B,
+ C
+ }
+
+ @NeverMerge
+ interface Itf {
+ @NeverInline
+ default int ordinal() {
+ return System.currentTimeMillis() > 0 ? 3 : -3;
+ }
+ }
+ }
+
+ static class SuccessUnusedDefaultMethodOverride {
+
+ public static void main(String[] args) {
+ System.out.println(EnumInterface.A.method());
+ System.out.println(5);
+ }
+
+ @NeverClassInline
+ enum EnumInterface implements Itf {
+ A,
+ B,
+ C;
+
+ @Override
+ @NeverInline
+ public int method() {
+ return System.currentTimeMillis() > 0 ? 5 : -5;
+ }
+ }
+
+ @NeverMerge
+ interface Itf {
+ @NeverInline
+ default int method() {
+ return System.currentTimeMillis() > 0 ? 3 : -3;
+ }
+ }
+ }
+
+ static class SuccessUnusedDefaultMethod {
+
+ public static void main(String[] args) {
+ System.out.println(EnumInterface.A.ordinal());
+ System.out.println(0);
+ }
+
+ @NeverClassInline
+ enum EnumInterface implements Itf {
+ A,
+ B,
+ C
+ }
+
+ @NeverMerge
+ interface Itf {
+ @NeverInline
+ default int method() {
+ return System.currentTimeMillis() > 0 ? 3 : -3;
+ }
+ }
+ }
+
+ static class SuccessAbstractMethod {
+
+ public static void main(String[] args) {
+ System.out.println(EnumInterface.A.method());
+ System.out.println(5);
+ }
+
+ @NeverClassInline
+ enum EnumInterface implements Itf {
+ A,
+ B,
+ C;
+
+ @Override
+ @NeverInline
+ public int method() {
+ return System.currentTimeMillis() > 0 ? 5 : -5;
+ }
+ }
+
+ @NeverMerge
+ interface Itf {
+ int method();
+ }
+ }
+
+ static class FailureDefaultMethodUsed {
+
+ public static void main(String[] args) {
+ System.out.println(EnumInterface.A.method());
+ System.out.println(3);
+ }
+
+ @NeverClassInline
+ enum EnumInterface implements Itf {
+ A,
+ B,
+ C
+ }
+
+ @NeverMerge
+ interface Itf {
+ @NeverInline
+ default int method() {
+ return System.currentTimeMillis() > 0 ? 3 : -3;
+ }
+ }
+ }
+
+ static class FailureUsedAsInterface {
+
+ public static void main(String[] args) {
+ print(EnumInterface.A);
+ System.out.println(5);
+ }
+
+ @NeverInline
+ public static void print(Itf itf) {
+ System.out.println(itf.method());
+ }
+
+ @NeverClassInline
+ enum EnumInterface implements Itf {
+ A,
+ B,
+ C;
+
+ @Override
+ @NeverInline
+ public int method() {
+ return System.currentTimeMillis() > 0 ? 5 : -5;
+ }
+ }
+
+ @NeverMerge
+ interface Itf {
+ @NeverInline
+ default int method() {
+ return System.currentTimeMillis() > 0 ? 3 : -3;
+ }
+ }
+ }
+}
