From 5b68b922b5542cb807c95635aaec67c38fda621a Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 24 Nov 2025 20:22:57 -0600 Subject: [PATCH 01/21] 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 efd7bf15a33..15283ffd776 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; @@ -368,6 +369,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: + * + * + * 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 38941fb7e07..f7ec450a44f 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 19c9335d74d..c7f4e8ec978 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; @@ -160,7 +161,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 = @@ -178,7 +179,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 d7dd069accc..fa59a85e435 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 262b13ba70140e39a27dfe7cbed31496546ae086 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 25 Nov 2025 20:34:40 -0600 Subject: [PATCH 02/21] 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 95174edf63c6d63cd329e47f7f538ddadf35d709 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 26 Nov 2025 11:43:28 -0600 Subject: [PATCH 03/21] 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 6b603e785e05c014364b455ee19e3339d10703e8 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 26 Nov 2025 21:05:35 -0600 Subject: [PATCH 04/21] 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 f7ec450a44f..b2a2c31b9c3 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 2938c74f6997ba6f425d62484185ffa9fb7b735f Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 27 Nov 2025 14:50:14 -0600 Subject: [PATCH 05/21] 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 15283ffd776..4834d01d970 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java +++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java @@ -369,7 +369,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 b2a2c31b9c3..adbb1c3869d 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 de872288e678babd1f82b6d4d7a1149efe2dbe8d Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 29 Nov 2025 19:34:21 -0600 Subject: [PATCH 06/21] 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 fa59a85e435..e38c8a6fafe 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); } @@ -520,6 +512,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 3f3c0f23c310b303b6f97998494a0b8fcef67acf Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 29 Nov 2025 21:25:21 -0600 Subject: [PATCH 07/21] 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 4834d01d970..15283ffd776 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java +++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java @@ -369,7 +369,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 4a64133ac110261c94569f5b4a3370e31dc97976 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sun, 30 Nov 2025 19:11:46 -0600 Subject: [PATCH 08/21] 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 2786f4bf8b285f99938d4b6322b79896db5888a8 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 3 Jan 2026 20:17:00 -0600 Subject: [PATCH 09/21] 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 fdc03524bfc65a27976a8be883e066fb77ad26a4 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 28 Jan 2026 13:15:59 -0600 Subject: [PATCH 10/21] 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 15283ffd776..efd7bf15a33 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; @@ -369,8 +368,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 bd9638bfbf16ae2603d6c9019b408d0a81398a14 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 28 Jan 2026 13:53:17 -0600 Subject: [PATCH 11/21] 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 d3b05036a71e1b2a6f2531aeec6d50c000e44587 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 28 Jan 2026 19:48:47 -0600 Subject: [PATCH 12/21] 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 adbb1c3869d..53d4d06b89c 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 c7f4e8ec978..19c9335d74d 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; @@ -161,7 +160,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 = @@ -179,7 +178,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 e38c8a6fafe..bb75ddb5306 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; @@ -509,25 +509,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 5bc051d8033..90958d259ca 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 @@ -545,7 +545,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 b4cb9d5bff90b3d6ed7edde33b0323ed951fb1a5 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 29 Jan 2026 18:08:46 -0600 Subject: [PATCH 13/21] 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 c8619e7e485..1898ac08cb9 100644 --- a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java @@ -133,6 +133,11 @@ public class JsToStringGenerationVisitor extends JsVisitor { private ArrayList statementStarts = new ArrayList(); private final boolean useLongIdents; + /** + * 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. */ @@ -546,15 +551,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) { @@ -576,6 +592,12 @@ public boolean visit(JsIf x, JsContext ctx) { _nestedPop(elseStmt); } } + + if (outerBlock) { + _blockClose(); + // Restore flag for later calls + inIfWithoutBraces = true; + } return false; } @@ -910,6 +932,7 @@ protected void printJsBlock(JsBlock x, boolean truncate, boolean finalNewline) { // Open braces. // _blockOpen(); + inIfWithoutBraces = false; } int count = 0; From c0fa0947824ab57e9d8d43becc894fd3d4b7187a Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 29 Jan 2026 19:37:10 -0600 Subject: [PATCH 14/21] Improved version of adding necessary braces --- dev/build.xml | 2 +- .../dev/js/JsToStringGenerationVisitor.java | 28 ++----- .../js/JsToStringGenerationVisitorTest.java | 80 +++++++++++++++++++ 3 files changed, 86 insertions(+), 24 deletions(-) diff --git a/dev/build.xml b/dev/build.xml index 850ef9cc59e..ddfeb90dcc3 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 1898ac08cb9..2fce6e8b247 100644 --- a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java @@ -133,11 +133,6 @@ public class JsToStringGenerationVisitor extends JsVisitor { private ArrayList statementStarts = new ArrayList(); private final boolean useLongIdents; - /** - * 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. */ @@ -551,26 +546,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) { @@ -592,12 +581,6 @@ public boolean visit(JsIf x, JsContext ctx) { _nestedPop(elseStmt); } } - - if (outerBlock) { - _blockClose(); - // Restore flag for later calls - inIfWithoutBraces = true; - } return false; } @@ -932,7 +915,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 4194954b6d2..5568a91468e 100644 --- a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java +++ b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java @@ -18,11 +18,19 @@ import com.google.gwt.dev.cfg.BindingProperty; import com.google.gwt.dev.cfg.ConditionNone; import com.google.gwt.dev.cfg.ConfigurationProperty; +import com.google.gwt.dev.jjs.SourceInfo; +import com.google.gwt.dev.jjs.SourceOrigin; import com.google.gwt.dev.jjs.impl.FullCompileTestBase; +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; @@ -74,6 +82,78 @@ public void testClassRangeMarking() throws UnableToCompleteException { // Verifies there is an epilogue after the program class range. assertTrue(programClassRange.getEndPosition() < text.getPosition()); } + public class EntryPoint { + private void go(int i) { + } + public void onModuleLoad() { + boolean a = true, b = true, c = true; + if (a) + if (b) + go(1); + else + go(2); + } + + } + + 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; + } @Override protected void optimizeJava() { From 48e32a97ddb8060a66f27104ac8e7512090fd4fe Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 30 Jan 2026 11:31:48 -0600 Subject: [PATCH 15/21] checkstyle --- .../dev/js/JsToStringGenerationVisitorTest.java | 14 -------------- 1 file changed, 14 deletions(-) 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 5568a91468e..d02d1307ddf 100644 --- a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java +++ b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java @@ -18,7 +18,6 @@ import com.google.gwt.dev.cfg.BindingProperty; import com.google.gwt.dev.cfg.ConditionNone; import com.google.gwt.dev.cfg.ConfigurationProperty; -import com.google.gwt.dev.jjs.SourceInfo; import com.google.gwt.dev.jjs.SourceOrigin; import com.google.gwt.dev.jjs.impl.FullCompileTestBase; import com.google.gwt.dev.js.ast.JsExprStmt; @@ -82,19 +81,6 @@ public void testClassRangeMarking() throws UnableToCompleteException { // Verifies there is an epilogue after the program class range. assertTrue(programClassRange.getEndPosition() < text.getPosition()); } - public class EntryPoint { - private void go(int i) { - } - public void onModuleLoad() { - boolean a = true, b = true, c = true; - if (a) - if (b) - go(1); - else - go(2); - } - - } public void testDanglingElse() throws Exception { // No braces, ELSE will be attached to the inner IF From 446049a871bb976fe16e9cc9f218293c48651e57 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 30 Jan 2026 13:43:12 -0600 Subject: [PATCH 16/21] Drop Java 11 support --- .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 a863014a190..8df79676dca 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', '22' ] 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 234cd92123e..c2e71d73ace 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', '22'] steps: - name: Checkout GWT itself into one directory uses: actions/checkout@v6 From 8a8cf1832a880b8652bbe869a2f9cfc20debeae0 Mon Sep 17 00:00:00 2001 From: Zbynek Konecny Date: Sat, 31 Jan 2026 19:24:47 +0100 Subject: [PATCH 17/21] Compute total size of samples correctly --- samples/build.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/build.xml b/samples/build.xml index 24f1cf56540..0ec60564fc0 100644 --- a/samples/build.xml +++ b/samples/build.xml @@ -60,7 +60,7 @@ - + From cb191261c819977e4b54e1d844e2074c98e566d7 Mon Sep 17 00:00:00 2001 From: Zbynek Konecny Date: Sat, 31 Jan 2026 23:12:42 +0100 Subject: [PATCH 18/21] Include deferred size in compilation report Co-authored-by: Colin Alworth --- samples/build.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/build.xml b/samples/build.xml index 0ec60564fc0..fedd6f547f9 100644 --- a/samples/build.xml +++ b/samples/build.xml @@ -60,7 +60,7 @@ - + From 459f3016453e7bcaafba97cff0f6ff8627b1fd87 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 14 Feb 2026 22:07:37 -0600 Subject: [PATCH 19/21] 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 6f26726624ad0789822d8daa0f5af939ef090a50 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 14 Feb 2026 22:08:17 -0600 Subject: [PATCH 20/21] 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 53d4d06b89c..acf22aa203b 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 90958d259ca..281e7d009ac 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;"); } /** @@ -371,9 +371,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 cb302b2c34445defa21869bb7b147490ca069f4e Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sun, 15 Feb 2026 08:22:04 -0600 Subject: [PATCH 21/21] 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); }",