Version 2.1.50
Reland Version 2.1.49
Cherry-pick: Fix enum name/toString issue
CL: https://r8-review.googlesource.com/c/r8/+/52465
Cherry-pick: Enum unboxing: Disable if missing static methods
CL: https://r8-review.googlesource.com/c/r8/+/52464
Bug: 160351050
Bug: 160535628
Change-Id: Iaa50db7b83e823875ef9566ee1b77151a3976432
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 5f5162a..e9f227a 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "2.1.48";
+ public static final String LABEL = "2.1.50";
private Version() {
}
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 5dc7874..059c49b 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
@@ -24,6 +24,7 @@
import com.android.tools.r8.graph.FieldResolutionResult;
import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.GraphLense.NestedGraphLense;
+import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.RewrittenPrototypeDescription;
import com.android.tools.r8.graph.RewrittenPrototypeDescription.ArgumentInfoCollection;
import com.android.tools.r8.graph.RewrittenPrototypeDescription.RewrittenTypeInfo;
@@ -160,7 +161,7 @@
analyzeCheckCast(instruction.asCheckCast(), eligibleEnums);
break;
case Opcodes.INVOKE_STATIC:
- analyzeInvokeStatic(instruction.asInvokeStatic(), eligibleEnums);
+ analyzeInvokeStatic(instruction.asInvokeStatic(), eligibleEnums, code.context());
break;
case Opcodes.STATIC_GET:
case Opcodes.INSTANCE_GET:
@@ -208,11 +209,17 @@
}
}
- private void analyzeInvokeStatic(InvokeStatic invokeStatic, Set<DexType> eligibleEnums) {
+ private void analyzeInvokeStatic(
+ InvokeStatic invokeStatic, Set<DexType> eligibleEnums, ProgramMethod context) {
DexMethod invokedMethod = invokeStatic.getInvokedMethod();
DexProgramClass enumClass = getEnumUnboxingCandidateOrNull(invokedMethod.holder);
if (enumClass != null) {
- eligibleEnums.add(enumClass.type);
+ DexEncodedMethod method = invokeStatic.lookupSingleTarget(appView, context);
+ if (method != null) {
+ eligibleEnums.add(enumClass.type);
+ } else {
+ markEnumAsUnboxable(Reason.INVALID_INVOKE, enumClass);
+ }
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java
index bac8a81..f9dcdf6 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java
@@ -75,6 +75,12 @@
boolean isOrdinalInvoke = invokedMethod == factory.enumMethods.ordinal;
boolean isNameInvoke = invokedMethod == factory.enumMethods.name;
boolean isToStringInvoke = invokedMethod == factory.enumMethods.toString;
+
+ // TODO(b/160667929): Re-enable name()/toString() optimizations.
+ if (!isOrdinalInvoke) {
+ continue;
+ }
+
if (!isOrdinalInvoke && !isNameInvoke && !isToStringInvoke) {
continue;
}
diff --git a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
index b262e4e..b9020eb 100644
--- a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
@@ -154,7 +154,7 @@
return addKeepMainRule(mainClass.getTypeName());
}
- public T addKeepMainRules(Class<?>[] mainClasses) {
+ public T addKeepMainRules(Class<?>... mainClasses) {
for (Class<?> mainClass : mainClasses) {
this.addKeepMainRule(mainClass);
}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingB160535628Test.java b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingB160535628Test.java
new file mode 100644
index 0000000..5ccd903
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingB160535628Test.java
@@ -0,0 +1,143 @@
+// 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 static org.hamcrest.core.StringContains.containsString;
+
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestDiagnosticMessages;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.BooleanUtils;
+import java.nio.file.Path;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class EnumUnboxingB160535628Test extends EnumUnboxingTestBase {
+
+ private final TestParameters parameters;
+ private final boolean missingStaticMethods;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withDexRuntimes().withAllApiLevels().build(), BooleanUtils.values());
+ }
+
+ public EnumUnboxingB160535628Test(TestParameters parameters, boolean missingStaticMethods) {
+ this.parameters = parameters;
+ this.missingStaticMethods = missingStaticMethods;
+ }
+
+ @Test
+ public void testCallToMissingStaticMethodInUnboxedEnum() throws Exception {
+ // Compile the lib cf to cf.
+ Path javaLibShrunk = compileLibrary();
+ // Compile the program with the lib.
+ // This should compile without error into code raising the correct NoSuchMethod errors.
+ R8TestCompileResult compile =
+ testForR8(parameters.getBackend())
+ .addProgramClasses(ProgramValueOf.class, ProgramStaticMethod.class)
+ .addProgramFiles(javaLibShrunk)
+ .addKeepMainRules(ProgramValueOf.class, ProgramStaticMethod.class)
+ .addOptionsModification(
+ options -> {
+ options.enableEnumUnboxing = true;
+ options.testing.enableEnumUnboxingDebugLogs = true;
+ })
+ .allowDiagnosticMessages()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspectDiagnosticMessages(
+ // The enums cannot be unboxed if static methods are missing,
+ // but they should be unboxed otherwise.
+ this::assertEnumUnboxedIfStaticMethodsPresent);
+ if (missingStaticMethods) {
+ compile
+ .run(parameters.getRuntime(), ProgramValueOf.class)
+ .assertFailureWithErrorThatMatches(containsString("NoSuchMethodError"))
+ .assertFailureWithErrorThatMatches(containsString("valueOf"));
+ compile
+ .run(parameters.getRuntime(), ProgramStaticMethod.class)
+ .assertFailureWithErrorThatMatches(containsString("NoSuchMethodError"))
+ .assertFailureWithErrorThatMatches(containsString("staticMethod"));
+ } else {
+ compile.run(parameters.getRuntime(), ProgramValueOf.class).assertSuccessWithOutputLines("0");
+ compile
+ .run(parameters.getRuntime(), ProgramStaticMethod.class)
+ .assertSuccessWithOutputLines("42");
+ }
+ }
+
+ private Path compileLibrary() throws Exception {
+ return testForR8(Backend.CF)
+ .addProgramClasses(Lib.class, Lib.LibEnumStaticMethod.class, Lib.LibEnum.class)
+ .addKeepRules("-keep enum * { <fields>; }")
+ .addKeepRules(missingStaticMethods ? "" : "-keep enum * { static <methods>; }")
+ .addOptionsModification(
+ options -> {
+ options.enableEnumUnboxing = true;
+ options.testing.enableEnumUnboxingDebugLogs = true;
+ })
+ .addKeepClassRules(Lib.LibEnumStaticMethod.class)
+ .allowDiagnosticMessages()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspectDiagnosticMessages(
+ msg -> {
+ assertEnumIsBoxed(
+ Lib.LibEnumStaticMethod.class,
+ Lib.LibEnumStaticMethod.class.getSimpleName(),
+ msg);
+ assertEnumIsBoxed(Lib.LibEnum.class, Lib.LibEnum.class.getSimpleName(), msg);
+ })
+ .writeToZip();
+ }
+
+ private void assertEnumUnboxedIfStaticMethodsPresent(TestDiagnosticMessages msg) {
+ if (missingStaticMethods) {
+ assertEnumIsBoxed(
+ Lib.LibEnumStaticMethod.class, Lib.LibEnumStaticMethod.class.getSimpleName(), msg);
+ assertEnumIsBoxed(Lib.LibEnum.class, Lib.LibEnum.class.getSimpleName(), msg);
+ } else {
+ assertEnumIsUnboxed(
+ Lib.LibEnumStaticMethod.class, Lib.LibEnumStaticMethod.class.getSimpleName(), msg);
+ assertEnumIsUnboxed(Lib.LibEnum.class, Lib.LibEnum.class.getSimpleName(), msg);
+ }
+ }
+
+ public static class Lib {
+
+ public enum LibEnumStaticMethod {
+ A,
+ B;
+
+ static int staticMethod() {
+ return 42;
+ }
+ }
+
+ public enum LibEnum {
+ A,
+ B
+ }
+ }
+
+ public static class ProgramValueOf {
+
+ public static void main(String[] args) {
+ System.out.println(Lib.LibEnumStaticMethod.valueOf(Lib.LibEnum.A.name()).ordinal());
+ }
+ }
+
+ public static class ProgramStaticMethod {
+
+ public static void main(String[] args) {
+ System.out.println(Lib.LibEnumStaticMethod.staticMethod());
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/rewrite/enums/EnumOptimizationTest.java b/src/test/java/com/android/tools/r8/rewrite/enums/EnumOptimizationTest.java
index ae4947a..1bf5f83 100644
--- a/src/test/java/com/android/tools/r8/rewrite/enums/EnumOptimizationTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/enums/EnumOptimizationTest.java
@@ -20,6 +20,7 @@
import com.android.tools.r8.utils.codeinspector.MethodSubject;
import java.util.List;
import java.util.Objects;
+import org.junit.Assume;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -74,13 +75,14 @@
if (enableOptimization) {
assertOrdinalReplacedWithConst(clazz.uniqueMethodWithName("simple"), 1);
assertOrdinalReplacedWithConst(clazz.uniqueMethodWithName("local"), 1);
+ // TODO(b/160667929): Re-enable name()/toString() optimizations.
// String concatenation optimization is enabled for DEX output.
// Even replaced ordinal is concatenated (and gone).
- if (parameters.isDexRuntime()) {
- assertOrdinalReplacedAndGone(clazz.uniqueMethodWithName("multipleUsages"));
- } else {
- assertOrdinalReplacedWithConst(clazz.uniqueMethodWithName("multipleUsages"), 1);
- }
+ // if (parameters.isDexRuntime()) {
+ // assertOrdinalReplacedAndGone(clazz.uniqueMethodWithName("multipleUsages"));
+ // } else {
+ assertOrdinalReplacedWithConst(clazz.uniqueMethodWithName("multipleUsages"), 1);
+ // }
assertOrdinalReplacedWithConst(clazz.uniqueMethodWithName("inlined"), 1);
assertOrdinalReplacedWithConst(clazz.uniqueMethodWithName("inSwitch"), 11);
assertOrdinalReplacedWithConst(clazz.uniqueMethodWithName("differentTypeStaticField"), 1);
@@ -121,6 +123,7 @@
assertTrue(clazz.isPresent());
if (enableOptimization) {
+ Assume.assumeTrue("TODO(b/160667929): Re-enable name()/toString() optimizations.", false);
assertNameReplacedWithConst(clazz.uniqueMethodWithName("simple"), "TWO");
assertNameReplacedWithConst(clazz.uniqueMethodWithName("local"), "TWO");
// String concatenation optimization is enabled for DEX output.
@@ -171,6 +174,7 @@
assertToStringWasNotReplaced(clazz.uniqueMethodWithName("valueWithoutToString"));
if (enableOptimization) {
+ Assume.assumeTrue("TODO(b/160667929): Re-enable name()/toString() optimizations.", false);
assertToStringReplacedWithConst(clazz.uniqueMethodWithName("noToString"), "TWO");
assertToStringReplacedWithConst(clazz.uniqueMethodWithName("local"), "TWO");
assertToStringReplacedWithConst(clazz.uniqueMethodWithName("multipleUsages"), "TWO");
