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: I1debfb14bbfa3b75cea399afc9a711570993aa77
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 5f5162a..59db0a2 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.49";
 
   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/EnumToStringLibTest.java b/src/test/java/com/android/tools/r8/enumunboxing/EnumToStringLibTest.java
new file mode 100644
index 0000000..83393bf
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/enumunboxing/EnumToStringLibTest.java
@@ -0,0 +1,218 @@
+// 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 com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.StringContains.containsString;
+
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.references.Reference;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.util.List;
+import org.junit.Assume;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class EnumToStringLibTest extends EnumUnboxingTestBase {
+
+  private final TestParameters parameters;
+  private final boolean enumValueOptimization;
+  private final EnumKeepRules enumKeepRules;
+  private final boolean enumUnboxing;
+
+  @Parameterized.Parameters(name = "{0} valueOpt: {1} keep: {2} unbox: {3}")
+  public static List<Object[]> data() {
+    return buildParameters(
+        getTestParameters().withDexRuntimes().withAllApiLevels().build(),
+        BooleanUtils.values(),
+        getAllEnumKeepRules(),
+        BooleanUtils.values());
+  }
+
+  public EnumToStringLibTest(
+      TestParameters parameters,
+      boolean enumValueOptimization,
+      EnumKeepRules enumKeepRules,
+      boolean enumUnboxing) {
+    this.parameters = parameters;
+    this.enumValueOptimization = enumValueOptimization;
+    this.enumKeepRules = enumKeepRules;
+    this.enumUnboxing = enumUnboxing;
+  }
+
+  @Test
+  public void testToStringLib() throws Exception {
+    Assume.assumeFalse(
+        "The test rely on valueOf, so only studio or snap keep rules are valid.",
+        enumKeepRules == EnumKeepRules.NONE);
+    // Compile the lib cf to cf.
+    R8TestCompileResult javaLibShrunk = compileLibrary();
+    assertEnumFieldsMinified(javaLibShrunk.inspector());
+    // Compile the program with the lib.
+    R8TestCompileResult compile =
+        testForR8(parameters.getBackend())
+            .addProgramClasses(AlwaysCorrectProgram.class, AlwaysCorrectProgram2.class)
+            .addProgramFiles(javaLibShrunk.writeToZip())
+            .addKeepMainRule(AlwaysCorrectProgram.class)
+            .addKeepMainRule(AlwaysCorrectProgram2.class)
+            .addKeepRules(enumKeepRules.getKeepRules())
+            .addOptionsModification(
+                options -> {
+                  options.enableEnumUnboxing = enumUnboxing;
+                  options.enableEnumValueOptimization = enumValueOptimization;
+                  options.enableEnumSwitchMapRemoval = enumValueOptimization;
+                  options.testing.enableEnumUnboxingDebugLogs = enumUnboxing;
+                })
+            .allowDiagnosticInfoMessages(enumUnboxing)
+            .setMinApi(parameters.getApiLevel())
+            .compile();
+    compile
+        .run(parameters.getRuntime(), AlwaysCorrectProgram.class)
+        .assertSuccessWithOutputLines("0", "1", "2", "0", "1", "2", "0", "1", "2");
+    if (!enumKeepRules.isSnap() && enumUnboxing) {
+      // TODO(b/160667929): Fix toString and enum unboxing.
+      compile
+          .run(parameters.getRuntime(), AlwaysCorrectProgram2.class)
+          .assertFailureWithErrorThatMatches(containsString("IllegalArgumentException"));
+      return;
+    }
+    compile
+        .run(parameters.getRuntime(), AlwaysCorrectProgram2.class)
+        .assertSuccessWithOutputLines("0", "1", "2", "0", "1", "2");
+  }
+
+  private void assertEnumFieldsMinified(CodeInspector codeInspector) throws Exception {
+    if (enumKeepRules.isSnap()) {
+      return;
+    }
+    ClassSubject clazz = codeInspector.clazz(ToStringLib.LibEnum.class);
+    assertThat(clazz, isPresent());
+    for (String fieldName : new String[] {"COFFEE", "BEAN", "SUGAR"}) {
+      assertThat(
+          codeInspector.field(ToStringLib.LibEnum.class.getField(fieldName)),
+          isPresentAndRenamed());
+    }
+  }
+
+  private R8TestCompileResult compileLibrary() throws Exception {
+    return testForR8(Backend.CF)
+        .addProgramClasses(ToStringLib.class, ToStringLib.LibEnum.class)
+        .addKeepRules(enumKeepRules.getKeepRules())
+        // TODO(b/160535629): Work-around on some optimizations relying on $VALUES name.
+        .addKeepRules(
+            "-keep enum "
+                + ToStringLib.LibEnum.class.getName()
+                + " { static "
+                + ToStringLib.LibEnum.class.getName()
+                + "[] $VALUES; }")
+        .addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
+        .addKeepMethodRules(
+            Reference.methodFromMethod(
+                ToStringLib.class.getDeclaredMethod("lookupByName", String.class)),
+            Reference.methodFromMethod(ToStringLib.class.getDeclaredMethod("getCoffee")),
+            Reference.methodFromMethod(ToStringLib.class.getDeclaredMethod("getBean")),
+            Reference.methodFromMethod(ToStringLib.class.getDeclaredMethod("getSugar")),
+            Reference.methodFromMethod(ToStringLib.class.getDeclaredMethod("directCoffee")),
+            Reference.methodFromMethod(ToStringLib.class.getDeclaredMethod("directBean")),
+            Reference.methodFromMethod(ToStringLib.class.getDeclaredMethod("directSugar")))
+        .addKeepClassRules(ToStringLib.LibEnum.class)
+        .allowDiagnosticMessages()
+        .setMinApi(parameters.getApiLevel())
+        .compile()
+        .inspectDiagnosticMessages(
+            msg ->
+                assertEnumIsBoxed(
+                    ToStringLib.LibEnum.class, ToStringLib.LibEnum.class.getSimpleName(), msg));
+  }
+
+  // This class emulates a library with the three public methods getEnumXXX.
+  public static class ToStringLib {
+
+    // We pick names here that we assume won't be picked by the minifier (i.e., not a,b,c).
+    public enum LibEnum {
+      COFFEE,
+      BEAN,
+      SUGAR;
+    }
+
+    // If there is a keep rule on LibEnum fields, then ToStringLib.lookupByName("COFFEE")
+    // should answer 0, else, the behavior of ToStringLib.lookupByName("COFFEE") is undefined.
+    // ToStringLib.lookupByName(LibEnum.COFFEE.toString()) should always answer 0, no matter
+    // what keep rules are on LibEnum.
+    public static int lookupByName(String key) {
+      if (key == null) {
+        return -1;
+      } else if (key.contains(LibEnum.COFFEE.name())) {
+        return LibEnum.COFFEE.ordinal();
+      } else if (key.contains(LibEnum.BEAN.name())) {
+        return LibEnum.BEAN.ordinal();
+      } else if (key.contains(LibEnum.SUGAR.name())) {
+        return LibEnum.SUGAR.ordinal();
+      } else {
+        return -2;
+      }
+    }
+
+    // The following method should always return 0, no matter what keep rules are on LibEnum.
+    public static int directCoffee() {
+      return LibEnum.valueOf(LibEnum.COFFEE.toString()).ordinal();
+    }
+
+    public static int directBean() {
+      return LibEnum.valueOf(LibEnum.BEAN.toString()).ordinal();
+    }
+
+    public static int directSugar() {
+      return LibEnum.valueOf(LibEnum.SUGAR.toString()).ordinal();
+    }
+
+    public static LibEnum getCoffee() {
+      return LibEnum.COFFEE;
+    }
+
+    public static LibEnum getBean() {
+      return LibEnum.BEAN;
+    }
+
+    public static LibEnum getSugar() {
+      return LibEnum.SUGAR;
+    }
+  }
+
+  // The next two classes emulate a program using the ToStringLib library.
+  public static class AlwaysCorrectProgram {
+
+    public static void main(String[] args) {
+      System.out.println(ToStringLib.directCoffee());
+      System.out.println(ToStringLib.directBean());
+      System.out.println(ToStringLib.directSugar());
+      System.out.println(ToStringLib.lookupByName(ToStringLib.getCoffee().toString()));
+      System.out.println(ToStringLib.lookupByName(ToStringLib.getBean().toString()));
+      System.out.println(ToStringLib.lookupByName(ToStringLib.getSugar().toString()));
+      System.out.println(ToStringLib.LibEnum.valueOf(ToStringLib.getCoffee().toString()).ordinal());
+      System.out.println(ToStringLib.LibEnum.valueOf(ToStringLib.getBean().toString()).ordinal());
+      System.out.println(ToStringLib.LibEnum.valueOf(ToStringLib.getSugar().toString()).ordinal());
+    }
+  }
+
+  public static class AlwaysCorrectProgram2 {
+
+    public static void main(String[] args) {
+      System.out.println(ToStringLib.lookupByName("COFFEE"));
+      System.out.println(ToStringLib.lookupByName("BEAN"));
+      System.out.println(ToStringLib.lookupByName("SUGAR"));
+      System.out.println(ToStringLib.LibEnum.valueOf("COFFEE").ordinal());
+      System.out.println(ToStringLib.LibEnum.valueOf("BEAN").ordinal());
+      System.out.println(ToStringLib.LibEnum.valueOf("SUGAR").ordinal());
+    }
+  }
+}
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");