Skip to content

Commit

Permalink
Improve folding of "do { ... } while (0);" statements with "break;" a…
Browse files Browse the repository at this point in the history
…nd "continue;" inside.

This is required for better optimization of generator functions.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=187496339
  • Loading branch information
SergejSalnikov authored and Tyler Breisacher committed Mar 3, 2018
1 parent f491995 commit 31a16fa
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 28 deletions.
73 changes: 45 additions & 28 deletions src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java
Expand Up @@ -21,7 +21,6 @@
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;


import com.google.common.base.Predicate; import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.javascript.rhino.IR; import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token; import com.google.javascript.rhino.Token;
Expand All @@ -35,6 +34,37 @@
*/ */
class PeepholeRemoveDeadCode extends AbstractPeepholeOptimization { class PeepholeRemoveDeadCode extends AbstractPeepholeOptimization {


private static final Predicate<Node> IS_UNNAMED_BREAK_PREDICATE = new Predicate<Node>() {
@Override
public boolean apply(Node node) {
return node.isBreak() && !node.hasChildren();
}
};

private static final Predicate<Node> IS_UNNAMED_CONTINUE_PREDICATE = new Predicate<Node>() {
@Override
public boolean apply(Node node) {
return node.isContinue() && !node.hasChildren();
}
};

private static final Predicate<Node> CAN_CONTAIN_BREAK_PREDICATE = new Predicate<Node>() {
@Override
public boolean apply(Node node) {
return !IR.mayBeExpression(node) // Functions are not visited
&& !NodeUtil.isLoopStructure(node)
&& !node.isSwitch();
}
};

private static final Predicate<Node> CAN_CONTAIN_CONTINUE_PREDICATE = new Predicate<Node>() {
@Override
public boolean apply(Node node) {
return !IR.mayBeExpression(node) // Functions are not visited
&& !NodeUtil.isLoopStructure(node);
}
};

// TODO(dcc): Some (all) of these can probably be better achieved // TODO(dcc): Some (all) of these can probably be better achieved
// using the control flow graph (like CheckUnreachableCode). // using the control flow graph (like CheckUnreachableCode).
// There is an existing CFG pass (UnreachableCodeElimination) that // There is an existing CFG pass (UnreachableCodeElimination) that
Expand Down Expand Up @@ -1013,20 +1043,15 @@ Node tryFoldDoAway(Node n) {
return n; return n;
} }


// TODO(johnlenz): The do-while can be turned into a label with Node block = NodeUtil.getLoopCodeBlock(n);
// named breaks and the label optimized away (maybe). if (n.getParent().isLabel() || hasUnnamedBreakOrContinue(block)) {
if (hasBreakOrContinue(n)) {
return n; return n;
} }


checkState(NodeUtil.isControlStructureCodeBlock(n, n.getFirstChild())); Node parent = n.getParent();
Node block = n.removeFirstChild(); n.replaceWith(block.detach());

Node parent = n.getParent();
parent.replaceChild(n, block);
if (mayHaveSideEffects(cond)) { if (mayHaveSideEffects(cond)) {
Node condStatement = IR.exprResult(cond.detach()) Node condStatement = IR.exprResult(cond.detach()).srcref(cond);
.srcref(cond);
parent.addChildAfter(condStatement, block); parent.addChildAfter(condStatement, block);
} }
compiler.reportChangeToEnclosingScope(parent); compiler.reportChangeToEnclosingScope(parent);
Expand All @@ -1044,32 +1069,24 @@ Node tryFoldEmptyDo(Node n) {
Node body = NodeUtil.getLoopCodeBlock(n); Node body = NodeUtil.getLoopCodeBlock(n);
if (body.isNormalBlock() && !body.hasChildren()) { if (body.isNormalBlock() && !body.hasChildren()) {
Node cond = NodeUtil.getConditionExpression(n); Node cond = NodeUtil.getConditionExpression(n);
Node whileNode = Node forNode =
IR.forNode(IR.empty().srcref(n), IR.forNode(IR.empty().srcref(n),
cond.detach(), cond.detach(),
IR.empty().srcref(n), IR.empty().srcref(n),
body.detach()) body.detach());
.srcref(n); n.replaceWith(forNode);
n.replaceWith(whileNode); compiler.reportChangeToEnclosingScope(forNode);
compiler.reportChangeToEnclosingScope(whileNode); return forNode;
return whileNode;
} }
return n; return n;
} }


