From 9b2de0dadb3ffe36c35423a90138340bd54c235c Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 8 Jun 2026 13:09:17 -0500 Subject: [PATCH 01/42] Raise Java language level to 17, update errorprone --- .github/workflows/full-check.yml | 2 +- .github/workflows/quick-check.yml | 2 +- common.ant.xml | 7 ++++--- dev/core/test/com/google/gwt/dev/GwtVersionTest.java | 1 + .../com/google/gwt/dev/javac/CompilationStateTestBase.java | 4 ++-- dev/core/test/com/google/gwt/dev/util/StringKeyTest.java | 2 ++ .../org/apache/commons/collections/AbstractTestObject.java | 1 + .../com/google/gwt/resources/rg/CssResourceGenerator.java | 3 ++- .../com/google/gwt/resources/rg/GssResourceGenerator.java | 3 ++- user/src/com/google/gwt/uibinder/rebind/BundleWriter.java | 3 ++- user/test/com/google/gwt/core/interop/JsExportTest.java | 1 + user/test/com/google/gwt/dev/jjs/test/JsoTest.java | 1 + user/test/com/google/gwt/dev/jjs/test/NativeLongTest.java | 1 + .../gwt/emultest/java/lang/CompilerConstantStringTest.java | 1 + .../test/com/google/gwt/emultest/java/util/TestObject.java | 1 + .../com/google/gwt/emultest/java8/lang/StringTest.java | 1 + user/test/com/google/gwt/junit/client/GWTTestCaseTest.java | 4 ++++ .../com/google/gwt/storage/client/MapInterfaceTest.java | 2 ++ 18 files changed, 30 insertions(+), 10 deletions(-) diff --git a/.github/workflows/full-check.yml b/.github/workflows/full-check.yml index a20411227d4..b3fa4b4ea51 100644 --- a/.github/workflows/full-check.yml +++ b/.github/workflows/full-check.yml @@ -24,7 +24,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - java-version: [ '11', '17', '21', '22' ] + java-version: [ '17', '21', '25' ] steps: - name: Checkout GWT itself into one directory uses: actions/checkout@v6 diff --git a/.github/workflows/quick-check.yml b/.github/workflows/quick-check.yml index e87cc11e973..b75fc198dcc 100644 --- a/.github/workflows/quick-check.yml +++ b/.github/workflows/quick-check.yml @@ -9,7 +9,7 @@ jobs: strategy: fail-fast: false matrix: - java-version: ['11', '17', '21', '22'] + java-version: ['17', '21', '25'] steps: - name: Checkout GWT itself into one directory uses: actions/checkout@v6 diff --git a/common.ant.xml b/common.ant.xml index d5e03123423..d2f2d06f981 100755 --- a/common.ant.xml +++ b/common.ant.xml @@ -54,13 +54,13 @@ - + - - + + @@ -187,6 +187,7 @@ + diff --git a/dev/core/test/com/google/gwt/dev/GwtVersionTest.java b/dev/core/test/com/google/gwt/dev/GwtVersionTest.java index 283cbfd27a5..e6c40eedcb9 100644 --- a/dev/core/test/com/google/gwt/dev/GwtVersionTest.java +++ b/dev/core/test/com/google/gwt/dev/GwtVersionTest.java @@ -78,6 +78,7 @@ public void testCompareTo() { /** * Test that GwtVersion.compareTo produced expected results. */ + @SuppressWarnings("SelfAssertion") public void testEquals() { GwtVersion v1 = new GwtVersion("0.0.0"); assertEquals(v1, v1); diff --git a/dev/core/test/com/google/gwt/dev/javac/CompilationStateTestBase.java b/dev/core/test/com/google/gwt/dev/javac/CompilationStateTestBase.java index 3a0834ac226..6a5cc93e840 100644 --- a/dev/core/test/com/google/gwt/dev/javac/CompilationStateTestBase.java +++ b/dev/core/test/com/google/gwt/dev/javac/CompilationStateTestBase.java @@ -170,12 +170,12 @@ protected void validateCompilationState(String... generatedTypeNames) { // Not always true due to caching! A source unit for FOO can b // identical to the generated FOO and already be cached. // assertTrue(unit.isGenerated()); - assertNotNull(generatedTypes.remove(className)); + assertTrue(generatedTypes.remove(className)); } else { String partialPath = className.replace('.', '/') + ".java"; assertTrue(resourcePathNames.contains(partialPath)); // TODO: Validate the source file matches the resource. - assertNotNull(resourcePathNames.remove(partialPath)); + assertTrue(resourcePathNames.remove(partialPath)); } } // The mutable sets should be empty now. diff --git a/dev/core/test/com/google/gwt/dev/util/StringKeyTest.java b/dev/core/test/com/google/gwt/dev/util/StringKeyTest.java index 10e4fb42361..91312064583 100644 --- a/dev/core/test/com/google/gwt/dev/util/StringKeyTest.java +++ b/dev/core/test/com/google/gwt/dev/util/StringKeyTest.java @@ -48,6 +48,7 @@ private static > void assertLT(T a, T b) { assertTrue(a.compareTo(b) < 0); } + @SuppressWarnings("SelfAssertion") public void test() { StringKey a = new KeyA("hello"); StringKey b = new KeyB("world"); @@ -64,6 +65,7 @@ public void test() { assertGT(b, a); } + @SuppressWarnings("SelfAssertion") public void testNull() { StringKey a = new KeyA(null); StringKey b = new KeyB(null); diff --git a/dev/core/test/org/apache/commons/collections/AbstractTestObject.java b/dev/core/test/org/apache/commons/collections/AbstractTestObject.java index 4bc2e948a9d..640b66b7d13 100644 --- a/dev/core/test/org/apache/commons/collections/AbstractTestObject.java +++ b/dev/core/test/org/apache/commons/collections/AbstractTestObject.java @@ -105,6 +105,7 @@ public boolean isEqualsCheckable() { } //----------------------------------------------------------------------- + @SuppressWarnings("SelfAssertion") public void testObjectEqualsSelf() { Object obj = makeObject(); assertEquals("A Object should equal itself", obj, obj); diff --git a/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java b/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java index e8376f22443..4ed93e7859f 100644 --- a/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java +++ b/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java @@ -90,6 +90,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.SortedSet; import java.util.TreeMap; @@ -931,7 +932,7 @@ private void initReplacements(TreeLogger logger, ResourceContext context, * be be worth the effort to simplify this. */ - if (context.getCachedData(KEY_HAS_CACHED_DATA, Boolean.class) != Boolean.TRUE) { + if (!Objects.equals(context.getCachedData(KEY_HAS_CACHED_DATA, Boolean.class), Boolean.TRUE)) { ConfigurationProperty prop; TreeSet reservedPrefixes = new TreeSet(); diff --git a/user/src/com/google/gwt/resources/rg/GssResourceGenerator.java b/user/src/com/google/gwt/resources/rg/GssResourceGenerator.java index bfd787ff8d1..d5a88c43a94 100644 --- a/user/src/com/google/gwt/resources/rg/GssResourceGenerator.java +++ b/user/src/com/google/gwt/resources/rg/GssResourceGenerator.java @@ -140,6 +140,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; @@ -695,7 +696,7 @@ public void init(TreeLogger logger, ResourceContext context) throws UnableToComp @SuppressWarnings("unchecked") private void initReplacement(ResourceContext context) { - if (context.getCachedData(KEY_HAS_CACHED_DATA, Boolean.class) != Boolean.TRUE) { + if (!Objects.equals(context.getCachedData(KEY_HAS_CACHED_DATA, Boolean.class), Boolean.TRUE)) { context.putCachedData(KEY_SHARED_METHODS, new IdentityHashMap()); context.putCachedData(KEY_BY_CLASS_AND_METHOD, new IdentityHashMap 0L); diff --git a/user/test/com/google/gwt/emultest/java/lang/CompilerConstantStringTest.java b/user/test/com/google/gwt/emultest/java/lang/CompilerConstantStringTest.java index 80763a0b4f4..43fa0e9f572 100644 --- a/user/test/com/google/gwt/emultest/java/lang/CompilerConstantStringTest.java +++ b/user/test/com/google/gwt/emultest/java/lang/CompilerConstantStringTest.java @@ -38,6 +38,7 @@ public void testCharAt() { assertEquals('b', "abc".charAt(1)); } + @SuppressWarnings("SelfAssertion") public void testConcat() { assertEquals("abcdef", "abc" + "def"); assertEquals("abcdef", "abc".concat("def")); diff --git a/user/test/com/google/gwt/emultest/java/util/TestObject.java b/user/test/com/google/gwt/emultest/java/util/TestObject.java index 63ce3e42a59..60d5dfa7677 100644 --- a/user/test/com/google/gwt/emultest/java/util/TestObject.java +++ b/user/test/com/google/gwt/emultest/java/util/TestObject.java @@ -47,6 +47,7 @@ public int getCompatibilityVersion() { /** Return a new, empty {@link Object} to used for testing. */ public abstract Object makeObject(); + @SuppressWarnings("SelfAssertion") public void testObjectEqualsSelf() { Object obj = makeObject(); assertEquals("A Object should equal itself", obj, obj); diff --git a/user/test/com/google/gwt/emultest/java8/lang/StringTest.java b/user/test/com/google/gwt/emultest/java8/lang/StringTest.java index 67dace4100d..11f4fd757cb 100644 --- a/user/test/com/google/gwt/emultest/java8/lang/StringTest.java +++ b/user/test/com/google/gwt/emultest/java8/lang/StringTest.java @@ -29,6 +29,7 @@ public String getModuleName() { return "com.google.gwt.emultest.EmulSuite"; } + @SuppressWarnings("StringJoin") public void testJoin() { assertEquals("", String.join("", "")); assertEquals("", String.join(",", "")); diff --git a/user/test/com/google/gwt/junit/client/GWTTestCaseTest.java b/user/test/com/google/gwt/junit/client/GWTTestCaseTest.java index 44681379679..ec527522a4f 100644 --- a/user/test/com/google/gwt/junit/client/GWTTestCaseTest.java +++ b/user/test/com/google/gwt/junit/client/GWTTestCaseTest.java @@ -117,6 +117,7 @@ public void testThrowsNonSerializableException_hasMetaData() throws Throwable { } + @SuppressWarnings("SelfAssertion") public void testAssertEqualsDouble() { assertEquals(0.0, 0.0, 0.0); assertEquals(1.1, 1.1, 0.0); @@ -134,6 +135,7 @@ public void testAssertEqualsDouble() { assertNotEquals(-2.0, -1.0, 0.1); } + @SuppressWarnings("SelfAssertion") public void testAssertEqualsFloat() { assertEquals(0.0f, 0.0f, 0.0f); assertEquals(1.1f, 1.1f, 0.0f); @@ -151,6 +153,7 @@ public void testAssertEqualsFloat() { assertNotEquals(-2.0f, -1.0f, 0.1f); } + @SuppressWarnings("SelfAssertion") public void testAssertEqualsIntInt() { assertEquals(5, 5); assertEquals("msg", 5, 5); @@ -166,6 +169,7 @@ public void testAssertEqualsIntIntFailWithMessage() { assertEquals("msg", 5, 4); } + @SuppressWarnings("SelfAssertion") public void testAssertEqualsObjectObject() { assertEquals(obj1, obj1Equal); assertEquals("msg", obj1, obj1); diff --git a/user/test/com/google/gwt/storage/client/MapInterfaceTest.java b/user/test/com/google/gwt/storage/client/MapInterfaceTest.java index bb866bb92a0..3fdd4d1c3f5 100644 --- a/user/test/com/google/gwt/storage/client/MapInterfaceTest.java +++ b/user/test/com/google/gwt/storage/client/MapInterfaceTest.java @@ -782,6 +782,7 @@ public void testEntrySetSetValueSameValue() { assertInvariants(map); } + @SuppressWarnings("SelfAssertion") public void testEqualsForEqualMap() { final Map map; try { @@ -841,6 +842,7 @@ public void disabled_testEqualsForSmallerMap() { assertFalse(map.equals(smallerMap)); } + @SuppressWarnings("SelfAssertion") public void testEqualsForEmptyMap() { final Map map; try { From 1458a5d2216e1b28c91e2bfdafa7f26667e25c3f Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 8 Jun 2026 13:30:21 -0500 Subject: [PATCH 02/42] temporarily use tools branch --- .github/workflows/full-check.yml | 3 ++- .github/workflows/quick-check.yml | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/full-check.yml b/.github/workflows/full-check.yml index b3fa4b4ea51..58282a9e90e 100644 --- a/.github/workflows/full-check.yml +++ b/.github/workflows/full-check.yml @@ -33,7 +33,8 @@ jobs: - name: Checkout GWT tools into a sibling directory uses: actions/checkout@v6 with: - repository: 'gwtproject/tools' + repository: 'Vertispan/tools' + ref: 'errorprone-2.42' path: 'tools' - name: Set up JDK ${{ matrix.java-version }} # GWT requires Java 11+ to build diff --git a/.github/workflows/quick-check.yml b/.github/workflows/quick-check.yml index b75fc198dcc..52add851053 100644 --- a/.github/workflows/quick-check.yml +++ b/.github/workflows/quick-check.yml @@ -20,7 +20,8 @@ jobs: - name: Checkout GWT tools into a sibling directory uses: actions/checkout@v6 with: - repository: 'gwtproject/tools' + repository: 'Vertispan/tools' + ref: 'errorprone-2.42' path: 'tools' - name: Set up JDK ${{ matrix.java-version }} # GWT presently requires Java 11+ to build From 6bf72333d1717f931f02f22e7fba477b816d4ac0 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 8 Jun 2026 14:22:59 -0500 Subject: [PATCH 03/42] new apis to triage --- .../doctool/custom/missing/charseq.properties | 1 + .../doctool/custom/missing/comparator.properties | 2 ++ .../google/doctool/custom/missing/math.properties | 14 ++++++++++++++ .../doctool/custom/missing/oflazy.properties | 2 ++ .../doctool/custom/missing/reader.properties | 3 +++ .../google/doctool/custom/missing/rootn.properties | 2 ++ .../doctool/custom/missing/string.properties | 3 +++ 7 files changed, 27 insertions(+) create mode 100644 build_tools/doctool/src/com/google/doctool/custom/missing/charseq.properties create mode 100644 build_tools/doctool/src/com/google/doctool/custom/missing/comparator.properties create mode 100644 build_tools/doctool/src/com/google/doctool/custom/missing/math.properties create mode 100644 build_tools/doctool/src/com/google/doctool/custom/missing/oflazy.properties create mode 100644 build_tools/doctool/src/com/google/doctool/custom/missing/reader.properties create mode 100644 build_tools/doctool/src/com/google/doctool/custom/missing/rootn.properties create mode 100644 build_tools/doctool/src/com/google/doctool/custom/missing/string.properties diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/charseq.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/charseq.properties new file mode 100644 index 00000000000..40e1a619f0d --- /dev/null +++ b/build_tools/doctool/src/com/google/doctool/custom/missing/charseq.properties @@ -0,0 +1 @@ +members=java.lang.CharSequence#getChars(int, int, char[], int) diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/comparator.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/comparator.properties new file mode 100644 index 00000000000..d2e5d4aea0c --- /dev/null +++ b/build_tools/doctool/src/com/google/doctool/custom/missing/comparator.properties @@ -0,0 +1,2 @@ +members=java.util.Comparator#min(Object, Object)\ +java.util.Comparator#max(Object, Object) diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/math.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/math.properties new file mode 100644 index 00000000000..3db156e1ac5 --- /dev/null +++ b/build_tools/doctool/src/com/google/doctool/custom/missing/math.properties @@ -0,0 +1,14 @@ +members=java.lang.Math#unsignedMultiplyExact(int, int)\ +java.lang.Math#unsignedMultiplyExact(long, long)\ +java.lang.Math#unsignedMultiplyExact(long, int)\ +java.lang.Math#powExact(int, int)\ +java.lang.Math#powExact(long, int)\ +java.lang.Math#unsignedPowExact(long, int)\ +java.lang.Math#unsignedPowExact(int, int)\ +java.lang.StrictMath#unsignedMultiplyExact(long, int)\ +java.lang.StrictMath#unsignedMultiplyExact(long, long)\ +java.lang.StrictMath#unsignedMultiplyExact(int, int)\ +java.lang.StrictMath#powExact(long, int)\ +java.lang.StrictMath#powExact(int, int)\ +java.lang.StrictMath#unsignedPowExact(long, int)\ +java.lang.StrictMath#unsignedPowExact(int, int) diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/oflazy.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/oflazy.properties new file mode 100644 index 00000000000..ac65eb59b51 --- /dev/null +++ b/build_tools/doctool/src/com/google/doctool/custom/missing/oflazy.properties @@ -0,0 +1,2 @@ +members=java.util.List#ofLazy(int, IntFunction)\ +java.util.Map#ofLazy(Set, Function) diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/reader.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/reader.properties new file mode 100644 index 00000000000..2988c0c918f --- /dev/null +++ b/build_tools/doctool/src/com/google/doctool/custom/missing/reader.properties @@ -0,0 +1,3 @@ +members=java.io.Reader#of(CharSequence)\ +java.io.Reader#readAllLines()\ +java.io.Reader#readAllAsString() diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/rootn.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/rootn.properties new file mode 100644 index 00000000000..f644db36295 --- /dev/null +++ b/build_tools/doctool/src/com/google/doctool/custom/missing/rootn.properties @@ -0,0 +1,2 @@ +members=java.math.BigInteger#rootn(int)\ +java.math.BigInteger#rootnAndRemainder(int) diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/string.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/string.properties new file mode 100644 index 00000000000..106d8693e27 --- /dev/null +++ b/build_tools/doctool/src/com/google/doctool/custom/missing/string.properties @@ -0,0 +1,3 @@ +members=java.lang.String#equalsFoldCase(String)\ +java.lang.String#compareToFoldCase(String)\ +java.lang.String#UNICODE_CASEFOLD_ORDER From 52acf3ae039689c9d34bdd8dbaae9a6356bdd7af Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 8 Jun 2026 21:36:28 -0500 Subject: [PATCH 04/42] Rename missing emul files --- .../src/com/google/doctool/custom/missing/gh10337.properties | 3 +++ .../missing/{comparator.properties => gh10338.properties} | 0 .../custom/missing/{charseq.properties => gh10339.properties} | 0 .../custom/missing/{math.properties => gh10340.properties} | 0 .../custom/missing/{reader.properties => gh10341.properties} | 0 .../custom/missing/{rootn.properties => gh10342.properties} | 0 .../custom/missing/{string.properties => gh10343.properties} | 0 .../src/com/google/doctool/custom/missing/oflazy.properties | 2 -- 8 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 build_tools/doctool/src/com/google/doctool/custom/missing/gh10337.properties rename build_tools/doctool/src/com/google/doctool/custom/missing/{comparator.properties => gh10338.properties} (100%) rename build_tools/doctool/src/com/google/doctool/custom/missing/{charseq.properties => gh10339.properties} (100%) rename build_tools/doctool/src/com/google/doctool/custom/missing/{math.properties => gh10340.properties} (100%) rename build_tools/doctool/src/com/google/doctool/custom/missing/{reader.properties => gh10341.properties} (100%) rename build_tools/doctool/src/com/google/doctool/custom/missing/{rootn.properties => gh10342.properties} (100%) rename build_tools/doctool/src/com/google/doctool/custom/missing/{string.properties => gh10343.properties} (100%) delete mode 100644 build_tools/doctool/src/com/google/doctool/custom/missing/oflazy.properties diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/gh10337.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/gh10337.properties new file mode 100644 index 00000000000..303cec80cdd --- /dev/null +++ b/build_tools/doctool/src/com/google/doctool/custom/missing/gh10337.properties @@ -0,0 +1,3 @@ +description=This is the wrong issue, but these methods are still preview, and we need an issue. +members=java.util.List#ofLazy(int, IntFunction)\ +java.util.Map#ofLazy(Set, Function) diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/comparator.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/gh10338.properties similarity index 100% rename from build_tools/doctool/src/com/google/doctool/custom/missing/comparator.properties rename to build_tools/doctool/src/com/google/doctool/custom/missing/gh10338.properties diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/charseq.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/gh10339.properties similarity index 100% rename from build_tools/doctool/src/com/google/doctool/custom/missing/charseq.properties rename to build_tools/doctool/src/com/google/doctool/custom/missing/gh10339.properties diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/math.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/gh10340.properties similarity index 100% rename from build_tools/doctool/src/com/google/doctool/custom/missing/math.properties rename to build_tools/doctool/src/com/google/doctool/custom/missing/gh10340.properties diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/reader.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/gh10341.properties similarity index 100% rename from build_tools/doctool/src/com/google/doctool/custom/missing/reader.properties rename to build_tools/doctool/src/com/google/doctool/custom/missing/gh10341.properties diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/rootn.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/gh10342.properties similarity index 100% rename from build_tools/doctool/src/com/google/doctool/custom/missing/rootn.properties rename to build_tools/doctool/src/com/google/doctool/custom/missing/gh10342.properties diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/string.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/gh10343.properties similarity index 100% rename from build_tools/doctool/src/com/google/doctool/custom/missing/string.properties rename to build_tools/doctool/src/com/google/doctool/custom/missing/gh10343.properties diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/oflazy.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/oflazy.properties deleted file mode 100644 index ac65eb59b51..00000000000 --- a/build_tools/doctool/src/com/google/doctool/custom/missing/oflazy.properties +++ /dev/null @@ -1,2 +0,0 @@ -members=java.util.List#ofLazy(int, IntFunction)\ -java.util.Map#ofLazy(Set, Function) From d2cea9af71d4d571eaa0bdc3546d6c6fe25dd393 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 9 Jun 2026 07:12:57 -0500 Subject: [PATCH 05/42] Revert "temporarily use tools branch" This reverts commit 1458a5d2216e1b28c91e2bfdafa7f26667e25c3f. --- .github/workflows/full-check.yml | 3 +-- .github/workflows/quick-check.yml | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/full-check.yml b/.github/workflows/full-check.yml index 58282a9e90e..b3fa4b4ea51 100644 --- a/.github/workflows/full-check.yml +++ b/.github/workflows/full-check.yml @@ -33,8 +33,7 @@ jobs: - name: Checkout GWT tools into a sibling directory uses: actions/checkout@v6 with: - repository: 'Vertispan/tools' - ref: 'errorprone-2.42' + repository: 'gwtproject/tools' path: 'tools' - name: Set up JDK ${{ matrix.java-version }} # GWT requires Java 11+ to build diff --git a/.github/workflows/quick-check.yml b/.github/workflows/quick-check.yml index 52add851053..b75fc198dcc 100644 --- a/.github/workflows/quick-check.yml +++ b/.github/workflows/quick-check.yml @@ -20,8 +20,7 @@ jobs: - name: Checkout GWT tools into a sibling directory uses: actions/checkout@v6 with: - repository: 'Vertispan/tools' - ref: 'errorprone-2.42' + repository: 'gwtproject/tools' path: 'tools' - name: Set up JDK ${{ matrix.java-version }} # GWT presently requires Java 11+ to build From 54e5fd77c936f97e607af7720c3c272be7c4db85 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 9 Jun 2026 07:21:05 -0500 Subject: [PATCH 06/42] Update notes about what versions are required --- .github/workflows/full-check.yml | 4 ++-- .github/workflows/quick-check.yml | 2 +- README.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/full-check.yml b/.github/workflows/full-check.yml index b3fa4b4ea51..1613b06e5e7 100644 --- a/.github/workflows/full-check.yml +++ b/.github/workflows/full-check.yml @@ -1,4 +1,4 @@ -# Run all tests and builds all aspects of GWT using Java 11, 17, and 21. Runs +# Run all tests and builds all aspects of GWT using Java 17, 21, and 25. Runs # nightly (plus or minus timzeones) on the main branch, and will also run right # away on a push to a release branch. Release zips are uploaded as part of the # build, though maven snapshots are not yet deployed. @@ -36,7 +36,7 @@ jobs: repository: 'gwtproject/tools' path: 'tools' - name: Set up JDK ${{ matrix.java-version }} - # GWT requires Java 11+ to build + # GWT requires Java 17+ to build uses: actions/setup-java@v5 with: java-version: ${{ matrix.java-version }} diff --git a/.github/workflows/quick-check.yml b/.github/workflows/quick-check.yml index b75fc198dcc..b3162d95a6f 100644 --- a/.github/workflows/quick-check.yml +++ b/.github/workflows/quick-check.yml @@ -23,7 +23,7 @@ jobs: repository: 'gwtproject/tools' path: 'tools' - name: Set up JDK ${{ matrix.java-version }} - # GWT presently requires Java 11+ to build + # GWT presently requires Java 17+ to build uses: actions/setup-java@v5 with: java-version: ${{ matrix.java-version }} diff --git a/README.md b/README.md index 0438255c30d..60bbe497a06 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ ### Building the GWT SDK: - - In order to build GWT, `java` and `ant` are required in your system. + - In order to build GWT, `java` (minimum version 17) and `ant` are required in your system. - You need the [GWT tools repository](https://github.com/gwtproject/tools/) checked out and up-to-date. By default it is expected to be found at `../tools`. From f27bcb72ff47ef0ed49a750feaa119ffc9dc9716 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 9 Jun 2026 21:29:39 -0500 Subject: [PATCH 07/42] Hardcode preview methods, support java 26 with an EP workaround --- .github/workflows/full-check.yml | 4 ++-- .github/workflows/quick-check.yml | 2 +- .../com/google/doctool/custom/JavaEmulSummaryDoclet.java | 6 ++++-- .../com/google/doctool/custom/missing/gh10337.properties | 3 --- .../gwt/client/impl/RequestFactoryEditorDelegate.java | 2 +- .../requestfactory/shared/impl/BaseProxyCategory.java | 2 +- 6 files changed, 9 insertions(+), 10 deletions(-) delete mode 100644 build_tools/doctool/src/com/google/doctool/custom/missing/gh10337.properties diff --git a/.github/workflows/full-check.yml b/.github/workflows/full-check.yml index 1613b06e5e7..d7ece3f571e 100644 --- a/.github/workflows/full-check.yml +++ b/.github/workflows/full-check.yml @@ -1,4 +1,4 @@ -# Run all tests and builds all aspects of GWT using Java 17, 21, and 25. Runs +# Run all tests and builds all aspects of GWT using Java 17, 21, 25, and 26. Runs # nightly (plus or minus timzeones) on the main branch, and will also run right # away on a push to a release branch. Release zips are uploaded as part of the # build, though maven snapshots are not yet deployed. @@ -24,7 +24,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - java-version: [ '17', '21', '25' ] + java-version: [ '17', '21', '25', '26' ] steps: - name: Checkout GWT itself into one directory uses: actions/checkout@v6 diff --git a/.github/workflows/quick-check.yml b/.github/workflows/quick-check.yml index b3162d95a6f..3c4664f3548 100644 --- a/.github/workflows/quick-check.yml +++ b/.github/workflows/quick-check.yml @@ -9,7 +9,7 @@ jobs: strategy: fail-fast: false matrix: - java-version: ['17', '21', '25'] + java-version: ['17', '21', '25', '26'] steps: - name: Checkout GWT itself into one directory uses: actions/checkout@v6 diff --git a/build_tools/doctool/src/com/google/doctool/custom/JavaEmulSummaryDoclet.java b/build_tools/doctool/src/com/google/doctool/custom/JavaEmulSummaryDoclet.java index 4226e3eda61..46d55574db6 100644 --- a/build_tools/doctool/src/com/google/doctool/custom/JavaEmulSummaryDoclet.java +++ b/build_tools/doctool/src/com/google/doctool/custom/JavaEmulSummaryDoclet.java @@ -71,7 +71,7 @@ public class JavaEmulSummaryDoclet implements Doclet { private static final String OPT_TRIAGE_FILE = "-triageFile"; private static final String OPT_MISSING_PROPERTIES_DIR = "-missingProperties"; // should be aligned with the latest version that's tested by CI - private static final int MAX_JRE_VERSION = 22; + private static final int MAX_JRE_VERSION = 26; // Lowest version for which missing method detection is reliable. // The tool can run with older versions, but will ignore missing methods. private static final int MIN_JRE_VERSION = 17; @@ -83,7 +83,9 @@ public class JavaEmulSummaryDoclet implements Doclet { "java.lang.Character#isJavaLetterOrDigit(char)", "java.lang.String#getBytes(int, int, byte[], int)", "java.lang.Class#isUnnamedClass()", // exists in 21 as a preview, removed - "java.lang.Enum#finalize()" // finalizers are ignored by GWT and to be removed from JVM + "java.lang.Enum#finalize()", // finalizers are ignored by GWT and to be removed from JVM + "java.util.Map#ofLazy(Set, Function)", // exists in 26 as a preview only + "java.util.List#ofLazy(int, IntFunction)" // exists in 26 as a preview only ); private static final List EXCLUDED_CLASSES = List.of("JsException"); diff --git a/build_tools/doctool/src/com/google/doctool/custom/missing/gh10337.properties b/build_tools/doctool/src/com/google/doctool/custom/missing/gh10337.properties deleted file mode 100644 index 303cec80cdd..00000000000 --- a/build_tools/doctool/src/com/google/doctool/custom/missing/gh10337.properties +++ /dev/null @@ -1,3 +0,0 @@ -description=This is the wrong issue, but these methods are still preview, and we need an issue. -members=java.util.List#ofLazy(int, IntFunction)\ -java.util.Map#ofLazy(Set, Function) diff --git a/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/RequestFactoryEditorDelegate.java b/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/RequestFactoryEditorDelegate.java index 4cbcf92a290..6f231cac1e6 100644 --- a/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/RequestFactoryEditorDelegate.java +++ b/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/RequestFactoryEditorDelegate.java @@ -138,7 +138,7 @@ protected T ensureMutable(T object) { return object; } if (object instanceof BaseProxy) { - @SuppressWarnings("unchecked") + // noinspection unchecked T toReturn = (T) request.edit((BaseProxy) object); return toReturn; } diff --git a/user/src/com/google/web/bindery/requestfactory/shared/impl/BaseProxyCategory.java b/user/src/com/google/web/bindery/requestfactory/shared/impl/BaseProxyCategory.java index 69172b9d5fc..dab43a370b8 100644 --- a/user/src/com/google/web/bindery/requestfactory/shared/impl/BaseProxyCategory.java +++ b/user/src/com/google/web/bindery/requestfactory/shared/impl/BaseProxyCategory.java @@ -49,7 +49,7 @@ public static T __intercept(AutoBean bean, T returnValue) { * delegate to the edit() method for wiring up the context. */ if (returnValue instanceof BaseProxy) { - @SuppressWarnings("unchecked") + // noinspection unchecked T toReturn = (T) context.editProxy((BaseProxy) returnValue); return toReturn; } From 67d99e32d19ba4525fcf92054b38591f5249ecc7 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 10 Jun 2026 07:46:33 -0500 Subject: [PATCH 08/42] Bump asm to support Java 27 --- dev/build.xml | 18 +++++++++--------- maven/poms/gwt/pom-template.xml | 2 +- user/build.xml | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/dev/build.xml b/dev/build.xml index 39da43bbe12..200f61a4732 100755 --- a/dev/build.xml +++ b/dev/build.xml @@ -64,9 +64,9 @@ - - - + + + @@ -119,9 +119,9 @@ - - - + + + @@ -217,9 +217,9 @@ - - - + + + diff --git a/maven/poms/gwt/pom-template.xml b/maven/poms/gwt/pom-template.xml index fc88ac50339..895d29cb684 100644 --- a/maven/poms/gwt/pom-template.xml +++ b/maven/poms/gwt/pom-template.xml @@ -19,7 +19,7 @@ 9.4.58.v20250814 - 9.6 + 9.10.1 diff --git a/user/build.xml b/user/build.xml index c8aa603c0ec..3b748634322 100755 --- a/user/build.xml +++ b/user/build.xml @@ -61,8 +61,8 @@ - - + + Date: Wed, 10 Jun 2026 07:47:52 -0500 Subject: [PATCH 09/42] Temp tools branch --- .github/workflows/full-check.yml | 3 ++- .github/workflows/quick-check.yml | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/full-check.yml b/.github/workflows/full-check.yml index d7ece3f571e..ad09fcc1e8e 100644 --- a/.github/workflows/full-check.yml +++ b/.github/workflows/full-check.yml @@ -33,7 +33,8 @@ jobs: - name: Checkout GWT tools into a sibling directory uses: actions/checkout@v6 with: - repository: 'gwtproject/tools' + repository: 'Vertispan/tools' + ref: 'asm-9.10.1' path: 'tools' - name: Set up JDK ${{ matrix.java-version }} # GWT requires Java 17+ to build diff --git a/.github/workflows/quick-check.yml b/.github/workflows/quick-check.yml index 3c4664f3548..7ca3c6460e2 100644 --- a/.github/workflows/quick-check.yml +++ b/.github/workflows/quick-check.yml @@ -20,7 +20,8 @@ jobs: - name: Checkout GWT tools into a sibling directory uses: actions/checkout@v6 with: - repository: 'gwtproject/tools' + repository: 'Vertispan/tools' + ref: 'asm-9.10.1' path: 'tools' - name: Set up JDK ${{ matrix.java-version }} # GWT presently requires Java 17+ to build From 470e637365a9b8f014ecccf4c4cf41a676c2ac70 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 10 Jun 2026 08:32:22 -0500 Subject: [PATCH 10/42] Another errorprone bug --- .../gwt/user/cellview/client/AbstractHasDataTestBase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user/test/com/google/gwt/user/cellview/client/AbstractHasDataTestBase.java b/user/test/com/google/gwt/user/cellview/client/AbstractHasDataTestBase.java index 232646efbbf..5fcd0f7d30f 100644 --- a/user/test/com/google/gwt/user/cellview/client/AbstractHasDataTestBase.java +++ b/user/test/com/google/gwt/user/cellview/client/AbstractHasDataTestBase.java @@ -309,7 +309,7 @@ protected void populateData(AbstractHasData view) { /** * A mutable number, so that we can modify the value from within anonymous classes. */ - protected static final class Counter { + static final class Counter { private int number = 0; protected void increment() { From 14f91f1d5108da3181ef5a0a37ba7b3b2b91f9e6 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 10 Jun 2026 10:27:27 -0500 Subject: [PATCH 11/42] Remove Java 26 again, seems to break DOMSuite --- .github/workflows/full-check.yml | 2 +- .github/workflows/quick-check.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/full-check.yml b/.github/workflows/full-check.yml index ad09fcc1e8e..df4bca9c8ef 100644 --- a/.github/workflows/full-check.yml +++ b/.github/workflows/full-check.yml @@ -24,7 +24,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - java-version: [ '17', '21', '25', '26' ] + java-version: [ '17', '21', '25' ] steps: - name: Checkout GWT itself into one directory uses: actions/checkout@v6 diff --git a/.github/workflows/quick-check.yml b/.github/workflows/quick-check.yml index 7ca3c6460e2..a216bd18a9d 100644 --- a/.github/workflows/quick-check.yml +++ b/.github/workflows/quick-check.yml @@ -9,7 +9,7 @@ jobs: strategy: fail-fast: false matrix: - java-version: ['17', '21', '25', '26'] + java-version: ['17', '21', '25'] steps: - name: Checkout GWT itself into one directory uses: actions/checkout@v6 From b8e9479d880017e3797ddbf813b23a13af46cbd6 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 24 Nov 2025 20:22:57 -0600 Subject: [PATCH 12/42] quick wip --- .../gwt/dev/jjs/JavaToJavaScriptCompiler.java | 3 + .../google/gwt/dev/jjs/ast/JIfStatement.java | 37 +++-- .../dev/jjs/impl/CatchBlockNormalizer.java | 2 +- .../google/gwt/dev/jjs/impl/ExpandBlocks.java | 153 ++++++++++++++++++ .../dev/jjs/impl/GenerateJavaScriptAST.java | 13 +- .../gwt/dev/jjs/impl/GwtAstBuilder.java | 6 +- .../jjs/impl/ImplementRecordComponents.java | 5 +- .../google/gwt/dev/jjs/impl/Simplifier.java | 12 +- .../jjs/impl/ToStringGenerationVisitor.java | 14 +- 9 files changed, 212 insertions(+), 33 deletions(-) create mode 100644 dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java diff --git a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java index d11ceb05479..505937d67ea 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java +++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java @@ -76,6 +76,7 @@ import com.google.gwt.dev.jjs.impl.EnumNameObfuscator; import com.google.gwt.dev.jjs.impl.EnumOrdinalizer; import com.google.gwt.dev.jjs.impl.EqualityNormalizer; +import com.google.gwt.dev.jjs.impl.ExpandBlocks; import com.google.gwt.dev.jjs.impl.Finalizer; import com.google.gwt.dev.jjs.impl.FixAssignmentsToUnboxOrCast; import com.google.gwt.dev.jjs.impl.FullOptimizerContext; @@ -369,6 +370,8 @@ private PermutationResult compilePermutation(Permutation permutation, UnifiedAst // TODO(rluble): eventually move to normizeSemantics. CompileTimeConstantsReplacer.exec(jprogram); + ExpandBlocks.exec(jprogram); + // TODO(stalcup): move to after normalize. // (3) Optimize the resolved Java AST optimizeJava(); diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java index 53eb6fec998..f4f0d9fa972 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java @@ -22,18 +22,18 @@ */ public class JIfStatement extends JStatement { - private JStatement elseStmt; + private JBlock elseStmt; private JExpression ifExpr; - private JStatement thenStmt; + private JBlock thenStmt; - public JIfStatement(SourceInfo info, JExpression ifExpr, JStatement thenStmt, JStatement elseStmt) { + public JIfStatement(SourceInfo info, JExpression ifExpr, JBlock thenStmt, JBlock elseStmt) { super(info); this.ifExpr = ifExpr; - this.thenStmt = thenStmt; - this.elseStmt = elseStmt; + this.thenStmt = thenStmt == null ? new JBlock(info) : thenStmt; + this.elseStmt = elseStmt == null ? new JBlock(info) : elseStmt; } - public JStatement getElseStmt() { + public JBlock getElseStmt() { return elseStmt; } @@ -41,7 +41,7 @@ public JExpression getIfExpr() { return ifExpr; } - public JStatement getThenStmt() { + public JBlock getThenStmt() { return thenStmt; } @@ -49,16 +49,27 @@ public JStatement getThenStmt() { public void traverse(JVisitor visitor, Context ctx) { if (visitor.visit(this, ctx)) { ifExpr = visitor.accept(ifExpr); - if (thenStmt != null) { - thenStmt = visitor.accept(thenStmt, true); - } - if (elseStmt != null) { - elseStmt = visitor.accept(elseStmt, true); - } +// if (thenStmt != null) { + thenStmt = ensureBlock(visitor.accept(thenStmt, true)); +// } +// if (elseStmt != null) { + elseStmt = ensureBlock(visitor.accept(elseStmt, true)); +// } } visitor.endVisit(this, ctx); } + private JBlock ensureBlock(JStatement statement) { + if (statement == null) { + return new JBlock(getSourceInfo()); + } + if (statement instanceof JBlock) { + return (JBlock) statement; + } + + return new JBlock(statement.getSourceInfo(), statement); + } + @Override public boolean unconditionalControlBreak() { boolean thenBreaks = thenStmt != null && thenStmt.unconditionalControlBreak(); diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java index c0c067c7993..1083f0bb580 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java @@ -119,7 +119,7 @@ public void endVisit(JTryStatement x, Context ctx) { new JDeclarationStatement(catchInfo, arg, exceptionVariable.makeRef(catchInfo)); block.addStmt(0, declaration); // nest the previous as an else for me - cur = new JIfStatement(catchInfo, ifTest, block, cur); + cur = new JIfStatement(catchInfo, ifTest, block, new JBlock(cur.getSourceInfo(), cur)); } newCatchBlock.addStmt(cur); diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java new file mode 100644 index 00000000000..d1034ad87dd --- /dev/null +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java @@ -0,0 +1,153 @@ +package com.google.gwt.dev.jjs.impl; + +import com.google.gwt.dev.jjs.ast.Context; +import com.google.gwt.dev.jjs.ast.JBlock; +import com.google.gwt.dev.jjs.ast.JDoStatement; +import com.google.gwt.dev.jjs.ast.JForStatement; +import com.google.gwt.dev.jjs.ast.JIfStatement; +import com.google.gwt.dev.jjs.ast.JMethod; +import com.google.gwt.dev.jjs.ast.JMethodBody; +import com.google.gwt.dev.jjs.ast.JModVisitor; +import com.google.gwt.dev.jjs.ast.JProgram; +import com.google.gwt.dev.jjs.ast.JStatement; +import com.google.gwt.dev.jjs.ast.JSwitchStatement; +import com.google.gwt.dev.jjs.ast.JTryStatement; +import com.google.gwt.dev.jjs.ast.JWhileStatement; +import com.google.gwt.dev.js.ast.JsBlock; + +import java.util.Set; +import java.util.Stack; + +/** + * Examines blocks where child blocks (if/else, try/catch) use control flow statements + * (return/throw/break/continue) which make it impossible for that block to flow back to the parent. + * In these cases, the other block can be expanded to include the rest of the parent block. + * + * Examples: + * if (condition) { statements; return; } rest of parent block; + * or + * if (condition) { statements; return; } else { more; } rest of parent block; + * -> if (condition) { statements; } else { [more;] rest of parent block; } + * + * try { statements; return; } catch (e) { handler(e); } rest of parent block; + * -> try { statements; } catch (e) { handler(e); rest of parent block; } + * + * These transformations have two simple benefits today: + *
    + *
  • They make it easier for the DeadCodeElimination/Simplifier to identify if/else blocks that return simple expressions and can be rewritten to a single conditional
    • + *
  • They reduce complexity slightly for passes like DuplicateClinitRemover, so that it doesn't need to merge the scopes it considers to continue
    • + *
+ * + * For cases where an else block is added, we will also add a JS pass to remove it, transforming + * if (condition) { ... return; } else { ... } -> if (condition) { ... return; } ... + * so that there is no net increase in size. This will could also save a few bytes when the else + * already existed, and can now be removed. + * + * Only needs to be run once as part of normalization, since MethodInliner will never move more than + * an expression (either in a JExpressionStatement or JReturnStatement). + * + * + * if (a) { return; } if (b) {return;} rest; + * --> if (a) { return; } else { if (b) { return; } else { rest; } } + * + * if (a) { for (..) { if (b) { break; } rest1; } } rest2; + * --> if (a) { for (..) { if (b) { break; } else { rest1; } } } else { rest2; } + * + */ +public class ExpandBlocks { + /** + * Normalize the program's nested blocks. + */ + public static void exec(JProgram program) { + new JModVisitor() { + + private JBlock acceptor; + @Override + public void endVisit(JIfStatement x, Context ctx) { + if (x.getThenStmt().unconditionalControlBreak()) { + if (x.getElseStmt().unconditionalControlBreak()) { + // Both branches break, nothing to do - parent block is unreachable code after if/else + acceptor = null; + } else { + // else can handle rest of parent block + acceptor = x.getElseStmt(); + } + } else { + if (x.getThenStmt().unconditionalControlBreak()) { + // else can handle rest of parent block + acceptor = x.getElseStmt(); + } else { + // neither branch breaks, cannot optimize + acceptor = null; + } + } + } + + @Override + public void endVisit(JTryStatement x, Context ctx) { + if (x.getFinallyBlock() != null) { + // Cannot optimize if there is a finally block, as finally executes after catch and before remainder + acceptor = null; + return; + } + + if (!x.getTryBlock().unconditionalControlBreak()) { + // Try must unconditionally break to optimize + acceptor = null; + return; + } + + if (x.getCatchClauses().size() != 1) { + // Only can optimize exactly one catch, and unconditional return try + acceptor = null; + return; + } + acceptor = x.getCatchClauses().get(0).getBlock(); + } + + @Override + public void endVisit(JSwitchStatement x, Context ctx) { + //TODO handle switch case (though probably rarely worth it) + acceptor = null; + } + + @Override + public void endVisit(JForStatement x, Context ctx) { + // Interrupts moving statements to the last acceptor block + acceptor = null; + } + + @Override + public void endVisit(JWhileStatement x, Context ctx) { + // Interrupts moving statements to the last acceptor block + acceptor = null; + } + + @Override + public void endVisit(JDoStatement x, Context ctx) { + // Interrupts moving statements to the last acceptor block + acceptor = null; + } + + + @Override + public boolean visit(JStatement x, Context ctx) { + if (acceptor != null) { + // If there is a block ready to accept this, any statement should be moved. + // Any visit() override must call super before it does its own work, + // to ensure that it is relocated + ctx.removeMe(); + acceptor.addStmt(x); + // Moved the item itself, continue to see if it needs to adopt later statements + } + return super.visit(x, ctx); + } + + @Override + public void endVisit(JMethodBody x, Context ctx) { + // Clear any acceptor at end of method body + acceptor = null; + } + }.accept(program); + } +} diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java index 27edf1f9aa0..16339961daf 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java @@ -574,7 +574,7 @@ public JsExpression transformBinaryOperation(JBinaryOperation binaryOperation) { } @Override - public JsStatement transformBlock(JBlock block) { + public JsBlock transformBlock(JBlock block) { JsBlock jsBlock = new JsBlock(block.getSourceInfo()); List stmts = jsBlock.getStatements(); @@ -753,12 +753,19 @@ public JsNode transformIfStatement(JIfStatement ifStatement) { result.setIfExpr(transform(ifStatement.getIfExpr())); result.setThenStmt(jsEmptyIfNull(ifStatement.getSourceInfo(), - transform(ifStatement.getThenStmt()))); - result.setElseStmt(transform(ifStatement.getElseStmt())); + unwrapSingleStatement(transformBlock(ifStatement.getThenStmt())))); + result.setElseStmt(unwrapSingleStatement(transformBlock(ifStatement.getElseStmt()))); return result; } + private JsStatement unwrapSingleStatement(JsBlock block) { + if (block.getStatements().size() == 1) { + return block.getStatements().get(0); + } + return block; + } + @Override public JsLabel transformLabel(JLabel label) { return new JsLabel(label.getSourceInfo(), names.get(label)); diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java index eb8b806cc27..3505c524e5f 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java @@ -1093,8 +1093,8 @@ public JExpression apply(JStatement statement) { public void endVisit(IfStatement x, BlockScope scope) { try { SourceInfo info = makeSourceInfo(x); - JStatement elseStatement = pop(x.elseStatement); - JStatement thenStatement = pop(x.thenStatement); + JBlock elseStatement = popBlock(info, x.elseStatement); + JBlock thenStatement = popBlock(info, x.thenStatement); JExpression condition = pop(x.condition); push(new JIfStatement(info, condition, thenStatement, elseStatement)); } catch (Throwable e) { @@ -2454,7 +2454,7 @@ private JBlock normalizeTryWithResources(SourceInfo info, TryStatement x, JBlock JExpression exceptionNotNull = new JBinaryOperation(info, JPrimitiveType.BOOLEAN, JBinaryOperator.NEQ, exceptionVar.makeRef(info), JNullLiteral.INSTANCE); finallyBlock.addStmt(new JIfStatement(info, exceptionNotNull, - new JThrowStatement(info, exceptionVar.makeRef(info)), null)); + new JBlock(info, new JThrowStatement(info, exceptionVar.makeRef(info))), null)); // Stitch all together into a inner try block outerTryBlock.addStmt(new JTryStatement(info, tryBlock, catchClauses, diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementRecordComponents.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementRecordComponents.java index 27b451d72f8..2d24a329a2e 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementRecordComponents.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementRecordComponents.java @@ -18,6 +18,7 @@ import com.google.gwt.dev.jjs.SourceInfo; import com.google.gwt.dev.jjs.ast.JBinaryOperation; import com.google.gwt.dev.jjs.ast.JBinaryOperator; +import com.google.gwt.dev.jjs.ast.JBlock; import com.google.gwt.dev.jjs.ast.JBooleanLiteral; import com.google.gwt.dev.jjs.ast.JClassLiteral; import com.google.gwt.dev.jjs.ast.JClassType; @@ -163,7 +164,7 @@ private void implementEquals(JRecordType type, JMethod method, SourceInfo info) new JThisRef(info, type), otherParam.createRef(info)); body.getBlock().addStmt(new JIfStatement(info, eq, - JBooleanLiteral.TRUE.makeReturnStatement(), null)); + new JBlock(info, JBooleanLiteral.TRUE.makeReturnStatement()), null)); // other == null JBinaryOperation nonNullCheck = @@ -181,7 +182,7 @@ private void implementEquals(JRecordType type, JMethod method, SourceInfo info) // if (other == null || MyRecordType.class != other.getClass()) return false; body.getBlock().addStmt(new JIfStatement(info, nullAndTypeCheck, - JBooleanLiteral.FALSE.makeReturnStatement(), null)); + new JBlock(info, JBooleanLiteral.FALSE.makeReturnStatement()), null)); // Create a local to assign to and compare each component JLocal typedOther = JProgram.createLocal(info, "other", type, true, body); diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java b/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java index e054081b931..32888182c27 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java @@ -240,8 +240,8 @@ public static JExpression simplifyConditional(JConditional expression) { public static JStatement simplifyIfStatement(JIfStatement ifStatement, JType methodReturnType) { SourceInfo info = ifStatement.getSourceInfo(); JExpression conditionExpression = ifStatement.getIfExpr(); - JStatement thenStmt = ifStatement.getThenStmt(); - JStatement elseStmt = ifStatement.getElseStmt(); + JBlock thenStmt = ifStatement.getThenStmt(); + JBlock elseStmt = ifStatement.getElseStmt(); if (conditionExpression instanceof JMultiExpression) { // if(a,b,c) d else e -> {a; b; if(c) d else e; } JMultiExpression condMulti = (JMultiExpression) conditionExpression; @@ -279,10 +279,14 @@ public static JStatement simplifyIfStatement(JIfStatement ifStatement, JType met JExpression negationArugment = Simplifier.maybeGetNegatedExpressionArgument(conditionExpression); if (negationArugment != null) { + + // Force sub-parts to blocks, otherwise we break else-if chains. // TODO: this goes away when we normalize the Java AST properly. - thenStmt = ensureBlock(thenStmt); - elseStmt = ensureBlock(elseStmt); + + +// thenStmt = ensureBlock(thenStmt); +// elseStmt = ensureBlock(elseStmt); return simplifyIfStatement( new JIfStatement(info, negationArugment, elseStmt, thenStmt), methodReturnType); } diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java index f42eded388c..594090e4344 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java @@ -552,16 +552,16 @@ public boolean visit(JIfStatement x, Context ctx) { needSemi = true; } print(CHARS_ELSE); - boolean elseIf = x.getElseStmt() instanceof JIfStatement; - if (!elseIf) { +// boolean elseIf = x.getElseStmt() instanceof JIfStatement; +// if (!elseIf) { nestedStatementPush(x.getElseStmt()); - } else { - space(); - } +// } else { +// space(); +// } accept(x.getElseStmt()); - if (!elseIf) { +// if (!elseIf) { nestedStatementPop(x.getElseStmt()); - } +// } } return false; From 790478d183964982dd2248ba7730e9cf80614aef Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 25 Nov 2025 20:34:40 -0600 Subject: [PATCH 13/42] Vaguely functional implementation, except for switches --- .../google/gwt/dev/jjs/impl/ExpandBlocks.java | 179 ++++++++++++++---- 1 file changed, 146 insertions(+), 33 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java index d1034ad87dd..4eaafa62aec 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java @@ -1,3 +1,18 @@ +/* + * Copyright 2025 GWT Project Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ package com.google.gwt.dev.jjs.impl; import com.google.gwt.dev.jjs.ast.Context; @@ -5,7 +20,6 @@ import com.google.gwt.dev.jjs.ast.JDoStatement; import com.google.gwt.dev.jjs.ast.JForStatement; import com.google.gwt.dev.jjs.ast.JIfStatement; -import com.google.gwt.dev.jjs.ast.JMethod; import com.google.gwt.dev.jjs.ast.JMethodBody; import com.google.gwt.dev.jjs.ast.JModVisitor; import com.google.gwt.dev.jjs.ast.JProgram; @@ -13,25 +27,23 @@ import com.google.gwt.dev.jjs.ast.JSwitchStatement; import com.google.gwt.dev.jjs.ast.JTryStatement; import com.google.gwt.dev.jjs.ast.JWhileStatement; -import com.google.gwt.dev.js.ast.JsBlock; -import java.util.Set; import java.util.Stack; /** * Examines blocks where child blocks (if/else, try/catch) use control flow statements * (return/throw/break/continue) which make it impossible for that block to flow back to the parent. * In these cases, the other block can be expanded to include the rest of the parent block. - * + *

* Examples: * if (condition) { statements; return; } rest of parent block; * or * if (condition) { statements; return; } else { more; } rest of parent block; * -> if (condition) { statements; } else { [more;] rest of parent block; } - * + *

* try { statements; return; } catch (e) { handler(e); } rest of parent block; * -> try { statements; } catch (e) { handler(e); rest of parent block; } - * + *

* These transformations have two simple benefits today: *

    *
  • They make it easier for the DeadCodeElimination/Simplifier to identify if/else blocks that return simple expressions and can be rewritten to a single conditional
    • @@ -42,14 +54,14 @@ * if (condition) { ... return; } else { ... } -> if (condition) { ... return; } ... * so that there is no net increase in size. This will could also save a few bytes when the else * already existed, and can now be removed. - * + *

    * Only needs to be run once as part of normalization, since MethodInliner will never move more than * an expression (either in a JExpressionStatement or JReturnStatement). - * + *

    * * if (a) { return; } if (b) {return;} rest; * --> if (a) { return; } else { if (b) { return; } else { rest; } } - * + *

    * if (a) { for (..) { if (b) { break; } rest1; } } rest2; * --> if (a) { for (..) { if (b) { break; } else { rest1; } } } else { rest2; } * @@ -61,92 +73,193 @@ public class ExpandBlocks { public static void exec(JProgram program) { new JModVisitor() { - private JBlock acceptor; + /** + * The location in which statements can be moved to is represented by a stack - it isn't quite the usual use of a stack in a visitor, as it doesn't represent + * the path from the root, but instead the most recent block that can accept statements. Nulls are added to the stack as well to indicate that there is currently + * no position to move statements to. Testing to see if a statement can be moved then is just checking if the stack is non-empty and the top is non-null. + *

    + * If we hit a statement that is immovable, we need to pop the latest item off the stack (if present and non-null). + */ + private final Stack acceptor = new Stack<>(); @Override public void endVisit(JIfStatement x, Context ctx) { + acceptor.pop(); + if (x.getThenStmt().unconditionalControlBreak()) { - if (x.getElseStmt().unconditionalControlBreak()) { - // Both branches break, nothing to do - parent block is unreachable code after if/else - acceptor = null; - } else { + if (!x.getElseStmt().unconditionalControlBreak()) { // else can handle rest of parent block - acceptor = x.getElseStmt(); + acceptor.push(x.getElseStmt()); } } else { if (x.getThenStmt().unconditionalControlBreak()) { // else can handle rest of parent block - acceptor = x.getElseStmt(); - } else { - // neither branch breaks, cannot optimize - acceptor = null; + acceptor.push(x.getElseStmt()); } } } @Override public void endVisit(JTryStatement x, Context ctx) { + acceptor.pop(); + if (x.getFinallyBlock() != null) { // Cannot optimize if there is a finally block, as finally executes after catch and before remainder - acceptor = null; return; } if (!x.getTryBlock().unconditionalControlBreak()) { - // Try must unconditionally break to optimize - acceptor = null; + // Try must unconditionally break to optimize, else we may catch the wrong exceptions return; } if (x.getCatchClauses().size() != 1) { // Only can optimize exactly one catch, and unconditional return try - acceptor = null; return; } - acceptor = x.getCatchClauses().get(0).getBlock(); + acceptor.push(x.getCatchClauses().get(0).getBlock()); } @Override public void endVisit(JSwitchStatement x, Context ctx) { - //TODO handle switch case (though probably rarely worth it) - acceptor = null; + acceptor.pop(); + //TODO handle switch case, necessary for correctness, else we have to skip any method with a + // switch/case statement in it. Note that we already skip any method with a switch expr, + // as we never anticipate statements in expressions. + acceptor.push(null); } @Override public void endVisit(JForStatement x, Context ctx) { + + acceptor.pop(); // Interrupts moving statements to the last acceptor block - acceptor = null; + acceptor.push(null); } @Override public void endVisit(JWhileStatement x, Context ctx) { + acceptor.pop(); // Interrupts moving statements to the last acceptor block - acceptor = null; + acceptor.push(null); } @Override public void endVisit(JDoStatement x, Context ctx) { + acceptor.pop(); // Interrupts moving statements to the last acceptor block - acceptor = null; + acceptor.push(null); } @Override public boolean visit(JStatement x, Context ctx) { - if (acceptor != null) { + if (!acceptor.isEmpty() && acceptor.peek() != null) { // If there is a block ready to accept this, any statement should be moved. - // Any visit() override must call super before it does its own work, + // Any visit() override must call super before it does its own work // to ensure that it is relocated ctx.removeMe(); - acceptor.addStmt(x); + acceptor.peek().addStmt(x); // Moved the item itself, continue to see if it needs to adopt later statements } return super.visit(x, ctx); } + @Override + public boolean visit(JIfStatement x, Context ctx) { + // Attempt to move the entire if + super.visit(x, ctx); + // While inside the if blocks, don't move statements out of it + acceptor.push(null); + return true; + } + + @Override + public boolean visit(JTryStatement x, Context ctx) { + // Attempt to move the entire try + super.visit(x, ctx); + // While inside the try block, don't move statements out of it + acceptor.push(null); + return true; + } + + @Override + public boolean visit(JForStatement x, Context ctx) { + // Attempt to move the entire for + super.visit(x, ctx); + // While inside the for block, don't move statements out of it + acceptor.push(null); + return true; + } + + @Override + public boolean visit(JWhileStatement x, Context ctx) { + // Attempt to move the entire while + super.visit(x, ctx); + // While inside the while block, don't move statements out of it + acceptor.push(null); + return true; + } + + @Override + public boolean visit(JDoStatement x, Context ctx) { + // Attempt to move the entire do + super.visit(x, ctx); + // While inside the do block, don't move statements out of it + acceptor.push(null); + return true; + } + + @Override + public boolean visit(JSwitchStatement x, Context ctx) { +// // Attempt to move the entire switch +// super.visit(x, ctx); +// // While inside the switch block, don't move statements out of it +// acceptor.push(null); +// return true; + // Disable switch/case for now - immovable object means we pop the last block + if (!acceptor.isEmpty() && acceptor.peek() != null) { + acceptor.pop(); + } + // Don't descend (at this time) - in the future we could either try to work around cases, + // or could wait until we hit some child block and instantiate a new visitor for local + // changes. + return false; + } + + @Override + public boolean visit(JBlock x, Context ctx) { + // Attempt to move the entire block + super.visit(x, ctx); + // While inside the block, don't move statements out of it + acceptor.push(null); + return true; + } + + @Override + public void endVisit(JBlock x, Context ctx) { + acceptor.pop(); + // Can't move statements into a block unless we know it was a statement and not part + // of a statement (like try/if/switch/etc). If we're part of an if/etc, after exiting + // the block, we'll exit the "if" and push that instead. +// acceptor.push(null); + } + +// @Override +// public boolean visit(JCaseStatement x, Context ctx) { +// // unlike other statements, can never be moved +// acceptor.push(null); +// return true; +// } +// +// @Override +// public void endVisit(JCaseStatement x, Context ctx) { +// acceptor.pop(); +// } + @Override public void endVisit(JMethodBody x, Context ctx) { // Clear any acceptor at end of method body - acceptor = null; + acceptor.clear(); } }.accept(program); } From 207a28333e06391d7dd45064a6a971ec208b882f Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 26 Nov 2025 11:43:28 -0600 Subject: [PATCH 14/42] A little more clean up, all samples build cleanly --- .../google/gwt/dev/jjs/impl/ExpandBlocks.java | 139 +++++++----------- 1 file changed, 56 insertions(+), 83 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java index 4eaafa62aec..032a02c2fc9 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java @@ -81,6 +81,22 @@ public static void exec(JProgram program) { * If we hit a statement that is immovable, we need to pop the latest item off the stack (if present and non-null). */ private final Stack acceptor = new Stack<>(); + + @Override + public boolean visit(JStatement x, Context ctx) { + attemptRelocate(x, ctx); + return true; + } + + @Override + public boolean visit(JIfStatement x, Context ctx) { + // Attempt to move the entire if + attemptRelocate(x, ctx); + // While inside the if blocks, don't move statements out of it + acceptor.push(null); + return true; + } + @Override public void endVisit(JIfStatement x, Context ctx) { acceptor.pop(); @@ -98,6 +114,15 @@ public void endVisit(JIfStatement x, Context ctx) { } } + @Override + public boolean visit(JTryStatement x, Context ctx) { + // Attempt to move the entire try + attemptRelocate(x, ctx); + // While inside the try block, don't move statements out of it + acceptor.push(null); + return true; + } + @Override public void endVisit(JTryStatement x, Context ctx) { acceptor.pop(); @@ -120,116 +145,80 @@ public void endVisit(JTryStatement x, Context ctx) { } @Override - public void endVisit(JSwitchStatement x, Context ctx) { - acceptor.pop(); - //TODO handle switch case, necessary for correctness, else we have to skip any method with a - // switch/case statement in it. Note that we already skip any method with a switch expr, - // as we never anticipate statements in expressions. - acceptor.push(null); - } - - @Override - public void endVisit(JForStatement x, Context ctx) { - - acceptor.pop(); - // Interrupts moving statements to the last acceptor block + public boolean visit(JWhileStatement x, Context ctx) { + // Attempt to move the entire while + attemptRelocate(x, ctx); + // While inside the while block, don't move statements out of it acceptor.push(null); + return true; } @Override public void endVisit(JWhileStatement x, Context ctx) { acceptor.pop(); - // Interrupts moving statements to the last acceptor block - acceptor.push(null); } @Override - public void endVisit(JDoStatement x, Context ctx) { - acceptor.pop(); - // Interrupts moving statements to the last acceptor block - acceptor.push(null); - } - + public boolean visit(JSwitchStatement x, Context ctx) { + // Attempt to move the entire switch + attemptRelocate(x, ctx); - @Override - public boolean visit(JStatement x, Context ctx) { - if (!acceptor.isEmpty() && acceptor.peek() != null) { - // If there is a block ready to accept this, any statement should be moved. - // Any visit() override must call super before it does its own work - // to ensure that it is relocated - ctx.removeMe(); - acceptor.peek().addStmt(x); - // Moved the item itself, continue to see if it needs to adopt later statements - } - return super.visit(x, ctx); + // Don't descend (at this time) - in the future we could either try to work around cases, + // or could wait until we hit some child block and instantiate a new visitor for local + // changes. + return false; } - @Override - public boolean visit(JIfStatement x, Context ctx) { - // Attempt to move the entire if - super.visit(x, ctx); - // While inside the if blocks, don't move statements out of it - acceptor.push(null); - return true; - } @Override - public boolean visit(JTryStatement x, Context ctx) { - // Attempt to move the entire try - super.visit(x, ctx); - // While inside the try block, don't move statements out of it - acceptor.push(null); - return true; + public void endVisit(JSwitchStatement x, Context ctx) { + // No endVisit for switch, since we didn't push anything or descend + // acceptor.pop(); } @Override public boolean visit(JForStatement x, Context ctx) { // Attempt to move the entire for - super.visit(x, ctx); + attemptRelocate(x, ctx); // While inside the for block, don't move statements out of it acceptor.push(null); return true; } @Override - public boolean visit(JWhileStatement x, Context ctx) { - // Attempt to move the entire while - super.visit(x, ctx); - // While inside the while block, don't move statements out of it - acceptor.push(null); - return true; + public void endVisit(JForStatement x, Context ctx) { + acceptor.pop(); } @Override public boolean visit(JDoStatement x, Context ctx) { // Attempt to move the entire do - super.visit(x, ctx); + attemptRelocate(x, ctx); // While inside the do block, don't move statements out of it acceptor.push(null); return true; } @Override - public boolean visit(JSwitchStatement x, Context ctx) { -// // Attempt to move the entire switch -// super.visit(x, ctx); -// // While inside the switch block, don't move statements out of it -// acceptor.push(null); -// return true; - // Disable switch/case for now - immovable object means we pop the last block + public void endVisit(JDoStatement x, Context ctx) { + acceptor.pop(); + } + + private void attemptRelocate(JStatement x, Context ctx) { if (!acceptor.isEmpty() && acceptor.peek() != null) { - acceptor.pop(); + // If there is a block ready to accept this, any statement should be moved. + // Any visit() override must call super before it does its own work + // to ensure that it is relocated + ctx.removeMe(); + acceptor.peek().addStmt(x); + // Moved the item itself, continue to see if it needs to adopt later statements } - // Don't descend (at this time) - in the future we could either try to work around cases, - // or could wait until we hit some child block and instantiate a new visitor for local - // changes. - return false; } @Override public boolean visit(JBlock x, Context ctx) { // Attempt to move the entire block - super.visit(x, ctx); + attemptRelocate(x, ctx); // While inside the block, don't move statements out of it acceptor.push(null); return true; @@ -238,24 +227,8 @@ public boolean visit(JBlock x, Context ctx) { @Override public void endVisit(JBlock x, Context ctx) { acceptor.pop(); - // Can't move statements into a block unless we know it was a statement and not part - // of a statement (like try/if/switch/etc). If we're part of an if/etc, after exiting - // the block, we'll exit the "if" and push that instead. -// acceptor.push(null); } -// @Override -// public boolean visit(JCaseStatement x, Context ctx) { -// // unlike other statements, can never be moved -// acceptor.push(null); -// return true; -// } -// -// @Override -// public void endVisit(JCaseStatement x, Context ctx) { -// acceptor.pop(); -// } - @Override public void endVisit(JMethodBody x, Context ctx) { // Clear any acceptor at end of method body From ccdc74d160a5d8ad54cbd3d27d2ded695ec41b09 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 26 Nov 2025 21:05:35 -0600 Subject: [PATCH 15/42] Don't try to move {}s, better java gen, fix a js bug --- .../com/google/gwt/dev/jjs/impl/ExpandBlocks.java | 2 +- .../gwt/dev/jjs/impl/GenerateJavaScriptAST.java | 4 ++-- .../gwt/dev/jjs/impl/ToStringGenerationVisitor.java | 13 +++---------- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java index 032a02c2fc9..5ac2e9146f4 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java @@ -218,7 +218,7 @@ private void attemptRelocate(JStatement x, Context ctx) { @Override public boolean visit(JBlock x, Context ctx) { // Attempt to move the entire block - attemptRelocate(x, ctx); +// attemptRelocate(x, ctx); // While inside the block, don't move statements out of it acceptor.push(null); return true; diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java index 16339961daf..ba71133d9f4 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java @@ -753,8 +753,8 @@ public JsNode transformIfStatement(JIfStatement ifStatement) { result.setIfExpr(transform(ifStatement.getIfExpr())); result.setThenStmt(jsEmptyIfNull(ifStatement.getSourceInfo(), - unwrapSingleStatement(transformBlock(ifStatement.getThenStmt())))); - result.setElseStmt(unwrapSingleStatement(transformBlock(ifStatement.getElseStmt()))); + transformBlock(ifStatement.getThenStmt()))); + result.setElseStmt(transformBlock(ifStatement.getElseStmt())); return result; } diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java index 594090e4344..41ab9b06b17 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java @@ -543,7 +543,7 @@ public boolean visit(JIfStatement x, Context ctx) { nestedStatementPop(x.getThenStmt()); } - if (x.getElseStmt() != null) { + if (x.getElseStmt() != null && !x.getElseStmt().isEmpty()) { if (needSemi) { semi(); newline(); @@ -552,16 +552,9 @@ public boolean visit(JIfStatement x, Context ctx) { needSemi = true; } print(CHARS_ELSE); -// boolean elseIf = x.getElseStmt() instanceof JIfStatement; -// if (!elseIf) { - nestedStatementPush(x.getElseStmt()); -// } else { -// space(); -// } + nestedStatementPush(x.getElseStmt()); accept(x.getElseStmt()); -// if (!elseIf) { - nestedStatementPop(x.getElseStmt()); -// } + nestedStatementPop(x.getElseStmt()); } return false; From 073dd28d16bb0299cffc80a8b5353956184cb606 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 27 Nov 2025 14:50:14 -0600 Subject: [PATCH 16/42] Checkpoint, user tests pass but compiler fails --- .../src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java | 2 +- .../com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java index 505937d67ea..31cb8b52d54 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java +++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java @@ -370,7 +370,7 @@ private PermutationResult compilePermutation(Permutation permutation, UnifiedAst // TODO(rluble): eventually move to normizeSemantics. CompileTimeConstantsReplacer.exec(jprogram); - ExpandBlocks.exec(jprogram); +// ExpandBlocks.exec(jprogram); // TODO(stalcup): move to after normalize. // (3) Optimize the resolved Java AST diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java index ba71133d9f4..00d0e4bdc06 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java @@ -753,8 +753,8 @@ public JsNode transformIfStatement(JIfStatement ifStatement) { result.setIfExpr(transform(ifStatement.getIfExpr())); result.setThenStmt(jsEmptyIfNull(ifStatement.getSourceInfo(), - transformBlock(ifStatement.getThenStmt()))); - result.setElseStmt(transformBlock(ifStatement.getElseStmt())); + transform(ifStatement.getThenStmt()))); + result.setElseStmt(transform(ifStatement.getElseStmt())); return result; } From 206198d03056611693d7d06c91d4abcec7d6819e Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 29 Nov 2025 19:34:21 -0600 Subject: [PATCH 17/42] More consistent output, still have a small size regression (inlining?) --- .../google/gwt/dev/jjs/impl/Simplifier.java | 19 +++++++------------ .../jjs/impl/ToStringGenerationVisitor.java | 12 +++++++++--- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java b/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java index 32888182c27..40fb72013ae 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java @@ -258,10 +258,10 @@ public static JStatement simplifyIfStatement(JIfStatement ifStatement, JType met if (conditionExpression instanceof JBooleanLiteral) { boolean conditionValue = ((JBooleanLiteral) conditionExpression).getValue(); - if (conditionValue && !JjsUtils.isEmptyBlock(thenStmt)) { + if (conditionValue && !thenStmt.isEmpty()) { // If true, replace myself with then statement return thenStmt; - } else if (!conditionValue && !JjsUtils.isEmptyBlock(elseStmt)) { + } else if (!conditionValue && !elseStmt.isEmpty()) { // If false, replace myself with else statement return elseStmt; } else { @@ -270,23 +270,15 @@ public static JStatement simplifyIfStatement(JIfStatement ifStatement, JType met } } - if (JjsUtils.isEmptyBlock(thenStmt) && JjsUtils.isEmptyBlock(elseStmt)) { + if (thenStmt.isEmpty() && elseStmt.isEmpty()) { return conditionExpression.makeStatement(); } - if (!JjsUtils.isEmptyBlock(elseStmt)) { + if (!elseStmt.isEmpty()) { // if (!cond) foo else bar -> if (cond) bar else foo JExpression negationArugment = Simplifier.maybeGetNegatedExpressionArgument(conditionExpression); if (negationArugment != null) { - - - // Force sub-parts to blocks, otherwise we break else-if chains. - // TODO: this goes away when we normalize the Java AST properly. - - -// thenStmt = ensureBlock(thenStmt); -// elseStmt = ensureBlock(elseStmt); return simplifyIfStatement( new JIfStatement(info, negationArugment, elseStmt, thenStmt), methodReturnType); } @@ -527,6 +519,9 @@ private static JExpression extractExpression(JStatement statement) { private static JStatement extractSingleStatement(JStatement statement) { if (statement instanceof JBlock) { JBlock block = (JBlock) statement; + if (block.isEmpty()) { + return null; + } if (block.getStatements().size() == 1) { return extractSingleStatement(block.getStatements().get(0)); } diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java index 41ab9b06b17..331242b0256 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java @@ -552,9 +552,15 @@ public boolean visit(JIfStatement x, Context ctx) { needSemi = true; } print(CHARS_ELSE); - nestedStatementPush(x.getElseStmt()); - accept(x.getElseStmt()); - nestedStatementPop(x.getElseStmt()); + boolean elseIf = x.getElseStmt().getStatements().size() == 1 && x.getElseStmt().getStatements().get(0) instanceof JIfStatement; + if (!elseIf) { + nestedStatementPush(x.getElseStmt()); + accept(x.getElseStmt()); + nestedStatementPop(x.getElseStmt()); + } else { + space(); + accept(x.getElseStmt().getStatements().get(0)); + } } return false; From 279130a1f315116cdc98cba71efc2a57f8c22a34 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 29 Nov 2025 21:25:21 -0600 Subject: [PATCH 18/42] Try to restore ExpandBlocks, more problems found --- .../gwt/dev/jjs/JavaToJavaScriptCompiler.java | 2 +- .../google/gwt/dev/jjs/impl/ExpandBlocks.java | 83 +++++++++++++++++-- 2 files changed, 75 insertions(+), 10 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java index 31cb8b52d54..505937d67ea 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java +++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java @@ -370,7 +370,7 @@ private PermutationResult compilePermutation(Permutation permutation, UnifiedAst // TODO(rluble): eventually move to normizeSemantics. CompileTimeConstantsReplacer.exec(jprogram); -// ExpandBlocks.exec(jprogram); + ExpandBlocks.exec(jprogram); // TODO(stalcup): move to after normalize. // (3) Optimize the resolved Java AST diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java index 5ac2e9146f4..998eab0e582 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java @@ -22,6 +22,7 @@ import com.google.gwt.dev.jjs.ast.JIfStatement; import com.google.gwt.dev.jjs.ast.JMethodBody; import com.google.gwt.dev.jjs.ast.JModVisitor; +import com.google.gwt.dev.jjs.ast.JNode; import com.google.gwt.dev.jjs.ast.JProgram; import com.google.gwt.dev.jjs.ast.JStatement; import com.google.gwt.dev.jjs.ast.JSwitchStatement; @@ -67,6 +68,14 @@ * */ public class ExpandBlocks { + private static class Acceptor { + JBlock acceptingBlock; + final JNode currentNode; + + private Acceptor(JNode currentNode) { + this.currentNode = currentNode; + } + } /** * Normalize the program's nested blocks. */ @@ -77,10 +86,12 @@ public static void exec(JProgram program) { * The location in which statements can be moved to is represented by a stack - it isn't quite the usual use of a stack in a visitor, as it doesn't represent * the path from the root, but instead the most recent block that can accept statements. Nulls are added to the stack as well to indicate that there is currently * no position to move statements to. Testing to see if a statement can be moved then is just checking if the stack is non-empty and the top is non-null. - *

    - * If we hit a statement that is immovable, we need to pop the latest item off the stack (if present and non-null). */ private final Stack acceptor = new Stack<>(); + /** + * Represents the current node we're visiting - will not necessarily be the same depth as the acceptor stack. + */ + private final Stack nodeStack = new Stack<>(); @Override public boolean visit(JStatement x, Context ctx) { @@ -94,38 +105,88 @@ public boolean visit(JIfStatement x, Context ctx) { attemptRelocate(x, ctx); // While inside the if blocks, don't move statements out of it acceptor.push(null); + nodeStack.push(x); return true; } @Override public void endVisit(JIfStatement x, Context ctx) { - acceptor.pop(); + // problematic case: +// if (a) { +// if (b) { +// return; +// } else { +// //1 +// } +// return; +// } else { +// //2 +// } +// // at the end, we must use 2, as 1 isn't suitable (it was in the branch with the unconditional return) + + //TODO try to use a more nested if structure to accept more statements - need to be sure it + // was from the same branch as we use below, else we have to go with "current". Without + // this, we can't solve this in a single pass. + // It may be necessary to implement this via accepting each then/else directly, and + // tracking which branch we came from to be able to do this correctly. + + + JBlock nested = popUntilNode(x); + JBlock current = acceptor.isEmpty() ? null : acceptor.peek(); if (x.getThenStmt().unconditionalControlBreak()) { if (!x.getElseStmt().unconditionalControlBreak()) { // else can handle rest of parent block acceptor.push(x.getElseStmt()); + } else { + // both break, can use the same block (if any) that we relocated the current node to + acceptor.push(current); } } else { if (x.getThenStmt().unconditionalControlBreak()) { // else can handle rest of parent block acceptor.push(x.getElseStmt()); + } else { + // neither breaks, use the same block (if any) that we relocated the current node to + acceptor.push(current); } } } + /** + * Walks up the stack of acceptors until we get the one in use before we started the current + * node. Returns the first non-null block found, or null if none - can be used in cases where + * we aren't deliberately clearing out the stack (e.g. after leaving a while/do/etc to discard + * nested ifs), so that nested if/elses where only one block doesn't return can have the + * innermost acceptor used. + * @param x + * @return + */ + private JBlock popUntilNode(JNode x) { + JBlock nonNull = null; + do { + JBlock seen = acceptor.pop(); + if (seen != null && nonNull == null) { + nonNull = seen; + } + } while (nodeStack.pop() != x); + return nonNull; + } + @Override public boolean visit(JTryStatement x, Context ctx) { // Attempt to move the entire try attemptRelocate(x, ctx); // While inside the try block, don't move statements out of it acceptor.push(null); + nodeStack.push(x); return true; } @Override public void endVisit(JTryStatement x, Context ctx) { - acceptor.pop(); + //TODO try to find a more deeply nested block + popUntilNode(x); if (x.getFinallyBlock() != null) { // Cannot optimize if there is a finally block, as finally executes after catch and before remainder @@ -150,12 +211,13 @@ public boolean visit(JWhileStatement x, Context ctx) { attemptRelocate(x, ctx); // While inside the while block, don't move statements out of it acceptor.push(null); + nodeStack.push(x); return true; } @Override public void endVisit(JWhileStatement x, Context ctx) { - acceptor.pop(); + popUntilNode(x); } @Override @@ -173,7 +235,7 @@ public boolean visit(JSwitchStatement x, Context ctx) { @Override public void endVisit(JSwitchStatement x, Context ctx) { // No endVisit for switch, since we didn't push anything or descend - // acceptor.pop(); + // popUntilNode(x); } @Override @@ -182,12 +244,13 @@ public boolean visit(JForStatement x, Context ctx) { attemptRelocate(x, ctx); // While inside the for block, don't move statements out of it acceptor.push(null); + nodeStack.push(x); return true; } @Override public void endVisit(JForStatement x, Context ctx) { - acceptor.pop(); + popUntilNode(x); } @Override @@ -196,12 +259,13 @@ public boolean visit(JDoStatement x, Context ctx) { attemptRelocate(x, ctx); // While inside the do block, don't move statements out of it acceptor.push(null); + nodeStack.push(x); return true; } @Override public void endVisit(JDoStatement x, Context ctx) { - acceptor.pop(); + popUntilNode(x); } private void attemptRelocate(JStatement x, Context ctx) { @@ -221,12 +285,13 @@ public boolean visit(JBlock x, Context ctx) { // attemptRelocate(x, ctx); // While inside the block, don't move statements out of it acceptor.push(null); + nodeStack.push(x); return true; } @Override public void endVisit(JBlock x, Context ctx) { - acceptor.pop(); + popUntilNode(x); } @Override From c93bc4cf66087273161607a8830cf81986631efa Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sun, 30 Nov 2025 19:11:46 -0600 Subject: [PATCH 19/42] Nearly correct impl of ExpandBlocks --- .../google/gwt/dev/jjs/impl/ExpandBlocks.java | 174 +++++++++--------- .../gwt/dev/jjs/impl/ExpandBlocksTest.java | 123 +++++++++++++ 2 files changed, 213 insertions(+), 84 deletions(-) create mode 100644 dev/core/test/com/google/gwt/dev/jjs/impl/ExpandBlocksTest.java diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java index 998eab0e582..8c9b52fc6ea 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java @@ -87,11 +87,12 @@ public static void exec(JProgram program) { * the path from the root, but instead the most recent block that can accept statements. Nulls are added to the stack as well to indicate that there is currently * no position to move statements to. Testing to see if a statement can be moved then is just checking if the stack is non-empty and the top is non-null. */ - private final Stack acceptor = new Stack<>(); - /** - * Represents the current node we're visiting - will not necessarily be the same depth as the acceptor stack. - */ - private final Stack nodeStack = new Stack<>(); + private final Stack acceptorStack = new Stack<>(); + + private JBlock acceptBlockWithoutPush(JBlock block) { + acceptWithInsertRemove(block.getStatements()); + return block; + } @Override public boolean visit(JStatement x, Context ctx) { @@ -101,76 +102,59 @@ public boolean visit(JStatement x, Context ctx) { @Override public boolean visit(JIfStatement x, Context ctx) { + // We do our own visiting here, rather than delegate to super, so + // we control then vs else. + // Attempt to move the entire if attemptRelocate(x, ctx); // While inside the if blocks, don't move statements out of it - acceptor.push(null); - nodeStack.push(x); - return true; - } + Acceptor self = new Acceptor(x); + acceptorStack.push(self); - @Override - public void endVisit(JIfStatement x, Context ctx) { - // problematic case: -// if (a) { -// if (b) { -// return; -// } else { -// //1 -// } -// return; -// } else { -// //2 -// } -// // at the end, we must use 2, as 1 isn't suitable (it was in the branch with the unconditional return) - - //TODO try to use a more nested if structure to accept more statements - need to be sure it - // was from the same branch as we use below, else we have to go with "current". Without - // this, we can't solve this in a single pass. - // It may be necessary to implement this via accepting each then/else directly, and - // tracking which branch we came from to be able to do this correctly. - - - JBlock nested = popUntilNode(x); - JBlock current = acceptor.isEmpty() ? null : acceptor.peek(); + // Ignore condition, visit then and else + acceptBlockWithoutPush(x.getThenStmt()); + JBlock thenAcceptor = self.acceptingBlock; + acceptBlockWithoutPush(x.getElseStmt()); + JBlock elseAcceptor = self.acceptingBlock; + + // Pop the local node, and look to see if we should edit the parent + acceptorStack.pop(); + Acceptor parent = acceptorStack.peek(); + // Mark where (if any) we accept later statements if (x.getThenStmt().unconditionalControlBreak()) { if (!x.getElseStmt().unconditionalControlBreak()) { // else can handle rest of parent block - acceptor.push(x.getElseStmt()); + if (elseAcceptor != null) { + // Use the acceptor that we found while visiting else + parent.acceptingBlock = elseAcceptor; + } else { + parent.acceptingBlock = x.getElseStmt(); + } } else { // both break, can use the same block (if any) that we relocated the current node to - acceptor.push(current); } } else { if (x.getThenStmt().unconditionalControlBreak()) { // else can handle rest of parent block - acceptor.push(x.getElseStmt()); + if (thenAcceptor != null) { + // Use the acceptor that we found while visiting then + parent.acceptingBlock = thenAcceptor; + } else { + parent.acceptingBlock = x.getThenStmt(); + } } else { // neither breaks, use the same block (if any) that we relocated the current node to - acceptor.push(current); } } + + // Already visited children + return false; } - /** - * Walks up the stack of acceptors until we get the one in use before we started the current - * node. Returns the first non-null block found, or null if none - can be used in cases where - * we aren't deliberately clearing out the stack (e.g. after leaving a while/do/etc to discard - * nested ifs), so that nested if/elses where only one block doesn't return can have the - * innermost acceptor used. - * @param x - * @return - */ - private JBlock popUntilNode(JNode x) { - JBlock nonNull = null; - do { - JBlock seen = acceptor.pop(); - if (seen != null && nonNull == null) { - nonNull = seen; - } - } while (nodeStack.pop() != x); - return nonNull; + @Override + public void endVisit(JIfStatement x, Context ctx) { + // Do nothing, handled in visit(if) } @Override @@ -178,31 +162,56 @@ public boolean visit(JTryStatement x, Context ctx) { // Attempt to move the entire try attemptRelocate(x, ctx); // While inside the try block, don't move statements out of it - acceptor.push(null); - nodeStack.push(x); - return true; - } + Acceptor self = new Acceptor(x); + acceptorStack.push(self); + + // Visit each child statement (ignore exprs), though we only need to track changes to "self" + // for the catch block if there is exactly one + JBlock catchExceptor = null; + accept(x.getTryBlock()); + for (int i = 0; i < x.getCatchClauses().size(); i++) { + JTryStatement.CatchClause clause = x.getCatchClauses().get(i); + acceptBlockWithoutPush(clause.getBlock()); + // If there is exactly one catch, we may be able to move later statements into it + if (x.getCatchClauses().size() == 1) { + catchExceptor = self.acceptingBlock; + } + } + if (x.getFinallyBlock() != null) { + accept(x.getFinallyBlock()); + } - @Override - public void endVisit(JTryStatement x, Context ctx) { - //TODO try to find a more deeply nested block - popUntilNode(x); + acceptorStack.pop(); + Acceptor parent = acceptorStack.peek(); if (x.getFinallyBlock() != null) { // Cannot optimize if there is a finally block, as finally executes after catch and before remainder - return; + return false; } if (!x.getTryBlock().unconditionalControlBreak()) { // Try must unconditionally break to optimize, else we may catch the wrong exceptions - return; + return false; } - if (x.getCatchClauses().size() != 1) { + if (x.getCatchClauses().size() != 1 || x.getCatchClauses().get(0).getBlock().unconditionalControlBreak()) { // Only can optimize exactly one catch, and unconditional return try - return; + return false; + } + // This is a valid case to rewrite, move later statements into the catch block, or nested + // if available + if (catchExceptor != null) { + // Use the acceptor that we found while visiting the catch + parent.acceptingBlock = catchExceptor; + } else { + parent.acceptingBlock = x.getCatchClauses().get(0).getBlock(); } - acceptor.push(x.getCatchClauses().get(0).getBlock()); + return false; + } + + @Override + public void endVisit(JTryStatement x, Context ctx) { + // Do nothing, handled in visit(try) } @Override @@ -210,14 +219,13 @@ public boolean visit(JWhileStatement x, Context ctx) { // Attempt to move the entire while attemptRelocate(x, ctx); // While inside the while block, don't move statements out of it - acceptor.push(null); - nodeStack.push(x); + acceptorStack.push(new Acceptor(x)); return true; } @Override public void endVisit(JWhileStatement x, Context ctx) { - popUntilNode(x); + acceptorStack.pop(); } @Override @@ -235,7 +243,7 @@ public boolean visit(JSwitchStatement x, Context ctx) { @Override public void endVisit(JSwitchStatement x, Context ctx) { // No endVisit for switch, since we didn't push anything or descend - // popUntilNode(x); + // acceptorStack.pop(); } @Override @@ -243,14 +251,13 @@ public boolean visit(JForStatement x, Context ctx) { // Attempt to move the entire for attemptRelocate(x, ctx); // While inside the for block, don't move statements out of it - acceptor.push(null); - nodeStack.push(x); + acceptorStack.push(new Acceptor(x)); return true; } @Override public void endVisit(JForStatement x, Context ctx) { - popUntilNode(x); + acceptorStack.pop(); } @Override @@ -258,23 +265,22 @@ public boolean visit(JDoStatement x, Context ctx) { // Attempt to move the entire do attemptRelocate(x, ctx); // While inside the do block, don't move statements out of it - acceptor.push(null); - nodeStack.push(x); + acceptorStack.push(new Acceptor(x)); return true; } @Override public void endVisit(JDoStatement x, Context ctx) { - popUntilNode(x); + acceptorStack.pop(); } private void attemptRelocate(JStatement x, Context ctx) { - if (!acceptor.isEmpty() && acceptor.peek() != null) { + if (!acceptorStack.isEmpty() && acceptorStack.peek().acceptingBlock != null) { // If there is a block ready to accept this, any statement should be moved. // Any visit() override must call super before it does its own work // to ensure that it is relocated ctx.removeMe(); - acceptor.peek().addStmt(x); + acceptorStack.peek().acceptingBlock.addStmt(x); // Moved the item itself, continue to see if it needs to adopt later statements } } @@ -284,20 +290,20 @@ public boolean visit(JBlock x, Context ctx) { // Attempt to move the entire block // attemptRelocate(x, ctx); // While inside the block, don't move statements out of it - acceptor.push(null); - nodeStack.push(x); + acceptorStack.push(new Acceptor(x)); return true; } @Override public void endVisit(JBlock x, Context ctx) { - popUntilNode(x); + acceptorStack.pop(); } @Override public void endVisit(JMethodBody x, Context ctx) { // Clear any acceptor at end of method body - acceptor.clear(); + assert acceptorStack.isEmpty(); + acceptorStack.clear(); } }.accept(program); } diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/ExpandBlocksTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/ExpandBlocksTest.java new file mode 100644 index 00000000000..478885c1566 --- /dev/null +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/ExpandBlocksTest.java @@ -0,0 +1,123 @@ +package com.google.gwt.dev.jjs.impl; + +import com.google.gwt.core.ext.TreeLogger; +import com.google.gwt.core.ext.UnableToCompleteException; +import com.google.gwt.dev.jjs.ast.JMethod; +import com.google.gwt.dev.jjs.ast.JProgram; + +public class ExpandBlocksTest extends OptimizerTestBase { + + public void testSimpleIf() throws Exception { + addSnippetClassDecl( + "static class A { ", + " static boolean a = true;", + " static int b = 0;", + " static int c = 0;", + "}"); + optimize("void", "if (A.a) { return; } A.b++;") + .into("if (A.a) { return; } else { A.b++; } "); + optimize("void", "if (A.a) { return; } else { A.b++; } A.c++;") + .into("if (A.a) { return; } else { A.b++; A.c++; }"); + //TODO incomplete: + optimize("void", "if (A.a) { return; } else if (A.b == 0) { A.b++; } else { return; } A.c++;") + .into("if (A.a) { return; } else { if (A.b == 0) { A.b++; } else { return; } A.c++;}"); +// TODO do this instead + // optimize("void", "if (A.a) { return; } else if (A.b == 0) { A.b++; } else { return; } A.c++;") +// .into("if (A.a) { return; } else if (A.b == 0) { A.b++; A.c++; } else { return; }"); + } + + public void testNestedIf() throws Exception { + addSnippetClassDecl( + "static class A { ", + " static boolean a = true;",// bool condition + " static int b = 0;", + " static int c = 0;",// statement expected to be moved + " static int d = 0;", + " static int method() { return b + c + d; }", + "}"); + optimize("void", "while(A.a) { A.b++; if (A.a) { break; } A.c++; } A.d++;") + .into("while(A.a) { A.b++; if (A.a) { break; } else { A.c++; } } A.d++;"); + optimize("void", "while(A.a) { A.b++; if (A.a) { continue; } else { A.b++; } A.c++; } A.d++;") + .into("while(A.a) { A.b++; if (A.a) { continue; } else { A.b++; A.c++; } } A.d++;"); + + optimize("void", + "if (A.a) {" + + " return;", + "}", + "A.c++;", + "if (A.a) {", + " while (A.a) {", + " A.b++;", + " if (A.a) {", + " break;", + " }", + " A.c++;", + " }", + "}" + ).into( + "if (A.a) {", + " return;", + "} else {", + " A.c++;", + " if (A.a) {", + " while (A.a) {", + " A.b++;", + " if (A.a) {", + " break;", + " } else {", + " A.c++;", + " }", + " }", + " }", + "}" + ); + } + + public void testImplEntry() throws Exception { + addSnippetClassDecl( + "static class A { ", + " static boolean a = true;",// bool condition + " static int b = 0;", + " static int c = 0;",// statement expected to be moved + " static int d = 0;", + " static int method() { return b + c + d; }", + "}"); + + // A method that looks a little like Impl.entry0(), as a real life test case + optimize("int", + "try {" + + " if (A.a) {", + " try {", + " return A.method();", + " } catch (Exception e) {", + " return -1;", + " }", + " } else {", + " return A.method();", + " }", + "} finally {", + " A.c++;", + "}" + ).into( + "try {" + + " if (A.a) {", + " try {", + " return A.method();", + " } catch (Exception e) {", + " return -1;", + " }", + " } else {", + " return A.method();", + " }", + "} finally {", + " A.c++;", + "}" + ); + } + + @Override + protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod method) throws UnableToCompleteException { + ExpandBlocks.exec(program); + return true; + } +} From 382b730449fa8fa459b1b6b81b8232f68d8515bf Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 3 Jan 2026 20:17:00 -0600 Subject: [PATCH 20/42] Cleanup dead code --- dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java index f4f0d9fa972..e51ba40e993 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java @@ -49,12 +49,8 @@ public JBlock getThenStmt() { public void traverse(JVisitor visitor, Context ctx) { if (visitor.visit(this, ctx)) { ifExpr = visitor.accept(ifExpr); -// if (thenStmt != null) { - thenStmt = ensureBlock(visitor.accept(thenStmt, true)); -// } -// if (elseStmt != null) { - elseStmt = ensureBlock(visitor.accept(elseStmt, true)); -// } + thenStmt = ensureBlock(visitor.accept(thenStmt, true)); + elseStmt = ensureBlock(visitor.accept(elseStmt, true)); } visitor.endVisit(this, ctx); } From 4bee28a1a23fb3b8481ad151274688143f5ce39b Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 28 Jan 2026 13:15:59 -0600 Subject: [PATCH 21/42] Remove the 'expand blocks' experiment --- .../gwt/dev/jjs/JavaToJavaScriptCompiler.java | 3 - .../google/gwt/dev/jjs/impl/ExpandBlocks.java | 310 ------------------ .../gwt/dev/jjs/impl/ExpandBlocksTest.java | 123 ------- 3 files changed, 436 deletions(-) delete mode 100644 dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java delete mode 100644 dev/core/test/com/google/gwt/dev/jjs/impl/ExpandBlocksTest.java diff --git a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java index 505937d67ea..d11ceb05479 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java +++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java @@ -76,7 +76,6 @@ import com.google.gwt.dev.jjs.impl.EnumNameObfuscator; import com.google.gwt.dev.jjs.impl.EnumOrdinalizer; import com.google.gwt.dev.jjs.impl.EqualityNormalizer; -import com.google.gwt.dev.jjs.impl.ExpandBlocks; import com.google.gwt.dev.jjs.impl.Finalizer; import com.google.gwt.dev.jjs.impl.FixAssignmentsToUnboxOrCast; import com.google.gwt.dev.jjs.impl.FullOptimizerContext; @@ -370,8 +369,6 @@ private PermutationResult compilePermutation(Permutation permutation, UnifiedAst // TODO(rluble): eventually move to normizeSemantics. CompileTimeConstantsReplacer.exec(jprogram); - ExpandBlocks.exec(jprogram); - // TODO(stalcup): move to after normalize. // (3) Optimize the resolved Java AST optimizeJava(); diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java deleted file mode 100644 index 8c9b52fc6ea..00000000000 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpandBlocks.java +++ /dev/null @@ -1,310 +0,0 @@ -/* - * Copyright 2025 GWT Project Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not - * use this file except in compliance with the License. You may obtain a copy of - * the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations under - * the License. - */ -package com.google.gwt.dev.jjs.impl; - -import com.google.gwt.dev.jjs.ast.Context; -import com.google.gwt.dev.jjs.ast.JBlock; -import com.google.gwt.dev.jjs.ast.JDoStatement; -import com.google.gwt.dev.jjs.ast.JForStatement; -import com.google.gwt.dev.jjs.ast.JIfStatement; -import com.google.gwt.dev.jjs.ast.JMethodBody; -import com.google.gwt.dev.jjs.ast.JModVisitor; -import com.google.gwt.dev.jjs.ast.JNode; -import com.google.gwt.dev.jjs.ast.JProgram; -import com.google.gwt.dev.jjs.ast.JStatement; -import com.google.gwt.dev.jjs.ast.JSwitchStatement; -import com.google.gwt.dev.jjs.ast.JTryStatement; -import com.google.gwt.dev.jjs.ast.JWhileStatement; - -import java.util.Stack; - -/** - * Examines blocks where child blocks (if/else, try/catch) use control flow statements - * (return/throw/break/continue) which make it impossible for that block to flow back to the parent. - * In these cases, the other block can be expanded to include the rest of the parent block. - *

    - * Examples: - * if (condition) { statements; return; } rest of parent block; - * or - * if (condition) { statements; return; } else { more; } rest of parent block; - * -> if (condition) { statements; } else { [more;] rest of parent block; } - *

    - * try { statements; return; } catch (e) { handler(e); } rest of parent block; - * -> try { statements; } catch (e) { handler(e); rest of parent block; } - *

    - * These transformations have two simple benefits today: - *

      - *
    • They make it easier for the DeadCodeElimination/Simplifier to identify if/else blocks that return simple expressions and can be rewritten to a single conditional
      • - *
    • They reduce complexity slightly for passes like DuplicateClinitRemover, so that it doesn't need to merge the scopes it considers to continue
      • - *
    - * - * For cases where an else block is added, we will also add a JS pass to remove it, transforming - * if (condition) { ... return; } else { ... } -> if (condition) { ... return; } ... - * so that there is no net increase in size. This will could also save a few bytes when the else - * already existed, and can now be removed. - *

    - * Only needs to be run once as part of normalization, since MethodInliner will never move more than - * an expression (either in a JExpressionStatement or JReturnStatement). - *

    - * - * if (a) { return; } if (b) {return;} rest; - * --> if (a) { return; } else { if (b) { return; } else { rest; } } - *

    - * if (a) { for (..) { if (b) { break; } rest1; } } rest2; - * --> if (a) { for (..) { if (b) { break; } else { rest1; } } } else { rest2; } - * - */ -public class ExpandBlocks { - private static class Acceptor { - JBlock acceptingBlock; - final JNode currentNode; - - private Acceptor(JNode currentNode) { - this.currentNode = currentNode; - } - } - /** - * Normalize the program's nested blocks. - */ - public static void exec(JProgram program) { - new JModVisitor() { - - /** - * The location in which statements can be moved to is represented by a stack - it isn't quite the usual use of a stack in a visitor, as it doesn't represent - * the path from the root, but instead the most recent block that can accept statements. Nulls are added to the stack as well to indicate that there is currently - * no position to move statements to. Testing to see if a statement can be moved then is just checking if the stack is non-empty and the top is non-null. - */ - private final Stack acceptorStack = new Stack<>(); - - private JBlock acceptBlockWithoutPush(JBlock block) { - acceptWithInsertRemove(block.getStatements()); - return block; - } - - @Override - public boolean visit(JStatement x, Context ctx) { - attemptRelocate(x, ctx); - return true; - } - - @Override - public boolean visit(JIfStatement x, Context ctx) { - // We do our own visiting here, rather than delegate to super, so - // we control then vs else. - - // Attempt to move the entire if - attemptRelocate(x, ctx); - // While inside the if blocks, don't move statements out of it - Acceptor self = new Acceptor(x); - acceptorStack.push(self); - - // Ignore condition, visit then and else - acceptBlockWithoutPush(x.getThenStmt()); - JBlock thenAcceptor = self.acceptingBlock; - acceptBlockWithoutPush(x.getElseStmt()); - JBlock elseAcceptor = self.acceptingBlock; - - // Pop the local node, and look to see if we should edit the parent - acceptorStack.pop(); - Acceptor parent = acceptorStack.peek(); - - // Mark where (if any) we accept later statements - if (x.getThenStmt().unconditionalControlBreak()) { - if (!x.getElseStmt().unconditionalControlBreak()) { - // else can handle rest of parent block - if (elseAcceptor != null) { - // Use the acceptor that we found while visiting else - parent.acceptingBlock = elseAcceptor; - } else { - parent.acceptingBlock = x.getElseStmt(); - } - } else { - // both break, can use the same block (if any) that we relocated the current node to - } - } else { - if (x.getThenStmt().unconditionalControlBreak()) { - // else can handle rest of parent block - if (thenAcceptor != null) { - // Use the acceptor that we found while visiting then - parent.acceptingBlock = thenAcceptor; - } else { - parent.acceptingBlock = x.getThenStmt(); - } - } else { - // neither breaks, use the same block (if any) that we relocated the current node to - } - } - - // Already visited children - return false; - } - - @Override - public void endVisit(JIfStatement x, Context ctx) { - // Do nothing, handled in visit(if) - } - - @Override - public boolean visit(JTryStatement x, Context ctx) { - // Attempt to move the entire try - attemptRelocate(x, ctx); - // While inside the try block, don't move statements out of it - Acceptor self = new Acceptor(x); - acceptorStack.push(self); - - // Visit each child statement (ignore exprs), though we only need to track changes to "self" - // for the catch block if there is exactly one - JBlock catchExceptor = null; - accept(x.getTryBlock()); - for (int i = 0; i < x.getCatchClauses().size(); i++) { - JTryStatement.CatchClause clause = x.getCatchClauses().get(i); - acceptBlockWithoutPush(clause.getBlock()); - // If there is exactly one catch, we may be able to move later statements into it - if (x.getCatchClauses().size() == 1) { - catchExceptor = self.acceptingBlock; - } - } - if (x.getFinallyBlock() != null) { - accept(x.getFinallyBlock()); - } - - acceptorStack.pop(); - Acceptor parent = acceptorStack.peek(); - - if (x.getFinallyBlock() != null) { - // Cannot optimize if there is a finally block, as finally executes after catch and before remainder - return false; - } - - if (!x.getTryBlock().unconditionalControlBreak()) { - // Try must unconditionally break to optimize, else we may catch the wrong exceptions - return false; - } - - if (x.getCatchClauses().size() != 1 || x.getCatchClauses().get(0).getBlock().unconditionalControlBreak()) { - // Only can optimize exactly one catch, and unconditional return try - return false; - } - // This is a valid case to rewrite, move later statements into the catch block, or nested - // if available - if (catchExceptor != null) { - // Use the acceptor that we found while visiting the catch - parent.acceptingBlock = catchExceptor; - } else { - parent.acceptingBlock = x.getCatchClauses().get(0).getBlock(); - } - return false; - } - - @Override - public void endVisit(JTryStatement x, Context ctx) { - // Do nothing, handled in visit(try) - } - - @Override - public boolean visit(JWhileStatement x, Context ctx) { - // Attempt to move the entire while - attemptRelocate(x, ctx); - // While inside the while block, don't move statements out of it - acceptorStack.push(new Acceptor(x)); - return true; - } - - @Override - public void endVisit(JWhileStatement x, Context ctx) { - acceptorStack.pop(); - } - - @Override - public boolean visit(JSwitchStatement x, Context ctx) { - // Attempt to move the entire switch - attemptRelocate(x, ctx); - - // Don't descend (at this time) - in the future we could either try to work around cases, - // or could wait until we hit some child block and instantiate a new visitor for local - // changes. - return false; - } - - - @Override - public void endVisit(JSwitchStatement x, Context ctx) { - // No endVisit for switch, since we didn't push anything or descend - // acceptorStack.pop(); - } - - @Override - public boolean visit(JForStatement x, Context ctx) { - // Attempt to move the entire for - attemptRelocate(x, ctx); - // While inside the for block, don't move statements out of it - acceptorStack.push(new Acceptor(x)); - return true; - } - - @Override - public void endVisit(JForStatement x, Context ctx) { - acceptorStack.pop(); - } - - @Override - public boolean visit(JDoStatement x, Context ctx) { - // Attempt to move the entire do - attemptRelocate(x, ctx); - // While inside the do block, don't move statements out of it - acceptorStack.push(new Acceptor(x)); - return true; - } - - @Override - public void endVisit(JDoStatement x, Context ctx) { - acceptorStack.pop(); - } - - private void attemptRelocate(JStatement x, Context ctx) { - if (!acceptorStack.isEmpty() && acceptorStack.peek().acceptingBlock != null) { - // If there is a block ready to accept this, any statement should be moved. - // Any visit() override must call super before it does its own work - // to ensure that it is relocated - ctx.removeMe(); - acceptorStack.peek().acceptingBlock.addStmt(x); - // Moved the item itself, continue to see if it needs to adopt later statements - } - } - - @Override - public boolean visit(JBlock x, Context ctx) { - // Attempt to move the entire block -// attemptRelocate(x, ctx); - // While inside the block, don't move statements out of it - acceptorStack.push(new Acceptor(x)); - return true; - } - - @Override - public void endVisit(JBlock x, Context ctx) { - acceptorStack.pop(); - } - - @Override - public void endVisit(JMethodBody x, Context ctx) { - // Clear any acceptor at end of method body - assert acceptorStack.isEmpty(); - acceptorStack.clear(); - } - }.accept(program); - } -} diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/ExpandBlocksTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/ExpandBlocksTest.java deleted file mode 100644 index 478885c1566..00000000000 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/ExpandBlocksTest.java +++ /dev/null @@ -1,123 +0,0 @@ -package com.google.gwt.dev.jjs.impl; - -import com.google.gwt.core.ext.TreeLogger; -import com.google.gwt.core.ext.UnableToCompleteException; -import com.google.gwt.dev.jjs.ast.JMethod; -import com.google.gwt.dev.jjs.ast.JProgram; - -public class ExpandBlocksTest extends OptimizerTestBase { - - public void testSimpleIf() throws Exception { - addSnippetClassDecl( - "static class A { ", - " static boolean a = true;", - " static int b = 0;", - " static int c = 0;", - "}"); - optimize("void", "if (A.a) { return; } A.b++;") - .into("if (A.a) { return; } else { A.b++; } "); - optimize("void", "if (A.a) { return; } else { A.b++; } A.c++;") - .into("if (A.a) { return; } else { A.b++; A.c++; }"); - //TODO incomplete: - optimize("void", "if (A.a) { return; } else if (A.b == 0) { A.b++; } else { return; } A.c++;") - .into("if (A.a) { return; } else { if (A.b == 0) { A.b++; } else { return; } A.c++;}"); -// TODO do this instead - // optimize("void", "if (A.a) { return; } else if (A.b == 0) { A.b++; } else { return; } A.c++;") -// .into("if (A.a) { return; } else if (A.b == 0) { A.b++; A.c++; } else { return; }"); - } - - public void testNestedIf() throws Exception { - addSnippetClassDecl( - "static class A { ", - " static boolean a = true;",// bool condition - " static int b = 0;", - " static int c = 0;",// statement expected to be moved - " static int d = 0;", - " static int method() { return b + c + d; }", - "}"); - optimize("void", "while(A.a) { A.b++; if (A.a) { break; } A.c++; } A.d++;") - .into("while(A.a) { A.b++; if (A.a) { break; } else { A.c++; } } A.d++;"); - optimize("void", "while(A.a) { A.b++; if (A.a) { continue; } else { A.b++; } A.c++; } A.d++;") - .into("while(A.a) { A.b++; if (A.a) { continue; } else { A.b++; A.c++; } } A.d++;"); - - optimize("void", - "if (A.a) {" + - " return;", - "}", - "A.c++;", - "if (A.a) {", - " while (A.a) {", - " A.b++;", - " if (A.a) {", - " break;", - " }", - " A.c++;", - " }", - "}" - ).into( - "if (A.a) {", - " return;", - "} else {", - " A.c++;", - " if (A.a) {", - " while (A.a) {", - " A.b++;", - " if (A.a) {", - " break;", - " } else {", - " A.c++;", - " }", - " }", - " }", - "}" - ); - } - - public void testImplEntry() throws Exception { - addSnippetClassDecl( - "static class A { ", - " static boolean a = true;",// bool condition - " static int b = 0;", - " static int c = 0;",// statement expected to be moved - " static int d = 0;", - " static int method() { return b + c + d; }", - "}"); - - // A method that looks a little like Impl.entry0(), as a real life test case - optimize("int", - "try {" + - " if (A.a) {", - " try {", - " return A.method();", - " } catch (Exception e) {", - " return -1;", - " }", - " } else {", - " return A.method();", - " }", - "} finally {", - " A.c++;", - "}" - ).into( - "try {" + - " if (A.a) {", - " try {", - " return A.method();", - " } catch (Exception e) {", - " return -1;", - " }", - " } else {", - " return A.method();", - " }", - "} finally {", - " A.c++;", - "}" - ); - } - - @Override - protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod method) throws UnableToCompleteException { - ExpandBlocks.exec(program); - return true; - } -} From 118bfbffb57afb3fda208d2d8de06d355703b379 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 28 Jan 2026 13:53:17 -0600 Subject: [PATCH 22/42] break long line --- .../com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java index 331242b0256..1c9aa621e78 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java @@ -552,7 +552,8 @@ public boolean visit(JIfStatement x, Context ctx) { needSemi = true; } print(CHARS_ELSE); - boolean elseIf = x.getElseStmt().getStatements().size() == 1 && x.getElseStmt().getStatements().get(0) instanceof JIfStatement; + boolean elseIf = x.getElseStmt().getStatements().size() == 1 + && x.getElseStmt().getStatements().get(0) instanceof JIfStatement; if (!elseIf) { nestedStatementPush(x.getElseStmt()); accept(x.getElseStmt()); From 96231ffcfcdad09d6885c95a0e6763f15ff9db34 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 28 Jan 2026 19:48:47 -0600 Subject: [PATCH 23/42] Update tests to drop extra braces when unnecessary --- .../com/google/gwt/dev/jjs/ast/JBlock.java | 14 +++++++ .../google/gwt/dev/jjs/ast/JIfStatement.java | 14 +++---- .../dev/jjs/impl/CatchBlockNormalizer.java | 2 +- .../gwt/dev/jjs/impl/DeadCodeElimination.java | 4 +- .../dev/jjs/impl/GenerateJavaScriptAST.java | 11 +---- .../gwt/dev/jjs/impl/GwtAstBuilder.java | 2 +- .../jjs/impl/ImplementRecordComponents.java | 5 +-- .../google/gwt/dev/jjs/impl/Simplifier.java | 24 +++-------- .../jjs/impl/ToStringGenerationVisitor.java | 22 +++++----- .../dev/jjs/impl/gflow/cfg/CfgBuilder.java | 8 ++-- .../dev/jjs/impl/DeadCodeEliminationTest.java | 5 ++- .../impl/ImplementCastsAndTypeChecksTest.java | 6 +-- .../gwt/dev/jjs/impl/MethodInlinerTest.java | 4 +- .../google/gwt/dev/jjs/impl/PrunerTest.java | 3 +- .../impl/SameParameterValueOptimizerTest.java | 3 +- .../jjs/impl/gflow/cfg/CfgBuilderTest.java | 40 ++++--------------- .../constants/ConstantsAnalysisTest.java | 5 +-- .../jjs/impl/gflow/copy/CopyAnalysisTest.java | 1 - 18 files changed, 67 insertions(+), 106 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java index 4d1b10630d2..28ecbd926d5 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java @@ -105,4 +105,18 @@ public boolean unconditionalControlBreak() { } return false; } + + public JStatement singleStatement() { + if (statements.isEmpty()) { + return null; + } + if (statements.size() == 1) { + JStatement jStatement = statements.get(0); + if (jStatement instanceof JBlock) { + return ((JBlock) jStatement).singleStatement(); + } + return jStatement; + } + return this; + } } diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java index e51ba40e993..7abefd3a94a 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java @@ -26,11 +26,11 @@ public class JIfStatement extends JStatement { private JExpression ifExpr; private JBlock thenStmt; - public JIfStatement(SourceInfo info, JExpression ifExpr, JBlock thenStmt, JBlock elseStmt) { + public JIfStatement(SourceInfo info, JExpression ifExpr, JStatement thenStmt, JStatement elseStmt) { super(info); this.ifExpr = ifExpr; - this.thenStmt = thenStmt == null ? new JBlock(info) : thenStmt; - this.elseStmt = elseStmt == null ? new JBlock(info) : elseStmt; + this.thenStmt = ensureBlock(info, thenStmt); + this.elseStmt = ensureBlock(info, elseStmt); } public JBlock getElseStmt() { @@ -49,15 +49,15 @@ public JBlock getThenStmt() { public void traverse(JVisitor visitor, Context ctx) { if (visitor.visit(this, ctx)) { ifExpr = visitor.accept(ifExpr); - thenStmt = ensureBlock(visitor.accept(thenStmt, true)); - elseStmt = ensureBlock(visitor.accept(elseStmt, true)); + thenStmt = ensureBlock(getSourceInfo(), visitor.accept(thenStmt, false)); + elseStmt = ensureBlock(getSourceInfo(), visitor.accept(elseStmt, false)); } visitor.endVisit(this, ctx); } - private JBlock ensureBlock(JStatement statement) { + private static JBlock ensureBlock(SourceInfo info, JStatement statement) { if (statement == null) { - return new JBlock(getSourceInfo()); + return new JBlock(info); } if (statement instanceof JBlock) { return (JBlock) statement; diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java index 1083f0bb580..c0c067c7993 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java @@ -119,7 +119,7 @@ public void endVisit(JTryStatement x, Context ctx) { new JDeclarationStatement(catchInfo, arg, exceptionVariable.makeRef(catchInfo)); block.addStmt(0, declaration); // nest the previous as an else for me - cur = new JIfStatement(catchInfo, ifTest, block, new JBlock(cur.getSourceInfo(), cur)); + cur = new JIfStatement(catchInfo, ifTest, block, cur); } newCatchBlock.addStmt(cur); diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java index 17caa5ecafa..8f035ac60d2 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java @@ -1964,10 +1964,8 @@ private void tryRemoveSwitch(JSwitchStatement x, Context ctx) { } else { // Create an if statement equivalent to the single-case switch. JBinaryOperation compareOperation = caseStatement.convertToCompareExpression(x.getExpr()); - JBlock block = new JBlock(x.getSourceInfo()); - block.addStmt(statement); JIfStatement ifStatement = - new JIfStatement(x.getSourceInfo(), compareOperation, block, null); + new JIfStatement(x.getSourceInfo(), compareOperation, statement, null); replaceMe(ifStatement, ctx); } } diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java index 00d0e4bdc06..ca538f51123 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java @@ -753,19 +753,12 @@ public JsNode transformIfStatement(JIfStatement ifStatement) { result.setIfExpr(transform(ifStatement.getIfExpr())); result.setThenStmt(jsEmptyIfNull(ifStatement.getSourceInfo(), - transform(ifStatement.getThenStmt()))); - result.setElseStmt(transform(ifStatement.getElseStmt())); + transform(ifStatement.getThenStmt().singleStatement()))); + result.setElseStmt(transform(ifStatement.getElseStmt().singleStatement())); return result; } - private JsStatement unwrapSingleStatement(JsBlock block) { - if (block.getStatements().size() == 1) { - return block.getStatements().get(0); - } - return block; - } - @Override public JsLabel transformLabel(JLabel label) { return new JsLabel(label.getSourceInfo(), names.get(label)); diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java index 3505c524e5f..968a1c4c469 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java @@ -2454,7 +2454,7 @@ private JBlock normalizeTryWithResources(SourceInfo info, TryStatement x, JBlock JExpression exceptionNotNull = new JBinaryOperation(info, JPrimitiveType.BOOLEAN, JBinaryOperator.NEQ, exceptionVar.makeRef(info), JNullLiteral.INSTANCE); finallyBlock.addStmt(new JIfStatement(info, exceptionNotNull, - new JBlock(info, new JThrowStatement(info, exceptionVar.makeRef(info))), null)); + new JThrowStatement(info, exceptionVar.makeRef(info)), null)); // Stitch all together into a inner try block outerTryBlock.addStmt(new JTryStatement(info, tryBlock, catchClauses, diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementRecordComponents.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementRecordComponents.java index 2d24a329a2e..27b451d72f8 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementRecordComponents.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementRecordComponents.java @@ -18,7 +18,6 @@ import com.google.gwt.dev.jjs.SourceInfo; import com.google.gwt.dev.jjs.ast.JBinaryOperation; import com.google.gwt.dev.jjs.ast.JBinaryOperator; -import com.google.gwt.dev.jjs.ast.JBlock; import com.google.gwt.dev.jjs.ast.JBooleanLiteral; import com.google.gwt.dev.jjs.ast.JClassLiteral; import com.google.gwt.dev.jjs.ast.JClassType; @@ -164,7 +163,7 @@ private void implementEquals(JRecordType type, JMethod method, SourceInfo info) new JThisRef(info, type), otherParam.createRef(info)); body.getBlock().addStmt(new JIfStatement(info, eq, - new JBlock(info, JBooleanLiteral.TRUE.makeReturnStatement()), null)); + JBooleanLiteral.TRUE.makeReturnStatement(), null)); // other == null JBinaryOperation nonNullCheck = @@ -182,7 +181,7 @@ private void implementEquals(JRecordType type, JMethod method, SourceInfo info) // if (other == null || MyRecordType.class != other.getClass()) return false; body.getBlock().addStmt(new JIfStatement(info, nullAndTypeCheck, - new JBlock(info, JBooleanLiteral.FALSE.makeReturnStatement()), null)); + JBooleanLiteral.FALSE.makeReturnStatement(), null)); // Create a local to assign to and compare each component JLocal typedOther = JProgram.createLocal(info, "other", type, true, body); diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java b/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java index 40fb72013ae..6e080578744 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java @@ -240,8 +240,8 @@ public static JExpression simplifyConditional(JConditional expression) { public static JStatement simplifyIfStatement(JIfStatement ifStatement, JType methodReturnType) { SourceInfo info = ifStatement.getSourceInfo(); JExpression conditionExpression = ifStatement.getIfExpr(); - JBlock thenStmt = ifStatement.getThenStmt(); - JBlock elseStmt = ifStatement.getElseStmt(); + final JBlock thenStmt = ifStatement.getThenStmt(); + final JBlock elseStmt = ifStatement.getElseStmt(); if (conditionExpression instanceof JMultiExpression) { // if(a,b,c) d else e -> {a; b; if(c) d else e; } JMultiExpression condMulti = (JMultiExpression) conditionExpression; @@ -516,25 +516,11 @@ private static JExpression extractExpression(JStatement statement) { return null; } - private static JStatement extractSingleStatement(JStatement statement) { - if (statement instanceof JBlock) { - JBlock block = (JBlock) statement; - if (block.isEmpty()) { - return null; - } - if (block.getStatements().size() == 1) { - return extractSingleStatement(block.getStatements().get(0)); - } - } - - return statement; - } - private static JStatement rewriteIfStatementAsExpression(SourceInfo sourceInfo, - JExpression conditionExpression, JStatement thenStmt, JStatement elseStmt, + JExpression conditionExpression, JBlock thenBlock, JBlock elseBlock, JType methodReturnType) { - thenStmt = extractSingleStatement(thenStmt); - elseStmt = extractSingleStatement(elseStmt); + JStatement thenStmt = thenBlock.singleStatement(); + JStatement elseStmt = elseBlock.singleStatement(); if (thenStmt instanceof JReturnStatement && elseStmt instanceof JReturnStatement && methodReturnType != null) { diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java index 1c9aa621e78..309c9d24012 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java @@ -537,13 +537,15 @@ public boolean visit(JIfStatement x, Context ctx) { accept(x.getIfExpr()); rparen(); - if (x.getThenStmt() != null) { - nestedStatementPush(x.getThenStmt()); - accept(x.getThenStmt()); - nestedStatementPop(x.getThenStmt()); + JStatement then = x.getThenStmt().singleStatement(); + if (then != null) { + nestedStatementPush(then); + accept(then); + nestedStatementPop(then); } - if (x.getElseStmt() != null && !x.getElseStmt().isEmpty()) { + JStatement elseStmt = x.getElseStmt().singleStatement(); + if (elseStmt != null) { if (needSemi) { semi(); newline(); @@ -552,15 +554,15 @@ public boolean visit(JIfStatement x, Context ctx) { needSemi = true; } print(CHARS_ELSE); - boolean elseIf = x.getElseStmt().getStatements().size() == 1 - && x.getElseStmt().getStatements().get(0) instanceof JIfStatement; + boolean elseIf = elseStmt instanceof JIfStatement; if (!elseIf) { nestedStatementPush(x.getElseStmt()); - accept(x.getElseStmt()); - nestedStatementPop(x.getElseStmt()); } else { space(); - accept(x.getElseStmt().getStatements().get(0)); + } + accept(x.getElseStmt()); + if (!elseIf) { + nestedStatementPop(x.getElseStmt()); } } diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilder.java b/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilder.java index 7bfa7c3cda2..7cfa654a807 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilder.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilder.java @@ -545,14 +545,14 @@ public boolean visit(JIfStatement x, Context ctx) { CfgIfNode node = addNode(new CfgIfNode(parent, x)); addNormalExit(node, CfgConditionalNode.THEN); - if (x.getThenStmt() != null) { - accept(x.getThenStmt()); + if (x.getThenStmt().singleStatement() != null) { + accept(x.getThenStmt().singleStatement()); } List thenExits = removeNormalExits(); addNormalExit(node, CfgConditionalNode.ELSE); - if (x.getElseStmt() != null) { - accept(x.getElseStmt()); + if (x.getElseStmt().singleStatement() != null) { + accept(x.getElseStmt().singleStatement()); } addExits(thenExits); diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java index d641d08ef0d..3f70d3d94be 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java @@ -571,7 +571,10 @@ protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod // If m is processed first, it will see the constructor as having side effects. // Then the constructor will become empty enabling m() become empty in the next round. // - assertEquals(0, DeadCodeElimination.exec(program, method)); + String before = method.toSource(); + int moreMods = DeadCodeElimination.exec(program, method); + String after = method.toSource(); + assertEquals("before: " + before + ", after: " + after, 0, moreMods); } return mods > 0; } diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementCastsAndTypeChecksTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementCastsAndTypeChecksTest.java index 1868d706c36..ad5f819d017 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementCastsAndTypeChecksTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementCastsAndTypeChecksTest.java @@ -31,8 +31,7 @@ public void testCastCheckIntoNullCheck() throws Exception { result.intoString( "EntryPoint$A a = new EntryPoint$A();", "a = null;", - "if (a != null) {", - "}"); + "if (a != null);"); } public void testRemoveCastCheck_exactType() throws Exception { @@ -42,8 +41,7 @@ public void testRemoveCastCheck_exactType() throws Exception { optimize("void", "A a = new A(); if (a instanceof A) {}"); result.intoString( "EntryPoint$A a = new EntryPoint$A();", - "if (a != null) {", - "}"); + "if (a != null);"); } @Override diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/MethodInlinerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/MethodInlinerTest.java index 0eac0f0df98..0d81dc71f87 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/MethodInlinerTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/MethodInlinerTest.java @@ -127,8 +127,8 @@ public void testDeadCodeElimination_notInlinable() throws Exception { + "else {switch(a) { case 1: a++; break; default: a=a+2; break; }; return a; }" + "}"); addSnippetClassDecl("static int fun2(int a)" + "{return fun1(a);}"); Result result = optimize("int", "return fun2(0);"); - assertEquals("static int fun1(int a){ if (a > 1) { return a;" - + " } else { switch (a) { case 1: ++a;" + assertEquals("static int fun1(int a){ if (a > 1) return a;" + + " else { switch (a) { case 1: ++a;" + " break; default: a = a + 2; }" + " return a; } }", getCanonicalSource(result.findMethod("fun1"))); assertEquals("static int fun2(int a){ return EntryPoint.fun1(a); }", diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java index 81ad2b67e42..700f9310226 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java @@ -218,9 +218,8 @@ public void testPrunerThenEqualityNormalizer() throws Exception { "return i;" )).intoString( "int i = 0;", - "if (null.nullField[i] == 0) {", + "if (null.nullField[i] == 0)", " i = 2;", - "}", "return i;" ); diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizerTest.java index bc496cf9f75..b4f35a3dccd 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizerTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizerTest.java @@ -78,8 +78,7 @@ public void testDontKillParameterValue_Binop() throws Exception { addSnippetClassDecl("static void foo(int i) { if (i == 2) {} int j = i; }"); optimizeMethod("foo", "void", "foo(1); ").intoString( - "if (1 == 2) {", - "}", + "if (1 == 2);", "int j = 1;"); } diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilderTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilderTest.java index 6318018ee9d..d63521900ce 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilderTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilderTest.java @@ -200,7 +200,6 @@ public void testIfStatement1() throws Exception { "STMT -> [*]", "READ(i) -> [*]", "COND (EntryPoint.i == 1) -> [THEN=*, ELSE=1]", - "BLOCK -> [*]", "STMT -> [*]", "WRITE(j, 2) -> [*]", "1: STMT -> [*]", @@ -220,11 +219,9 @@ public void testIfStatement2() throws Exception { "STMT -> [*]", "WRITE(i, 1) -> [*]", "COND ((EntryPoint.i = 1) == 2) -> [THEN=*, ELSE=1]", - "BLOCK -> [*]", "STMT -> [*]", "WRITE(j, 2) -> [2]", - "1: BLOCK -> [*]", - "STMT -> [*]", + "1: STMT -> [*]", "READ(j) -> [*]", "WRITE(k, EntryPoint.j) -> [*]", "2: END"); @@ -238,7 +235,6 @@ public void testIfStatement3() throws Exception { "COND (EntryPoint.b1) -> [ELSE=*, THEN=1]", "READ(b2) -> [*]", "1: COND (EntryPoint.b1 || EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "WRITE(j, 2) -> [*]", "2: END"); @@ -279,11 +275,9 @@ public void testDoStatementBreakNoLabel() throws Exception { "STMT -> [*]", "READ(b1) -> [*]", "COND (EntryPoint.b1) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [4]", - "2: BLOCK -> [*]", - "STMT -> [*]", + "2: STMT -> [*]", "3: BLOCK -> [*]", "STMT -> [*]", "WRITE(j, 2) -> [*]", @@ -302,11 +296,9 @@ public void testDoStatementContinueNoLabel() throws Exception { "STMT -> [*]", "READ(b1) -> [*]", "COND (EntryPoint.b1) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [1]", - "2: BLOCK -> [*]", - "STMT -> [*]", + "2: STMT -> [*]", "3: BLOCK -> [*]", "STMT -> [*]", "WRITE(j, 2) -> [*]", @@ -414,7 +406,6 @@ public void testThrowWithoutCatch2() throws Exception { "STMT -> [*]", "READ(b1) -> [*]", "COND (EntryPoint.b1) -> [THEN=*, ELSE=1]", - "BLOCK -> [*]", "STMT -> [*]", "READ(runtimeException) -> [*]", "THROW -> [2]", @@ -434,7 +425,6 @@ public void testWhileContinueNoLabel() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [1]", "2: STMT -> [*]", @@ -458,9 +448,8 @@ public void testWhileContinueWithLabel1() throws Exception { "COND (EntryPoint.b1) -> [THEN=*, ELSE=1]", "BLOCK -> [*]", "STMT -> [*]", - "READ(b2) -> [*]", + "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=3]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [2]", "3: STMT -> [*]", @@ -487,7 +476,6 @@ public void testWhileContinueWithLabel2() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=3]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [1]", "3: STMT -> [*]", @@ -509,7 +497,6 @@ public void testWhileContinueWithLabel3() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [1]", "2: STMT -> [*]", @@ -527,7 +514,6 @@ public void testWhileBreakNoLabel() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [3]", "2: STMT -> [*]", @@ -545,11 +531,9 @@ public void testWhileBreakNoLabel2() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [4]", - "2: BLOCK -> [*]", - "STMT -> [*]", + "2: STMT -> [*]", "3: READ(i) -> [*]", "COND (EntryPoint.i < 10) -> [THEN=*, ELSE=1]", "BLOCK -> [*]", @@ -576,7 +560,6 @@ public void testWhileBreakWithLabel1() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=3]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [1]", "3: STMT -> [*]", @@ -602,7 +585,6 @@ public void testWhileBreakWithLabel2() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=3]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [4]", "3: STMT -> [*]", @@ -621,7 +603,6 @@ public void testWhileBreakWithLabel3() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [3]", "2: STMT -> [*]", @@ -642,7 +623,6 @@ public void testForBreakNoLabel() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [3]", "2: STMT -> [*]", @@ -664,7 +644,6 @@ public void testForContinueNoLabel() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [3]", "2: STMT -> [*]", @@ -684,11 +663,9 @@ public void testForBreakNestedForWithLabel() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [4]", - "2: BLOCK -> [*]", - "STMT -> [*]", + "2: STMT -> [*]", "STMT -> [*]", "WRITE(i, 0) -> [*]", "3: READ(i) -> [*]", @@ -715,11 +692,9 @@ public void testForBreakNestedForNoLabel() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [4]", - "2: BLOCK -> [*]", - "STMT -> [*]", + "2: STMT -> [*]", "STMT -> [*]", "WRITE(i, 0) -> [*]", "3: READ(i) -> [*]", @@ -1678,7 +1653,6 @@ public void testBreakLoopAndSwitch() throws Exception { "STMT -> [*]", "READ(j) -> [*]", "COND (EntryPoint.j == 1) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [7]", "2: STMT -> [*]", diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsAnalysisTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsAnalysisTest.java index 1810c049342..b73a84c6594 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsAnalysisTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsAnalysisTest.java @@ -117,12 +117,10 @@ public void testIfStatement() throws Exception { "STMT -> [* T]", "READ(i) -> [* T]", "COND (i == 1) -> [THEN=* {i = 1}, ELSE=1 T]", - "BLOCK -> [* {i = 1}]", "STMT -> [* {i = 1}]", "READ(i) -> [* {i = 1}]", "WRITE(j, i) -> [2 {i = 1, j = 1}]", - "1: BLOCK -> [* T]", - "STMT -> [* T]", + "1: STMT -> [* T]", "READ(i) -> [* T]", "WRITE(j, i) -> [* T]", "2: END"); @@ -238,7 +236,6 @@ public void testParamNonConstant() throws Exception { "STMT -> [* T]", "READ(j) -> [* T]", "COND (j == 0) -> [THEN=* {j = 0}, ELSE=1 T]", - "BLOCK -> [* {j = 0}]", "STMT -> [* {j = 0}]", "WRITE(i, 0) -> [* {i = 0, j = 0}]", "1: STMT -> [* T]", diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/copy/CopyAnalysisTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/copy/CopyAnalysisTest.java index 70090ed9600..d63ae674af3 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/copy/CopyAnalysisTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/copy/CopyAnalysisTest.java @@ -79,7 +79,6 @@ public void testConditionalKill() throws Exception { "STMT -> [* {j = i}]", "READ(b) -> [* {j = i}]", "COND (EntryPoint.b) -> [THEN=* {j = i}, ELSE=1 {j = i}]", - "BLOCK -> [* {j = i}]", "STMT -> [* {j = i}]", "WRITE(j, 1) -> [* {j = T}]", "1: STMT -> [* {j = T}]", From 38fb2bd841a14e90014ddca2e1d54d8d801ede87 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 29 Jan 2026 18:08:46 -0600 Subject: [PATCH 24/42] Defensively add {}s to JS ifs --- .../dev/js/JsToStringGenerationVisitor.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java index 9bf04b54d17..28bba706ceb 100644 --- a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java @@ -145,6 +145,11 @@ public PrintOptions(boolean useLongIdents, boolean minifyLiterals) { } } + /** + * Tracks if we are within an IF without a block to guard against dangling-else ambiguity. + */ + private boolean inIfWithoutBraces = false; + /** * Generate the output string using short identifiers. */ @@ -555,15 +560,26 @@ public boolean visit(JsFunction x, JsContext ctx) { @Override public boolean visit(JsIf x, JsContext ctx) { + boolean outerBlock = false; + if (x.getElseStmt() != null && inIfWithoutBraces) { + // To avoid dangling-else ambiguity, if a nested IF with an ELSE must always be wrapped + // in a block + _blockOpen(); + outerBlock = true; + } _if(); _spaceOpt(); _lparen(); accept(x.getIfExpr()); _rparen(); + + inIfWithoutBraces = true; JsStatement thenStmt = x.getThenStmt(); _nestedPush(thenStmt, false); accept(thenStmt); _nestedPop(thenStmt); + + inIfWithoutBraces = true; JsStatement elseStmt = x.getElseStmt(); if (elseStmt != null) { if (needSemi) { @@ -585,6 +601,12 @@ public boolean visit(JsIf x, JsContext ctx) { _nestedPop(elseStmt); } } + + if (outerBlock) { + _blockClose(); + // Restore flag for later calls + inIfWithoutBraces = true; + } return false; } @@ -948,6 +970,7 @@ protected void printJsBlock(JsBlock x, boolean truncate, boolean finalNewline) { // Open braces. // _blockOpen(); + inIfWithoutBraces = false; } int count = 0; From d53e77d7adc9a06e8f3992aace458cb904f8d15f Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 29 Jan 2026 19:37:10 -0600 Subject: [PATCH 25/42] Improved version of adding necessary braces --- dev/build.xml | 2 +- .../dev/js/JsToStringGenerationVisitor.java | 28 ++------ .../js/JsToStringGenerationVisitorTest.java | 66 +++++++++++++++++++ 3 files changed, 72 insertions(+), 24 deletions(-) diff --git a/dev/build.xml b/dev/build.xml index 200f61a4732..8dd7924f04a 100755 --- a/dev/build.xml +++ b/dev/build.xml @@ -22,7 +22,7 @@ - + diff --git a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java index 28bba706ceb..a4c5d37c597 100644 --- a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java @@ -145,11 +145,6 @@ public PrintOptions(boolean useLongIdents, boolean minifyLiterals) { } } - /** - * Tracks if we are within an IF without a block to guard against dangling-else ambiguity. - */ - private boolean inIfWithoutBraces = false; - /** * Generate the output string using short identifiers. */ @@ -560,26 +555,20 @@ public boolean visit(JsFunction x, JsContext ctx) { @Override public boolean visit(JsIf x, JsContext ctx) { - boolean outerBlock = false; - if (x.getElseStmt() != null && inIfWithoutBraces) { - // To avoid dangling-else ambiguity, if a nested IF with an ELSE must always be wrapped - // in a block - _blockOpen(); - outerBlock = true; - } _if(); _spaceOpt(); _lparen(); accept(x.getIfExpr()); _rparen(); - - inIfWithoutBraces = true; JsStatement thenStmt = x.getThenStmt(); + if (!(thenStmt instanceof JsBlock) && x.getElseStmt() != null) { + JsBlock b = new JsBlock(thenStmt.getSourceInfo()); + b.getStatements().add(thenStmt); + thenStmt = b; + } _nestedPush(thenStmt, false); accept(thenStmt); _nestedPop(thenStmt); - - inIfWithoutBraces = true; JsStatement elseStmt = x.getElseStmt(); if (elseStmt != null) { if (needSemi) { @@ -601,12 +590,6 @@ public boolean visit(JsIf x, JsContext ctx) { _nestedPop(elseStmt); } } - - if (outerBlock) { - _blockClose(); - // Restore flag for later calls - inIfWithoutBraces = true; - } return false; } @@ -970,7 +953,6 @@ protected void printJsBlock(JsBlock x, boolean truncate, boolean finalNewline) { // Open braces. // _blockOpen(); - inIfWithoutBraces = false; } int count = 0; diff --git a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java index 4db0da3de13..f3ab1b61ca8 100644 --- a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java +++ b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java @@ -21,10 +21,17 @@ import com.google.gwt.dev.jjs.impl.DeadCodeElimination; import com.google.gwt.dev.jjs.impl.FullCompileTestBase; import com.google.gwt.dev.jjs.impl.FullOptimizerContext; +import com.google.gwt.dev.jjs.SourceOrigin; +import com.google.gwt.dev.js.ast.JsExprStmt; +import com.google.gwt.dev.js.ast.JsFunction; +import com.google.gwt.dev.js.ast.JsIf; +import com.google.gwt.dev.js.ast.JsStatement; import com.google.gwt.dev.util.DefaultTextOutput; import com.google.gwt.dev.util.TextOutput; import com.google.gwt.thirdparty.guava.common.collect.Maps; +import java.io.IOException; +import java.io.StringReader; import java.util.List; import java.util.Map; @@ -79,6 +86,65 @@ public void testClassRangeMarking() throws UnableToCompleteException { assertTrue(programClassRange.getEndPosition() < text.getPosition()); } + public void testDanglingElse() throws Exception { + // No braces, ELSE will be attached to the inner IF + String statements = """ + if (a) + if (b) + go(1); + else + go(2); + """; + JsStatement result = compileAndParseStatement(statements); + assertNotNull(result); + assertTrue(result instanceof JsIf); + JsIf outerIf = (JsIf) result; + assertTrue(outerIf.getThenStmt() instanceof JsIf); + JsIf innerIf = (JsIf) outerIf.getThenStmt(); + assertNotNull(innerIf.getElseStmt()); + assertNull(outerIf.getElseStmt()); + } + + private JsStatement compileAndParseStatement(String statements) throws UnableToCompleteException, IOException, JsParserException { + String code = """ + package test; + public class EntryPoint { + private static boolean a = true, b = true, c = true; + private static void go(int i) { + } + public static void onModuleLoad() { +""" + statements + """ + } + } + """; + compileSnippetToJS(code); + TextOutput text = new DefaultTextOutput(true); + JsSourceGenerationVisitor jsSourceGenerationVisitor = new JsSourceGenerationVisitor(text); + jsSourceGenerationVisitor.accept(jsProgram); + + List classRanges = jsSourceGenerationVisitor.getClassRanges(); + String entrypoint = classRanges.stream().filter(r -> r.getName().equals("test.EntryPoint")).findFirst().map(r -> { + return text.toString().substring(r.getStartPosition(), r.getEndPosition()); + }).get(); + + // Parse the source to be sure that the printed output has the expected characteristics + List parsed = JsParser.parse(SourceOrigin.UNKNOWN, jsProgram.getScope(), new StringReader(entrypoint)); + + JsStatement result = null; + for (JsStatement jsStatement : parsed) { + if (jsStatement instanceof JsExprStmt && ((JsExprStmt) jsStatement).getExpression() instanceof JsFunction) { + JsFunction jsFunction = (JsFunction) ((JsExprStmt) jsStatement).getExpression(); + if (jsFunction.getName().getShortIdent().equals("onModuleLoad")) { + // In case of clinit, return the last statement only + List s = jsFunction.getBody().getStatements(); + result = s.get(s.size() - 1); + break; + } + } + } + return result; + } + public void testLiteralPrint() throws UnableToCompleteException { TextOutput text = buildTextOutput(new JsToStringGenerationVisitor.PrintOptions(false, false)); From 362537d3721bc786568fb8d189f19fd0812258a7 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 30 Jan 2026 11:31:48 -0600 Subject: [PATCH 26/42] checkstyle --- .../com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java index f3ab1b61ca8..18498fe4b54 100644 --- a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java +++ b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java @@ -19,9 +19,9 @@ import com.google.gwt.dev.cfg.ConditionNone; import com.google.gwt.dev.cfg.ConfigurationProperty; import com.google.gwt.dev.jjs.impl.DeadCodeElimination; +import com.google.gwt.dev.jjs.SourceOrigin; import com.google.gwt.dev.jjs.impl.FullCompileTestBase; import com.google.gwt.dev.jjs.impl.FullOptimizerContext; -import com.google.gwt.dev.jjs.SourceOrigin; import com.google.gwt.dev.js.ast.JsExprStmt; import com.google.gwt.dev.js.ast.JsFunction; import com.google.gwt.dev.js.ast.JsIf; From ac18a9809899c0848ed0a312d2eb0b1515ef25b6 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 14 Feb 2026 22:07:37 -0600 Subject: [PATCH 27/42] fix java ast dump to avoid dangling else --- .../com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java index 309c9d24012..494cdc39917 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java @@ -539,6 +539,9 @@ public boolean visit(JIfStatement x, Context ctx) { JStatement then = x.getThenStmt().singleStatement(); if (then != null) { + if (x.getElseStmt() != null && !(then instanceof JBlock)) { + then = new JBlock(then.getSourceInfo(), then); + } nestedStatementPush(then); accept(then); nestedStatementPop(then); From ac44bd110e57515ba811981e811754a4fac322d7 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 14 Feb 2026 22:08:17 -0600 Subject: [PATCH 28/42] do/for/while support --- .../com/google/gwt/dev/jjs/ast/JBlock.java | 11 +++++++ .../google/gwt/dev/jjs/ast/JDoStatement.java | 10 +++---- .../google/gwt/dev/jjs/ast/JForStatement.java | 10 +++---- .../google/gwt/dev/jjs/ast/JIfStatement.java | 19 +++--------- .../gwt/dev/jjs/ast/JWhileStatement.java | 10 +++---- .../gwt/dev/jjs/impl/DeadCodeElimination.java | 11 +++++-- .../dev/jjs/impl/GenerateJavaScriptAST.java | 6 ++-- .../jjs/impl/ToStringGenerationVisitor.java | 27 +++++++++-------- .../dev/jjs/impl/gflow/cfg/CfgBuilder.java | 16 +++++----- .../dev/jjs/impl/DeadCodeEliminationTest.java | 6 ++-- .../jjs/impl/gflow/cfg/CfgBuilderTest.java | 30 +++++-------------- .../constants/ConstantsAnalysisTest.java | 5 ++-- 12 files changed, 74 insertions(+), 87 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java index 28ecbd926d5..986a4d492c2 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java @@ -33,6 +33,17 @@ public JBlock(SourceInfo info, JStatement... statements) { this.statements.addAll(Arrays.asList(statements)); } + public static JBlock ensureBlock(SourceInfo info, JStatement statement) { + if (statement == null) { + return new JBlock(info); + } + if (statement instanceof JBlock) { + return (JBlock) statement; + } + + return new JBlock(statement.getSourceInfo(), statement); + } + /** * Insert a statement into this block. */ diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JDoStatement.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JDoStatement.java index 65825e8e16c..642b17f61b9 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JDoStatement.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JDoStatement.java @@ -22,16 +22,16 @@ */ public class JDoStatement extends JStatement { - private JStatement body; + private JBlock body; private JExpression testExpr; public JDoStatement(SourceInfo info, JExpression testExpr, JStatement body) { super(info); this.testExpr = testExpr; - this.body = body; + this.body = JBlock.ensureBlock(info, body); } - public JStatement getBody() { + public JBlock getBody() { return body; } @@ -43,9 +43,7 @@ public JExpression getTestExpr() { public void traverse(JVisitor visitor, Context ctx) { if (visitor.visit(this, ctx)) { testExpr = visitor.accept(testExpr); - if (body != null) { - body = visitor.accept(body, true); - } + body = JBlock.ensureBlock(getSourceInfo(), visitor.accept(body, false)); } visitor.endVisit(this, ctx); } diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JForStatement.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JForStatement.java index d9b7c7f10cb..26e99e62431 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JForStatement.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JForStatement.java @@ -25,7 +25,7 @@ */ public class JForStatement extends JStatement { - private JStatement body; + private JBlock body; private List initializers; private JExpression condition; private JExpression increments; @@ -40,13 +40,13 @@ public JForStatement(SourceInfo info, List initializers, JExpression this.initializers = Lists.newArrayList(initializers); this.condition = condition; this.increments = increments; - this.body = body; + this.body = JBlock.ensureBlock(info, body); } /** * Returns the {@code for} statement body. */ - public JStatement getBody() { + public JBlock getBody() { return body; } @@ -81,9 +81,7 @@ public void traverse(JVisitor visitor, Context ctx) { if (increments != null) { increments = visitor.accept(increments); } - if (body != null) { - body = visitor.accept(body, true); - } + body = JBlock.ensureBlock(getSourceInfo(), visitor.accept(body, false));//TODO no tests fail without this change... } visitor.endVisit(this, ctx); } diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java index 7abefd3a94a..ecdd2223ad5 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java @@ -29,8 +29,8 @@ public class JIfStatement extends JStatement { public JIfStatement(SourceInfo info, JExpression ifExpr, JStatement thenStmt, JStatement elseStmt) { super(info); this.ifExpr = ifExpr; - this.thenStmt = ensureBlock(info, thenStmt); - this.elseStmt = ensureBlock(info, elseStmt); + this.thenStmt = JBlock.ensureBlock(info, thenStmt); + this.elseStmt = JBlock.ensureBlock(info, elseStmt); } public JBlock getElseStmt() { @@ -49,23 +49,12 @@ public JBlock getThenStmt() { public void traverse(JVisitor visitor, Context ctx) { if (visitor.visit(this, ctx)) { ifExpr = visitor.accept(ifExpr); - thenStmt = ensureBlock(getSourceInfo(), visitor.accept(thenStmt, false)); - elseStmt = ensureBlock(getSourceInfo(), visitor.accept(elseStmt, false)); + thenStmt = JBlock.ensureBlock(getSourceInfo(), visitor.accept(thenStmt, false)); + elseStmt = JBlock.ensureBlock(getSourceInfo(), visitor.accept(elseStmt, false)); } visitor.endVisit(this, ctx); } - private static JBlock ensureBlock(SourceInfo info, JStatement statement) { - if (statement == null) { - return new JBlock(info); - } - if (statement instanceof JBlock) { - return (JBlock) statement; - } - - return new JBlock(statement.getSourceInfo(), statement); - } - @Override public boolean unconditionalControlBreak() { boolean thenBreaks = thenStmt != null && thenStmt.unconditionalControlBreak(); diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JWhileStatement.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JWhileStatement.java index a6d7eabbd3b..4c14be3adf0 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JWhileStatement.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JWhileStatement.java @@ -22,16 +22,16 @@ */ public class JWhileStatement extends JStatement { - private JStatement body; + private JBlock body; private JExpression testExpr; public JWhileStatement(SourceInfo info, JExpression testExpr, JStatement body) { super(info); this.testExpr = testExpr; - this.body = body; + this.body = JBlock.ensureBlock(info, body); } - public JStatement getBody() { + public JBlock getBody() { return body; } @@ -43,9 +43,7 @@ public JExpression getTestExpr() { public void traverse(JVisitor visitor, Context ctx) { if (visitor.visit(this, ctx)) { testExpr = visitor.accept(testExpr); - if (body != null) { - body = visitor.accept(body, true); - } + body = JBlock.ensureBlock(getSourceInfo(), visitor.accept(body, false));//TODO no tests fail without this change... } visitor.endVisit(this, ctx); } diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java index 8f035ac60d2..11ef42f1ad4 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java @@ -328,7 +328,7 @@ public void endVisit(JDeclarationStatement x, Context ctx) { } /** - * Convert do { } while (false); into a block. + * Convert do { } while (false); into a block, or do { } while (true); into while(true) { }. */ @Override public void endVisit(JDoStatement x, Context ctx) { @@ -336,9 +336,9 @@ public void endVisit(JDoStatement x, Context ctx) { if (expression instanceof JBooleanLiteral) { JBooleanLiteral booleanLiteral = (JBooleanLiteral) expression; - // If false, replace do with do's body if (!booleanLiteral.getValue()) { - if (JjsUtils.isEmptyBlock(x.getBody())) { + // If false, replace do with do's body + if (x.getBody().isEmpty()) { ctx.removeMe(); } else { // Unless it contains break/continue statements FindBreakContinueStatementsVisitor visitor = new FindBreakContinueStatementsVisitor(); @@ -347,6 +347,11 @@ public void endVisit(JDoStatement x, Context ctx) { ctx.replaceMe(x.getBody()); } } +// } else { +// // If true, replace with while(true) { body } +// JWhileStatement whileStatement = new JWhileStatement(x.getSourceInfo(), expression, +// x.getBody()); +// ctx.replaceMe(whileStatement); } } } diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java index ca538f51123..9f26bab91a2 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java @@ -692,7 +692,7 @@ public JsNode transformDeclarationStatement(JDeclarationStatement declarationSta public JsNode transformDoStatement(JDoStatement doStatement) { JsDoWhile stmt = new JsDoWhile(doStatement.getSourceInfo()); stmt.setCondition(transform(doStatement.getTestExpr())); - stmt.setBody(jsEmptyIfNull(doStatement.getSourceInfo(), transform(doStatement.getBody()))); + stmt.setBody(jsEmptyIfNull(doStatement.getSourceInfo(), transform(doStatement.getBody().singleStatement()))); return stmt; } @@ -742,7 +742,7 @@ public JsNode transformForStatement(JForStatement forStatement) { result.setInitExpr(initExpr); result.setCondition(transform(forStatement.getCondition())); result.setIncrExpr(transform(forStatement.getIncrements())); - result.setBody(jsEmptyIfNull(forStatement.getSourceInfo(), transform(forStatement.getBody()))); + result.setBody(jsEmptyIfNull(forStatement.getSourceInfo(), transform(forStatement.getBody().singleStatement()))); return result; } @@ -1168,7 +1168,7 @@ public JsNode transformWhileStatement(JWhileStatement whileStatement) { SourceInfo info = whileStatement.getSourceInfo(); JsWhile stmt = new JsWhile(info); stmt.setCondition(transform(whileStatement.getTestExpr())); - stmt.setBody(jsEmptyIfNull(info, transform(whileStatement.getBody()))); + stmt.setBody(jsEmptyIfNull(info, transform(whileStatement.getBody().singleStatement()))); return stmt; } diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java index 494cdc39917..d848161a396 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java @@ -431,10 +431,11 @@ public boolean visit(JDeclarationStatement x, Context ctx) { public boolean visit(JDoStatement x, Context ctx) { print(CHARS_DO); needSemi = true; - if (x.getBody() != null) { - nestedStatementPush(x.getBody()); - accept(x.getBody()); - nestedStatementPop(x.getBody()); + JStatement body = x.getBody().singleStatement(); + if (body != null) { + nestedStatementPush(body); + accept(body); + nestedStatementPop(body); } if (needSemi) { semi(); @@ -522,10 +523,11 @@ public boolean visit(JForStatement x, Context ctx) { } rparen(); - if (x.getBody() != null) { - nestedStatementPush(x.getBody()); - accept(x.getBody()); - nestedStatementPop(x.getBody()); + JStatement body = x.getBody().singleStatement(); + if (body != null) { + nestedStatementPush(body); + accept(body); + nestedStatementPop(body); } return false; } @@ -925,10 +927,11 @@ public boolean visit(JWhileStatement x, Context ctx) { lparen(); accept(x.getTestExpr()); rparen(); - if (x.getBody() != null) { - nestedStatementPush(x.getBody()); - accept(x.getBody()); - nestedStatementPop(x.getBody()); + JStatement body = x.getBody().singleStatement(); + if (body != null) { + nestedStatementPush(body); + accept(body); + nestedStatementPop(body); } return false; } diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilder.java b/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilder.java index 7cfa654a807..838adce4651 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilder.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilder.java @@ -465,8 +465,8 @@ public boolean visit(JDoStatement x, Context ctx) { pushNode(new CfgStatementNode(parent, x)); int pos = nodes.size(); - if (x.getBody() != null) { - accept(x.getBody()); + if (!x.getBody().isEmpty()) { + accept(x.getBody().singleStatement()); } if (x.getTestExpr() != null) { @@ -510,8 +510,8 @@ public boolean visit(JForStatement x, Context ctx) { addNormalExit(cond, CfgConditionalNode.THEN); } - if (x.getBody() != null) { - accept(x.getBody()); + if (!x.getBody().isEmpty()) { + accept(x.getBody().singleStatement()); } int incrementsPos = nodes.size(); if (x.getIncrements() != null) { @@ -545,13 +545,13 @@ public boolean visit(JIfStatement x, Context ctx) { CfgIfNode node = addNode(new CfgIfNode(parent, x)); addNormalExit(node, CfgConditionalNode.THEN); - if (x.getThenStmt().singleStatement() != null) { + if (!x.getThenStmt().isEmpty()) { accept(x.getThenStmt().singleStatement()); } List thenExits = removeNormalExits(); addNormalExit(node, CfgConditionalNode.ELSE); - if (x.getElseStmt().singleStatement() != null) { + if (!x.getElseStmt().isEmpty()) { accept(x.getElseStmt().singleStatement()); } @@ -1012,8 +1012,8 @@ public boolean visit(JWhileStatement x, Context ctx) { CfgWhileNode node = addNode(new CfgWhileNode(parent, x)); addNormalExit(node, CfgConditionalNode.THEN); - if (x.getBody() != null) { - accept(x.getBody()); + if (!x.getBody().isEmpty()) { + accept(x.getBody().singleStatement()); } List thenExits = removeNormalExits(); diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java index 3f70d3d94be..d3e336a9dc5 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java @@ -268,7 +268,7 @@ public void testForOptimizations() { optimize("void", "for (i = 1; B.isFalse(); i++) { i = 2; }") .intoString("EntryPoint.i = 1;"); optimize("void", "for (i = 1; i < 5; i++) { i = 2; }") - .intoString("for (EntryPoint.i = 1; EntryPoint.i < 5; EntryPoint.i++) {\n EntryPoint.i = 2;\n}"); + .intoString("for (EntryPoint.i = 1; EntryPoint.i < 5; EntryPoint.i++)\n EntryPoint.i = 2;"); } /** @@ -397,9 +397,9 @@ public void testDoOptimization() throws Exception { optimize("void", "do {} while (false);").intoString(""); optimize("void", "do { i++; } while (false);").intoString("++EntryPoint.i;"); optimize("void", "do { break; } while (false);").intoString( - "do {", + "do", " break;", - "} while (false);"); + "while (false);"); } public void testNegationOptimizations() throws Exception { diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilderTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilderTest.java index d63521900ce..ff4abf6d023 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilderTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilderTest.java @@ -246,7 +246,6 @@ public void testWhileStatement() throws Exception { "STMT -> [*]", "1: READ(i) -> [*]", "COND (EntryPoint.i == 1) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "WRITE(j, 2) -> [1]", "2: STMT -> [*]", @@ -259,8 +258,7 @@ public void testDoStatement() throws Exception { assertCfg("void", "do { j = 2; } while (i == 1);").is( "BLOCK -> [*]", "STMT -> [*]", - "1: BLOCK -> [*]", - "STMT -> [*]", + "1: STMT -> [*]", "WRITE(j, 2) -> [*]", "READ(i) -> [*]", "COND (EntryPoint.i == 1) -> [THEN=1, ELSE=*]", @@ -271,15 +269,13 @@ public void testDoStatementBreakNoLabel() throws Exception { assertCfg("void", "do { if (b1) { break; } else { do { j = 2; } while (b2); } } while (i == 1);").is( "BLOCK -> [*]", "STMT -> [*]", - "1: BLOCK -> [*]", - "STMT -> [*]", + "1: STMT -> [*]", "READ(b1) -> [*]", "COND (EntryPoint.b1) -> [THEN=*, ELSE=2]", "STMT -> [*]", "GOTO -> [4]", "2: STMT -> [*]", - "3: BLOCK -> [*]", - "STMT -> [*]", + "3: STMT -> [*]", "WRITE(j, 2) -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=3, ELSE=*]", @@ -292,15 +288,13 @@ public void testDoStatementContinueNoLabel() throws Exception { assertCfg("void", "do { if (b1) { continue; } else { do { j = 2; } while (b2); } } while (i == 1);").is( "BLOCK -> [*]", "STMT -> [*]", - "1: BLOCK -> [*]", - "STMT -> [*]", + "1: STMT -> [*]", "READ(b1) -> [*]", "COND (EntryPoint.b1) -> [THEN=*, ELSE=2]", "STMT -> [*]", "GOTO -> [1]", "2: STMT -> [*]", - "3: BLOCK -> [*]", - "STMT -> [*]", + "3: STMT -> [*]", "WRITE(j, 2) -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=3, ELSE=*]", @@ -370,7 +364,6 @@ public void testForStatement() throws Exception { "WRITE(i, 0) -> [*]", "1: READ(i) -> [*]", "COND (i < 10) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "READWRITE(j, null) -> [*]", "READWRITE(i, null) -> [1]", @@ -385,8 +378,7 @@ public void testEmptyForStatement() throws Exception { "for (;;) { j++; }").is( "BLOCK -> [*]", "STMT -> [*]", - "1: BLOCK -> [*]", - "STMT -> [*]", + "1: STMT -> [*]", "READWRITE(j, null) -> [1]", "END"); } @@ -527,7 +519,6 @@ public void testWhileBreakNoLabel2() throws Exception { "STMT -> [*]", "1: READ(b1) -> [*]", "COND (EntryPoint.b1) -> [THEN=*, ELSE=4]", - "BLOCK -> [*]", "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", @@ -536,7 +527,6 @@ public void testWhileBreakNoLabel2() throws Exception { "2: STMT -> [*]", "3: READ(i) -> [*]", "COND (EntryPoint.i < 10) -> [THEN=*, ELSE=1]", - "BLOCK -> [*]", "STMT -> [*]", "READWRITE(i, null) -> [3]", "4: END"); @@ -659,8 +649,7 @@ public void testForBreakNestedForWithLabel() throws Exception { "STMT -> [*]", "WRITE(j, 0) -> [*]", "STMT -> [*]", - "1: BLOCK -> [*]", - "STMT -> [*]", + "1: STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", "STMT -> [*]", @@ -670,7 +659,6 @@ public void testForBreakNestedForWithLabel() throws Exception { "WRITE(i, 0) -> [*]", "3: READ(i) -> [*]", "COND (i < 1) -> [THEN=*, ELSE=1]", - "BLOCK -> [*]", "STMT -> [*]", "READ(i) -> [*]", "WRITE(j, i) -> [*]", @@ -688,8 +676,7 @@ public void testForBreakNestedForNoLabel() throws Exception { "STMT -> [*]", "WRITE(j, 0) -> [*]", "STMT -> [*]", - "1: BLOCK -> [*]", - "STMT -> [*]", + "1: STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", "STMT -> [*]", @@ -699,7 +686,6 @@ public void testForBreakNestedForNoLabel() throws Exception { "WRITE(i, 0) -> [*]", "3: READ(i) -> [*]", "COND (i < 1) -> [THEN=*, ELSE=1]", - "BLOCK -> [*]", "STMT -> [*]", "READ(i) -> [*]", "WRITE(j, i) -> [*]", diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsAnalysisTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsAnalysisTest.java index b73a84c6594..0c5abe41904 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsAnalysisTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsAnalysisTest.java @@ -178,9 +178,8 @@ public void testWhileLoop2() throws Exception { "WRITE(j, 0) -> [* {j = 0}]", "STMT -> [* {j = 0}]", "1: READ(j) -> [* {j = 0}]", - "COND (j > 0) -> [THEN=* {j = 0}, ELSE=2 {j = 0}]", - "BLOCK -> [1 {j = 0}]", - "2: END"); + "COND (j > 0) -> [THEN=1 {j = 0}, ELSE=* {j = 0}]", + "END"); } public void testConditionalExpressions() throws Exception { From 42baabefda2370fed0167234e192ccfd46a46295 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sun, 15 Feb 2026 08:22:04 -0600 Subject: [PATCH 29/42] if's else can never be null, fixed other tests --- .../gwt/dev/jjs/impl/ToStringGenerationVisitor.java | 2 +- .../gwt/dev/jjs/impl/ImplementJsVarargsTest.java | 12 ++++-------- .../com/google/gwt/dev/jjs/impl/Java10AstTest.java | 6 ++---- .../google/gwt/dev/jjs/impl/MethodInlinerTest.java | 4 ++-- 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java index d848161a396..27f95a2b511 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java @@ -541,7 +541,7 @@ public boolean visit(JIfStatement x, Context ctx) { JStatement then = x.getThenStmt().singleStatement(); if (then != null) { - if (x.getElseStmt() != null && !(then instanceof JBlock)) { + if (!x.getElseStmt().isEmpty() && !(then instanceof JBlock)) { then = new JBlock(then.getSourceInfo(), then); } nestedStatementPush(then); diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementJsVarargsTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementJsVarargsTest.java index f5b368d71e0..b51718e3f1f 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementJsVarargsTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementJsVarargsTest.java @@ -89,9 +89,8 @@ public void testOptimizedArguments_writeToArguments() throws Exception { "public static void m(Object[] _arguments_){", " {", " Object[] obj = new Object[][_arguments_.length];", - " for (int $i = 0; $i < _arguments_.length; $i++) {", + " for (int $i = 0; $i < _arguments_.length; $i++)", " obj[$i] = _arguments_[$i];", - " }", " }", " obj[5] = Integer.valueOf(1);", "}"), result.findMethod("test.EntryPoint$A.m([Ljava/lang/Object;)V").toSource()); @@ -111,9 +110,8 @@ public void testOptimizedArguments_postIncrement() throws Exception { "public static void m(int[] _arguments_){", " {", " int[] obj = new int[][_arguments_.length];", - " for (int $i = 0; $i < _arguments_.length; $i++) {", + " for (int $i = 0; $i < _arguments_.length; $i++)", " obj[$i] = _arguments_[$i];", - " }", " }", " obj[5]++;", "}"), result.findMethod("test.EntryPoint$A.m([I)V").toSource()); @@ -133,9 +131,8 @@ public void testOptimizedArguments_preDecrement() throws Exception { "public static void m(int[] _arguments_){", " {", " int[] obj = new int[][_arguments_.length];", - " for (int $i = 0; $i < _arguments_.length; $i++) {", + " for (int $i = 0; $i < _arguments_.length; $i++)", " obj[$i] = _arguments_[$i];", - " }", " }", " --obj[5];", "}"), result.findMethod("test.EntryPoint$A.m([I)V").toSource()); @@ -156,9 +153,8 @@ public void testOptimizedArguments_call() throws Exception { "public static void m(int[] _arguments_){", " {", " int[] obj = new int[][_arguments_.length];", - " for (int $i = 0; $i < _arguments_.length; $i++) {", + " for (int $i = 0; $i < _arguments_.length; $i++)", " obj[$i] = _arguments_[$i];", - " }", " }", " EntryPoint$A.n(obj);", "}"), result.findMethod("test.EntryPoint$A.m([I)V").toSource()); diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/Java10AstTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/Java10AstTest.java index d28c81ba9ba..4c30a7f3061 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/Java10AstTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/Java10AstTest.java @@ -72,18 +72,16 @@ public void testLocalVarType_ForLoop() throws Exception { public void testLocalVarType_EnhancedForLoopArray() throws Exception { assertEqualBlock( "for(final String[] s$array=new String[]{},int s$index=0,final int s$max=s$array.length;" - + " s$index)null);" ); diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/MethodInlinerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/MethodInlinerTest.java index 0d81dc71f87..0eac0f0df98 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/MethodInlinerTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/MethodInlinerTest.java @@ -127,8 +127,8 @@ public void testDeadCodeElimination_notInlinable() throws Exception { + "else {switch(a) { case 1: a++; break; default: a=a+2; break; }; return a; }" + "}"); addSnippetClassDecl("static int fun2(int a)" + "{return fun1(a);}"); Result result = optimize("int", "return fun2(0);"); - assertEquals("static int fun1(int a){ if (a > 1) return a;" - + " else { switch (a) { case 1: ++a;" + assertEquals("static int fun1(int a){ if (a > 1) { return a;" + + " } else { switch (a) { case 1: ++a;" + " break; default: a = a + 2; }" + " return a; } }", getCanonicalSource(result.findMethod("fun1"))); assertEquals("static int fun2(int a){ return EntryPoint.fun1(a); }", From 19a0aa50006f76ebfa8407ae3d95cf4154178328 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 24 Feb 2026 15:02:56 -0600 Subject: [PATCH 30/42] Updated draft, trying to unify logic, avoid copy/paste --- .../impl/RemoveUnnecessaryControlFlow.java | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java new file mode 100644 index 00000000000..2883c8e4e43 --- /dev/null +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java @@ -0,0 +1,122 @@ +package com.google.gwt.dev.jjs.impl; + +import com.google.gwt.dev.jjs.ast.Context; +import com.google.gwt.dev.jjs.ast.JBlock; +import com.google.gwt.dev.jjs.ast.JBreakStatement; +import com.google.gwt.dev.jjs.ast.JDoStatement; +import com.google.gwt.dev.jjs.ast.JForStatement; +import com.google.gwt.dev.jjs.ast.JIfStatement; +import com.google.gwt.dev.jjs.ast.JMethod; +import com.google.gwt.dev.jjs.ast.JMethodBody; +import com.google.gwt.dev.jjs.ast.JModVisitor; +import com.google.gwt.dev.jjs.ast.JPrimitiveType; +import com.google.gwt.dev.jjs.ast.JProgram; +import com.google.gwt.dev.jjs.ast.JReturnStatement; +import com.google.gwt.dev.jjs.ast.JStatement; +import com.google.gwt.dev.jjs.ast.JWhileStatement; + +import java.util.List; + +/** + * Finds return statements that are effectively at the end of a void method and removes them, + * or replaces them with something shorter in compiled output (e.g. break instead of return in a + * loop). + */ +public class RemoveUnnecessaryControlFlow { + private static String NAME = RemoveUnnecessaryControlFlow.class.getSimpleName(); + + public static int exec(JProgram program, OptimizerContext optimizerCtx) { + try (OptimizerStats stats = OptimizerStats.optimization(NAME)) { + + new RewriteUnnecessaryReturnsVisitor(optimizerCtx).accept(program); + + optimizerCtx.incOptimizationStep(); + return stats.getNumMods(); + } + + } + + private static class RewriteUnnecessaryReturnsVisitor extends JModVisitor { + private final OptimizerContext optimizerCtx; + + public RewriteUnnecessaryReturnsVisitor(OptimizerContext optimizerCtx) { + this.optimizerCtx = optimizerCtx; + } + + @Override + public boolean visit(JMethod x, Context ctx) { + if (x.getType() == JPrimitiveType.VOID && x.getBody() instanceof JMethodBody b) { + BlockLevel.BLOCK.update(x, b.getBlock(), optimizerCtx); + } + return false; + } + + private enum BlockLevel { + /** + * Neither this block nor any containing block are part of a loop. + */ + BLOCK, + /** + * This block (or a containing block) is part of a loop, so "return"s cannot be dropped but + * converted to breaks, and nested loops cannot be converted to breaks. + */ + LOOP { + @Override + protected void rewrite(List stmts, int lastIndex) { + stmts.set(lastIndex, new JBreakStatement(stmts.get(lastIndex).getSourceInfo(), null)); + } + }, + /** + * This block is in a loop which is contained in another loop, so "return"s cannot be omitted + * or converted to breaks. + */ + LOOP_IN_LOOP { + @Override + public void update(JMethod containingMethod, JBlock block, OptimizerContext ctx) { + // no-op, we can't do anything + } + }; + + public void update(JMethod containingMethod, JBlock block, OptimizerContext ctx) { + List stmts = block.getStatements(); + if (stmts.isEmpty()) { + return; + } + int lastIndex = stmts.size() - 1; + if (stmts.get(lastIndex) instanceof JReturnStatement ret && ret.getExpr() == null) { + rewrite(stmts, lastIndex); + ctx.markModified(containingMethod); + + if (stmts.isEmpty()) { + return; + } + lastIndex = stmts.size() - 1; + } + // Even if we already made a change, continue on, we could have an earlier return in a branch + // or a loop. None of these will remove the statement in question, so we don't need to iterate. + + if (stmts.get(lastIndex) instanceof JBlock b) { + update(containingMethod, b, ctx); + } else if (stmts.get(lastIndex) instanceof JIfStatement ifStmt) { + update(containingMethod, ifStmt.getThenStmt(), ctx); + update(containingMethod, ifStmt.getElseStmt(), ctx); + } else if (stmts.get(lastIndex) instanceof JWhileStatement whileStmt) { + loop().update(containingMethod, whileStmt.getBody(), ctx); + } else if (stmts.get(lastIndex) instanceof JForStatement forStmt) { + loop().update(containingMethod, forStmt.getBody(), ctx); + } else if (stmts.get(lastIndex) instanceof JDoStatement doStmt) { + loop().update(containingMethod, doStmt.getBody(), ctx); + } // TODO handle switches, try/catch/finally + + } + + protected void rewrite(List stmts, int lastIndex) { + stmts.remove(lastIndex); + } + + private BlockLevel loop() { + return this == BLOCK ? LOOP : LOOP_IN_LOOP; + } + } + } +} From 3c6fa133eab2e15f03bf6325dc4b6efdedfe4758 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 2 Mar 2026 21:21:54 -0600 Subject: [PATCH 31/42] Also remove continue;s, add a simple test --- .../gwt/dev/jjs/JavaToJavaScriptCompiler.java | 2 + .../impl/RemoveUnnecessaryControlFlow.java | 116 ++++++++++++++---- .../RemoveUnnecessaryControlFlowTest.java | 80 ++++++++++++ 3 files changed, 176 insertions(+), 22 deletions(-) create mode 100644 dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java diff --git a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java index d11ceb05479..d3bc3339c97 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java +++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java @@ -107,6 +107,7 @@ import com.google.gwt.dev.jjs.impl.RecordRebinds; import com.google.gwt.dev.jjs.impl.RemoveEmptySuperCalls; import com.google.gwt.dev.jjs.impl.RemoveSpecializations; +import com.google.gwt.dev.jjs.impl.RemoveUnnecessaryControlFlow; import com.google.gwt.dev.jjs.impl.ReplaceCallsToNativeJavaLangObjectOverrides; import com.google.gwt.dev.jjs.impl.ReplaceGetClassOverrides; import com.google.gwt.dev.jjs.impl.ResolvePermutationDependentValues; @@ -1471,6 +1472,7 @@ private void optimizeJavaToFixedPoint() throws InterruptedException { stats.recordModified(MethodCallTightener.exec(jprogram, optimizerCtx)); // Note: Specialization should be done before inlining. stats.recordModified(MethodCallSpecializer.exec(jprogram, optimizerCtx)); + stats.recordModified(RemoveUnnecessaryControlFlow.exec(jprogram, optimizerCtx)); stats.recordModified(DeadCodeElimination.exec(jprogram, optimizerCtx)); stats.recordModified(MethodInliner.exec(jprogram, optimizerCtx)); if (options.shouldInlineLiteralParameters()) { diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java index 2883c8e4e43..3b50f99b253 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java @@ -1,8 +1,22 @@ +/* + * Copyright 2026 GWT Project Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ package com.google.gwt.dev.jjs.impl; import com.google.gwt.dev.jjs.ast.Context; import com.google.gwt.dev.jjs.ast.JBlock; import com.google.gwt.dev.jjs.ast.JBreakStatement; +import com.google.gwt.dev.jjs.ast.JContinueStatement; import com.google.gwt.dev.jjs.ast.JDoStatement; import com.google.gwt.dev.jjs.ast.JForStatement; import com.google.gwt.dev.jjs.ast.JIfStatement; @@ -21,9 +35,16 @@ * Finds return statements that are effectively at the end of a void method and removes them, * or replaces them with something shorter in compiled output (e.g. break instead of return in a * loop). + *

    + * Additionally, finds "continue" statements (without labels) at the end of a loop (or in a non-loop + * block) and removes them as unnecessary. + *

    + * Presently may require multiple passes to converge, but since it is unlikely to remove many such + * statements, this will be addressed by just allowing another loop to find the changes. Additional + * changes from other optimizations (mostly DCE) can expose other opportunities for this pass. */ public class RemoveUnnecessaryControlFlow { - private static String NAME = RemoveUnnecessaryControlFlow.class.getSimpleName(); + private static final String NAME = RemoveUnnecessaryControlFlow.class.getSimpleName(); public static int exec(JProgram program, OptimizerContext optimizerCtx) { try (OptimizerStats stats = OptimizerStats.optimization(NAME)) { @@ -38,6 +59,7 @@ public static int exec(JProgram program, OptimizerContext optimizerCtx) { private static class RewriteUnnecessaryReturnsVisitor extends JModVisitor { private final OptimizerContext optimizerCtx; + private JMethod currentMethod; public RewriteUnnecessaryReturnsVisitor(OptimizerContext optimizerCtx) { this.optimizerCtx = optimizerCtx; @@ -45,10 +67,32 @@ public RewriteUnnecessaryReturnsVisitor(OptimizerContext optimizerCtx) { @Override public boolean visit(JMethod x, Context ctx) { + currentMethod = x; if (x.getType() == JPrimitiveType.VOID && x.getBody() instanceof JMethodBody b) { - BlockLevel.BLOCK.update(x, b.getBlock(), optimizerCtx); + BlockLevel.BLOCK.updateReturns(x, b.getBlock(), optimizerCtx); } - return false; + return true; + } + + @Override + public void endVisit(JMethod x, Context ctx) { + currentMethod = null; + } + + @Override + public void endVisit(JForStatement x, Context ctx) { + assert currentMethod != null; + BlockLevel.LOOP.updateContinues(currentMethod, x.getBody(), optimizerCtx); + } + + @Override + public void endVisit(JWhileStatement x, Context ctx) { + BlockLevel.LOOP.updateContinues(currentMethod, x.getBody(), optimizerCtx); + } + + @Override + public void endVisit(JDoStatement x, Context ctx) { + BlockLevel.LOOP.updateContinues(currentMethod, x.getBody(), optimizerCtx); } private enum BlockLevel { @@ -62,7 +106,7 @@ private enum BlockLevel { */ LOOP { @Override - protected void rewrite(List stmts, int lastIndex) { + protected void rewriteReturns(List stmts, int lastIndex) { stmts.set(lastIndex, new JBreakStatement(stmts.get(lastIndex).getSourceInfo(), null)); } }, @@ -72,45 +116,73 @@ protected void rewrite(List stmts, int lastIndex) { */ LOOP_IN_LOOP { @Override - public void update(JMethod containingMethod, JBlock block, OptimizerContext ctx) { + public void updateReturns(JMethod containingMethod, JBlock block, OptimizerContext ctx) { // no-op, we can't do anything } }; - public void update(JMethod containingMethod, JBlock block, OptimizerContext ctx) { + public void updateReturns(JMethod containingMethod, JBlock block, OptimizerContext ctx) { List stmts = block.getStatements(); if (stmts.isEmpty()) { return; } int lastIndex = stmts.size() - 1; - if (stmts.get(lastIndex) instanceof JReturnStatement ret && ret.getExpr() == null) { - rewrite(stmts, lastIndex); + JStatement lastStmt = stmts.get(lastIndex); + if (lastStmt instanceof JReturnStatement ret && ret.getExpr() == null) { + rewriteReturns(stmts, lastIndex); ctx.markModified(containingMethod); if (stmts.isEmpty()) { return; } - lastIndex = stmts.size() - 1; + lastStmt = stmts.get(stmts.size() - 1); } // Even if we already made a change, continue on, we could have an earlier return in a branch // or a loop. None of these will remove the statement in question, so we don't need to iterate. + if (lastStmt instanceof JBlock b) { + updateReturns(containingMethod, b, ctx); + } else if (lastStmt instanceof JIfStatement ifStmt) { + updateReturns(containingMethod, ifStmt.getThenStmt(), ctx); + updateReturns(containingMethod, ifStmt.getElseStmt(), ctx); + } else if (lastStmt instanceof JWhileStatement whileStmt) { + loop().updateReturns(containingMethod, whileStmt.getBody(), ctx); + } else if (lastStmt instanceof JForStatement forStmt) { + loop().updateReturns(containingMethod, forStmt.getBody(), ctx); + } else if (lastStmt instanceof JDoStatement doStmt) { + loop().updateReturns(containingMethod, doStmt.getBody(), ctx); + } + + // TODO handle switches, try/catch/finally + } + + public void updateContinues(JMethod containingMethod, JBlock block, OptimizerContext ctx) { + if (block.isEmpty()) { + return; + } + List stmts = block.getStatements(); + int lastIndex = stmts.size() - 1; + JStatement lastStmt = stmts.get(lastIndex); + if (stmts.get(lastIndex) instanceof JContinueStatement cont && cont.getLabel() == null) { + stmts.remove(lastIndex); + ctx.markModified(containingMethod); + + if (stmts.isEmpty()) { + return; + } + lastStmt = stmts.get(stmts.size() - 1); + } - if (stmts.get(lastIndex) instanceof JBlock b) { - update(containingMethod, b, ctx); - } else if (stmts.get(lastIndex) instanceof JIfStatement ifStmt) { - update(containingMethod, ifStmt.getThenStmt(), ctx); - update(containingMethod, ifStmt.getElseStmt(), ctx); - } else if (stmts.get(lastIndex) instanceof JWhileStatement whileStmt) { - loop().update(containingMethod, whileStmt.getBody(), ctx); - } else if (stmts.get(lastIndex) instanceof JForStatement forStmt) { - loop().update(containingMethod, forStmt.getBody(), ctx); - } else if (stmts.get(lastIndex) instanceof JDoStatement doStmt) { - loop().update(containingMethod, doStmt.getBody(), ctx); - } // TODO handle switches, try/catch/finally + if (lastStmt instanceof JBlock b) { + updateContinues(containingMethod, b, ctx); + } else if (lastStmt instanceof JIfStatement ifStmt) { + updateContinues(containingMethod, ifStmt.getThenStmt(), ctx); + updateContinues(containingMethod, ifStmt.getElseStmt(), ctx); + } // Don't handle nested loops here, will be handled when visited. + // TODO handle switches, try/catch/finally } - protected void rewrite(List stmts, int lastIndex) { + protected void rewriteReturns(List stmts, int lastIndex) { stmts.remove(lastIndex); } diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java new file mode 100644 index 00000000000..e1da7cef132 --- /dev/null +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java @@ -0,0 +1,80 @@ +/* + * Copyright 2026 GWT Project Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.gwt.dev.jjs.impl; + +import com.google.gwt.core.ext.TreeLogger; +import com.google.gwt.core.ext.UnableToCompleteException; +import com.google.gwt.dev.jjs.ast.JMethod; +import com.google.gwt.dev.jjs.ast.JProgram; + +public class RemoveUnnecessaryControlFlowTest extends OptimizerTestBase { + + @Override + protected void setUp() throws Exception { + super.setUp(); + + addSnippetClassDecl("static boolean condition() { return true; }"); + addSnippetClassDecl("static void foo() { }"); + } + + public void testReturnInSimpleBlock() throws UnableToCompleteException { + optimize("void", "return;").into(""); + optimize("void", "{return;}").into("{}"); + + optimize("void", "foo();return;").into("foo();"); + optimize("void", "{foo();return;}").into("{foo();}"); + optimize("void", "foo();{return;}").into("foo();{}"); + + // We could also remove these, resulting in returning undefined. + optimize("String", "return null;").noChange(); + optimize("String", "{return null;}").noChange(); + } + + public void testReturnInIf() throws UnableToCompleteException { + optimize("void", "if (condition()) { return; }") + .into("if (condition()) { }"); + optimize("void", "if (condition()) { return; } return;") + .into("if (condition()) { }"); + + optimize("void", "if (condition()) { return; } foo();").noChange(); + + optimize("void", "if (condition()) { return; } else { return; }") + .into("if (condition()) { } else { }"); + optimize("void", "if (condition()) { foo(); } else { return; }") + .into("if (condition()) { foo(); } else { }"); + + optimize("void", "if (condition()) { if (condition()) { return; } foo(); return; }") + .into("if (condition()) { if (condition()) { return; } foo(); }"); + } + + public void testReturnInLoop() throws UnableToCompleteException { + optimize("void", "while (condition()) { return; }") + .into("while (condition()) { break; }"); + optimize("void", "while (condition()) { return; } return;") + .into("while (condition()) { break; }"); + + optimize("void", "while (condition()) { return; } foo();").noChange(); + } + + @Override + protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod method) + throws UnableToCompleteException { + // Not presently guaranteed to converge in a single pass, so loop until it does. + int mods; + do { + mods = RemoveUnnecessaryControlFlow.exec(program, OptimizerContext.NULL_OPTIMIZATION_CONTEXT); + } while (mods > 0); + return false; + } +} \ No newline at end of file From fafdf661a02bc668784f6479cadec69174f152a5 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 2 Mar 2026 21:29:25 -0600 Subject: [PATCH 32/42] Support 17 for dev core too --- dev/build.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/build.xml b/dev/build.xml index 8dd7924f04a..c17554e1239 100755 --- a/dev/build.xml +++ b/dev/build.xml @@ -197,7 +197,7 @@ description="Validates that the standalone gwt-compiler project can build."> - + From 62b4cd42b06af1f84490140f8880a9e41a5cd1e7 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 10 Jun 2026 13:25:09 -0500 Subject: [PATCH 33/42] checkstyle, revert unneeded java 17 changes --- dev/build.xml | 4 ++-- .../gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/dev/build.xml b/dev/build.xml index c17554e1239..200f61a4732 100755 --- a/dev/build.xml +++ b/dev/build.xml @@ -22,7 +22,7 @@ - + @@ -197,7 +197,7 @@ description="Validates that the standalone gwt-compiler project can build."> - + diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java index 3b50f99b253..2c1a976a782 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java @@ -48,20 +48,18 @@ public class RemoveUnnecessaryControlFlow { public static int exec(JProgram program, OptimizerContext optimizerCtx) { try (OptimizerStats stats = OptimizerStats.optimization(NAME)) { - new RewriteUnnecessaryReturnsVisitor(optimizerCtx).accept(program); optimizerCtx.incOptimizationStep(); return stats.getNumMods(); } - } private static class RewriteUnnecessaryReturnsVisitor extends JModVisitor { private final OptimizerContext optimizerCtx; private JMethod currentMethod; - public RewriteUnnecessaryReturnsVisitor(OptimizerContext optimizerCtx) { + private RewriteUnnecessaryReturnsVisitor(OptimizerContext optimizerCtx) { this.optimizerCtx = optimizerCtx; } @@ -137,8 +135,9 @@ public void updateReturns(JMethod containingMethod, JBlock block, OptimizerConte } lastStmt = stmts.get(stmts.size() - 1); } - // Even if we already made a change, continue on, we could have an earlier return in a branch - // or a loop. None of these will remove the statement in question, so we don't need to iterate. + // Even if we already made a change, continue on, we could have an earlier return in a + // branch or a loop. None of these will remove the statement in question, so we don't need + // to iterate. if (lastStmt instanceof JBlock b) { updateReturns(containingMethod, b, ctx); } else if (lastStmt instanceof JIfStatement ifStmt) { From 8d5288e4606781326a901605bf2a2d3e30963ba6 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 24 Jun 2026 11:09:27 -0500 Subject: [PATCH 34/42] Remove java 26 from list for now --- .github/workflows/full-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/full-check.yml b/.github/workflows/full-check.yml index 295160026a3..cb81d9b05fb 100644 --- a/.github/workflows/full-check.yml +++ b/.github/workflows/full-check.yml @@ -1,4 +1,4 @@ -# Run all tests and builds all aspects of GWT using Java 17, 21, 25, and 26. Runs +# Run all tests and builds all aspects of GWT using Java 17, 21, and 25. Runs # nightly (plus or minus timzeones) on the main branch, and will also run right # away on a push to a release branch. Release zips are uploaded as part of the # build, though maven snapshots are not yet deployed. From 4cce622afb4d85060aaf01e13941ece2553ec1c8 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 24 Jun 2026 11:09:49 -0500 Subject: [PATCH 35/42] Revert "Temp tools branch" This reverts commit c70fb473b4f5e957ecb3c7f2ada0498eae20b4c3. --- .github/workflows/full-check.yml | 3 +-- .github/workflows/quick-check.yml | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/full-check.yml b/.github/workflows/full-check.yml index cb81d9b05fb..ebd0c063766 100644 --- a/.github/workflows/full-check.yml +++ b/.github/workflows/full-check.yml @@ -33,8 +33,7 @@ jobs: - name: Checkout GWT tools into a sibling directory uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 #v6.0.3 with: - repository: 'Vertispan/tools' - ref: 'asm-9.10.1' + repository: 'gwtproject/tools' path: 'tools' - name: Set up JDK ${{ matrix.java-version }} # GWT requires Java 17+ to build diff --git a/.github/workflows/quick-check.yml b/.github/workflows/quick-check.yml index bc76f48558b..27213d661bd 100644 --- a/.github/workflows/quick-check.yml +++ b/.github/workflows/quick-check.yml @@ -20,8 +20,7 @@ jobs: - name: Checkout GWT tools into a sibling directory uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 #v6.0.3 with: - repository: 'Vertispan/tools' - ref: 'asm-9.10.1' + repository: 'gwtproject/tools' path: 'tools' - name: Set up JDK ${{ matrix.java-version }} # GWT presently requires Java 17+ to build From ad0946e23654dc8dfe4cda6eed066da06570f85e Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 24 Jun 2026 16:31:51 -0500 Subject: [PATCH 36/42] Revert do->while rewrite, unrelated --- .../com/google/gwt/dev/jjs/impl/DeadCodeElimination.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java index 11ef42f1ad4..b8e52713460 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java @@ -328,7 +328,7 @@ public void endVisit(JDeclarationStatement x, Context ctx) { } /** - * Convert do { } while (false); into a block, or do { } while (true); into while(true) { }. + * Convert do { } while (false); into a block. */ @Override public void endVisit(JDoStatement x, Context ctx) { @@ -347,11 +347,6 @@ public void endVisit(JDoStatement x, Context ctx) { ctx.replaceMe(x.getBody()); } } -// } else { -// // If true, replace with while(true) { body } -// JWhileStatement whileStatement = new JWhileStatement(x.getSourceInfo(), expression, -// x.getBody()); -// ctx.replaceMe(whileStatement); } } } From 955e3d569f810021a2186b1a6f4ff28f18ed81b9 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 24 Jun 2026 16:47:02 -0500 Subject: [PATCH 37/42] Remove a comment that can't be handled in the java pass anyway --- .../gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java index e1da7cef132..139a1642337 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java @@ -36,7 +36,6 @@ public void testReturnInSimpleBlock() throws UnableToCompleteException { optimize("void", "{foo();return;}").into("{foo();}"); optimize("void", "foo();{return;}").into("foo();{}"); - // We could also remove these, resulting in returning undefined. optimize("String", "return null;").noChange(); optimize("String", "{return null;}").noChange(); } From ae4df095464ccfe8df3328c35bd3f49304b29b61 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 1 Jul 2026 17:05:29 -0500 Subject: [PATCH 38/42] Add more cases, exclude switch-expr, and write out more tests --- .../impl/RemoveUnnecessaryControlFlow.java | 190 +++++++++++++++--- .../RemoveUnnecessaryControlFlowTest.java | 26 ++- 2 files changed, 179 insertions(+), 37 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java index 2c1a976a782..47caf77ad60 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java @@ -27,17 +27,27 @@ import com.google.gwt.dev.jjs.ast.JProgram; import com.google.gwt.dev.jjs.ast.JReturnStatement; import com.google.gwt.dev.jjs.ast.JStatement; +import com.google.gwt.dev.jjs.ast.JSwitchExpression; +import com.google.gwt.dev.jjs.ast.JSwitchStatement; +import com.google.gwt.dev.jjs.ast.JTryStatement; import com.google.gwt.dev.jjs.ast.JWhileStatement; import java.util.List; /** * Finds return statements that are effectively at the end of a void method and removes them, - * or replaces them with something shorter in compiled output (e.g. break instead of return in a - * loop). + * or replaces them with something shorter in compiled output. *

    - * Additionally, finds "continue" statements (without labels) at the end of a loop (or in a non-loop - * block) and removes them as unnecessary. + * Additionally, loops and switch blocks get special treatment: + *

      + *
    • loops: finds "continue" statements (without labels) at the end of a loop (or in a non-loop + * block) and removes them as unnecessary, and replaces returns at the end of a loop with a break. + *
      • + *
    • switch: finds break/return at the end of the block and removes it.
      • + *
    + *

    + * Loops or switches inside other loops are unable to remove return or replace break, but can only + * remove the final continue/break respectively. *

    * Presently may require multiple passes to converge, but since it is unlikely to remove many such * statements, this will be addressed by just allowing another loop to find the changes. Additional @@ -67,7 +77,7 @@ private RewriteUnnecessaryReturnsVisitor(OptimizerContext optimizerCtx) { public boolean visit(JMethod x, Context ctx) { currentMethod = x; if (x.getType() == JPrimitiveType.VOID && x.getBody() instanceof JMethodBody b) { - BlockLevel.BLOCK.updateReturns(x, b.getBlock(), optimizerCtx); + BlockLevel.BLOCK.updateLastStatement(x, b.getBlock(), optimizerCtx); } return true; } @@ -93,33 +103,90 @@ public void endVisit(JDoStatement x, Context ctx) { BlockLevel.LOOP.updateContinues(currentMethod, x.getBody(), optimizerCtx); } + @Override + public boolean visit(JSwitchExpression x, Context ctx) { + // At this time, skip switch expressions entirely - break/continue within loops effectively + // behave like they are in a different method, we should restart the whole "method" from + // this node. + // Left for later work, since it won't be a regression, and rarely do switch expressions + // get this complex. + return false; + } + + /** + * Enumerates meaningful structures for removing unnecessary control flow. + */ private enum BlockLevel { /** - * Neither this block nor any containing block are part of a loop. + * Neither this block nor any containing block within a loop or a switch, returns can be + * removed. */ BLOCK, + /** - * This block (or a containing block) is part of a loop, so "return"s cannot be dropped but - * converted to breaks, and nested loops cannot be converted to breaks. + * This block (or a containing block) is part of a loop, so {@code return}s cannot be dropped + * but converted to breaks, and nested loops cannot be converted to breaks. Anywhere they are + * found, if the last statement of a loop is a continue, it can be removed. */ LOOP { @Override - protected void rewriteReturns(List stmts, int lastIndex) { + protected boolean rewriteReturn(List stmts, int lastIndex) { stmts.set(lastIndex, new JBreakStatement(stmts.get(lastIndex).getSourceInfo(), null)); + return true; } }, + /** * This block is in a loop which is contained in another loop, so "return"s cannot be omitted - * or converted to breaks. + * or converted to breaks. Continues can still be removed. */ LOOP_IN_LOOP { @Override - public void updateReturns(JMethod containingMethod, JBlock block, OptimizerContext ctx) { - // no-op, we can't do anything + public void updateLastStatement(JMethod containingMethod, JBlock block, OptimizerContext ctx) { + // No-op, we can't remove either breaks or returns. Overriding this means we don't need + // to descend further. + } + }, + + /** + * This block is in a switch, so breaks and returns at the end can both be removed. + */ + SWITCH { + @Override + protected boolean rewriteBreak(List stmts, int lastIndex) { + stmts.remove(lastIndex); + return true; + } + }, + + /** + * This block is in a switch which is contained in a loop - breaks can be omitted, but returns + * cannot. + */ + SWITCH_IN_LOOP { + @Override + protected boolean rewriteBreak(List stmts, int lastIndex) { + stmts.remove(lastIndex); + return true; + } + + @Override + protected boolean rewriteReturn(List stmts, int lastIndex) { + return false; } }; - public void updateReturns(JMethod containingMethod, JBlock block, OptimizerContext ctx) { + /** + * Test the last statement of a method to see if it is capable of being removed or rewritten. + *

    + * Check a block for unnecessary returns, descending then into the last statement as + * appropriate. + * + * @param containingMethod the method that contains the current block + * @param block the block to check + * @param ctx the current context + */ + public void updateLastStatement(JMethod containingMethod, JBlock block, OptimizerContext ctx) { List stmts = block.getStatements(); if (stmts.isEmpty()) { return; @@ -127,33 +194,60 @@ public void updateReturns(JMethod containingMethod, JBlock block, OptimizerConte int lastIndex = stmts.size() - 1; JStatement lastStmt = stmts.get(lastIndex); if (lastStmt instanceof JReturnStatement ret && ret.getExpr() == null) { - rewriteReturns(stmts, lastIndex); - ctx.markModified(containingMethod); + if (rewriteReturn(stmts, lastIndex)) { + ctx.markModified(containingMethod); - if (stmts.isEmpty()) { - return; + if (stmts.isEmpty()) { + return; + } + lastStmt = stmts.get(stmts.size() - 1); + } + } else if (lastStmt instanceof JBreakStatement breakStmt && breakStmt.getLabel() == null) { + if (rewriteBreak(stmts, lastIndex)) { + ctx.markModified(containingMethod); + + if (stmts.isEmpty()) { + return; + } + lastStmt = stmts.get(stmts.size() - 1); } - lastStmt = stmts.get(stmts.size() - 1); } // Even if we already made a change, continue on, we could have an earlier return in a // branch or a loop. None of these will remove the statement in question, so we don't need // to iterate. if (lastStmt instanceof JBlock b) { - updateReturns(containingMethod, b, ctx); + updateLastStatement(containingMethod, b, ctx); } else if (lastStmt instanceof JIfStatement ifStmt) { - updateReturns(containingMethod, ifStmt.getThenStmt(), ctx); - updateReturns(containingMethod, ifStmt.getElseStmt(), ctx); + updateLastStatement(containingMethod, ifStmt.getThenStmt(), ctx); + updateLastStatement(containingMethod, ifStmt.getElseStmt(), ctx); } else if (lastStmt instanceof JWhileStatement whileStmt) { - loop().updateReturns(containingMethod, whileStmt.getBody(), ctx); + loop().updateLastStatement(containingMethod, whileStmt.getBody(), ctx); } else if (lastStmt instanceof JForStatement forStmt) { - loop().updateReturns(containingMethod, forStmt.getBody(), ctx); + loop().updateLastStatement(containingMethod, forStmt.getBody(), ctx); } else if (lastStmt instanceof JDoStatement doStmt) { - loop().updateReturns(containingMethod, doStmt.getBody(), ctx); + loop().updateLastStatement(containingMethod, doStmt.getBody(), ctx); + } else if (lastStmt instanceof JTryStatement tryStmt) { + updateLastStatement(containingMethod, tryStmt.getTryBlock(), ctx); + updateLastStatement(containingMethod, tryStmt.getFinallyBlock(), ctx); + for (JTryStatement.CatchClause catchBlock : tryStmt.getCatchClauses()) { + updateLastStatement(containingMethod, catchBlock.getBlock(), ctx); + } + } else if (lastStmt instanceof JSwitchStatement switchStmt) { + // Since we're in a switch and the switch is effectively the end of the method, we can + // replace any return with break, and if the last statement is a break/return, outright + // remove it. We don't look at earlier case/defaults + switchStmt().updateLastStatement(containingMethod, switchStmt.getBody(), ctx); } - - // TODO handle switches, try/catch/finally } + /** + * Anywhere within a method, a loop that ends with {@code continue;} can have it + * removed. This can recursively check the final statement of a block. + * + * @param containingMethod the method containing the {@code continue;} + * @param block the block being checked + * @param ctx the current context + */ public void updateContinues(JMethod containingMethod, JBlock block, OptimizerContext ctx) { if (block.isEmpty()) { return; @@ -171,22 +265,56 @@ public void updateContinues(JMethod containingMethod, JBlock block, OptimizerCon lastStmt = stmts.get(stmts.size() - 1); } + // Don't handle nested loops here, will be handled when visited. if (lastStmt instanceof JBlock b) { updateContinues(containingMethod, b, ctx); } else if (lastStmt instanceof JIfStatement ifStmt) { updateContinues(containingMethod, ifStmt.getThenStmt(), ctx); updateContinues(containingMethod, ifStmt.getElseStmt(), ctx); - } // Don't handle nested loops here, will be handled when visited. - - // TODO handle switches, try/catch/finally + } else if (lastStmt instanceof JTryStatement tryStmt) { + updateContinues(containingMethod, tryStmt.getTryBlock(), ctx); + updateContinues(containingMethod, tryStmt.getFinallyBlock(), ctx); + for (JTryStatement.CatchClause catchBlock : tryStmt.getCatchClauses()) { + updateContinues(containingMethod, catchBlock.getBlock(), ctx); + } + } else if (lastStmt instanceof JSwitchStatement) { + updateContinues(containingMethod, ((JSwitchStatement) lastStmt).getBody(), ctx); + } } - protected void rewriteReturns(List stmts, int lastIndex) { + /** + * Helper to rewrite/remove/ignore returns differently in different contexts. + * + * @param stmts the statements of the block containing the return + * @param lastIndex the position of the return in its parent block + * @return true if a change was made, false otherwise + */ + protected boolean rewriteReturn(List stmts, int lastIndex) { stmts.remove(lastIndex); + return true; } + /** + * Helper to rewrite/remove/ignore breaks differently in different contexts. + * + * @param stmts the statements of the block containing the break + * @param lastIndex the position of the return in its parent block + * @return true if a change was made, false otherwise + */ + protected boolean rewriteBreak(List stmts, int lastIndex) { + // default implementation does nothing, breaks aren't found outside of specific parents + assert this == LOOP || this == LOOP_IN_LOOP : "Shouldn't encounter breaks here"; + return false; + } + + // Move into a loop from the current block private BlockLevel loop() { - return this == BLOCK ? LOOP : LOOP_IN_LOOP; + return this == LOOP || this == SWITCH_IN_LOOP ? LOOP_IN_LOOP : LOOP; + } + + // Move into a switch from the current block + private BlockLevel switchStmt() { + return this == LOOP || this == SWITCH_IN_LOOP ? SWITCH_IN_LOOP : SWITCH; } } } diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java index 139a1642337..f0719282e70 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java @@ -64,16 +64,30 @@ public void testReturnInLoop() throws UnableToCompleteException { .into("while (condition()) { break; }"); optimize("void", "while (condition()) { return; } foo();").noChange(); + + // for, do/while } + // TODO test try/catches/finally + // switch/case/default + // loop in loop break/return + // loop, loop-in-loop continue (in other blocks) + // switch-in-loop break/return + // if/block/try in switch + // + + // switch exprs, with loops in them, etc + + + @Override protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod method) throws UnableToCompleteException { - // Not presently guaranteed to converge in a single pass, so loop until it does. - int mods; - do { - mods = RemoveUnnecessaryControlFlow.exec(program, OptimizerContext.NULL_OPTIMIZATION_CONTEXT); - } while (mods > 0); - return false; + int mods = RemoveUnnecessaryControlFlow.exec(program, OptimizerContext.NULL_OPTIMIZATION_CONTEXT); + if (mods > 0) { + // verify we converged in a single pass + assert RemoveUnnecessaryControlFlow.exec(program, OptimizerContext.NULL_OPTIMIZATION_CONTEXT) == 0; + } + return mods > 0; } } \ No newline at end of file From d82c4f49bd9143746e9705f5d7c96dfab7bc6e47 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 7 Jul 2026 19:15:02 -0500 Subject: [PATCH 39/42] More test --- .../RemoveUnnecessaryControlFlowTest.java | 58 ++++++++++++++++--- 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java index f0719282e70..36f65f2114d 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java @@ -57,7 +57,13 @@ public void testReturnInIf() throws UnableToCompleteException { .into("if (condition()) { if (condition()) { return; } foo(); }"); } + public void testReturnInTry() throws UnableToCompleteException { + optimize("void", "try { return; } catch (Exception e) { return; } finally { return; }") + .into("try { } catch (Exception e) { } finally { }"); + } + public void testReturnInLoop() throws UnableToCompleteException { + // while, with various returns optimize("void", "while (condition()) { return; }") .into("while (condition()) { break; }"); optimize("void", "while (condition()) { return; } return;") @@ -65,12 +71,50 @@ public void testReturnInLoop() throws UnableToCompleteException { optimize("void", "while (condition()) { return; } foo();").noChange(); - // for, do/while + // for, do/while - add some nesting + optimize("void", "for (int i = 0; i < 10; i++) { return; }") + .into("for (int i = 0; i < 10; i++) { break; }"); + optimize("void", "do { if (condition()) { return; } else { foo(); } } while(true);") + .into("do { if (condition()) { break; } else { foo(); } } while(true);"); + } + + public void testContinueInLoop() throws UnableToCompleteException { + // Same sorts of tests on each loop construct, with various other siblings/wrappings + optimize("void", "for (int i = 0; i < 10; i++) { continue; }") + .into("for (int i = 0; i < 10; i++) { }"); + optimize("void", "do { if (condition()) { continue; } else { foo(); } } while(true);") + .into("do { if (condition()) { } else { foo(); } } while(true);"); + optimize("void", "while(true) { foo(); continue; } ") + .into("while(true) { foo(); }"); + } + + public void testSwitchStmt() throws UnableToCompleteException { + // Final return/break removed + optimize("void", "switch (3) { case 1: return; case 2: default: return; }") + .into("switch (3) { case 1: return; case 2: default: }"); + optimize("void", "switch (3) { case 1: break; case 2: default: break; }") + .into("switch (3) { case 1: break; case 2: default: }"); + } + + public void testLoopInLoop() throws UnableToCompleteException { + // Continue can be omitted even if the loop isn't the last statement + optimize("void", "while (condition()) { while (condition()) { foo(); continue; } continue; } foo();") + .into("while (condition()) { while (condition()) { foo(); } } foo();"); + optimize("void", "while (condition()) { while (condition()) { foo(); continue; } }") + .into("while (condition()) { while (condition()) { foo(); } }"); + + // return cannot be omitted in a loop-in-loop + optimize("void", "while (condition()) { while (true) { foo(); return; } }") + .noChange(); + // loop-in-loop structure doesn't impact return in the outer loop + optimize("void", "while (condition()) { while (condition()) { foo(); } return; }") + .into("while (condition()) { while (condition()) { foo(); } break; }"); + } + + public void testSwitchInLoop() { + } - // TODO test try/catches/finally - // switch/case/default - // loop in loop break/return // loop, loop-in-loop continue (in other blocks) // switch-in-loop break/return // if/block/try in switch @@ -84,10 +128,8 @@ public void testReturnInLoop() throws UnableToCompleteException { protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod method) throws UnableToCompleteException { int mods = RemoveUnnecessaryControlFlow.exec(program, OptimizerContext.NULL_OPTIMIZATION_CONTEXT); - if (mods > 0) { - // verify we converged in a single pass - assert RemoveUnnecessaryControlFlow.exec(program, OptimizerContext.NULL_OPTIMIZATION_CONTEXT) == 0; - } + // verify we converged in a single pass + assert RemoveUnnecessaryControlFlow.exec(program, OptimizerContext.NULL_OPTIMIZATION_CONTEXT) == 0; return mods > 0; } } \ No newline at end of file From a0b883c18041466b38d80583a053138ffec6c388 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 8 Jul 2026 15:45:48 -0500 Subject: [PATCH 40/42] More test, fix a bug --- .../impl/RemoveUnnecessaryControlFlow.java | 5 ++-- .../RemoveUnnecessaryControlFlowTest.java | 27 ++++++++++++++----- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java index 47caf77ad60..0bf9405fa47 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java @@ -108,9 +108,8 @@ public boolean visit(JSwitchExpression x, Context ctx) { // At this time, skip switch expressions entirely - break/continue within loops effectively // behave like they are in a different method, we should restart the whole "method" from // this node. - // Left for later work, since it won't be a regression, and rarely do switch expressions - // get this complex. - return false; + // However, if we see a void type, then we're actually just a regular switch statement + return x.getType() == JPrimitiveType.VOID; } /** diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java index 36f65f2114d..327132c0c3d 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java @@ -111,18 +111,31 @@ public void testLoopInLoop() throws UnableToCompleteException { .into("while (condition()) { while (condition()) { foo(); } break; }"); } - public void testSwitchInLoop() { - + public void testSwitchInLoop() throws UnableToCompleteException { + // validate that the break is treated as part of the switch and still removed + optimize("void", "for (int i = 0; i < 10; i++) { switch (i) { case 1: return; case 2: break; } }") + .into("for (int i = 0; i < 10; i++) { switch (i) { case 1: return; case 2: } }"); + // Likewise, the continue inside the switch is part of the loop and removed as the final statement + optimize("void", "for (int i = 0; i < 10; i++) { switch (i) { case 1: return; case 2: continue; } }") + .into("for (int i = 0; i < 10; i++) { switch (i) { case 1: return; case 2: } }"); } - // loop, loop-in-loop continue (in other blocks) - // switch-in-loop break/return - // if/block/try in switch - // + public void testLoopInSwitchStatement() throws UnableToCompleteException { + optimize("void", "switch(4) {case 1: for (int i = 0; i < 10; i++) { break; }}").noChange(); - // switch exprs, with loops in them, etc + optimize("void", "switch(4) {case 0: foo(); case 1: for (int i = 0; i < 10; i++) { continue; } break; }") + .into("switch(4) {case 0: foo(); case 1: for (int i = 0; i < 10; i++) { }}"); + } + public void testBlocksInSwitchStatement() throws UnableToCompleteException { + optimize("void", "switch(4) {case 0: {foo();} case 1: if (condition()) {return;} else {foo(); return;} }") + .into("switch(4) {case 0: {foo();} case 1: if (condition()) {} else {foo(); } }"); + } + public void testContinuesInNonVoidMethod() throws UnableToCompleteException { + optimize("int", "while (condition()) { foo(); continue; } return 1;") + .into("while (condition()) { foo(); }; return 1;"); + } @Override protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod method) From ec0109f2fb987dda53fc77702249f1235e27f017 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 8 Jul 2026 16:04:53 -0500 Subject: [PATCH 41/42] checkstyle --- .../gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java | 9 ++++++--- .../dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java index 0bf9405fa47..e6afd52c738 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java @@ -130,7 +130,8 @@ private enum BlockLevel { LOOP { @Override protected boolean rewriteReturn(List stmts, int lastIndex) { - stmts.set(lastIndex, new JBreakStatement(stmts.get(lastIndex).getSourceInfo(), null)); + stmts.set(lastIndex, + new JBreakStatement(stmts.get(lastIndex).getSourceInfo(), null)); return true; } }, @@ -141,7 +142,8 @@ protected boolean rewriteReturn(List stmts, int lastIndex) { */ LOOP_IN_LOOP { @Override - public void updateLastStatement(JMethod containingMethod, JBlock block, OptimizerContext ctx) { + public void updateLastStatement(JMethod containingMethod, JBlock block, + OptimizerContext ctx) { // No-op, we can't remove either breaks or returns. Overriding this means we don't need // to descend further. } @@ -185,7 +187,8 @@ protected boolean rewriteReturn(List stmts, int lastIndex) { * @param block the block to check * @param ctx the current context */ - public void updateLastStatement(JMethod containingMethod, JBlock block, OptimizerContext ctx) { + public void updateLastStatement(JMethod containingMethod, JBlock block, + OptimizerContext ctx) { List stmts = block.getStatements(); if (stmts.isEmpty()) { return; diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java index 327132c0c3d..8f5b0802dd5 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java @@ -123,7 +123,7 @@ public void testSwitchInLoop() throws UnableToCompleteException { public void testLoopInSwitchStatement() throws UnableToCompleteException { optimize("void", "switch(4) {case 1: for (int i = 0; i < 10; i++) { break; }}").noChange(); - optimize("void", "switch(4) {case 0: foo(); case 1: for (int i = 0; i < 10; i++) { continue; } break; }") + optimize("void", "switch(4) {case 0: foo(); case 1: for (int i = 0; i < 10; i++) { continue; } break; }") .into("switch(4) {case 0: foo(); case 1: for (int i = 0; i < 10; i++) { }}"); } From a3c31e9efb4fb9d0242d766bb030c3e2b5be11b1 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 9 Jul 2026 09:34:33 -0500 Subject: [PATCH 42/42] Guard against try without finally --- .../gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java index e6afd52c738..bda4c030c6b 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java @@ -230,7 +230,9 @@ public void updateLastStatement(JMethod containingMethod, JBlock block, loop().updateLastStatement(containingMethod, doStmt.getBody(), ctx); } else if (lastStmt instanceof JTryStatement tryStmt) { updateLastStatement(containingMethod, tryStmt.getTryBlock(), ctx); - updateLastStatement(containingMethod, tryStmt.getFinallyBlock(), ctx); + if (tryStmt.getFinallyBlock() != null) { + updateLastStatement(containingMethod, tryStmt.getFinallyBlock(), ctx); + } for (JTryStatement.CatchClause catchBlock : tryStmt.getCatchClauses()) { updateLastStatement(containingMethod, catchBlock.getBlock(), ctx); } @@ -275,7 +277,9 @@ public void updateContinues(JMethod containingMethod, JBlock block, OptimizerCon updateContinues(containingMethod, ifStmt.getElseStmt(), ctx); } else if (lastStmt instanceof JTryStatement tryStmt) { updateContinues(containingMethod, tryStmt.getTryBlock(), ctx); - updateContinues(containingMethod, tryStmt.getFinallyBlock(), ctx); + if (tryStmt.getFinallyBlock() != null) { + updateContinues(containingMethod, tryStmt.getFinallyBlock(), ctx); + } for (JTryStatement.CatchClause catchBlock : tryStmt.getCatchClauses()) { updateContinues(containingMethod, catchBlock.getBlock(), ctx); }