From 03d1e633026d1141418d31bae4590d4ffde3c869 Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Mon, 7 Aug 2017 21:56:25 +0200 Subject: [PATCH 1/8] Helper evaluate Path --- .../babel-helper-evaluate-path/src/index.js | 146 ++++++++++++++++++ .../package.json | 5 +- .../src/index.js | 35 +---- 3 files changed, 150 insertions(+), 36 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index 55bfb892a..e529b4eec 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -1,4 +1,36 @@ +"use strict"; + module.exports = function evaluate(path) { + if (path.isIdentifier()) { + return evaluateIdentifier(path); + } + + const state = { + deoptPath: null, + confident: false + }; + + // prepare + path.traverse({ + Scope(scopePath) { + scopePath.skip(); + }, + ReferencedIdentifier(idPath) { + const evalResult = evaluateIdentifier(idPath); + if (evalResult.confident) { + idPath.replaceWith(evalResult.value); + deref(idPath); + } else { + state.confident = evalResult.confident; + state.deoptPath = evalResult.deoptPath; + } + } + }); + + if (!state.confident) { + return state; + } + try { return path.evaluate(); } catch (e) { @@ -8,3 +40,117 @@ module.exports = function evaluate(path) { }; } }; + +// Original Source: +// https://github.com/babel/babel/blob/master/packages/babel-traverse/src/path/evaluation.js +// modified for Babili use +function evaluateIdentifier(path) { + if (!path.isReferencedIdentifier()) { + throw new Error(`Expected ReferencedIdentifier. Got ${path.type}`); + } + + const { node } = path; + + const binding = path.scope.getBinding(node.name); + + if (!binding) { + return deopt(path); + } + + if (binding.constantViolations.length > 0) { + return deopt(binding.path); + } + + // let/var/const referenced before init + // or "var" referenced in an outer scope + if (shouldDeoptBasedOnFlow(binding, path)) { + return deopt(path); + } + + return path.evaluate(); +} + +function shouldDeoptBasedOnFlow(binding, refPath) { + const decl = binding.path; + const refs = binding.referencePaths; + + if (binding.kind === "var") { + // early-exit + const declaration = binding.path.parentPath; + if ( + declaration.parentPath.isIfStatement() || + declaration.parentPath.isLoop() + ) { + return true; + } + + let blockParent = binding.path.scope.getBlockParent().path; + const fnParent = binding.path.getFunctionParent(); + + if (blockParent === fnParent) { + blockParent = blockParent.get("body"); + } + + detectUsageOutsideInitScope: { + if (!blockParent.get("body").some(stmt => stmt.isAncestor(refPath))) { + return true; + } + } + + detectUsageBeforeInit: { + const stmts = fnParent.get("body").get("body"); + + const state = stmts.map(stmt => { + if (stmt.isAncestor(binding.path)) { + return { type: "binding" }; + } else { + for (const ref of binding.referencePaths) { + if (stmt.isAncestor(ref)) { + return { + type: "ref" + }; + } + } + return { type: "neither" }; + } + }); + + if (state[0].type === "ref") { + return true; + } + } + } else if (binding.kind === "let" || binding.kind === "const") { + throw "not implemented"; + } + + return false; +} + +function deopt(deoptPath) { + return { + confident: false, + deoptPath + }; +} + +function deref(refPath) { + if (!refPath.isReferencedIdentifier()) { + throw new Error(`Expected ReferencedIdentifier. Got ${refPath.type}`); + } + + const binding = refPath.scope.getBinding(refPath.node.name); + + if (binding.references > 0) { + binding.references--; + } + if (binding.references === 0) { + binding.referenced = false; + } + const idx = binding.referencePaths.indexOf(refPath); + + if (idx < 0) { + throw new Error("Unexpected Error. Scope not updated properly"); + } + + binding.referecePaths.splice(idx, 1); +} diff --git a/packages/babel-plugin-minify-dead-code-elimination/package.json b/packages/babel-plugin-minify-dead-code-elimination/package.json index 332301d99..8b97fd4d3 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/package.json +++ b/packages/babel-plugin-minify-dead-code-elimination/package.json @@ -8,10 +8,9 @@ "author": "amasad", "license": "MIT", "main": "lib/index.js", - "keywords": [ - "babel-plugin" - ], + "keywords": ["babel-plugin"], "dependencies": { + "babel-helper-evaluate-path": "^0.1.0", "babel-helper-mark-eval-scopes": "^0.1.1", "babel-helper-remove-or-void": "^0.1.1", "lodash.some": "^4.6.0" 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 73c3abf77..31f0264d1 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/packages/babel-plugin-minify-dead-code-elimination/src/index.js @@ -3,6 +3,7 @@ const some = require("lodash.some"); const { markEvalScopes, hasEval } = require("babel-helper-mark-eval-scopes"); const removeUseStrict = require("./remove-use-strict"); +const evaluate = require("babel-helper-evaluate-path"); function prevSiblings(path) { const parentPath = path.parentPath; @@ -771,41 +772,9 @@ module.exports = ({ types: t, traverse }) => { const alternate = path.get("alternate"); const test = path.get("test"); - const evalResult = test.evaluate(); + const evalResult = evaluate(test); const isPure = test.isPure(); - 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( From 8998845bb7788e171d220a72c03636b9abc296e6 Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Tue, 8 Aug 2017 16:00:01 +0200 Subject: [PATCH 2/8] mutative evaluate --- .../babel-helper-evaluate-path/src/index.js | 146 +++++++++++++----- .../babel-plugin-minify-builtins/src/index.js | 2 +- .../src/index.js | 2 +- .../__tests__/dead-code-elimination-test.js | 84 +++++----- .../src/index.js | 2 +- 5 files changed, 156 insertions(+), 80 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index e529b4eec..d0b8256fa 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -1,13 +1,15 @@ "use strict"; -module.exports = function evaluate(path) { - if (path.isIdentifier()) { +module.exports = function evaluate(path, t) { + if (!t) { + throw new Error("Expected babel-types"); + } + if (path.isReferencedIdentifier()) { return evaluateIdentifier(path); } const state = { - deoptPath: null, - confident: false + confident: true }; // prepare @@ -16,10 +18,15 @@ module.exports = function evaluate(path) { scopePath.skip(); }, ReferencedIdentifier(idPath) { + const binding = idPath.scope.getBinding(idPath.node.name); + // don't deopt globals + // let babel take care of it + if (!binding) return; + const evalResult = evaluateIdentifier(idPath); if (evalResult.confident) { - idPath.replaceWith(evalResult.value); - deref(idPath); + idPath.replaceWith(t.valueToNode(evalResult.value)); + deref(idPath, binding); } else { state.confident = evalResult.confident; state.deoptPath = evalResult.deoptPath; @@ -61,19 +68,38 @@ function evaluateIdentifier(path) { return deopt(binding.path); } + // referenced in a different scope - deopt + if (shouldDeoptBasedOnScope(binding, path)) { + return deopt(path); + } + // let/var/const referenced before init // or "var" referenced in an outer scope - if (shouldDeoptBasedOnFlow(binding, path)) { + const flowEvalResult = evaluateBasedOnControlFlow(binding, path); + + if (flowEvalResult.confident) { + return flowEvalResult; + } + + if (flowEvalResult.shouldDeopt) { return deopt(path); } return path.evaluate(); } -function shouldDeoptBasedOnFlow(binding, refPath) { - const decl = binding.path; - const refs = binding.referencePaths; +// check if referenced in a different fn scope +// we can't determine if this function is called sync or async +// if the binding is in program scope +// all it's references inside a different function should be deopted +function shouldDeoptBasedOnScope(binding, refPath) { + if (binding.scope.path.isProgram() && refPath.scope !== binding.scope) { + return true; + } + return false; +} +function evaluateBasedOnControlFlow(binding, refPath) { if (binding.kind === "var") { // early-exit const declaration = binding.path.parentPath; @@ -81,49 +107,91 @@ function shouldDeoptBasedOnFlow(binding, refPath) { declaration.parentPath.isIfStatement() || declaration.parentPath.isLoop() ) { - return true; + return { shouldDeopt: true }; } let blockParent = binding.path.scope.getBlockParent().path; const fnParent = binding.path.getFunctionParent(); if (blockParent === fnParent) { - blockParent = blockParent.get("body"); + if (!fnParent.isProgram()) blockParent = blockParent.get("body"); + } + + //detect Usage Outside Init Scope + if (!blockParent.get("body").some(stmt => stmt.isAncestor(refPath))) { + return { shouldDeopt: true }; } - detectUsageOutsideInitScope: { - if (!blockParent.get("body").some(stmt => stmt.isAncestor(refPath))) { - return true; + // Detect usage before init + const stmts = fnParent.isProgram() + ? fnParent.get("body") + : fnParent.get("body").get("body"); + + const state = stmts.map(stmt => { + if (stmt.isAncestor(binding.path)) { + return { type: "binding" }; + } + for (const ref of binding.referencePaths) { + if (stmt.isAncestor(ref)) { + return { + type: ref === refPath ? "current-ref" : "other-ref" + }; + } } + return { type: "neither" }; + }); + + const types = state.map(s => s.type); + + if (types.indexOf("current-ref") < types.indexOf("binding")) { + return { shouldDeopt: true }; + } + } else if (binding.kind === "let" || binding.kind === "const") { + // binding.path is the declarator + const declarator = binding.path; + + let scopePath = declarator.scope.path; + + if (scopePath.isFunction()) { + scopePath = scopePath.get("body"); } - detectUsageBeforeInit: { - const stmts = fnParent.get("body").get("body"); - - const state = stmts.map(stmt => { - if (stmt.isAncestor(binding.path)) { - return { type: "binding" }; - } else { - for (const ref of binding.referencePaths) { - if (stmt.isAncestor(ref)) { - return { - type: "ref" - }; - } - } - return { type: "neither" }; + // Detect Usage before Init + const stmts = scopePath.get("body"); + + const state = { + binding: null, + reference: null + }; + + for (const [idx, stmt] of stmts.entries()) { + if (stmt.isAncestor(binding.path)) { + state.binding = { idx }; + } + for (const ref of binding.referencePaths) { + if (ref === refPath && stmt.isAncestor(ref)) { + state.reference = { + idx, + scope: binding.path.scope === ref.scope ? "current" : "other" + }; + break; } - }); + } + } - if (state[0].type === "ref") { - return true; + if (state.reference && state.binding) { + if ( + state.reference.scope === "current" && + state.reference.idx < state.binding.idx + ) { + return { confident: true, value: void 0 }; } + + return { shouldDeopt: true }; } - } else if (binding.kind === "let" || binding.kind === "const") { - throw "not implemented"; } - return false; + return { confident: false, shouldDeopt: false }; } function deopt(deoptPath) { @@ -133,13 +201,11 @@ function deopt(deoptPath) { }; } -function deref(refPath) { +function deref(refPath, binding) { if (!refPath.isReferencedIdentifier()) { throw new Error(`Expected ReferencedIdentifier. Got ${refPath.type}`); } - const binding = refPath.scope.getBinding(refPath.node.name); - if (binding.references > 0) { binding.references--; } @@ -152,5 +218,5 @@ function deref(refPath) { throw new Error("Unexpected Error. Scope not updated properly"); } - binding.referecePaths.splice(idx, 1); + binding.referencePaths.splice(idx, 1); } diff --git a/packages/babel-plugin-minify-builtins/src/index.js b/packages/babel-plugin-minify-builtins/src/index.js index 5f1119e01..5dd205b66 100644 --- a/packages/babel-plugin-minify-builtins/src/index.js +++ b/packages/babel-plugin-minify-builtins/src/index.js @@ -48,7 +48,7 @@ module.exports = function({ types: t }) { // computed property should be not optimized // Math[max]() -> Math.max() if (!isComputed(callee) && isBuiltin(callee)) { - const result = evaluate(path); + const result = evaluate(path, t); // deopt when we have side effecty evaluate-able arguments // Math.max(foo(), 1) --> untouched // Math.floor(1) --> 1 diff --git a/packages/babel-plugin-minify-constant-folding/src/index.js b/packages/babel-plugin-minify-constant-folding/src/index.js index 5e591a4f2..4cce64d2d 100644 --- a/packages/babel-plugin-minify-constant-folding/src/index.js +++ b/packages/babel-plugin-minify-constant-folding/src/index.js @@ -143,7 +143,7 @@ module.exports = babel => { return; } - const res = evaluate(path); + const res = evaluate(path, t); if (res.confident) { // Avoid fractions because they can be longer than the original expression. // There is also issues with number percision? 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 c4b5ab9e3..d01ccac94 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 @@ -518,59 +518,41 @@ describe("dce-plugin", () => { ); thePlugin( - "should handle orpahaned returns", + "should handle orphaned returns", ` - var a = true; - function foo() { - if (a) return; - x(); - } - `, + var a = true; + function foo() { + if (a) return; + x(); + } ` - var a = true; - function foo() {} - ` ); thePlugin( "should handle orpahaned returns with a value", ` - var a = true; - function foo() { - if (a) return 1; - x(); - } - `, + var a = true; + function foo() { + if (a) return 1; + x(); + } ` - var a = true; - function foo() { - return 1; - } - ` ); thePlugin( "should handle orphaned, redundant returns", ` - var x = true; - function foo() { - if (b) { - if (x) { - z(); - return; + var x = true; + function foo() { + if (b) { + if (x) { + z(); + return; + } + y(); } - y(); } - } - `, ` - var x = true; - function foo() { - if (b) { - z(); - } - } - ` ); thePlugin( @@ -2472,4 +2454,32 @@ describe("dce-plugin", () => { } ` ); + + thePlugin( + "should optimize to void 0 for lets referenced before init declarations", + ` + function foo() { + bar(a); + let a = 1; + } + `, + ` + function foo() { + bar(void 0); + } + ` + ); + + thePlugin( + "should optimize lets referenced before init declarations - 2", + ` + function foo() { + if (a) console.log(a); + let a = 1; + } + `, + ` + function foo() {} + ` + ); }); 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 31f0264d1..badc08314 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/packages/babel-plugin-minify-dead-code-elimination/src/index.js @@ -772,7 +772,7 @@ module.exports = ({ types: t, traverse }) => { const alternate = path.get("alternate"); const test = path.get("test"); - const evalResult = evaluate(test); + const evalResult = evaluate(test, t); const isPure = test.isPure(); const replacements = []; From 0002669e6f6e9d979dc00d1b21e441a3ddd643c5 Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Tue, 8 Aug 2017 17:59:08 +0200 Subject: [PATCH 3/8] Fix let and var --- .../babel-helper-evaluate-path/src/index.js | 41 +++++++++++++------ .../__tests__/dead-code-elimination-test.js | 12 +++++- .../src/index.js | 10 ++--- 3 files changed, 43 insertions(+), 20 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index d0b8256fa..b99f2ebf0 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -117,7 +117,7 @@ function evaluateBasedOnControlFlow(binding, refPath) { if (!fnParent.isProgram()) blockParent = blockParent.get("body"); } - //detect Usage Outside Init Scope + // detect Usage Outside Init Scope if (!blockParent.get("body").some(stmt => stmt.isAncestor(refPath))) { return { shouldDeopt: true }; } @@ -127,23 +127,34 @@ function evaluateBasedOnControlFlow(binding, refPath) { ? fnParent.get("body") : fnParent.get("body").get("body"); - const state = stmts.map(stmt => { + const state = { + binding: null, + reference: null + }; + + for (const [idx, stmt] of stmts.entries()) { if (stmt.isAncestor(binding.path)) { - return { type: "binding" }; + state.binding = { idx }; } for (const ref of binding.referencePaths) { - if (stmt.isAncestor(ref)) { - return { - type: ref === refPath ? "current-ref" : "other-ref" + if (ref === refPath && stmt.isAncestor(ref)) { + state.reference = { + idx, + scope: binding.path.scope === ref.scope ? "current" : "other" }; + break; } } - return { type: "neither" }; - }); + } - const types = state.map(s => s.type); + if (state.reference && state.binding) { + if ( + state.reference.scope === "current" && + state.reference.idx < state.binding.idx + ) { + return { confident: true, value: void 0 }; + } - if (types.indexOf("current-ref") < types.indexOf("binding")) { return { shouldDeopt: true }; } } else if (binding.kind === "let" || binding.kind === "const") { @@ -184,10 +195,14 @@ function evaluateBasedOnControlFlow(binding, refPath) { state.reference.scope === "current" && state.reference.idx < state.binding.idx ) { - return { confident: true, value: void 0 }; + throw new Error( + `ReferenceError: Used ${refPath.node.name}: ` + + `${binding.kind} binding before declaration` + ); + } + if (state.reference.scope === "other") { + return { shouldDeopt: true }; } - - return { shouldDeopt: true }; } } 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 d01ccac94..51d95086a 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 @@ -2474,12 +2474,20 @@ describe("dce-plugin", () => { "should optimize lets referenced before init declarations - 2", ` function foo() { - if (a) console.log(a); + function bar() { + if (a) console.log(a); + } let a = 1; + return bar; } `, ` - function foo() {} + function foo() { + let a = 1; + return function () { + if (a) console.log(a); + }; + } ` ); }); 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 badc08314..fd851bcfa 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/packages/babel-plugin-minify-dead-code-elimination/src/index.js @@ -509,7 +509,7 @@ module.exports = ({ types: t, traverse }) => { SwitchStatement: { exit(path) { - const evaluated = path.get("discriminant").evaluate(); + const evaluated = evaluate(path.get("discriminant"), t); if (!evaluated.confident) return; @@ -528,7 +528,7 @@ module.exports = ({ types: t, traverse }) => { continue; } - const testResult = test.evaluate(); + const testResult = evaluate(test, t); // if we are not able to deternine a test during // compile time, we terminate immediately @@ -614,7 +614,7 @@ module.exports = ({ types: t, traverse }) => { WhileStatement(path) { const test = path.get("test"); - const result = test.evaluate(); + const result = evaluate(test, t); if (result.confident && test.isPure() && !result.value) { path.remove(); } @@ -624,7 +624,7 @@ module.exports = ({ types: t, traverse }) => { const test = path.get("test"); if (!test.isPure()) return; - const result = test.evaluate(); + const result = evaluate(test, t); if (result.confident) { if (result.value) { test.remove(); @@ -636,7 +636,7 @@ module.exports = ({ types: t, traverse }) => { DoWhileStatement(path) { const test = path.get("test"); - const result = test.evaluate(); + const result = evaluate(test, t); if (result.confident && test.isPure() && !result.value) { const body = path.get("body"); From 110a1ecbc575e0667be0fe2e13bf4005d962cfdb Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Tue, 8 Aug 2017 18:02:10 +0200 Subject: [PATCH 4/8] Fix minify builtins testcases --- packages/babel-helper-evaluate-path/src/index.js | 2 +- .../__snapshots__/minify-builtins.js.snap | 14 +++++++------- .../__tests__/minify-builtins.js | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index b99f2ebf0..ec65c8739 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -25,8 +25,8 @@ module.exports = function evaluate(path, t) { const evalResult = evaluateIdentifier(idPath); if (evalResult.confident) { - idPath.replaceWith(t.valueToNode(evalResult.value)); deref(idPath, binding); + idPath.replaceWith(t.valueToNode(evalResult.value)); } else { state.confident = evalResult.confident; state.deoptPath = evalResult.deoptPath; diff --git a/packages/babel-plugin-minify-builtins/__tests__/__snapshots__/minify-builtins.js.snap b/packages/babel-plugin-minify-builtins/__tests__/__snapshots__/minify-builtins.js.snap index 444173407..ffe207d50 100644 --- a/packages/babel-plugin-minify-builtins/__tests__/__snapshots__/minify-builtins.js.snap +++ b/packages/babel-plugin-minify-builtins/__tests__/__snapshots__/minify-builtins.js.snap @@ -85,7 +85,7 @@ new A();", exports[`minify-builtins should collect and minify no matter any depth 1`] = ` Object { "_source": "function a (){ - Math.max(b, a); + Math.max(c, a); const b = () => { const a = Math.floor(c); Math.min(b, a) * Math.floor(b); @@ -95,7 +95,7 @@ Object { } }", "expected": "function a() { - Math.max(b, a); + Math.max(c, a); const b = () => { var _Mathmin = Math.min; var _Mathfloor = Math.floor; @@ -170,8 +170,8 @@ exports[`minify-builtins should minify builtins to method scope for class declar Object { "_source": "class Test { foo() { - Math.max(c, d) - Math.max(c, d) + Math.max(a, d) + Math.max(a, d) const c = function() { Math.max(c, d) Math.floor(m); @@ -187,8 +187,8 @@ Object { foo() { var _Mathmax = Math.max; - _Mathmax(c, d); - _Mathmax(c, d); + _Mathmax(a, d); + _Mathmax(a, d); const c = function () { var _Mathfloor = Math.floor; @@ -220,7 +220,7 @@ Object { let a = 10; const d = false; - _Mathmax(a, b) + _Mathmax(b, a); + _Mathmax(10, b) + _Mathmax(b, 10); return d && true; }", } diff --git a/packages/babel-plugin-minify-builtins/__tests__/minify-builtins.js b/packages/babel-plugin-minify-builtins/__tests__/minify-builtins.js index 344396c78..7837a6e47 100644 --- a/packages/babel-plugin-minify-builtins/__tests__/minify-builtins.js +++ b/packages/babel-plugin-minify-builtins/__tests__/minify-builtins.js @@ -40,7 +40,7 @@ describe("minify-builtins", () => { "should collect and minify no matter any depth", ` function a (){ - Math.max(b, a); + Math.max(c, a); const b = () => { const a = Math.floor(c); Math.min(b, a) * Math.floor(b); @@ -68,8 +68,8 @@ describe("minify-builtins", () => { ` class Test { foo() { - Math.max(c, d) - Math.max(c, d) + Math.max(a, d) + Math.max(a, d) const c = function() { Math.max(c, d) Math.floor(m); From b51b904ad4d5eabc80e3690d95ddcd4e786e811c Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Tue, 8 Aug 2017 18:22:40 +0200 Subject: [PATCH 5/8] Fix for switchcase parent --- packages/babel-helper-evaluate-path/src/index.js | 8 +++----- .../__tests__/__snapshots__/minify-builtins.js.snap | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index ec65c8739..6a43e4ecd 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -24,10 +24,7 @@ module.exports = function evaluate(path, t) { if (!binding) return; const evalResult = evaluateIdentifier(idPath); - if (evalResult.confident) { - deref(idPath, binding); - idPath.replaceWith(t.valueToNode(evalResult.value)); - } else { + if (!evalResult.confident) { state.confident = evalResult.confident; state.deoptPath = evalResult.deoptPath; } @@ -105,7 +102,8 @@ function evaluateBasedOnControlFlow(binding, refPath) { const declaration = binding.path.parentPath; if ( declaration.parentPath.isIfStatement() || - declaration.parentPath.isLoop() + declaration.parentPath.isLoop() || + declaration.parentPath.isSwitchCase() ) { return { shouldDeopt: true }; } diff --git a/packages/babel-plugin-minify-builtins/__tests__/__snapshots__/minify-builtins.js.snap b/packages/babel-plugin-minify-builtins/__tests__/__snapshots__/minify-builtins.js.snap index ffe207d50..2873bc632 100644 --- a/packages/babel-plugin-minify-builtins/__tests__/__snapshots__/minify-builtins.js.snap +++ b/packages/babel-plugin-minify-builtins/__tests__/__snapshots__/minify-builtins.js.snap @@ -220,7 +220,7 @@ Object { let a = 10; const d = false; - _Mathmax(10, b) + _Mathmax(b, 10); + _Mathmax(a, b) + _Mathmax(b, a); return d && true; }", } From ad9be49288048d74b99169800ea76c55d3b15f15 Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Tue, 8 Aug 2017 18:26:10 +0200 Subject: [PATCH 6/8] Remove deadcode --- .../babel-helper-evaluate-path/src/index.js | 25 +------------------ .../babel-plugin-minify-builtins/src/index.js | 2 +- .../src/index.js | 2 +- .../src/index.js | 12 ++++----- 4 files changed, 9 insertions(+), 32 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index 6a43e4ecd..cea47a1b0 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -1,9 +1,6 @@ "use strict"; -module.exports = function evaluate(path, t) { - if (!t) { - throw new Error("Expected babel-types"); - } +module.exports = function evaluate(path) { if (path.isReferencedIdentifier()) { return evaluateIdentifier(path); } @@ -213,23 +210,3 @@ function deopt(deoptPath) { deoptPath }; } - -function deref(refPath, binding) { - if (!refPath.isReferencedIdentifier()) { - throw new Error(`Expected ReferencedIdentifier. Got ${refPath.type}`); - } - - if (binding.references > 0) { - binding.references--; - } - if (binding.references === 0) { - binding.referenced = false; - } - const idx = binding.referencePaths.indexOf(refPath); - - if (idx < 0) { - throw new Error("Unexpected Error. Scope not updated properly"); - } - - binding.referencePaths.splice(idx, 1); -} diff --git a/packages/babel-plugin-minify-builtins/src/index.js b/packages/babel-plugin-minify-builtins/src/index.js index 5dd205b66..5f1119e01 100644 --- a/packages/babel-plugin-minify-builtins/src/index.js +++ b/packages/babel-plugin-minify-builtins/src/index.js @@ -48,7 +48,7 @@ module.exports = function({ types: t }) { // computed property should be not optimized // Math[max]() -> Math.max() if (!isComputed(callee) && isBuiltin(callee)) { - const result = evaluate(path, t); + const result = evaluate(path); // deopt when we have side effecty evaluate-able arguments // Math.max(foo(), 1) --> untouched // Math.floor(1) --> 1 diff --git a/packages/babel-plugin-minify-constant-folding/src/index.js b/packages/babel-plugin-minify-constant-folding/src/index.js index 4cce64d2d..5e591a4f2 100644 --- a/packages/babel-plugin-minify-constant-folding/src/index.js +++ b/packages/babel-plugin-minify-constant-folding/src/index.js @@ -143,7 +143,7 @@ module.exports = babel => { return; } - const res = evaluate(path, t); + const res = evaluate(path); if (res.confident) { // Avoid fractions because they can be longer than the original expression. // There is also issues with number percision? 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 fd851bcfa..e882b7b94 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/packages/babel-plugin-minify-dead-code-elimination/src/index.js @@ -509,7 +509,7 @@ module.exports = ({ types: t, traverse }) => { SwitchStatement: { exit(path) { - const evaluated = evaluate(path.get("discriminant"), t); + const evaluated = evaluate(path.get("discriminant")); if (!evaluated.confident) return; @@ -528,7 +528,7 @@ module.exports = ({ types: t, traverse }) => { continue; } - const testResult = evaluate(test, t); + const testResult = evaluate(test); // if we are not able to deternine a test during // compile time, we terminate immediately @@ -614,7 +614,7 @@ module.exports = ({ types: t, traverse }) => { WhileStatement(path) { const test = path.get("test"); - const result = evaluate(test, t); + const result = evaluate(test); if (result.confident && test.isPure() && !result.value) { path.remove(); } @@ -624,7 +624,7 @@ module.exports = ({ types: t, traverse }) => { const test = path.get("test"); if (!test.isPure()) return; - const result = evaluate(test, t); + const result = evaluate(test); if (result.confident) { if (result.value) { test.remove(); @@ -636,7 +636,7 @@ module.exports = ({ types: t, traverse }) => { DoWhileStatement(path) { const test = path.get("test"); - const result = evaluate(test, t); + const result = evaluate(test); if (result.confident && test.isPure() && !result.value) { const body = path.get("body"); @@ -772,7 +772,7 @@ module.exports = ({ types: t, traverse }) => { const alternate = path.get("alternate"); const test = path.get("test"); - const evalResult = evaluate(test, t); + const evalResult = evaluate(test); const isPure = test.isPure(); const replacements = []; From 81e364ac43aa534124f4d9d8ac166aee4438d127 Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Tue, 8 Aug 2017 23:28:05 +0200 Subject: [PATCH 7/8] Extract compare function --- .../babel-helper-evaluate-path/src/index.js | 88 +++++++++---------- 1 file changed, 41 insertions(+), 47 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index cea47a1b0..4247d3f4d 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -122,30 +122,16 @@ function evaluateBasedOnControlFlow(binding, refPath) { ? fnParent.get("body") : fnParent.get("body").get("body"); - const state = { - binding: null, - reference: null - }; - - for (const [idx, stmt] of stmts.entries()) { - if (stmt.isAncestor(binding.path)) { - state.binding = { idx }; - } - for (const ref of binding.referencePaths) { - if (ref === refPath && stmt.isAncestor(ref)) { - state.reference = { - idx, - scope: binding.path.scope === ref.scope ? "current" : "other" - }; - break; - } - } - } + const compareResult = compareBindingAndReference({ + binding, + refPath, + stmts + }); - if (state.reference && state.binding) { + if (compareResult.reference && compareResult.binding) { if ( - state.reference.scope === "current" && - state.reference.idx < state.binding.idx + compareResult.reference.scope === "current" && + compareResult.reference.idx < compareResult.binding.idx ) { return { confident: true, value: void 0 }; } @@ -155,9 +141,7 @@ function evaluateBasedOnControlFlow(binding, refPath) { } else if (binding.kind === "let" || binding.kind === "const") { // binding.path is the declarator const declarator = binding.path; - let scopePath = declarator.scope.path; - if (scopePath.isFunction()) { scopePath = scopePath.get("body"); } @@ -165,37 +149,23 @@ function evaluateBasedOnControlFlow(binding, refPath) { // Detect Usage before Init const stmts = scopePath.get("body"); - const state = { - binding: null, - reference: null - }; - - for (const [idx, stmt] of stmts.entries()) { - if (stmt.isAncestor(binding.path)) { - state.binding = { idx }; - } - for (const ref of binding.referencePaths) { - if (ref === refPath && stmt.isAncestor(ref)) { - state.reference = { - idx, - scope: binding.path.scope === ref.scope ? "current" : "other" - }; - break; - } - } - } + const compareResult = compareBindingAndReference({ + binding, + refPath, + stmts + }); - if (state.reference && state.binding) { + if (compareResult.reference && compareResult.binding) { if ( - state.reference.scope === "current" && - state.reference.idx < state.binding.idx + compareResult.reference.scope === "current" && + compareResult.reference.idx < compareResult.binding.idx ) { throw new Error( `ReferenceError: Used ${refPath.node.name}: ` + `${binding.kind} binding before declaration` ); } - if (state.reference.scope === "other") { + if (compareResult.reference.scope === "other") { return { shouldDeopt: true }; } } @@ -204,6 +174,30 @@ function evaluateBasedOnControlFlow(binding, refPath) { return { confident: false, shouldDeopt: false }; } +function compareBindingAndReference({ binding, refPath, stmts }) { + const state = { + binding: null, + reference: null + }; + + for (const [idx, stmt] of stmts.entries()) { + if (stmt.isAncestor(binding.path)) { + state.binding = { idx }; + } + for (const ref of binding.referencePaths) { + if (ref === refPath && stmt.isAncestor(ref)) { + state.reference = { + idx, + scope: binding.path.scope === ref.scope ? "current" : "other" + }; + break; + } + } + } + + return state; +} + function deopt(deoptPath) { return { confident: false, From c9a8059c603b337de31e7f83e8ff110f2e95a05b Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Tue, 8 Aug 2017 23:33:13 +0200 Subject: [PATCH 8/8] Skip test --- .../__tests__/dead-code-elimination-test.js | 9 ++------- 1 file changed, 2 insertions(+), 7 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 51d95086a..26d0ef427 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 @@ -2455,18 +2455,13 @@ describe("dce-plugin", () => { ` ); - thePlugin( + thePlugin.skip( "should optimize to void 0 for lets referenced before init declarations", ` function foo() { - bar(a); + bar(a); // Should be a ReferenceError let a = 1; } - `, - ` - function foo() { - bar(void 0); - } ` );