/** /**
* * Returns whether a node has any unhandled breaks or continue.
*/ */
static boolean hasBreakOrContinue(Node n) { static boolean hasUnnamedBreakOrContinue(Node n) {
// TODO(johnlenz): This is overkill as named breaks may refer to outer return NodeUtil.has(n, IS_UNNAMED_BREAK_PREDICATE, CAN_CONTAIN_BREAK_PREDICATE)
// loops or labels, and any break my refer to an inner loop. || NodeUtil.has(n, IS_UNNAMED_CONTINUE_PREDICATE, CAN_CONTAIN_CONTINUE_PREDICATE);
// More generally, this check may be more expensive than we like.
return NodeUtil.has(
n,
Predicates.or(
new NodeUtil.MatchNodeType(Token.BREAK),
new NodeUtil.MatchNodeType(Token.CONTINUE)),
NodeUtil.MATCH_NOT_FUNCTION);
} }


/** /**
Expand Down
15 changes: 15 additions & 0 deletions test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java
Expand Up @@ -76,6 +76,9 @@ public void testRemoveNoOpLabelledStatement() {


foldSame("a: { break a; console.log('unreachable'); }"); foldSame("a: { break a; console.log('unreachable'); }");
foldSame("a: { break a; var x = 1; } x = 2;"); foldSame("a: { break a; var x = 1; } x = 2;");

foldSame("b: { var x = 1; } x = 2;");
foldSame("a: b: { var x = 1; } x = 2;");
} }


public void testFoldBlock() { public void testFoldBlock() {
Expand Down Expand Up @@ -254,7 +257,19 @@ public void testFoldUselessDo() {


// Can't fold with break or continues. // Can't fold with break or continues.
foldSame("do { foo(); continue; } while(0)"); foldSame("do { foo(); continue; } while(0)");
foldSame("do { try { foo() } catch (e) { break; } } while (0);");
foldSame("do { foo(); break; } while(0)"); foldSame("do { foo(); break; } while(0)");
fold("do { while (1) {foo(); continue;} } while(0)", "while (1) {foo(); continue;}");
foldSame("l1: do { while (1) { foo() } } while(0)");
fold("do { switch (1) { default: foo(); break} } while(0)", "foo();");
fold("do { switch (1) { default: foo(); continue} } while(0)",
"do { foo(); continue } while(0)");

fold("l1: { do { x = 1; break l1; } while (0); x = 2; }", "l1: { x = 1; break l1; x = 2;}");

fold("do { x = 1; } while (x = 0);", "x = 1; x = 0;");
fold("let x = 1; (function() { do { let x = 2; } while (x = 10, false); })();",
"let x = 1; (function() { { let x = 2 } x = 10 })();");
} }


public void testFoldEmptyDo() { public void testFoldEmptyDo() {
Expand Down
Expand Up @@ -234,6 +234,17 @@ function testDoWhileCapturedLet() {
assertEquals(5, arr[5]()); assertEquals(5, arr[5]());
} }


function testDoWhileFold() {
let x = 1;
(function () {
do {
let x = 2;
assertEquals(2, x);
} while (x = 10, false);
})();
assertEquals(10, x);
}

function testMutatedLoopCapturedLet() { function testMutatedLoopCapturedLet() {
const arr = []; const arr = [];
for (let i = 0; i < 10; i++) { for (let i = 0; i < 10; i++) {
Expand Down

0 comments on commit 31a16fa

Please sign in to comment.