From 19720cb87c154b2572f74aa8689d544247655227 Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Mon, 7 Aug 2017 21:46:11 +0200 Subject: [PATCH] [fix #574] deopt when binding is present in diff scope (#597) * [fix #574] deopt when binding is present in diff scope * fix binding check * small changes * Fix detection of different scope --- .../__tests__/dead-code-elimination-test.js | 10 ++++ .../src/index.js | 47 +++++++++++++++---- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js b/packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js index abecf1de3..c4b5ab9e3 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js +++ b/packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js @@ -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); + } + ` + ); }); diff --git a/packages/babel-plugin-minify-dead-code-elimination/src/index.js b/packages/babel-plugin-minify-dead-code-elimination/src/index.js index 796c08787..73c3abf77 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/packages/babel-plugin-minify-dead-code-elimination/src/index.js @@ -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; @@ -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) { @@ -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; @@ -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))