Skip to content

Simplify boolean expressions in JavaScript AST #10261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zbynek wants to merge 3 commits into
base: main
Choose a base branch
from
+190 −42
Open

Simplify boolean expressions in JavaScript AST #10261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52f4726
Simplify boolean expressions in JavaScript AST
zbynek Feb 20, 2026
4980b13
Add test, fix JavaDoc visibility issue
zbynek Feb 24, 2026
cdd001c
Propagate context for conditional ternary operator
zbynek Feb 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 122 additions & 38 deletions dev/core/src/com/google/gwt/dev/js/JsStaticEval.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@
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;
import java.util.HashSet;
import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -104,7 +105,7 @@ private static class MustExecVisitor extends JsVisitor {

private final List<JsStatement> mustExec = new ArrayList<JsStatement>();

public MustExecVisitor() {
MustExecVisitor() {
}

@Override
Expand Down Expand Up @@ -144,21 +145,53 @@ 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.
*
* TODO: borrow more concepts from
* {@link com.google.gwt.dev.jjs.impl.DeadCodeElimination}, such as ignored
* expression results.
Copy link
Copy Markdown
Member

@niloc132 niloc132 Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're quite close to throwing away ignored expression results here. Probably not worth adding a few more VOID checks at this stage

*/
private class StaticEvalVisitor extends JsModVisitor {
@VisibleForTesting
static class StaticEvalVisitor extends JsModVisitor {

private Set<JsExpression> evalBooleanContext = new HashSet<JsExpression>();
/**
* Stores how are expression evaluations used.
* Missing entry = no coercion, difference between null and false matters.
*/
private final Map<JsExpression, EvalMode> evalContext = new HashMap<>();

/**
* This is used by {@link #additionCoercesToString}.
*/
private Map<JsExpression, Boolean> coercesToStringMap = new IdentityHashMap<JsExpression, Boolean>();
private Map<JsExpression, Boolean> 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);
Comment thread
zbynek marked this conversation as resolved.
if (evalContext.containsKey(x)) {
evalContext.put(x.getArg2(), evalContext.get(x));
}
}
return true;
}

@Override
public void endVisit(JsBinaryOperation x, JsContext ctx) {
Expand All @@ -167,11 +200,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) {
Expand All @@ -195,7 +230,8 @@ public void endVisit(JsBinaryOperation x, JsContext ctx) {
break;
}
}

evalContext.remove(arg1);
evalContext.remove(arg2);
result = maybeReorderOperations(result);

if (result != x) {
Expand Down Expand Up @@ -256,21 +292,29 @@ public void endVisit(JsBlock x, JsContext ctx) {

@Override
public void endVisit(JsConditional x, JsContext ctx) {
evalBooleanContext.remove(x.getTestExpression());
evalContext.remove(x.getTestExpression());
evalContext.remove(x.getThenExpression());
evalContext.remove(x.getElseExpression());

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));
}
}
Expand All @@ -281,7 +325,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) {
Expand All @@ -304,6 +348,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();
Expand All @@ -318,7 +363,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) {
Expand Down Expand Up @@ -348,7 +393,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) {
Expand Down Expand Up @@ -402,10 +447,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();
Expand All @@ -422,7 +467,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) {
Expand All @@ -443,39 +488,44 @@ 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);
EvalMode evalMode = evalContext.get(x);
if (evalMode != null) {
evalContext.put(x.getThenExpression(), evalMode);
evalContext.put(x.getElseExpression(), evalMode);
}
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;
}

Expand Down Expand Up @@ -607,41 +657,75 @@ public static int exec(JsProgram program) {
* Simplify short circuit AND expressions.
*
* <pre>
* 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()
* </pre>
* In boolean context also
* <pre>
* if (isWhatever() && true()) -> if (isWhatever()) unless true() has side effects
* if (isWhatever() && false()) -> if (false()) unless isWhatever() side effects
* </pre>
*/
protected static JsExpression shortCircuitAnd(JsBinaryOperation expr) {
protected static JsExpression shortCircuitAnd(JsBinaryOperation expr,
Map<JsExpression, EvalMode> 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)) {
Copy link
Copy Markdown
Member

@niloc132 niloc132 Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern, just a containsKey, so it could be explicitly VOID.

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.
*
* <pre>
* 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()
* </pre>
* In boolean context also
* <pre>
* if (isWhatever() || false()) -> if (isWhatever()) unless false() has side effects
* if (isWhatever() || true()) -> if (true()) unless isWhatever() side effects
* </pre>
*/
protected static JsExpression shortCircuitOr(JsBinaryOperation expr) {
protected static JsExpression shortCircuitOr(JsBinaryOperation expr,
Map<JsExpression, EvalMode> 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)) {
Copy link
Copy Markdown
Member

@niloc132 niloc132 Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this containsKey(expr) be a get(expr) == BOOL? It might have been missed in the last update, but seems implied by the javadoc.

If it is VOID, I would assume we actually throw away arg2... but maybe a later patch (see my comment on the TODO for this nested type).

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;
Expand Down
Loading
Loading