commit 021c3f09703d17fec5022a007125dbf3e06a285e
author Søren Gjesse <sgjesse@google.com> Tue Aug 17 14:22:02 2021 +0200
committer Søren Gjesse <sgjesse@google.com> Tue Aug 17 13:31:38 2021 +0000
tree 9e5488afad8d29fb037248734391451897d699db
parent 2853e9965fc75b17e5d63ea2791b8487526c5dfa

Fix optimization of Objects.requireNonNullElseGet

Fixes: 196945191
Change-Id: I1bd37f1b375d0e57b1d99ba3f37c32c0b9de2505

src/main/java/com/android/tools/r8/ir/optimize/library/ObjectsMethodOptimizer.java

diff --git a/src/main/java/com/android/tools/r8/ir/optimize/library/ObjectsMethodOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/library/ObjectsMethodOptimizer.java
index 08199da..4b7cffb 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/library/ObjectsMethodOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/library/ObjectsMethodOptimizer.java
@@ -183,16 +183,8 @@
         }
         instructionIterator.removeOrReplaceByDebugLocalRead();
       } else if (singleTarget.getReference() == objectsMethods.requireNonNullElseGet) {
-        // Optimize Objects.requireNonNullElseGet(null, supplier) into supplier.get().
-        if (invoke.hasOutValue()) {
-          invoke.outValue().replaceUsers(invoke.getLastArgument(), affectedValues);
-        }
-        instructionIterator.replaceCurrentInstruction(
-            InvokeVirtual.builder()
-                .setMethod(dexItemFactory.supplierMembers.get)
-                .setOutValue(invoke.outValue())
-                .setSingleArgument(invoke.getLastArgument())
-                .build());
+        // Don't optimize Objects.requireNonNullElseGet. The result of calling supplier.get() still
+        // needs a null-check, so two invokes will be needed.
       }
     }
   }

src/test/java/com/android/tools/r8/ir/optimize/library/ObjectsRequireNonNullElseGetTest.java

diff --git a/src/test/java/com/android/tools/r8/ir/optimize/library/ObjectsRequireNonNullElseGetTest.java b/src/test/java/com/android/tools/r8/ir/optimize/library/ObjectsRequireNonNullElseGetTest.java
index e0a4c6d..aadd5dd 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/library/ObjectsRequireNonNullElseGetTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/library/ObjectsRequireNonNullElseGetTest.java
@@ -41,7 +41,17 @@
   }
 
   @Test
-  public void test() throws Exception {
+  public void testD8() throws Exception {
+    testForD8(parameters.getBackend())
+        .addProgramClassFileData(getProgramClassFileData())
+        .setMinApi(parameters.getApiLevel())
+        .compile()
+        .run(parameters.getRuntime(), Main.class)
+        .assertSuccessWithOutputLines("Foo", "Bar", "Expected NPE");
+  }
+
+  @Test
+  public void testR8() throws Exception {
     testForR8(parameters.getBackend())
         .addProgramClassFileData(getProgramClassFileData())
         .addKeepMainRule(Main.class)
@@ -65,10 +75,12 @@
               assertThat(testNullArgumentMethodSubject, isPresent());
               assertThat(
                   testNullArgumentMethodSubject,
-                  not(invokesMethodWithName("requireNonNullElseGet")));
+                  parameters.getApiLevel().isGreaterThan(AndroidApiLevel.Q)
+                      ? invokesMethodWithName("requireNonNullElseGet")
+                      : not(invokesMethodWithName("requireNonNullElseGet")));
             })
         .run(parameters.getRuntime(), Main.class)
-        .assertSuccessWithOutputLines("Foo", "Bar");
+        .assertSuccessWithOutputLines("Foo", "Bar", "Expected NPE");
   }
 
   private byte[] getProgramClassFileData() throws IOException {
@@ -83,6 +95,12 @@
     public static void main(String[] args) {
       testNonNullArgument();
       testNullArgument();
+      try {
+        testNullArgumentAndNullSupplier();
+        System.out.println("Unexpected");
+      } catch (NullPointerException e) {
+        System.out.println("Expected NPE");
+      }
     }
 
     @NeverInline
@@ -94,6 +112,11 @@
     static void testNullArgument() {
       System.out.println(Mock.requireNonNullElseGet(null, () -> "Bar"));
     }
+
+    @NeverInline
+    static void testNullArgumentAndNullSupplier() {
+      System.out.println(Mock.requireNonNullElseGet(null, () -> null));
+    }
   }
 
   // References to this class are rewritten to java.util.Objects by transformation.