From c0729be6bd8a6312aee7aa65cb353bee7777cbc7 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 16 Oct 2024 10:38:27 -0500 Subject: [PATCH] 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 --- .../gwt/dev/jjs/ast/JCaseStatement.java | 5 +++-- .../google/gwt/dev/jjs/test/Java17Test.java | 19 +++++++++++++++---- .../google/gwt/dev/jjs/test/Java17Test.java | 4 ++++ 3 files changed, 22 insertions(+), 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 index 083e9e62ec..8b7e96034a 100644 --- 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 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 index 0a0e83dc18..375899a5ba 100644 --- 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,8 +447,7 @@ 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; }; } @@ -456,8 +455,7 @@ 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 index fe4658868e..39e0253397 100644 --- 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; }