Skip to content

Commit 52f4726

Browse files
zbynekzbynek
committed
Simplify boolean expressions in JavaScript AST
1 parent 2e307b7 commit 52f4726

2 files changed

Lines changed: 173 additions & 40 deletions

File tree

‎dev/core/src/com/google/gwt/dev/js/JsStaticEval.java‎

Lines changed: 112 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353

5454
import java.util.ArrayList;
5555
import java.util.EnumSet;
56-
import java.util.HashSet;
56+
import java.util.HashMap;
5757
import java.util.IdentityHashMap;
5858
import java.util.List;
5959
import java.util.Map;
@@ -104,7 +104,7 @@ private static class MustExecVisitor extends JsVisitor {
104104

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

107-
public MustExecVisitor() {
107+
MustExecVisitor() {
108108
}
109109

110110
@Override
@@ -144,6 +144,14 @@ public boolean visit(JsFunction x, JsContext ctx) {
144144
}
145145
}
146146

147+
/**
148+
* Describes how is the result of evaluation used.
149+
*/
150+
private enum EvalMode {
151+
BOOL, // result will be coerced to a boolean
152+
VOID // result will be discarded
153+
}
154+
147155
/**
148156
* Does static evals.
149157
*
@@ -153,12 +161,35 @@ public boolean visit(JsFunction x, JsContext ctx) {
153161
*/
154162
private class StaticEvalVisitor extends JsModVisitor {
155163

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

158170
/**
159171
* This is used by {@link #additionCoercesToString}.
160172
*/
161-
private Map<JsExpression, Boolean> coercesToStringMap = new IdentityHashMap<JsExpression, Boolean>();
173+
private Map<JsExpression, Boolean> coercesToStringMap = new IdentityHashMap<>();
174+
175+
public boolean visit(JsExprStmt x, JsContext ctx) {
176+
evalContext.put(x.getExpression(), EvalMode.VOID);
177+
return true;
178+
}
179+
180+
public boolean visit(JsBinaryOperation x, JsContext ctx) {
181+
if (evalContext.containsKey(x)
182+
&& (x.getOperator() == JsBinaryOperator.AND || x.getOperator() == JsBinaryOperator.OR)) {
183+
evalContext.put(x.getArg1(), EvalMode.BOOL);
184+
evalContext.put(x.getArg2(), evalContext.get(x));
185+
} else if (x.getOperator() == JsBinaryOperator.COMMA) {
186+
evalContext.put(x.getArg1(), EvalMode.VOID);
187+
if (evalContext.containsKey(x)) {
188+
evalContext.put(x.getArg2(), evalContext.get(x));
189+
}
190+
}
191+
return true;
192+
}
162193

163194
@Override
164195
public void endVisit(JsBinaryOperation x, JsContext ctx) {
@@ -167,11 +198,13 @@ public void endVisit(JsBinaryOperation x, JsContext ctx) {
167198
JsExpression arg2 = x.getArg2();
168199

169200
JsExpression result = x;
170-
171-
if (op == JsBinaryOperator.AND) {
172-
result = shortCircuitAnd(x);
201+
if (evalContext.get(x) == EvalMode.VOID
202+
&& !arg2.hasSideEffects() && !op.isAssignment()) {
203+
result = arg1;
204+
} else if (op == JsBinaryOperator.AND) {
205+
result = shortCircuitAnd(x, evalContext);
173206
} else if (op == JsBinaryOperator.OR) {
174-
result = shortCircuitOr(x);
207+
result = shortCircuitOr(x, evalContext);
175208
} else if (op == JsBinaryOperator.COMMA) {
176209
result = trySimplifyComma(x);
177210
} else if (op == JsBinaryOperator.EQ || op == JsBinaryOperator.REF_EQ) {
@@ -195,7 +228,8 @@ public void endVisit(JsBinaryOperation x, JsContext ctx) {
195228
break;
196229
}
197230
}
198-
231+
evalContext.remove(arg1);
232+
evalContext.remove(arg2);
199233
result = maybeReorderOperations(result);
200234

201235
if (result != x) {
@@ -256,21 +290,27 @@ public void endVisit(JsBlock x, JsContext ctx) {
256290

257291
@Override
258292
public void endVisit(JsConditional x, JsContext ctx) {
259-
evalBooleanContext.remove(x.getTestExpression());
293+
evalContext.remove(x.getTestExpression());
260294

261295
JsExpression condExpr = x.getTestExpression();
262296
JsExpression thenExpr = x.getThenExpression();
263297
JsExpression elseExpr = x.getElseExpression();
264-
if (condExpr instanceof CanBooleanEval) {
298+
if (condExpr instanceof JsBinaryOperation
299+
&& ((JsBinaryOperation) condExpr).getOperator() == JsBinaryOperator.COMMA) {
300+
JsBinaryOperation condition = (JsBinaryOperation) condExpr;
301+
JsExpression newConditional = accept(new JsConditional(x.getSourceInfo(),
302+
condition.getArg2(), thenExpr, elseExpr));
303+
ctx.replaceMe(withSideEffect(newConditional, condition.getArg1(), x.getSourceInfo()));
304+
} else if (condExpr instanceof CanBooleanEval) {
265305
CanBooleanEval condEval = (CanBooleanEval) condExpr;
266306
if (condEval.isBooleanTrue()) {
267307
JsBinaryOperation binOp = new JsBinaryOperation(x.getSourceInfo(),
268-
JsBinaryOperator.AND, condExpr, thenExpr);
308+
JsBinaryOperator.COMMA, condExpr, thenExpr);
269309
ctx.replaceMe(accept(binOp));
270310
} else if (condEval.isBooleanFalse()) {
271-
// e.g. (false() ? then : else) -> false() || else
311+
// e.g. (false() ? then : else) -> (false() , else)
272312
JsBinaryOperation binOp = new JsBinaryOperation(x.getSourceInfo(),
273-
JsBinaryOperator.OR, condExpr, elseExpr);
313+
JsBinaryOperator.COMMA, condExpr, elseExpr);
274314
ctx.replaceMe(accept(binOp));
275315
}
276316
}
@@ -281,7 +321,7 @@ public void endVisit(JsConditional x, JsContext ctx) {
281321
*/
282322
@Override
283323
public void endVisit(JsDoWhile x, JsContext ctx) {
284-
evalBooleanContext.remove(x.getCondition());
324+
evalContext.remove(x.getCondition());
285325

286326
JsExpression expr = x.getCondition();
287327
if (expr instanceof CanBooleanEval) {
@@ -304,6 +344,7 @@ public void endVisit(JsDoWhile x, JsContext ctx) {
304344

305345
@Override
306346
public void endVisit(JsExprStmt x, JsContext ctx) {
347+
evalContext.remove(x.getExpression());
307348
if (!x.getExpression().hasSideEffects()) {
308349
if (ctx.canRemove()) {
309350
ctx.removeMe();
@@ -318,7 +359,7 @@ public void endVisit(JsExprStmt x, JsContext ctx) {
318359
*/
319360
@Override
320361
public void endVisit(JsFor x, JsContext ctx) {
321-
evalBooleanContext.remove(x.getCondition());
362+
evalContext.remove(x.getCondition());
322363

323364
JsExpression expr = x.getCondition();
324365
if (expr instanceof CanBooleanEval) {
@@ -348,7 +389,7 @@ public void endVisit(JsFor x, JsContext ctx) {
348389
*/
349390
@Override
350391
public void endVisit(JsIf x, JsContext ctx) {
351-
evalBooleanContext.remove(x.getIfExpr());
392+
evalContext.remove(x.getIfExpr());
352393

353394
JsExpression condExpr = x.getIfExpr();
354395
if (condExpr instanceof CanBooleanEval) {
@@ -402,10 +443,10 @@ public void endVisit(JsIf x, JsContext ctx) {
402443
@Override
403444
public void endVisit(JsPrefixOperation x, JsContext ctx) {
404445
if (x.getOperator() == JsUnaryOperator.NOT) {
405-
evalBooleanContext.remove(x.getArg());
446+
evalContext.remove(x.getArg());
406447
}
407448

408-
if (evalBooleanContext.contains(x)) {
449+
if (evalContext.containsKey(x)) {
409450
if ((x.getOperator() == JsUnaryOperator.NOT)
410451
&& (x.getArg() instanceof JsPrefixOperation)) {
411452
JsPrefixOperation arg = (JsPrefixOperation) x.getArg();
@@ -422,7 +463,7 @@ public void endVisit(JsPrefixOperation x, JsContext ctx) {
422463
*/
423464
@Override
424465
public void endVisit(JsWhile x, JsContext ctx) {
425-
evalBooleanContext.remove(x.getCondition());
466+
evalContext.remove(x.getCondition());
426467

427468
JsExpression expr = x.getCondition();
428469
if (expr instanceof CanBooleanEval) {
@@ -443,39 +484,39 @@ public void endVisit(JsWhile x, JsContext ctx) {
443484

444485
@Override
445486
public boolean visit(JsConditional x, JsContext ctx) {
446-
evalBooleanContext.add(x.getTestExpression());
487+
evalContext.put(x.getTestExpression(), EvalMode.BOOL);
447488
return true;
448489
}
449490

450491
@Override
451492
public boolean visit(JsDoWhile x, JsContext ctx) {
452-
evalBooleanContext.add(x.getCondition());
493+
evalContext.put(x.getCondition(), EvalMode.BOOL);
453494
return true;
454495
}
455496

456497
@Override
457498
public boolean visit(JsFor x, JsContext ctx) {
458-
evalBooleanContext.add(x.getCondition());
499+
evalContext.put(x.getCondition(), EvalMode.BOOL);
459500
return true;
460501
}
461502

462503
@Override
463504
public boolean visit(JsIf x, JsContext ctx) {
464-
evalBooleanContext.add(x.getIfExpr());
505+
evalContext.put(x.getIfExpr(), EvalMode.BOOL);
465506
return true;
466507
}
467508

468509
@Override
469510
public boolean visit(JsPrefixOperation x, JsContext ctx) {
470511
if (x.getOperator() == JsUnaryOperator.NOT) {
471-
evalBooleanContext.add(x.getArg());
512+
evalContext.put(x.getArg(), EvalMode.BOOL);
472513
}
473514
return true;
474515
}
475516

476517
@Override
477518
public boolean visit(JsWhile x, JsContext ctx) {
478-
evalBooleanContext.add(x.getCondition());
519+
evalContext.put(x.getCondition(), EvalMode.BOOL);
479520
return true;
480521
}
481522

@@ -607,41 +648,75 @@ public static int exec(JsProgram program) {
607648
* Simplify short circuit AND expressions.
608649
*
609650
* <pre>
610-
* if (true && isWhatever()) -> if (isWhatever()), unless side effects
611-
* if (false() && isWhatever()) -> if (false())
651+
* return true() && isWhatever() -> return isWhatever(), unless true() has side effects
652+
* return false() && isWhatever() -> return false()
653+
* </pre>
654+
* In boolean context also
655+
* <pre>
656+
* if (isWhatever() && true()) -> if (isWhatever()) unless true() has side effects
657+
* if (isWhatever() && false()) -> if (false()) unless isWhatever() side effects
612658
* </pre>
613659
*/
614-
protected static JsExpression shortCircuitAnd(JsBinaryOperation expr) {
660+
protected static JsExpression shortCircuitAnd(JsBinaryOperation expr,
661+
Map<JsExpression, EvalMode> evalContext) {
615662
JsExpression arg1 = expr.getArg1();
616663
JsExpression arg2 = expr.getArg2();
617664
if (arg1 instanceof CanBooleanEval) {
618665
CanBooleanEval eval1 = (CanBooleanEval) arg1;
619-
if (eval1.isBooleanTrue() && !arg1.hasSideEffects()) {
620-
return arg2;
666+
if (eval1.isBooleanTrue()) {
667+
return withSideEffect(arg2, arg1, expr.getSourceInfo());
621668
} else if (eval1.isBooleanFalse()) {
622669
return arg1;
623670
}
624671
}
672+
if (arg2 instanceof CanBooleanEval && evalContext.containsKey(expr)) {
673+
CanBooleanEval eval2 = (CanBooleanEval) arg2;
674+
if (eval2.isBooleanTrue() && !arg2.hasSideEffects()) {
675+
return arg1;
676+
} else if (eval2.isBooleanFalse()) {
677+
return withSideEffect(arg2, arg1, expr.getSourceInfo());
678+
}
679+
}
625680
return expr;
626681
}
627682

683+
private static JsExpression withSideEffect(JsExpression main, JsExpression sideEffect,
684+
SourceInfo sourceInfo) {
685+
return !sideEffect.hasSideEffects() ? main
686+
: new JsBinaryOperation(sourceInfo, JsBinaryOperator.COMMA, sideEffect, main);
687+
}
688+
628689
/**
629690
* Simplify short circuit OR expressions.
630691
*
631692
* <pre>
632-
* if (true() || isWhatever()) -> if (true())
633-
* if (false || isWhatever()) -> if (isWhatever()), unless side effects
693+
* return false() || isWhatever() -> return isWhatever(), unless false() has side effects
694+
* return true() && isWhatever() -> return true()
695+
* </pre>
696+
* In boolean context also
697+
* <pre>
698+
* if (isWhatever() || false()) -> if (isWhatever()) unless false() has side effects
699+
* if (isWhatever() || true()) -> if (true()) unless isWhatever() side effects
634700
* </pre>
635701
*/
636-
protected static JsExpression shortCircuitOr(JsBinaryOperation expr) {
702+
protected static JsExpression shortCircuitOr(JsBinaryOperation expr,
703+
Map<JsExpression, EvalMode> evalContext) {
637704
JsExpression arg1 = expr.getArg1();
638705
JsExpression arg2 = expr.getArg2();
639706
if (arg1 instanceof CanBooleanEval) {
640707
CanBooleanEval eval1 = (CanBooleanEval) arg1;
641-
if (eval1.isBooleanTrue()) {
708+
if (eval1.isBooleanFalse()) {
709+
return withSideEffect(arg2, arg1, expr.getSourceInfo());
710+
} else if (eval1.isBooleanTrue()) {
711+
return arg1;
712+
}
713+
}
714+
if (arg2 instanceof CanBooleanEval && evalContext.containsKey(expr)) {
715+
CanBooleanEval eval2 = (CanBooleanEval) arg2;
716+
if (eval2.isBooleanFalse() && !arg2.hasSideEffects()) {
642717
return arg1;
643-
} else if (eval1.isBooleanFalse() && !arg1.hasSideEffects()) {
644-
return arg2;
718+
} else if (eval2.isBooleanTrue()) {
719+
return withSideEffect(arg2, arg1, expr.getSourceInfo());
645720
}
646721
}
647722
return expr;

0 commit comments

Comments
 (0)