commit 6781c669736037ce7b2e970a0da2e3c3bfcfadae
author Christoffer Quist Adamsen <christofferqa@google.com> Fri Nov 27 14:46:20 2020 +0100
committer Christoffer Quist Adamsen <christofferqa@google.com> Fri Nov 27 14:15:27 2020 +0000
tree 7e08ea29bb873e798b052593d5e12e10666c4387
parent 8c9359bf6102931f3d8eeafa7204a89e5c11f822

Fix side effect model for StringBuilder.<init>(CharSequence)

Change-Id: Idbc492d7b0aab4813d9e9aefafa814f52b45db33

src/main/java/com/android/tools/r8/graph/DexItemFactory.java

diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index 8f46be4..a93000e 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -1628,9 +1628,10 @@
     public boolean constructorInvokeIsSideEffectFree(InvokeMethod invoke) {
       DexMethod invokedMethod = invoke.getInvokedMethod();
       if (invokedMethod == charSequenceConstructor) {
-        // NullPointerException - if seq is null.
-        Value seqValue = invoke.inValues().get(1);
-        return !seqValue.getType().isNullable();
+        // Performs callbacks on the given CharSequence, which may have side effects.
+        TypeElement charSequenceType = invoke.getArgument(1).getType();
+        return charSequenceType.isClassType()
+            && charSequenceType.asClassType().getClassType() == stringType;
       }
 
       if (invokedMethod == defaultConstructor) {

src/test/java/com/android/tools/r8/ToolHelper.java

diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 04155b9..367a8f3 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -271,10 +271,18 @@
         return compareTo(other) > 0;
       }
 
+      public boolean isNewerThanOrEqual(Version other) {
+        return compareTo(other) >= 0;
+      }
+
       public boolean isAtLeast(Version other) {
         return compareTo(other) >= 0;
       }
 
+      public boolean isOlderThan(Version other) {
+        return compareTo(other) < 0;
+      }
+
       public boolean isOlderThanOrEqual(Version other) {
         return compareTo(other) <= 0;
       }

src/test/java/com/android/tools/r8/ir/optimize/string/UnusedStringBuilderFromCharSequenceWithAppendObjectTest.java

diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/UnusedStringBuilderFromCharSequenceWithAppendObjectTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/UnusedStringBuilderFromCharSequenceWithAppendObjectTest.java
index 46dfe04..8f1f58b 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/UnusedStringBuilderFromCharSequenceWithAppendObjectTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/UnusedStringBuilderFromCharSequenceWithAppendObjectTest.java
@@ -7,6 +7,7 @@
 import com.android.tools.r8.TestBase;
 import com.android.tools.r8.TestParameters;
 import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -33,13 +34,17 @@
         .setMinApi(parameters.getApiLevel())
         .compile()
         .run(parameters.getRuntime(), Main.class)
-        .assertSuccessWithEmptyOutput();
-    // TODO(christofferqa): Should succeed with output;
-    //  .assertSuccessWithOutputLines(
-    //      "CustomCharSequence.length()",
-    //      "CustomCharSequence.length()",
-    //      "CustomCharSequence.length()",
-    //      "CustomCharSequence.charAt(0)");
+        .assertSuccessWithOutputLinesIf(
+            parameters.isCfRuntime()
+                || parameters.getDexRuntimeVersion().isNewerThanOrEqual(Version.V7_0_0),
+            "CustomCharSequence.length()",
+            "CustomCharSequence.length()",
+            "CustomCharSequence.length()",
+            "CustomCharSequence.charAt(0)")
+        .assertSuccessWithOutputLinesIf(
+            parameters.isDexRuntime()
+                && parameters.getDexRuntimeVersion().isOlderThan(Version.V7_0_0),
+            "CustomCharSequence.toString()");
   }
 
   static class Main {
@@ -63,12 +68,18 @@
         throw new RuntimeException();
       }
       System.out.println("CustomCharSequence.charAt(0)");
-      return 0;
+      return 'A';
     }
 
     @Override
     public CharSequence subSequence(int i, int i1) {
       throw new RuntimeException();
     }
+
+    @Override
+    public String toString() {
+      System.out.println("CustomCharSequence.toString()");
+      return "A";
+    }
   }
 }