Skip to content

Commit

Permalink
DCE: Deopt impure statements in If.test (#386)
Browse files Browse the repository at this point in the history 
* DCE: Deopt impure statements in If.test

+ (Fix #385)
+ Add isPure check to IfStatement visitor in DCE
+ Move IfStatement visitor from Single Pass program.exit to DCEPlugin.visitor - so that it executes for all IfStatements as babel traverse happens and NOT during programPath.traverse. This eliminates the need for a second pass for things like the one mentioned in #385.

* Add tests

* Moar optimizations for side-effecty if statements

* Add dependency jsesc for constant-folding
  • Loading branch information
@boopathi @kangax
boopathi authored and kangax committed Jan 25, 2017
1 parent 17eba03 commit ba72329
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 66 deletions.
3 changes: 2 additions & 1 deletion packages/babel-plugin-minify-constant-folding/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"babel-plugin"
],
"dependencies": {
"babel-helper-evaluate-path": "^0.0.3"
"babel-helper-evaluate-path": "^0.0.3",
"jsesc": "^2.4.0"
},
"devDependencies": {}
}
39 changes: 39 additions & 0 deletions packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2421,4 +2421,43 @@ describe("dce-plugin", () => {

expect(transform(source)).toBe(expected);
});

it("should impure expressions in confidently evaluated if statements", () => {
const source = unpad(`
if (a.b(), true) {
foo();
}
`);
const expected = unpad(`
a.b();
foo();
`);
expect(transform(source)).toBe(expected);
});

it("should extract all necessary things from if statements", () => {
const source = unpad(`
if (a.b(), false) {
var foo = foo1;
foo();
} else if (b.c(), true) {
var bar = bar1;
bar();
} else {
var baz = baz1;
baz();
}
`);
const expected = unpad(`
a.b();
b.c();
var bar = bar1;
bar();
var baz;
var foo;
`);
expect(transform(source)).toBe(expected);
});
});
153 changes: 88 additions & 65 deletions packages/babel-plugin-minify-dead-code-elimination/src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,71 +419,6 @@ module.exports = ({ types: t, traverse }) => {
}
},

IfStatement: {
exit(path) {
const consequent = path.get("consequent");
const alternate = path.get("alternate");
const test = path.get("test");

const evaluateTest = test.evaluateTruthy();

// we can check if a test will be truthy 100% and if so then we can inline
// the consequent and completely ignore the alternate
//
// if (true) { foo; } -> { foo; }
// if ("foo") { foo; } -> { foo; }
//
if (evaluateTest === true) {
path.replaceWithMultiple(
[...toStatements(consequent), ...extractVars(alternate)]
);
return;
}

// we can check if a test will be falsy 100% and if so we can inline the
// alternate if there is one and completely remove the consequent
//
// if ("") { bar; } else { foo; } -> { foo; }
// if ("") { bar; } ->
//
if (evaluateTest === false) {
if (alternate.node) {
path.replaceWithMultiple(
[...toStatements(alternate), ...extractVars(consequent)]
);
return;
} else {
path.replaceWithMultiple(extractVars(consequent));
}
}

// remove alternate blocks that are empty
//
// if (foo) { foo; } else {} -> if (foo) { foo; }
//
if (alternate.isBlockStatement() && !alternate.node.body.length) {
alternate.remove();
// For if-statements babel-traverse replaces with an empty block
path.node.alternate = null;
}

// if the consequent block is empty turn alternate blocks into a consequent
// and flip the test
//
// if (foo) {} else { bar; } -> if (!foo) { bar; }
//
if (consequent.isBlockStatement() && !consequent.node.body.length &&
alternate.isBlockStatement() && alternate.node.body.length
) {
consequent.replaceWith(alternate.node);
alternate.remove();
// For if-statements babel-traverse replaces with an empty block
path.node.alternate = null;
test.replaceWith(t.unaryExpression("!", test.node, true));
}
},
},

SwitchStatement: {
exit(path) {
const evaluated = path.get("discriminant").evaluate();
Expand Down Expand Up @@ -699,11 +634,88 @@ module.exports = ({ types: t, traverse }) => {
return {
name: "minify-dead-code-elimination",
visitor: {
IfStatement: {
exit(path) {
const consequent = path.get("consequent");
const alternate = path.get("alternate");
const test = path.get("test");

const evalResult = test.evaluate();
const isPure = test.isPure();

const replacements = [];

if (evalResult.confident && !isPure && test.isSequenceExpression()) {
replacements.push(
t.expressionStatement(extractSequenceImpure(test))
);
}

// we can check if a test will be truthy 100% and if so then we can inline
// the consequent and completely ignore the alternate
//
// if (true) { foo; } -> { foo; }
// if ("foo") { foo; } -> { foo; }
//
if (evalResult.confident && evalResult.value) {
path.replaceWithMultiple([
...replacements, ...toStatements(consequent), ...extractVars(alternate)
]);
return;
}

// we can check if a test will be falsy 100% and if so we can inline the
// alternate if there is one and completely remove the consequent
//
// if ("") { bar; } else { foo; } -> { foo; }
// if ("") { bar; } ->
//
if (evalResult.confident && !evalResult.value) {
if (alternate.node) {
path.replaceWithMultiple([
...replacements, ...toStatements(alternate), ...extractVars(consequent)
]);
return;
} else {
path.replaceWithMultiple([
...replacements, ...extractVars(consequent)
]);
}
}

// remove alternate blocks that are empty
//
// if (foo) { foo; } else {} -> if (foo) { foo; }
//
if (alternate.isBlockStatement() && !alternate.node.body.length) {
alternate.remove();
// For if-statements babel-traverse replaces with an empty block
path.node.alternate = null;
}

// if the consequent block is empty turn alternate blocks into a consequent
// and flip the test
//
// if (foo) {} else { bar; } -> if (!foo) { bar; }
//
if (consequent.isBlockStatement() && !consequent.node.body.length &&
alternate.isBlockStatement() && alternate.node.body.length
) {
consequent.replaceWith(alternate.node);
alternate.remove();
// For if-statements babel-traverse replaces with an empty block
path.node.alternate = null;
test.replaceWith(t.unaryExpression("!", test.node, true));
}
},
},

EmptyStatement(path) {
if (path.parentPath.isBlockStatement() || path.parentPath.isProgram()) {
path.remove();
}
},

Program: {
exit(path, {
opts: {
Expand Down Expand Up @@ -1025,4 +1037,15 @@ module.exports = ({ types: t, traverse }) => {
} while (parent = parent.parentPath);
return false;
}

function extractSequenceImpure(seq) {
const expressions = seq.get("expressions");
const result = [];
for (let i = 0; i < expressions.length; i++) {
if (!expressions[i].isPure()) {
result.push(expressions[i].node);
}
}
return t.sequenceExpression(result);
}
};
14 changes: 14 additions & 0 deletions packages/babel-preset-babili/__tests__/preset-tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,18 @@ describe("preset", () => {
`);
expect(transform(source)).toBe(expected);
});

it("should fix issue#385 - impure if statements with Sequence and DCE", () => {
const source = unpad(`
a = b;
c = d;
if (false) {
const x = y
}
`);
const expected = unpad(`
a = b, c = d;
`);
expect(transform(source)).toBe(expected);
});
});

0 comments on commit ba72329

Please # to comment.