From 52f472695a3a2acb7b4e1b6ac41fdcda59aa3f2f Mon Sep 17 00:00:00 2001 From: Zbynek Konecny Date: Fri, 20 Feb 2026 03:00:29 +0100 Subject: [PATCH 1/3] Simplify boolean expressions in JavaScript AST --- .../com/google/gwt/dev/js/JsStaticEval.java | 149 +++++++++++++----- .../google/gwt/dev/js/JsStaticEvalTest.java | 64 +++++++- 2 files changed, 173 insertions(+), 40 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java b/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java index d5edef08cc6..bb6d4f30b03 100644 --- a/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java +++ b/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java @@ -53,7 +53,7 @@ import java.util.ArrayList; import java.util.EnumSet; -import java.util.HashSet; +import java.util.HashMap; import java.util.IdentityHashMap; import java.util.List; import java.util.Map; @@ -104,7 +104,7 @@ private static class MustExecVisitor extends JsVisitor { private final List mustExec = new ArrayList(); - public MustExecVisitor() { + MustExecVisitor() { } @Override @@ -144,6 +144,14 @@ public boolean visit(JsFunction x, JsContext ctx) { } } + /** + * Describes how is the result of evaluation used. + */ + private enum EvalMode { + BOOL, // result will be coerced to a boolean + VOID // result will be discarded + } + /** * Does static evals. * @@ -153,12 +161,35 @@ public boolean visit(JsFunction x, JsContext ctx) { */ private class StaticEvalVisitor extends JsModVisitor { - private Set evalBooleanContext = new HashSet(); + /** + * Stores how are expression evaluations used. + * Missing entry = no coercion, difference between null and false matters. + */ + private final Map evalContext = new HashMap<>(); /** * This is used by {@link #additionCoercesToString}. */ - private Map coercesToStringMap = new IdentityHashMap(); + private Map coercesToStringMap = new IdentityHashMap<>(); + + public boolean visit(JsExprStmt x, JsContext ctx) { + evalContext.put(x.getExpression(), EvalMode.VOID); + return true; + } + + public boolean visit(JsBinaryOperation x, JsContext ctx) { + if (evalContext.containsKey(x) + && (x.getOperator() == JsBinaryOperator.AND || x.getOperator() == JsBinaryOperator.OR)) { + evalContext.put(x.getArg1(), EvalMode.BOOL); + evalContext.put(x.getArg2(), evalContext.get(x)); + } else if (x.getOperator() == JsBinaryOperator.COMMA) { + evalContext.put(x.getArg1(), EvalMode.VOID); + if (evalContext.containsKey(x)) { + evalContext.put(x.getArg2(), evalContext.get(x)); + } + } + return true; + } @Override public void endVisit(JsBinaryOperation x, JsContext ctx) { @@ -167,11 +198,13 @@ public void endVisit(JsBinaryOperation x, JsContext ctx) { JsExpression arg2 = x.getArg2(); JsExpression result = x; - - if (op == JsBinaryOperator.AND) { - result = shortCircuitAnd(x); + if (evalContext.get(x) == EvalMode.VOID + && !arg2.hasSideEffects() && !op.isAssignment()) { + result = arg1; + } else if (op == JsBinaryOperator.AND) { + result = shortCircuitAnd(x, evalContext); } else if (op == JsBinaryOperator.OR) { - result = shortCircuitOr(x); + result = shortCircuitOr(x, evalContext); } else if (op == JsBinaryOperator.COMMA) { result = trySimplifyComma(x); } else if (op == JsBinaryOperator.EQ || op == JsBinaryOperator.REF_EQ) { @@ -195,7 +228,8 @@ public void endVisit(JsBinaryOperation x, JsContext ctx) { break; } } - + evalContext.remove(arg1); + evalContext.remove(arg2); result = maybeReorderOperations(result); if (result != x) { @@ -256,21 +290,27 @@ public void endVisit(JsBlock x, JsContext ctx) { @Override public void endVisit(JsConditional x, JsContext ctx) { - evalBooleanContext.remove(x.getTestExpression()); + evalContext.remove(x.getTestExpression()); JsExpression condExpr = x.getTestExpression(); JsExpression thenExpr = x.getThenExpression(); JsExpression elseExpr = x.getElseExpression(); - if (condExpr instanceof CanBooleanEval) { + if (condExpr instanceof JsBinaryOperation + && ((JsBinaryOperation) condExpr).getOperator() == JsBinaryOperator.COMMA) { + JsBinaryOperation condition = (JsBinaryOperation) condExpr; + JsExpression newConditional = accept(new JsConditional(x.getSourceInfo(), + condition.getArg2(), thenExpr, elseExpr)); + ctx.replaceMe(withSideEffect(newConditional, condition.getArg1(), x.getSourceInfo())); + } else if (condExpr instanceof CanBooleanEval) { CanBooleanEval condEval = (CanBooleanEval) condExpr; if (condEval.isBooleanTrue()) { JsBinaryOperation binOp = new JsBinaryOperation(x.getSourceInfo(), - JsBinaryOperator.AND, condExpr, thenExpr); + JsBinaryOperator.COMMA, condExpr, thenExpr); ctx.replaceMe(accept(binOp)); } else if (condEval.isBooleanFalse()) { - // e.g. (false() ? then : else) -> false() || else + // e.g. (false() ? then : else) -> (false() , else) JsBinaryOperation binOp = new JsBinaryOperation(x.getSourceInfo(), - JsBinaryOperator.OR, condExpr, elseExpr); + JsBinaryOperator.COMMA, condExpr, elseExpr); ctx.replaceMe(accept(binOp)); } } @@ -281,7 +321,7 @@ public void endVisit(JsConditional x, JsContext ctx) { */ @Override public void endVisit(JsDoWhile x, JsContext ctx) { - evalBooleanContext.remove(x.getCondition()); + evalContext.remove(x.getCondition()); JsExpression expr = x.getCondition(); if (expr instanceof CanBooleanEval) { @@ -304,6 +344,7 @@ public void endVisit(JsDoWhile x, JsContext ctx) { @Override public void endVisit(JsExprStmt x, JsContext ctx) { + evalContext.remove(x.getExpression()); if (!x.getExpression().hasSideEffects()) { if (ctx.canRemove()) { ctx.removeMe(); @@ -318,7 +359,7 @@ public void endVisit(JsExprStmt x, JsContext ctx) { */ @Override public void endVisit(JsFor x, JsContext ctx) { - evalBooleanContext.remove(x.getCondition()); + evalContext.remove(x.getCondition()); JsExpression expr = x.getCondition(); if (expr instanceof CanBooleanEval) { @@ -348,7 +389,7 @@ public void endVisit(JsFor x, JsContext ctx) { */ @Override public void endVisit(JsIf x, JsContext ctx) { - evalBooleanContext.remove(x.getIfExpr()); + evalContext.remove(x.getIfExpr()); JsExpression condExpr = x.getIfExpr(); if (condExpr instanceof CanBooleanEval) { @@ -402,10 +443,10 @@ public void endVisit(JsIf x, JsContext ctx) { @Override public void endVisit(JsPrefixOperation x, JsContext ctx) { if (x.getOperator() == JsUnaryOperator.NOT) { - evalBooleanContext.remove(x.getArg()); + evalContext.remove(x.getArg()); } - if (evalBooleanContext.contains(x)) { + if (evalContext.containsKey(x)) { if ((x.getOperator() == JsUnaryOperator.NOT) && (x.getArg() instanceof JsPrefixOperation)) { JsPrefixOperation arg = (JsPrefixOperation) x.getArg(); @@ -422,7 +463,7 @@ public void endVisit(JsPrefixOperation x, JsContext ctx) { */ @Override public void endVisit(JsWhile x, JsContext ctx) { - evalBooleanContext.remove(x.getCondition()); + evalContext.remove(x.getCondition()); JsExpression expr = x.getCondition(); if (expr instanceof CanBooleanEval) { @@ -443,39 +484,39 @@ public void endVisit(JsWhile x, JsContext ctx) { @Override public boolean visit(JsConditional x, JsContext ctx) { - evalBooleanContext.add(x.getTestExpression()); + evalContext.put(x.getTestExpression(), EvalMode.BOOL); return true; } @Override public boolean visit(JsDoWhile x, JsContext ctx) { - evalBooleanContext.add(x.getCondition()); + evalContext.put(x.getCondition(), EvalMode.BOOL); return true; } @Override public boolean visit(JsFor x, JsContext ctx) { - evalBooleanContext.add(x.getCondition()); + evalContext.put(x.getCondition(), EvalMode.BOOL); return true; } @Override public boolean visit(JsIf x, JsContext ctx) { - evalBooleanContext.add(x.getIfExpr()); + evalContext.put(x.getIfExpr(), EvalMode.BOOL); return true; } @Override public boolean visit(JsPrefixOperation x, JsContext ctx) { if (x.getOperator() == JsUnaryOperator.NOT) { - evalBooleanContext.add(x.getArg()); + evalContext.put(x.getArg(), EvalMode.BOOL); } return true; } @Override public boolean visit(JsWhile x, JsContext ctx) { - evalBooleanContext.add(x.getCondition()); + evalContext.put(x.getCondition(), EvalMode.BOOL); return true; } @@ -607,41 +648,75 @@ public static int exec(JsProgram program) { * Simplify short circuit AND expressions. * * 
-   * if (true && isWhatever()) -> if (isWhatever()), unless side effects
-   * if (false() && isWhatever()) -> if (false())
+   * return true() && isWhatever() -> return isWhatever(), unless true() has side effects
+   * return false() && isWhatever() -> return false()
+   *
+ * In boolean context also + * 
+   * if (isWhatever() && true()) -> if (isWhatever()) unless true() has side effects
+   * if (isWhatever() && false()) -> if (false()) unless isWhatever() side effects
    *
*/ - protected static JsExpression shortCircuitAnd(JsBinaryOperation expr) { + protected static JsExpression shortCircuitAnd(JsBinaryOperation expr, + Map evalContext) { JsExpression arg1 = expr.getArg1(); JsExpression arg2 = expr.getArg2(); if (arg1 instanceof CanBooleanEval) { CanBooleanEval eval1 = (CanBooleanEval) arg1; - if (eval1.isBooleanTrue() && !arg1.hasSideEffects()) { - return arg2; + if (eval1.isBooleanTrue()) { + return withSideEffect(arg2, arg1, expr.getSourceInfo()); } else if (eval1.isBooleanFalse()) { return arg1; } } + if (arg2 instanceof CanBooleanEval && evalContext.containsKey(expr)) { + CanBooleanEval eval2 = (CanBooleanEval) arg2; + if (eval2.isBooleanTrue() && !arg2.hasSideEffects()) { + return arg1; + } else if (eval2.isBooleanFalse()) { + return withSideEffect(arg2, arg1, expr.getSourceInfo()); + } + } return expr; } + private static JsExpression withSideEffect(JsExpression main, JsExpression sideEffect, + SourceInfo sourceInfo) { + return !sideEffect.hasSideEffects() ? main + : new JsBinaryOperation(sourceInfo, JsBinaryOperator.COMMA, sideEffect, main); + } + /** * Simplify short circuit OR expressions. * * 
-   * if (true() || isWhatever()) -> if (true())
-   * if (false || isWhatever()) -> if (isWhatever()), unless side effects
+   * return false() || isWhatever() -> return isWhatever(), unless false() has side effects
+   * return true() && isWhatever() -> return true()
+   *
+ * In boolean context also + * 
+   * if (isWhatever() || false()) -> if (isWhatever()) unless false() has side effects
+   * if (isWhatever() || true()) -> if (true()) unless isWhatever() side effects
    *
*/ - protected static JsExpression shortCircuitOr(JsBinaryOperation expr) { + protected static JsExpression shortCircuitOr(JsBinaryOperation expr, + Map evalContext) { JsExpression arg1 = expr.getArg1(); JsExpression arg2 = expr.getArg2(); if (arg1 instanceof CanBooleanEval) { CanBooleanEval eval1 = (CanBooleanEval) arg1; - if (eval1.isBooleanTrue()) { + if (eval1.isBooleanFalse()) { + return withSideEffect(arg2, arg1, expr.getSourceInfo()); + } else if (eval1.isBooleanTrue()) { + return arg1; + } + } + if (arg2 instanceof CanBooleanEval && evalContext.containsKey(expr)) { + CanBooleanEval eval2 = (CanBooleanEval) arg2; + if (eval2.isBooleanFalse() && !arg2.hasSideEffects()) { return arg1; - } else if (eval1.isBooleanFalse() && !arg1.hasSideEffects()) { - return arg2; + } else if (eval2.isBooleanTrue()) { + return withSideEffect(arg2, arg1, expr.getSourceInfo()); } } return expr; diff --git a/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java b/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java index 3177b093199..b5df9a456cc 100644 --- a/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java +++ b/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java @@ -43,7 +43,8 @@ public void testAssociativity() throws Exception { assertEquals("alert(a&&b||c&&d);", optimize("alert((a && b) || ( c && d));")); assertEquals("a(),b&&c();", optimize("a(), b && c()")); - assertEquals("a()&&b,c();", optimize("a() && b, c()")); + assertEquals("a()&&b(),c();", optimize("a() && b(), c()")); + assertEquals("a(),c();", optimize("a() && b, c()")); // Don't damage math expressions assertEquals("alert(seconds/3600);", @@ -53,8 +54,8 @@ public void testAssociativity() throws Exception { assertEquals("alert(1-(1-foo));", optimize("alert(1 - (1 - foo))")); // Don't damage assignments - assertEquals("alert((a=0,b=foo));", - optimize("alert((a = 0, b = (bar, foo)))")); + assertEquals("alert((a=7,b=foo));", + optimize("alert((a = 7, b = (bar, foo)))")); assertEquals("alert(1+(a='2')+3+4);", optimize("alert(1 + (a = '2') + 3 + 4);")); assertEquals("alert(1+(a='2')+7);", @@ -154,6 +155,63 @@ public void testLiteralCompares() throws Exception { assertEquals("alert(true);", optimize("alert(\"a\" !== null)")); } + public void testShortCircuitAnd() throws Exception { + assertEquals("alert(a);", optimize("alert(true && a)")); + assertEquals("alert(false);", optimize("alert(false && a)")); + + // these can't be simplified to maintain type + assertEquals("alert(a&&true);", optimize("alert(a && true)")); + assertEquals("alert(a&&false);", optimize("alert(a && false)")); + assertEquals("alert(!!a&&!!b);", optimize("alert(!!a && !!b)")); + assertEquals("alert(c|(bits&&1));", optimize("alert(c | (a, bits && 1))")); + + // in boolean context we can simplify more + assertEquals("alert(a&&b?c:d);", optimize("alert(!!a && !!b ? c :d)")); + assertEquals("alert(d);", optimize("alert(false && !!b ? c :d)")); + assertEquals("alert(b?c:d);", optimize("alert(true && !!b ? c :d)")); + assertEquals("alert(b?c:d1);", optimize("alert(b && true ? c :d1)")); + assertEquals("alert(d1);", optimize("alert(b && false ? c :d1)")); + assertEquals("alert(d2);", optimize("alert(a && false && b ? c :d2)")); + } + + public void testShortCircuitAndWithSideEffects() throws Exception { + assertEquals("a&&b();", optimize("!!a && !!b()")); + assertEquals("a();", optimize("a() && false && c();")); + assertEquals("a(),e();", optimize("a() && false && c() ? d() : e();")); + assertEquals("a()&&c()?d():e();", optimize("a() && true && c() ? d() : e();")); + } + + public void testSimplifyComma() throws Exception { + assertEquals("alert(!!a());", optimize("alert((true, !!a()))")); + assertEquals("alert((a(),true));", optimize("alert((!!a(), true))")); + } + + public void testShortCircuitOr() throws Exception { + assertEquals("alert(true);", optimize("alert(true || a)")); + assertEquals("alert(a);", optimize("alert(false || a)")); + + // these can't be simplified to maintain type + assertEquals("alert(a||true);", optimize("alert(a || true)")); + assertEquals("alert(a||false);", optimize("alert(a || false)")); + assertEquals("alert(!!a||!!b);", optimize("alert(!!a || !!b)")); + assertEquals("alert(c|(bits||0));", optimize("alert(c | (a, bits || 0))")); + + // in boolean context we can simplify more + assertEquals("alert(a||b?c:d);", optimize("alert(!!a || !!b ? c :d)")); + assertEquals("alert(c);", optimize("alert(true || !!b ? c :d)")); + assertEquals("alert(b?c:d);", optimize("alert(false || !!b ? c :d)")); + assertEquals("alert(b?c:d1);", optimize("alert(b || false ? c :d1)")); + assertEquals("alert(c1);", optimize("alert(b || true ? c1 :d)")); + assertEquals("alert(c2);", optimize("alert(a || true || b ? c2 :d)")); + } + + public void testShortCircuitOrWithSideEffects() throws Exception { + assertEquals("a||b();", optimize("!!a || !!b()")); + assertEquals("a();", optimize("a() || true || c();")); + assertEquals("a(),d();", optimize("a() || true || c() ? d() : e();")); + assertEquals("a()||c()?d():e();", optimize("a() || false || c() ? d() : e();")); + } + public void testLiteralEqNull() throws Exception { assertEquals("alert(false);", optimize("alert('test' == null)")); } From 4980b13d899cda7fc322882775bd1f249e9b386b Mon Sep 17 00:00:00 2001 From: Zbynek Konecny Date: Tue, 24 Feb 2026 20:35:05 +0100 Subject: [PATCH 2/3] Add test, fix JavaDoc visibility issue --- dev/core/src/com/google/gwt/dev/js/JsStaticEval.java | 4 +++- dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java b/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java index bb6d4f30b03..08e2d39380c 100644 --- a/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java +++ b/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java @@ -50,6 +50,7 @@ import com.google.gwt.dev.js.ast.JsWhile; import com.google.gwt.dev.js.rhino.ScriptRuntime; import com.google.gwt.dev.util.Ieee754_64_Arithmetic; +import com.google.gwt.thirdparty.guava.common.annotations.VisibleForTesting; import java.util.ArrayList; import java.util.EnumSet; @@ -159,7 +160,8 @@ private enum EvalMode { * {@link com.google.gwt.dev.jjs.impl.DeadCodeElimination}, such as ignored * expression results. */ - private class StaticEvalVisitor extends JsModVisitor { + @VisibleForTesting + static class StaticEvalVisitor extends JsModVisitor { /** * Stores how are expression evaluations used. diff --git a/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java b/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java index b5df9a456cc..1e63a93ab32 100644 --- a/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java +++ b/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java @@ -88,7 +88,8 @@ public void testAssociativity() throws Exception { /** * Test for issue 7088. JsStatic eval infinite loop in - * {@link JsStaticEval.StaticEvalVisitor#endVisit(JsBlock, JsContext)} + * {@link JsStaticEval.StaticEvalVisitor#endVisit(com.google.gwt.dev.js.ast.JsBlock, + * com.google.gwt.dev.js.ast.JsContext)} */ public void testDeclareAfterReturn() throws Exception { // TODO(rluble): Note that the source output has the wrong precedence for function definition @@ -179,6 +180,7 @@ public void testShortCircuitAndWithSideEffects() throws Exception { assertEquals("a();", optimize("a() && false && c();")); assertEquals("a(),e();", optimize("a() && false && c() ? d() : e();")); assertEquals("a()&&c()?d():e();", optimize("a() && true && c() ? d() : e();")); + assertEquals("a()&&b();", optimize("a()&&(b(),undefined)")); } public void testSimplifyComma() throws Exception { From cdd001c3ab76c10f52e410cbde0df88d313da3b3 Mon Sep 17 00:00:00 2001 From: Zbynek Konecny Date: Wed, 25 Feb 2026 03:07:05 +0100 Subject: [PATCH 3/3] Propagate context for conditional ternary operator --- dev/core/src/com/google/gwt/dev/js/JsStaticEval.java | 7 +++++++ dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java b/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java index 08e2d39380c..cf3420a5174 100644 --- a/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java +++ b/dev/core/src/com/google/gwt/dev/js/JsStaticEval.java @@ -293,6 +293,8 @@ public void endVisit(JsBlock x, JsContext ctx) { @Override public void endVisit(JsConditional x, JsContext ctx) { evalContext.remove(x.getTestExpression()); + evalContext.remove(x.getThenExpression()); + evalContext.remove(x.getElseExpression()); JsExpression condExpr = x.getTestExpression(); JsExpression thenExpr = x.getThenExpression(); @@ -487,6 +489,11 @@ public void endVisit(JsWhile x, JsContext ctx) { @Override public boolean visit(JsConditional x, JsContext ctx) { evalContext.put(x.getTestExpression(), EvalMode.BOOL); + EvalMode evalMode = evalContext.get(x); + if (evalMode != null) { + evalContext.put(x.getThenExpression(), evalMode); + evalContext.put(x.getElseExpression(), evalMode); + } return true; } diff --git a/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java b/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java index 1e63a93ab32..d2e7ca5c5fd 100644 --- a/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java +++ b/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java @@ -180,7 +180,11 @@ public void testShortCircuitAndWithSideEffects() throws Exception { assertEquals("a();", optimize("a() && false && c();")); assertEquals("a(),e();", optimize("a() && false && c() ? d() : e();")); assertEquals("a()&&c()?d():e();", optimize("a() && true && c() ? d() : e();")); + } + + public void testSimplifyCommaInVoidContext() throws Exception { assertEquals("a()&&b();", optimize("a()&&(b(),undefined)")); + assertEquals("a()?b():c();", optimize("a()?(b(),undefined):(c(),undefined)")); } public void testSimplifyComma() throws Exception {