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 71c7e34cee6..3b5fe7c6297 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java +++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java @@ -107,6 +107,7 @@ import com.google.gwt.dev.jjs.impl.RecordRebinds; import com.google.gwt.dev.jjs.impl.RemoveEmptySuperCalls; import com.google.gwt.dev.jjs.impl.RemoveSpecializations; +import com.google.gwt.dev.jjs.impl.RemoveUnnecessaryControlFlow; import com.google.gwt.dev.jjs.impl.ReplaceCallsToNativeJavaLangObjectOverrides; import com.google.gwt.dev.jjs.impl.ReplaceGetClassOverrides; import com.google.gwt.dev.jjs.impl.ResolvePermutationDependentValues; @@ -1469,6 +1470,7 @@ private void optimizeJavaToFixedPoint() throws InterruptedException { stats.recordModified(MethodCallTightener.exec(jprogram, optimizerCtx)); // Note: Specialization should be done before inlining. stats.recordModified(MethodCallSpecializer.exec(jprogram, optimizerCtx)); + stats.recordModified(RemoveUnnecessaryControlFlow.exec(jprogram, optimizerCtx)); stats.recordModified(DeadCodeElimination.exec(jprogram, optimizerCtx)); stats.recordModified(MethodInliner.exec(jprogram, optimizerCtx)); if (options.shouldInlineLiteralParameters()) { diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java index 4d1b10630d2..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. */ @@ -105,4 +116,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/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 53eb6fec998..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 @@ -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) { super(info); this.ifExpr = ifExpr; - this.thenStmt = thenStmt; - this.elseStmt = elseStmt; + this.thenStmt = JBlock.ensureBlock(info, thenStmt); + this.elseStmt = JBlock.ensureBlock(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,12 +49,8 @@ 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); - } + thenStmt = JBlock.ensureBlock(getSourceInfo(), visitor.accept(thenStmt, false)); + elseStmt = JBlock.ensureBlock(getSourceInfo(), visitor.accept(elseStmt, false)); } visitor.endVisit(this, ctx); } 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 17caa5ecafa..b8e52713460 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java @@ -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(); @@ -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 27edf1f9aa0..9f26bab91a2 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java @@ -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(); @@ -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; } @@ -753,8 +753,8 @@ 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; } @@ -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/GwtAstBuilder.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java index eb8b806cc27..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 @@ -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) { diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java new file mode 100644 index 00000000000..bda4c030c6b --- /dev/null +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlow.java @@ -0,0 +1,327 @@ +/* + * Copyright 2026 GWT Project Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.gwt.dev.jjs.impl; + +import com.google.gwt.dev.jjs.ast.Context; +import com.google.gwt.dev.jjs.ast.JBlock; +import com.google.gwt.dev.jjs.ast.JBreakStatement; +import com.google.gwt.dev.jjs.ast.JContinueStatement; +import com.google.gwt.dev.jjs.ast.JDoStatement; +import com.google.gwt.dev.jjs.ast.JForStatement; +import com.google.gwt.dev.jjs.ast.JIfStatement; +import com.google.gwt.dev.jjs.ast.JMethod; +import com.google.gwt.dev.jjs.ast.JMethodBody; +import com.google.gwt.dev.jjs.ast.JModVisitor; +import com.google.gwt.dev.jjs.ast.JPrimitiveType; +import com.google.gwt.dev.jjs.ast.JProgram; +import com.google.gwt.dev.jjs.ast.JReturnStatement; +import com.google.gwt.dev.jjs.ast.JStatement; +import com.google.gwt.dev.jjs.ast.JSwitchExpression; +import com.google.gwt.dev.jjs.ast.JSwitchStatement; +import com.google.gwt.dev.jjs.ast.JTryStatement; +import com.google.gwt.dev.jjs.ast.JWhileStatement; + +import java.util.List; + +/** + * Finds return statements that are effectively at the end of a void method and removes them, + * or replaces them with something shorter in compiled output. + *

+ * Additionally, loops and switch blocks get special treatment: + *

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

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

+ * Presently may require multiple passes to converge, but since it is unlikely to remove many such + * statements, this will be addressed by just allowing another loop to find the changes. Additional + * changes from other optimizations (mostly DCE) can expose other opportunities for this pass. + */ +public class RemoveUnnecessaryControlFlow { + private static final String NAME = RemoveUnnecessaryControlFlow.class.getSimpleName(); + + public static int exec(JProgram program, OptimizerContext optimizerCtx) { + try (OptimizerStats stats = OptimizerStats.optimization(NAME)) { + new RewriteUnnecessaryReturnsVisitor(optimizerCtx).accept(program); + + optimizerCtx.incOptimizationStep(); + return stats.getNumMods(); + } + } + + private static class RewriteUnnecessaryReturnsVisitor extends JModVisitor { + private final OptimizerContext optimizerCtx; + private JMethod currentMethod; + + private RewriteUnnecessaryReturnsVisitor(OptimizerContext optimizerCtx) { + this.optimizerCtx = optimizerCtx; + } + + @Override + public boolean visit(JMethod x, Context ctx) { + currentMethod = x; + if (x.getType() == JPrimitiveType.VOID && x.getBody() instanceof JMethodBody b) { + BlockLevel.BLOCK.updateLastStatement(x, b.getBlock(), optimizerCtx); + } + return true; + } + + @Override + public void endVisit(JMethod x, Context ctx) { + currentMethod = null; + } + + @Override + public void endVisit(JForStatement x, Context ctx) { + assert currentMethod != null; + BlockLevel.LOOP.updateContinues(currentMethod, x.getBody(), optimizerCtx); + } + + @Override + public void endVisit(JWhileStatement x, Context ctx) { + BlockLevel.LOOP.updateContinues(currentMethod, x.getBody(), optimizerCtx); + } + + @Override + public void endVisit(JDoStatement x, Context ctx) { + BlockLevel.LOOP.updateContinues(currentMethod, x.getBody(), optimizerCtx); + } + + @Override + public boolean visit(JSwitchExpression x, Context ctx) { + // At this time, skip switch expressions entirely - break/continue within loops effectively + // behave like they are in a different method, we should restart the whole "method" from + // this node. + // However, if we see a void type, then we're actually just a regular switch statement + return x.getType() == JPrimitiveType.VOID; + } + + /** + * Enumerates meaningful structures for removing unnecessary control flow. + */ + private enum BlockLevel { + /** + * Neither this block nor any containing block within a loop or a switch, returns can be + * removed. + */ + BLOCK, + + /** + * This block (or a containing block) is part of a loop, so {@code return}s cannot be dropped + * but converted to breaks, and nested loops cannot be converted to breaks. Anywhere they are + * found, if the last statement of a loop is a continue, it can be removed. + */ + LOOP { + @Override + protected boolean rewriteReturn(List stmts, int lastIndex) { + stmts.set(lastIndex, + new JBreakStatement(stmts.get(lastIndex).getSourceInfo(), null)); + return true; + } + }, + + /** + * This block is in a loop which is contained in another loop, so "return"s cannot be omitted + * or converted to breaks. Continues can still be removed. + */ + LOOP_IN_LOOP { + @Override + public void updateLastStatement(JMethod containingMethod, JBlock block, + OptimizerContext ctx) { + // No-op, we can't remove either breaks or returns. Overriding this means we don't need + // to descend further. + } + }, + + /** + * This block is in a switch, so breaks and returns at the end can both be removed. + */ + SWITCH { + @Override + protected boolean rewriteBreak(List stmts, int lastIndex) { + stmts.remove(lastIndex); + return true; + } + }, + + /** + * This block is in a switch which is contained in a loop - breaks can be omitted, but returns + * cannot. + */ + SWITCH_IN_LOOP { + @Override + protected boolean rewriteBreak(List stmts, int lastIndex) { + stmts.remove(lastIndex); + return true; + } + + @Override + protected boolean rewriteReturn(List stmts, int lastIndex) { + return false; + } + }; + + /** + * Test the last statement of a method to see if it is capable of being removed or rewritten. + *

+ * Check a block for unnecessary returns, descending then into the last statement as + * appropriate. + * + * @param containingMethod the method that contains the current block + * @param block the block to check + * @param ctx the current context + */ + public void updateLastStatement(JMethod containingMethod, JBlock block, + OptimizerContext ctx) { + List stmts = block.getStatements(); + if (stmts.isEmpty()) { + return; + } + int lastIndex = stmts.size() - 1; + JStatement lastStmt = stmts.get(lastIndex); + if (lastStmt instanceof JReturnStatement ret && ret.getExpr() == null) { + if (rewriteReturn(stmts, lastIndex)) { + ctx.markModified(containingMethod); + + if (stmts.isEmpty()) { + return; + } + lastStmt = stmts.get(stmts.size() - 1); + } + } else if (lastStmt instanceof JBreakStatement breakStmt && breakStmt.getLabel() == null) { + if (rewriteBreak(stmts, lastIndex)) { + ctx.markModified(containingMethod); + + if (stmts.isEmpty()) { + return; + } + lastStmt = stmts.get(stmts.size() - 1); + } + } + // Even if we already made a change, continue on, we could have an earlier return in a + // branch or a loop. None of these will remove the statement in question, so we don't need + // to iterate. + if (lastStmt instanceof JBlock b) { + updateLastStatement(containingMethod, b, ctx); + } else if (lastStmt instanceof JIfStatement ifStmt) { + updateLastStatement(containingMethod, ifStmt.getThenStmt(), ctx); + updateLastStatement(containingMethod, ifStmt.getElseStmt(), ctx); + } else if (lastStmt instanceof JWhileStatement whileStmt) { + loop().updateLastStatement(containingMethod, whileStmt.getBody(), ctx); + } else if (lastStmt instanceof JForStatement forStmt) { + loop().updateLastStatement(containingMethod, forStmt.getBody(), ctx); + } else if (lastStmt instanceof JDoStatement doStmt) { + loop().updateLastStatement(containingMethod, doStmt.getBody(), ctx); + } else if (lastStmt instanceof JTryStatement tryStmt) { + updateLastStatement(containingMethod, tryStmt.getTryBlock(), ctx); + if (tryStmt.getFinallyBlock() != null) { + updateLastStatement(containingMethod, tryStmt.getFinallyBlock(), ctx); + } + for (JTryStatement.CatchClause catchBlock : tryStmt.getCatchClauses()) { + updateLastStatement(containingMethod, catchBlock.getBlock(), ctx); + } + } else if (lastStmt instanceof JSwitchStatement switchStmt) { + // Since we're in a switch and the switch is effectively the end of the method, we can + // replace any return with break, and if the last statement is a break/return, outright + // remove it. We don't look at earlier case/defaults + switchStmt().updateLastStatement(containingMethod, switchStmt.getBody(), ctx); + } + } + + /** + * Anywhere within a method, a loop that ends with {@code continue;} can have it + * removed. This can recursively check the final statement of a block. + * + * @param containingMethod the method containing the {@code continue;} + * @param block the block being checked + * @param ctx the current context + */ + public void updateContinues(JMethod containingMethod, JBlock block, OptimizerContext ctx) { + if (block.isEmpty()) { + return; + } + List stmts = block.getStatements(); + int lastIndex = stmts.size() - 1; + JStatement lastStmt = stmts.get(lastIndex); + if (stmts.get(lastIndex) instanceof JContinueStatement cont && cont.getLabel() == null) { + stmts.remove(lastIndex); + ctx.markModified(containingMethod); + + if (stmts.isEmpty()) { + return; + } + lastStmt = stmts.get(stmts.size() - 1); + } + + // Don't handle nested loops here, will be handled when visited. + if (lastStmt instanceof JBlock b) { + updateContinues(containingMethod, b, ctx); + } else if (lastStmt instanceof JIfStatement ifStmt) { + updateContinues(containingMethod, ifStmt.getThenStmt(), ctx); + updateContinues(containingMethod, ifStmt.getElseStmt(), ctx); + } else if (lastStmt instanceof JTryStatement tryStmt) { + updateContinues(containingMethod, tryStmt.getTryBlock(), ctx); + if (tryStmt.getFinallyBlock() != null) { + updateContinues(containingMethod, tryStmt.getFinallyBlock(), ctx); + } + for (JTryStatement.CatchClause catchBlock : tryStmt.getCatchClauses()) { + updateContinues(containingMethod, catchBlock.getBlock(), ctx); + } + } else if (lastStmt instanceof JSwitchStatement) { + updateContinues(containingMethod, ((JSwitchStatement) lastStmt).getBody(), ctx); + } + } + + /** + * Helper to rewrite/remove/ignore returns differently in different contexts. + * + * @param stmts the statements of the block containing the return + * @param lastIndex the position of the return in its parent block + * @return true if a change was made, false otherwise + */ + protected boolean rewriteReturn(List stmts, int lastIndex) { + stmts.remove(lastIndex); + return true; + } + + /** + * Helper to rewrite/remove/ignore breaks differently in different contexts. + * + * @param stmts the statements of the block containing the break + * @param lastIndex the position of the return in its parent block + * @return true if a change was made, false otherwise + */ + protected boolean rewriteBreak(List stmts, int lastIndex) { + // default implementation does nothing, breaks aren't found outside of specific parents + assert this == LOOP || this == LOOP_IN_LOOP : "Shouldn't encounter breaks here"; + return false; + } + + // Move into a loop from the current block + private BlockLevel loop() { + return this == LOOP || this == SWITCH_IN_LOOP ? LOOP_IN_LOOP : LOOP; + } + + // Move into a switch from the current block + private BlockLevel switchStmt() { + return this == LOOP || this == SWITCH_IN_LOOP ? SWITCH_IN_LOOP : SWITCH; + } + } + } +} diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java b/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java index e054081b931..6e080578744 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java @@ -240,8 +240,8 @@ public static JExpression simplifyConditional(JConditional expression) { public static JStatement simplifyIfStatement(JIfStatement ifStatement, JType methodReturnType) { SourceInfo info = ifStatement.getSourceInfo(); JExpression conditionExpression = ifStatement.getIfExpr(); - JStatement thenStmt = ifStatement.getThenStmt(); - JStatement 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; @@ -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,19 +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,22 +516,11 @@ private static JExpression extractExpression(JStatement statement) { return null; } - private static JStatement extractSingleStatement(JStatement statement) { - if (statement instanceof JBlock) { - JBlock block = (JBlock) statement; - if (block.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 f42eded388c..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 @@ -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; } @@ -537,13 +539,18 @@ 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) { + if (!x.getElseStmt().isEmpty() && !(then instanceof JBlock)) { + then = new JBlock(then.getSourceInfo(), then); + } + nestedStatementPush(then); + accept(then); + nestedStatementPop(then); } - if (x.getElseStmt() != null) { + JStatement elseStmt = x.getElseStmt().singleStatement(); + if (elseStmt != null) { if (needSemi) { semi(); newline(); @@ -552,7 +559,7 @@ public boolean visit(JIfStatement x, Context ctx) { needSemi = true; } print(CHARS_ELSE); - boolean elseIf = x.getElseStmt() instanceof JIfStatement; + boolean elseIf = elseStmt instanceof JIfStatement; if (!elseIf) { nestedStatementPush(x.getElseStmt()); } else { @@ -920,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 7bfa7c3cda2..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,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().isEmpty()) { + accept(x.getThenStmt().singleStatement()); } List thenExits = removeNormalExits(); addNormalExit(node, CfgConditionalNode.ELSE); - if (x.getElseStmt() != null) { - accept(x.getElseStmt()); + if (!x.getElseStmt().isEmpty()) { + accept(x.getElseStmt().singleStatement()); } addExits(thenExits); @@ -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/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java index 9bf04b54d17..a4c5d37c597 100644 --- a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java +++ b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java @@ -561,6 +561,11 @@ public boolean visit(JsIf x, JsContext ctx) { accept(x.getIfExpr()); _rparen(); 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); diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java index d641d08ef0d..d3e336a9dc5 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java @@ -268,7 +268,7 @@ public void testForOptimizations() { optimize("void", "for (i = 1; B.isFalse(); i++) { i = 2; }") .intoString("EntryPoint.i = 1;"); optimize("void", "for (i = 1; i < 5; i++) { i = 2; }") - .intoString("for (EntryPoint.i = 1; EntryPoint.i < 5; EntryPoint.i++) {\n EntryPoint.i = 2;\n}"); + .intoString("for (EntryPoint.i = 1; EntryPoint.i < 5; EntryPoint.i++)\n EntryPoint.i = 2;"); } /** @@ -397,9 +397,9 @@ public void testDoOptimization() throws Exception { optimize("void", "do {} while (false);").intoString(""); optimize("void", "do { i++; } while (false);").intoString("++EntryPoint.i;"); optimize("void", "do { break; } while (false);").intoString( - "do {", + "do", " break;", - "} while (false);"); + "while (false);"); } public void testNegationOptimizations() throws Exception { @@ -571,7 +571,10 @@ protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod // If m is processed first, it will see the constructor as having side effects. // Then the constructor will become empty enabling m() become empty in the next round. // - assertEquals(0, DeadCodeElimination.exec(program, method)); + String before = method.toSource(); + int moreMods = DeadCodeElimination.exec(program, method); + String after = method.toSource(); + assertEquals("before: " + before + ", after: " + after, 0, moreMods); } return mods > 0; } diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementCastsAndTypeChecksTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementCastsAndTypeChecksTest.java index 1868d706c36..ad5f819d017 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementCastsAndTypeChecksTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/ImplementCastsAndTypeChecksTest.java @@ -31,8 +31,7 @@ public void testCastCheckIntoNullCheck() throws Exception { result.intoString( "EntryPoint$A a = new EntryPoint$A();", "a = null;", - "if (a != null) {", - "}"); + "if (a != null);"); } public void testRemoveCastCheck_exactType() throws Exception { @@ -42,8 +41,7 @@ public void testRemoveCastCheck_exactType() throws Exception { optimize("void", "A a = new A(); if (a instanceof A) {}"); result.intoString( "EntryPoint$A a = new EntryPoint$A();", - "if (a != null) {", - "}"); + "if (a != null);"); } @Override diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/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/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/RemoveUnnecessaryControlFlowTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java new file mode 100644 index 00000000000..8f5b0802dd5 --- /dev/null +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/RemoveUnnecessaryControlFlowTest.java @@ -0,0 +1,148 @@ +/* + * Copyright 2026 GWT Project Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.gwt.dev.jjs.impl; + +import com.google.gwt.core.ext.TreeLogger; +import com.google.gwt.core.ext.UnableToCompleteException; +import com.google.gwt.dev.jjs.ast.JMethod; +import com.google.gwt.dev.jjs.ast.JProgram; + +public class RemoveUnnecessaryControlFlowTest extends OptimizerTestBase { + + @Override + protected void setUp() throws Exception { + super.setUp(); + + addSnippetClassDecl("static boolean condition() { return true; }"); + addSnippetClassDecl("static void foo() { }"); + } + + public void testReturnInSimpleBlock() throws UnableToCompleteException { + optimize("void", "return;").into(""); + optimize("void", "{return;}").into("{}"); + + optimize("void", "foo();return;").into("foo();"); + optimize("void", "{foo();return;}").into("{foo();}"); + optimize("void", "foo();{return;}").into("foo();{}"); + + optimize("String", "return null;").noChange(); + optimize("String", "{return null;}").noChange(); + } + + public void testReturnInIf() throws UnableToCompleteException { + optimize("void", "if (condition()) { return; }") + .into("if (condition()) { }"); + optimize("void", "if (condition()) { return; } return;") + .into("if (condition()) { }"); + + optimize("void", "if (condition()) { return; } foo();").noChange(); + + optimize("void", "if (condition()) { return; } else { return; }") + .into("if (condition()) { } else { }"); + optimize("void", "if (condition()) { foo(); } else { return; }") + .into("if (condition()) { foo(); } else { }"); + + optimize("void", "if (condition()) { if (condition()) { return; } foo(); return; }") + .into("if (condition()) { if (condition()) { return; } foo(); }"); + } + + public void testReturnInTry() throws UnableToCompleteException { + optimize("void", "try { return; } catch (Exception e) { return; } finally { return; }") + .into("try { } catch (Exception e) { } finally { }"); + } + + public void testReturnInLoop() throws UnableToCompleteException { + // while, with various returns + optimize("void", "while (condition()) { return; }") + .into("while (condition()) { break; }"); + optimize("void", "while (condition()) { return; } return;") + .into("while (condition()) { break; }"); + + optimize("void", "while (condition()) { return; } foo();").noChange(); + + // for, do/while - add some nesting + optimize("void", "for (int i = 0; i < 10; i++) { return; }") + .into("for (int i = 0; i < 10; i++) { break; }"); + optimize("void", "do { if (condition()) { return; } else { foo(); } } while(true);") + .into("do { if (condition()) { break; } else { foo(); } } while(true);"); + } + + public void testContinueInLoop() throws UnableToCompleteException { + // Same sorts of tests on each loop construct, with various other siblings/wrappings + optimize("void", "for (int i = 0; i < 10; i++) { continue; }") + .into("for (int i = 0; i < 10; i++) { }"); + optimize("void", "do { if (condition()) { continue; } else { foo(); } } while(true);") + .into("do { if (condition()) { } else { foo(); } } while(true);"); + optimize("void", "while(true) { foo(); continue; } ") + .into("while(true) { foo(); }"); + } + + public void testSwitchStmt() throws UnableToCompleteException { + // Final return/break removed + optimize("void", "switch (3) { case 1: return; case 2: default: return; }") + .into("switch (3) { case 1: return; case 2: default: }"); + optimize("void", "switch (3) { case 1: break; case 2: default: break; }") + .into("switch (3) { case 1: break; case 2: default: }"); + } + + public void testLoopInLoop() throws UnableToCompleteException { + // Continue can be omitted even if the loop isn't the last statement + optimize("void", "while (condition()) { while (condition()) { foo(); continue; } continue; } foo();") + .into("while (condition()) { while (condition()) { foo(); } } foo();"); + optimize("void", "while (condition()) { while (condition()) { foo(); continue; } }") + .into("while (condition()) { while (condition()) { foo(); } }"); + + // return cannot be omitted in a loop-in-loop + optimize("void", "while (condition()) { while (true) { foo(); return; } }") + .noChange(); + // loop-in-loop structure doesn't impact return in the outer loop + optimize("void", "while (condition()) { while (condition()) { foo(); } return; }") + .into("while (condition()) { while (condition()) { foo(); } break; }"); + } + + public void testSwitchInLoop() throws UnableToCompleteException { + // validate that the break is treated as part of the switch and still removed + optimize("void", "for (int i = 0; i < 10; i++) { switch (i) { case 1: return; case 2: break; } }") + .into("for (int i = 0; i < 10; i++) { switch (i) { case 1: return; case 2: } }"); + // Likewise, the continue inside the switch is part of the loop and removed as the final statement + optimize("void", "for (int i = 0; i < 10; i++) { switch (i) { case 1: return; case 2: continue; } }") + .into("for (int i = 0; i < 10; i++) { switch (i) { case 1: return; case 2: } }"); + } + + public void testLoopInSwitchStatement() throws UnableToCompleteException { + optimize("void", "switch(4) {case 1: for (int i = 0; i < 10; i++) { break; }}").noChange(); + + optimize("void", "switch(4) {case 0: foo(); case 1: for (int i = 0; i < 10; i++) { continue; } break; }") + .into("switch(4) {case 0: foo(); case 1: for (int i = 0; i < 10; i++) { }}"); + } + + public void testBlocksInSwitchStatement() throws UnableToCompleteException { + optimize("void", "switch(4) {case 0: {foo();} case 1: if (condition()) {return;} else {foo(); return;} }") + .into("switch(4) {case 0: {foo();} case 1: if (condition()) {} else {foo(); } }"); + } + + public void testContinuesInNonVoidMethod() throws UnableToCompleteException { + optimize("int", "while (condition()) { foo(); continue; } return 1;") + .into("while (condition()) { foo(); }; return 1;"); + } + + @Override + protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod method) + throws UnableToCompleteException { + int mods = RemoveUnnecessaryControlFlow.exec(program, OptimizerContext.NULL_OPTIMIZATION_CONTEXT); + // verify we converged in a single pass + assert RemoveUnnecessaryControlFlow.exec(program, OptimizerContext.NULL_OPTIMIZATION_CONTEXT) == 0; + return mods > 0; + } +} \ No newline at end of file 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..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 @@ -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"); @@ -250,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 -> [*]", @@ -263,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=*]", @@ -275,17 +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]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [4]", - "2: BLOCK -> [*]", - "STMT -> [*]", - "3: BLOCK -> [*]", - "STMT -> [*]", + "2: STMT -> [*]", + "3: STMT -> [*]", "WRITE(j, 2) -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=3, ELSE=*]", @@ -298,17 +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]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [1]", - "2: BLOCK -> [*]", - "STMT -> [*]", - "3: BLOCK -> [*]", - "STMT -> [*]", + "2: STMT -> [*]", + "3: STMT -> [*]", "WRITE(j, 2) -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=3, ELSE=*]", @@ -378,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]", @@ -393,8 +378,7 @@ public void testEmptyForStatement() throws Exception { "for (;;) { j++; }").is( "BLOCK -> [*]", "STMT -> [*]", - "1: BLOCK -> [*]", - "STMT -> [*]", + "1: STMT -> [*]", "READWRITE(j, null) -> [1]", "END"); } @@ -414,7 +398,6 @@ public void testThrowWithoutCatch2() throws Exception { "STMT -> [*]", "READ(b1) -> [*]", "COND (EntryPoint.b1) -> [THEN=*, ELSE=1]", - "BLOCK -> [*]", "STMT -> [*]", "READ(runtimeException) -> [*]", "THROW -> [2]", @@ -434,7 +417,6 @@ public void testWhileContinueNoLabel() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [1]", "2: STMT -> [*]", @@ -458,9 +440,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 +468,6 @@ public void testWhileContinueWithLabel2() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=3]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [1]", "3: STMT -> [*]", @@ -509,7 +489,6 @@ public void testWhileContinueWithLabel3() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [1]", "2: STMT -> [*]", @@ -527,7 +506,6 @@ public void testWhileBreakNoLabel() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [3]", "2: STMT -> [*]", @@ -541,18 +519,14 @@ 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]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [4]", - "2: BLOCK -> [*]", - "STMT -> [*]", + "2: STMT -> [*]", "3: READ(i) -> [*]", "COND (EntryPoint.i < 10) -> [THEN=*, ELSE=1]", - "BLOCK -> [*]", "STMT -> [*]", "READWRITE(i, null) -> [3]", "4: END"); @@ -576,7 +550,6 @@ public void testWhileBreakWithLabel1() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=3]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [1]", "3: STMT -> [*]", @@ -602,7 +575,6 @@ public void testWhileBreakWithLabel2() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=3]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [4]", "3: STMT -> [*]", @@ -621,7 +593,6 @@ public void testWhileBreakWithLabel3() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [3]", "2: STMT -> [*]", @@ -642,7 +613,6 @@ public void testForBreakNoLabel() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [3]", "2: STMT -> [*]", @@ -664,7 +634,6 @@ public void testForContinueNoLabel() throws Exception { "STMT -> [*]", "READ(b2) -> [*]", "COND (EntryPoint.b2) -> [THEN=*, ELSE=2]", - "BLOCK -> [*]", "STMT -> [*]", "GOTO -> [3]", "2: STMT -> [*]", @@ -680,20 +649,16 @@ public void testForBreakNestedForWithLabel() throws Exception { "STMT -> [*]", "WRITE(j, 0) -> [*]", "STMT -> [*]", - "1: BLOCK -> [*]", - "STMT -> [*]", + "1: 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) -> [*]", "COND (i < 1) -> [THEN=*, ELSE=1]", - "BLOCK -> [*]", "STMT -> [*]", "READ(i) -> [*]", "WRITE(j, i) -> [*]", @@ -711,20 +676,16 @@ public void testForBreakNestedForNoLabel() throws Exception { "STMT -> [*]", "WRITE(j, 0) -> [*]", "STMT -> [*]", - "1: BLOCK -> [*]", - "STMT -> [*]", + "1: 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) -> [*]", "COND (i < 1) -> [THEN=*, ELSE=1]", - "BLOCK -> [*]", "STMT -> [*]", "READ(i) -> [*]", "WRITE(j, i) -> [*]", @@ -1678,7 +1639,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..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 @@ -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"); @@ -180,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 { @@ -238,7 +235,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}]", diff --git a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java index 4db0da3de13..18498fe4b54 100644 --- a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java +++ b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorTest.java @@ -19,12 +19,19 @@ import com.google.gwt.dev.cfg.ConditionNone; import com.google.gwt.dev.cfg.ConfigurationProperty; import com.google.gwt.dev.jjs.impl.DeadCodeElimination; +import com.google.gwt.dev.jjs.SourceOrigin; import com.google.gwt.dev.jjs.impl.FullCompileTestBase; import com.google.gwt.dev.jjs.impl.FullOptimizerContext; +import com.google.gwt.dev.js.ast.JsExprStmt; +import com.google.gwt.dev.js.ast.JsFunction; +import com.google.gwt.dev.js.ast.JsIf; +import com.google.gwt.dev.js.ast.JsStatement; import com.google.gwt.dev.util.DefaultTextOutput; import com.google.gwt.dev.util.TextOutput; import com.google.gwt.thirdparty.guava.common.collect.Maps; +import java.io.IOException; +import java.io.StringReader; import java.util.List; import java.util.Map; @@ -79,6 +86,65 @@ public void testClassRangeMarking() throws UnableToCompleteException { assertTrue(programClassRange.getEndPosition() < text.getPosition()); } + public void testDanglingElse() throws Exception { + // No braces, ELSE will be attached to the inner IF + String statements = """ + if (a) + if (b) + go(1); + else + go(2); + """; + JsStatement result = compileAndParseStatement(statements); + assertNotNull(result); + assertTrue(result instanceof JsIf); + JsIf outerIf = (JsIf) result; + assertTrue(outerIf.getThenStmt() instanceof JsIf); + JsIf innerIf = (JsIf) outerIf.getThenStmt(); + assertNotNull(innerIf.getElseStmt()); + assertNull(outerIf.getElseStmt()); + } + + private JsStatement compileAndParseStatement(String statements) throws UnableToCompleteException, IOException, JsParserException { + String code = """ + package test; + public class EntryPoint { + private static boolean a = true, b = true, c = true; + private static void go(int i) { + } + public static void onModuleLoad() { +""" + statements + """ + } + } + """; + compileSnippetToJS(code); + TextOutput text = new DefaultTextOutput(true); + JsSourceGenerationVisitor jsSourceGenerationVisitor = new JsSourceGenerationVisitor(text); + jsSourceGenerationVisitor.accept(jsProgram); + + List classRanges = jsSourceGenerationVisitor.getClassRanges(); + String entrypoint = classRanges.stream().filter(r -> r.getName().equals("test.EntryPoint")).findFirst().map(r -> { + return text.toString().substring(r.getStartPosition(), r.getEndPosition()); + }).get(); + + // Parse the source to be sure that the printed output has the expected characteristics + List parsed = JsParser.parse(SourceOrigin.UNKNOWN, jsProgram.getScope(), new StringReader(entrypoint)); + + JsStatement result = null; + for (JsStatement jsStatement : parsed) { + if (jsStatement instanceof JsExprStmt && ((JsExprStmt) jsStatement).getExpression() instanceof JsFunction) { + JsFunction jsFunction = (JsFunction) ((JsExprStmt) jsStatement).getExpression(); + if (jsFunction.getName().getShortIdent().equals("onModuleLoad")) { + // In case of clinit, return the last statement only + List s = jsFunction.getBody().getStatements(); + result = s.get(s.size() - 1); + break; + } + } + } + return result; + } + public void testLiteralPrint() throws UnableToCompleteException { TextOutput text = buildTextOutput(new JsToStringGenerationVisitor.PrintOptions(false, false));