Skip to content

Commit

Permalink
[fix #574] deopt when binding is present in diff scope (#597)
Browse files Browse the repository at this point in the history
* [fix #574] deopt when binding is present in diff scope

* fix binding check

* small changes

* Fix detection of different scope
  • Loading branch information
vigneshshanmugam authored and boopathi committed Aug 7, 2017
1 parent 4359199 commit 19720cb
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2462,4 +2462,14 @@ describe("dce-plugin", () => {
}
`
);

thePlugin(
"should deopt when binding is on different scope - issue #574",
`
function foo(v) {
if (v) var w = 10;
if (w) console.log("hello", v);
}
`
);
});
47 changes: 39 additions & 8 deletions packages/babel-plugin-minify-dead-code-elimination/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,13 @@ module.exports = ({ types: t, traverse }) => {
) {
continue;
} else if (binding.path.isVariableDeclarator()) {
const declaration = binding.path.parentPath;
const maybeBlockParent = declaration.parentPath;
if (
binding.path.parentPath.parentPath &&
binding.path.parentPath.parentPath.isForXStatement()
maybeBlockParent &&
maybeBlockParent.isForXStatement({
left: declaration.node
})
) {
// Can't remove if in a for-in/for-of/for-await statement `for (var x in wat)`.
continue;
Expand Down Expand Up @@ -293,14 +297,10 @@ module.exports = ({ types: t, traverse }) => {
let replacementPath = binding.path;
let isReferencedBefore = false;

if (binding.referencePaths.length > 1) {
throw new Error("Expected only one reference");
}
const refPath = binding.referencePaths[0];

if (t.isVariableDeclarator(replacement)) {
const _prevSiblings = prevSiblings(replacementPath);

// traverse ancestors of a reference checking if it's before declaration
forEachAncestor(refPath, ancestor => {
if (_prevSiblings.indexOf(ancestor) > -1) {
Expand All @@ -309,7 +309,7 @@ module.exports = ({ types: t, traverse }) => {
});

// deopt if reference is in different scope than binding
// since we don't know if it's sync or async execition
// since we don't know if it's sync or async execution
// (i.e. whether value has been assigned to a reference or not)
if (isReferencedBefore && refPath.scope !== binding.scope) {
continue;
Expand Down Expand Up @@ -774,8 +774,39 @@ module.exports = ({ types: t, traverse }) => {
const evalResult = test.evaluate();
const isPure = test.isPure();

const replacements = [];
const binding = path.scope.getBinding(test.node.name);

// Ref - https://github.com/babel/babili/issues/574
// deopt if var is declared in other scope
// if (a) { var b = blahl;} if (b) { //something }
if (
binding &&
binding.path.parentPath.isVariableDeclaration({ kind: "var" })
) {
let ifStatementParent = null;

const fnParent =
binding.path.getFunctionParent() ||
binding.path.getProgramParent();

forEachAncestor(binding.path.parentPath, parent => {
if (fnParent === parent) return;
if (parent.isIfStatement()) {
ifStatementParent = parent;
}
});

if (
ifStatementParent &&
binding.referencePaths.some(
ref => !ref.isDescendant(ifStatementParent)
)
) {
return;
}
}

const replacements = [];
if (evalResult.confident && !isPure && test.isSequenceExpression()) {
replacements.push(
t.expressionStatement(extractSequenceImpure(test))
Expand Down

0 comments on commit 19720cb

Please sign in to comment.