Avoid "immutable" apis for JNodes, preventing optimizer errors

The immutable APIs for JNodes apparently were never immutable to begin with, and have been watered down since then so that it isn't safe to actually treat them as being immutable. This confusion prevented multiple case expressions from being optimized correctly. Fixes #10005
gwtproject · Oct 16, 2024 · c0729be · c0729be
1 parent 32d07c8
commit c0729be
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 6 deletions.
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JCaseStatement.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JCaseStatement.java
@@ -16,6 +16,7 @@
 package com.google.gwt.dev.jjs.ast;
 
 import com.google.gwt.dev.jjs.SourceInfo;
+import com.google.gwt.thirdparty.guava.common.collect.Lists;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -31,7 +32,7 @@ public class JCaseStatement extends JStatement {
 
   public JCaseStatement(SourceInfo info, JExpression expr) {
     super(info);
-    this.exprs = Collections.singletonList(expr);
+    this.exprs = Lists.newArrayList(expr);
   }
 
   public JCaseStatement(SourceInfo info, Collection<JExpression> exprs) {
@@ -69,7 +70,7 @@ public JBinaryOperation convertToCompareExpression(JExpression value) {
   @Override
   public void traverse(JVisitor visitor, Context ctx) {
     if (visitor.visit(this, ctx)) {
-      exprs = Collections.unmodifiableList(visitor.acceptImmutable(exprs));
+      exprs = visitor.acceptWithInsertRemoveImmutable(exprs);
     }
     visitor.endVisit(this, ctx);
   }

diff --git a/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java17Test.java b/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java17Test.java
@@ -447,17 +447,15 @@ public static final int which(HasSwitchMethod whichSwitch) {
         return switch(whichSwitch) {
           case A -> 1;
           case RED -> 2;
-          case SUNDAY -> 3;
-          case JANUARY -> 4;
+          case SUNDAY, JANUARY -> 4;
           case ZERO -> 5;
         };
       }
       public static final int pick(HasSwitchMethod whichSwitch) {
         return 2 * switch(whichSwitch) {
           case A -> 1;
           case RED -> 2;
-          case SUNDAY -> 3;
-          case JANUARY -> 4;
+          case SUNDAY, JANUARY -> 4;
           case ZERO -> 5;
         };
       }
@@ -467,4 +465,17 @@ public static final int pick(HasSwitchMethod whichSwitch) {
     assertEquals(2, HasSwitchMethod.which(uninlinedValue));
     assertEquals(4, HasSwitchMethod.pick(uninlinedValue));
   }
+
+  private static final String ONE = "1";
+  private static final String TWO = "2";
+  private static final String FOUR = "4";
+
+  public void testInlinedStringConstantsInCase() {
+    int value = switch(Math.random() > 2 ? "2" : "4") {
+      case ONE, TWO -> 2;
+      case FOUR -> 4;
+      default -> 0;
+    };
+    assertEquals(4, value);
+  }
 }
diff --git a/user/test/com/google/gwt/dev/jjs/test/Java17Test.java b/user/test/com/google/gwt/dev/jjs/test/Java17Test.java
@@ -118,6 +118,10 @@ public void testSwitchExprInlining() {
     assertFalse(isGwtSourceLevel17());
   }
 
+  public void testInlinedStringConstantsInCase() {
+    assertFalse(isGwtSourceLevel17());
+  }
+
   private boolean isGwtSourceLevel17() {
     return JUnitShell.getCompilerOptions().getSourceLevel().compareTo(SourceLevel.JAVA17) >= 0;
   }