Skip to content

Commit 0cfd19b

Browse files
zbynekzbynek
committed
Simplify boolean expressions
1 parent 2a8b571 commit 0cfd19b

File tree

3 files changed

+136
-33
lines changed

3 files changed

+136
-33
lines changed

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

Lines changed: 87 additions & 29 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;
@@ -144,6 +144,11 @@ public boolean visit(JsFunction x, JsContext ctx) {
144144
}
145145
}
146146

147+
private enum EvalMode {
148+
BOOL,
149+
VOID
150+
}
151+
147152
/**
148153
* Does static evals.
149154
*
@@ -153,12 +158,29 @@ public boolean visit(JsFunction x, JsContext ctx) {
153158
*/
154159
private class StaticEvalVisitor extends JsModVisitor {
155160

156-
private Set<JsExpression> evalBooleanContext = new HashSet<JsExpression>();
161+
private Map<JsExpression, EvalMode> evalContext = new HashMap<>();
157162

158163
/**
159164
* This is used by {@link #additionCoercesToString}.
160165
*/
161-
private Map<JsExpression, Boolean> coercesToStringMap = new IdentityHashMap<JsExpression, Boolean>();
166+
private Map<JsExpression, Boolean> coercesToStringMap = new IdentityHashMap<>();
167+
168+
public boolean visit(JsExprStmt x, JsContext ctx) {
169+
evalContext.put(x.getExpression(), EvalMode.VOID);
170+
return true;
171+
}
172+
173+
public boolean visit(JsBinaryOperation x, JsContext ctx) {
174+
if (evalContext.containsKey(x)
175+
&& (x.getOperator() == JsBinaryOperator.AND || x.getOperator() == JsBinaryOperator.OR)) {
176+
evalContext.put(x.getArg1(), EvalMode.BOOL);
177+
evalContext.put(x.getArg2(), evalContext.get(x));
178+
}
179+
if (x.getOperator() == JsBinaryOperator.COMMA) {
180+
evalContext.put(x.getArg1(), EvalMode.VOID);
181+
}
182+
return true;
183+
}
162184

163185
@Override
164186
public void endVisit(JsBinaryOperation x, JsContext ctx) {
@@ -167,13 +189,20 @@ public void endVisit(JsBinaryOperation x, JsContext ctx) {
167189
JsExpression arg2 = x.getArg2();
168190

169191
JsExpression result = x;
170-
171-
if (op == JsBinaryOperator.AND) {
172-
result = shortCircuitAnd(x);
192+
if (evalContext.get(x) == EvalMode.VOID
193+
&& !arg2.hasSideEffects() && !op.isAssignment()) {
194+
result = arg1;
195+
} else if (op == JsBinaryOperator.AND) {
196+
result = shortCircuitAnd(x, evalContext);
197+
evalContext.remove(arg1);
198+
evalContext.remove(arg2);
173199
} else if (op == JsBinaryOperator.OR) {
174-
result = shortCircuitOr(x);
200+
result = shortCircuitOr(x, evalContext);
201+
evalContext.remove(arg1);
202+
evalContext.remove(arg2);
175203
} else if (op == JsBinaryOperator.COMMA) {
176204
result = trySimplifyComma(x);
205+
evalContext.remove(x.getArg1());
177206
} else if (op == JsBinaryOperator.EQ || op == JsBinaryOperator.REF_EQ) {
178207
result = simplifyEqAndRefEq(x);
179208
} else if (op == JsBinaryOperator.NEQ || op == JsBinaryOperator.REF_NEQ) {
@@ -256,7 +285,7 @@ public void endVisit(JsBlock x, JsContext ctx) {
256285

257286
@Override
258287
public void endVisit(JsConditional x, JsContext ctx) {
259-
evalBooleanContext.remove(x.getTestExpression());
288+
evalContext.remove(x.getTestExpression());
260289

261290
JsExpression condExpr = x.getTestExpression();
262291
JsExpression thenExpr = x.getThenExpression();
@@ -265,12 +294,12 @@ public void endVisit(JsConditional x, JsContext ctx) {
265294
CanBooleanEval condEval = (CanBooleanEval) condExpr;
266295
if (condEval.isBooleanTrue()) {
267296
JsBinaryOperation binOp = new JsBinaryOperation(x.getSourceInfo(),
268-
JsBinaryOperator.AND, condExpr, thenExpr);
297+
JsBinaryOperator.COMMA, condExpr, thenExpr);
269298
ctx.replaceMe(accept(binOp));
270299
} else if (condEval.isBooleanFalse()) {
271-
// e.g. (false() ? then : else) -> false() || else
300+
// e.g. (false() ? then : else) -> (false() , else)
272301
JsBinaryOperation binOp = new JsBinaryOperation(x.getSourceInfo(),
273-
JsBinaryOperator.OR, condExpr, elseExpr);
302+
JsBinaryOperator.COMMA, condExpr, elseExpr);
274303
ctx.replaceMe(accept(binOp));
275304
}
276305
}
@@ -281,7 +310,7 @@ public void endVisit(JsConditional x, JsContext ctx) {
281310
*/
282311
@Override
283312
public void endVisit(JsDoWhile x, JsContext ctx) {
284-
evalBooleanContext.remove(x.getCondition());
313+
evalContext.remove(x.getCondition());
285314

286315
JsExpression expr = x.getCondition();
287316
if (expr instanceof CanBooleanEval) {
@@ -304,6 +333,7 @@ public void endVisit(JsDoWhile x, JsContext ctx) {
304333

305334
@Override
306335
public void endVisit(JsExprStmt x, JsContext ctx) {
336+
evalContext.remove(x.getExpression());
307337
if (!x.getExpression().hasSideEffects()) {
308338
if (ctx.canRemove()) {
309339
ctx.removeMe();
@@ -318,7 +348,7 @@ public void endVisit(JsExprStmt x, JsContext ctx) {
318348
*/
319349
@Override
320350
public void endVisit(JsFor x, JsContext ctx) {
321-
evalBooleanContext.remove(x.getCondition());
351+
evalContext.remove(x.getCondition());
322352

323353
JsExpression expr = x.getCondition();
324354
if (expr instanceof CanBooleanEval) {
@@ -348,7 +378,7 @@ public void endVisit(JsFor x, JsContext ctx) {
348378
*/
349379
@Override
350380
public void endVisit(JsIf x, JsContext ctx) {
351-
evalBooleanContext.remove(x.getIfExpr());
381+
evalContext.remove(x.getIfExpr());
352382

353383
JsExpression condExpr = x.getIfExpr();
354384
if (condExpr instanceof CanBooleanEval) {
@@ -402,10 +432,10 @@ public void endVisit(JsIf x, JsContext ctx) {
402432
@Override
403433
public void endVisit(JsPrefixOperation x, JsContext ctx) {
404434
if (x.getOperator() == JsUnaryOperator.NOT) {
405-
evalBooleanContext.remove(x.getArg());
435+
evalContext.remove(x.getArg());
406436
}
407437

408-
if (evalBooleanContext.contains(x)) {
438+
if (evalContext.containsKey(x)) {
409439
if ((x.getOperator() == JsUnaryOperator.NOT)
410440
&& (x.getArg() instanceof JsPrefixOperation)) {
411441
JsPrefixOperation arg = (JsPrefixOperation) x.getArg();
@@ -422,7 +452,7 @@ public void endVisit(JsPrefixOperation x, JsContext ctx) {
422452
*/
423453
@Override
424454
public void endVisit(JsWhile x, JsContext ctx) {
425-
evalBooleanContext.remove(x.getCondition());
455+
evalContext.remove(x.getCondition());
426456

427457
JsExpression expr = x.getCondition();
428458
if (expr instanceof CanBooleanEval) {
@@ -443,39 +473,39 @@ public void endVisit(JsWhile x, JsContext ctx) {
443473

444474
@Override
445475
public boolean visit(JsConditional x, JsContext ctx) {
446-
evalBooleanContext.add(x.getTestExpression());
476+
evalContext.put(x.getTestExpression(), EvalMode.BOOL);
447477
return true;
448478
}
449479

450480
@Override
451481
public boolean visit(JsDoWhile x, JsContext ctx) {
452-
evalBooleanContext.add(x.getCondition());
482+
evalContext.put(x.getCondition(), EvalMode.BOOL);
453483
return true;
454484
}
455485

456486
@Override
457487
public boolean visit(JsFor x, JsContext ctx) {
458-
evalBooleanContext.add(x.getCondition());
488+
evalContext.put(x.getCondition(), EvalMode.BOOL);
459489
return true;
460490
}
461491

462492
@Override
463493
public boolean visit(JsIf x, JsContext ctx) {
464-
evalBooleanContext.add(x.getIfExpr());
494+
evalContext.put(x.getIfExpr(), EvalMode.BOOL);
465495
return true;
466496
}
467497

468498
@Override
469499
public boolean visit(JsPrefixOperation x, JsContext ctx) {
470500
if (x.getOperator() == JsUnaryOperator.NOT) {
471-
evalBooleanContext.add(x.getArg());
501+
evalContext.put(x.getArg(), EvalMode.BOOL);
472502
}
473503
return true;
474504
}
475505

476506
@Override
477507
public boolean visit(JsWhile x, JsContext ctx) {
478-
evalBooleanContext.add(x.getCondition());
508+
evalContext.put(x.getCondition(), EvalMode.BOOL);
479509
return true;
480510
}
481511

@@ -607,11 +637,17 @@ public static int exec(JsProgram program) {
607637
* Simplify short circuit AND expressions.
608638
*
609639
* <pre>
610-
* if (true && isWhatever()) -> if (isWhatever()), unless side effects
611-
* if (false() && isWhatever()) -> if (false())
640+
* return true() && isWhatever() -> return isWhatever(), unless true() has side effects
641+
* return false() && isWhatever() -> return false()
642+
* </pre>
643+
* In boolean context also
644+
* <pre>
645+
* if (isWhatever() && true()) -> if (isWhatever()) unless true() has side effects
646+
* if (isWhatever() && false()) -> if (false()) unless isWhatever() side effects
612647
* </pre>
613648
*/
614-
protected static JsExpression shortCircuitAnd(JsBinaryOperation expr) {
649+
protected static JsExpression shortCircuitAnd(JsBinaryOperation expr,
650+
Map<JsExpression, EvalMode> evalContext) {
615651
JsExpression arg1 = expr.getArg1();
616652
JsExpression arg2 = expr.getArg2();
617653
if (arg1 instanceof CanBooleanEval) {
@@ -622,18 +658,32 @@ protected static JsExpression shortCircuitAnd(JsBinaryOperation expr) {
622658
return arg1;
623659
}
624660
}
661+
if (arg2 instanceof CanBooleanEval && evalContext.containsKey(arg1)) {
662+
CanBooleanEval eval2 = (CanBooleanEval) arg2;
663+
if (eval2.isBooleanTrue() && !arg2.hasSideEffects()) {
664+
return arg1;
665+
} else if (eval2.isBooleanFalse() && !arg1.hasSideEffects()) {
666+
return arg2;
667+
}
668+
}
625669
return expr;
626670
}
627671

628672
/**
629673
* Simplify short circuit OR expressions.
630674
*
631675
* <pre>
632-
* if (true() || isWhatever()) -> if (true())
633-
* if (false || isWhatever()) -> if (isWhatever()), unless side effects
676+
* return false() || isWhatever() -> return isWhatever(), unless false() has side effects
677+
* return true() && isWhatever() -> return true()
678+
* </pre>
679+
* In boolean context also
680+
* <pre>
681+
* if (isWhatever() || false()) -> if (isWhatever()) unless false() has side effects
682+
* if (isWhatever() || true()) -> if (true()) unless isWhatever() side effects
634683
* </pre>
635684
*/
636-
protected static JsExpression shortCircuitOr(JsBinaryOperation expr) {
685+
protected static JsExpression shortCircuitOr(JsBinaryOperation expr,
686+
Map<JsExpression, EvalMode> evalContext) {
637687
JsExpression arg1 = expr.getArg1();
638688
JsExpression arg2 = expr.getArg2();
639689
if (arg1 instanceof CanBooleanEval) {
@@ -644,6 +694,14 @@ protected static JsExpression shortCircuitOr(JsBinaryOperation expr) {
644694
return arg2;
645695
}
646696
}
697+
if (arg2 instanceof CanBooleanEval && evalContext.containsKey(arg1)) {
698+
CanBooleanEval eval2 = (CanBooleanEval) arg2;
699+
if (eval2.isBooleanTrue() && !arg1.hasSideEffects()) {
700+
return arg2;
701+
} else if (eval2.isBooleanFalse() && !arg2.hasSideEffects()) {
702+
return arg1;
703+
}
704+
}
647705
return expr;
648706
}
649707

‎dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperation.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
/**
1919
* Represents a JavaScript binary operation.
2020
*/
21-
public final class JsBinaryOperation extends JsExpression {
21+
public final class JsBinaryOperation extends JsExpression implements CanBooleanEval {
2222

2323
private JsExpression arg1;
2424

@@ -68,6 +68,26 @@ public boolean isDefinitelyNull() {
6868
return false;
6969
}
7070

71+
@Override
72+
public boolean isBooleanFalse() {
73+
// assume false && x was already simplified, only check x && false
74+
if (getOperator() == JsBinaryOperator.AND && getArg2() instanceof CanBooleanEval) {
75+
CanBooleanEval eval = (CanBooleanEval) getArg2();
76+
return eval.isBooleanFalse();
77+
}
78+
return false;
79+
}
80+
81+
@Override
82+
public boolean isBooleanTrue() {
83+
// assume true || x was already simplified, only check for x || true
84+
if (getOperator() == JsBinaryOperator.OR && getArg2() instanceof CanBooleanEval) {
85+
CanBooleanEval eval = (CanBooleanEval) getArg2();
86+
return eval.isBooleanTrue();
87+
}
88+
return false;
89+
}
90+
7191
public void setArg1(JsExpression arg1) {
7292
this.arg1 = arg1;
7393
}

‎dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ public void testAssociativity() throws Exception {
4343
assertEquals("alert(a&&b||c&&d);",
4444
optimize("alert((a && b) || ( c && d));"));
4545
assertEquals("a(),b&&c();", optimize("a(), b && c()"));
46-
assertEquals("a()&&b,c();", optimize("a() && b, c()"));
46+
assertEquals("a()&&b(),c();", optimize("a() && b(), c()"));
47+
assertEquals("a(),c();", optimize("a() && b, c()"));
4748

4849
// Don't damage math expressions
4950
assertEquals("alert(seconds/3600);",
@@ -53,8 +54,8 @@ public void testAssociativity() throws Exception {
5354
assertEquals("alert(1-(1-foo));", optimize("alert(1 - (1 - foo))"));
5455

5556
// Don't damage assignments
56-
assertEquals("alert((a=0,b=foo));",
57-
optimize("alert((a = 0, b = (bar, foo)))"));
57+
assertEquals("alert((a=7,b=foo));",
58+
optimize("alert((a = 7, b = (bar, foo)))"));
5859
assertEquals("alert(1+(a='2')+3+4);",
5960
optimize("alert(1 + (a = '2') + 3 + 4);"));
6061
assertEquals("alert(1+(a='2')+7);",
@@ -154,6 +155,30 @@ public void testLiteralCompares() throws Exception {
154155
assertEquals("alert(true);", optimize("alert(\"a\" !== null)"));
155156
}
156157

158+
public void testShortCircuitAnd() throws Exception {
159+
assertEquals("alert(a);", optimize("alert(true && a)"));
160+
assertEquals("alert(false);", optimize("alert(false && a)"));
161+
162+
// these can't be simplified to maintain type
163+
assertEquals("alert(a&&true);", optimize("alert(a && true)"));
164+
assertEquals("alert(a&&false);", optimize("alert(a && false)"));
165+
assertEquals("alert(!!a&&!!b);", optimize("alert(!!a && !!b)"));
166+
167+
// in boolean context we can simplify more
168+
assertEquals("alert(a&&b?c:d);", optimize("alert(!!a && !!b ? c :d)"));
169+
assertEquals("alert(d);", optimize("alert(false && !!b ? c :d)"));
170+
assertEquals("alert(b?c:d);", optimize("alert(true && !!b ? c :d)"));
171+
assertEquals("alert(b?c:d1);", optimize("alert(b && true ? c :d1)"));
172+
assertEquals("alert(d1);", optimize("alert(b && false ? c :d1)"));
173+
assertEquals("alert(d2);", optimize("alert(a && false && b ? c :d2)"));
174+
}
175+
176+
public void testShortCircuitAndWithSideEffects() throws Exception {
177+
assertEquals("a&&b();", optimize("!!a && !!b()"));
178+
assertEquals("a();", optimize("a() && false && c();"));
179+
assertEquals("a(),e();", optimize("a() && false && c() ? d() : e();"));
180+
}
181+
157182
public void testLiteralEqNull() throws Exception {
158183
assertEquals("alert(false);", optimize("alert('test' == null)"));
159184
}

0 commit comments

Comments
 (0)