diff --git a/.eslintrc b/.eslintrc index a87319bb62..007d3298cc 100644 --- a/.eslintrc +++ b/.eslintrc @@ -17,7 +17,8 @@ "no-fallthrough": 0, "new-cap": 0, "no-loop-func": 0, - "no-unreachable": 0 + "no-unreachable": 0, + "no-labels": 0 }, "env": { "node": true diff --git a/src/babel/transformation/transformers/es6/parameters.rest.js b/src/babel/transformation/transformers/es6/parameters.rest.js index 3c1d8e044e..7d309ba5d2 100644 --- a/src/babel/transformation/transformers/es6/parameters.rest.js +++ b/src/babel/transformation/transformers/es6/parameters.rest.js @@ -9,41 +9,53 @@ var memberExpressionOptimisationVisitor = { } }, - enter(node, parent, scope, state) { - var stop = () => { - state.canOptimise = false; - this.stop(); - }; - - if (this.isArrowFunctionExpression()) return stop(); - + Function(node, parent, scope, state) { // skip over functions as whatever `arguments` we reference inside will refer // to the wrong function - if (this.isFunctionDeclaration() || this.isFunctionExpression()) { - state.noOptimise = true; - this.traverse(memberExpressionOptimisationVisitor, state); - state.noOptimise = false; - return this.skip(); + state.noOptimise = true; + this.traverse(memberExpressionOptimisationVisitor, state); + state.noOptimise = false; + this.skip(); + }, + + ReferencedIdentifier(node, parent, scope, state) { + // we can't guarantee the purity of arguments + if (node.name === "arguments") { + state.deopted = true; } // is this a referenced identifier and is it referencing the rest parameter? - if (!this.isReferencedIdentifier({ name: state.name })) return; + if (node.name !== state.name) return; - if (!state.noOptimise && t.isMemberExpression(parent) && parent.computed) { - // if we know that this member expression is referencing a number then we can safely - // optimise it - var prop = this.parentPath.get("property"); - if (prop.isBaseType("number")) { - state.candidates.push(this); - return; + if (!state.noOptimise) { + if (this.parentPath.isMemberExpression({ computed: true, object: node })) { + // if we know that this member expression is referencing a number then we can safely + // optimise it + var prop = this.parentPath.get("property"); + if (prop.isBaseType("number")) { + state.candidates.push(this); + return; + } + } + + // optimise single spread args in calls + if (this.parentPath.isSpreadElement() && state.offset === 0) { + var call = this.parentPath.parentPath; + if (call.isCallExpression() && call.node.arguments.length === 1) { + return state.argumentsNode; + } } } - stop(); + if (state.noOptimise) { + state.deopted = true; + } else { + state.references.push(this); + } } }; -function optimizeMemberExpression(parent, offset) { +function optimiseMemberExpression(parent, offset) { if (offset === 0) return; var newExpr; @@ -86,25 +98,38 @@ export var visitor = { node.body.body.unshift(declar); } - // check if rest is used in member expressions and optimise for those cases + // check and optimise for extremely common cases var state = { - outerBinding: scope.getBindingIdentifier(rest.name), - canOptimise: true, - candidates: [], - method: node, - name: rest.name + references: [], + offset: node.params.length, + + argumentsNode: argsId, + outerBinding: scope.getBindingIdentifier(rest.name), + + // candidate member expressions we could optimise if there are no other references + candidates: [], + + // local rest binding name + name: rest.name, + + // whether any references to the rest parameter were made in a function + deopted: false }; this.traverse(memberExpressionOptimisationVisitor, state); - // we only have shorthands and there's no other references - if (state.canOptimise && state.candidates.length) { - for (var candidate of (state.candidates: Array)) { - candidate.replaceWith(argsId); - optimizeMemberExpression(candidate.parent, node.params.length); + if (!state.deopted && !state.references.length) { + // we only have shorthands and there are no other references + if (state.candidates.length) { + for (var candidate of (state.candidates: Array)) { + candidate.replaceWith(argsId); + optimiseMemberExpression(candidate.parent, state.offset); + } } return; + } else { + state.references = state.references.concat(state.candidates); } // @@ -144,6 +169,13 @@ export var visitor = { KEY: key, LEN: len }); + + if (!state.deopted) { + // perform allocation at the lowest common denominator of all references + this.getEarliestCommonAncestorFrom(state.references).getStatementParent().insertBefore(loop); + return; + } + loop._blockHoist = node.params.length + 1; node.body.body.unshift(loop); } diff --git a/src/babel/transformation/transformers/other/regenerator.js b/src/babel/transformation/transformers/other/regenerator.js index e49d585270..28695eadad 100644 --- a/src/babel/transformation/transformers/other/regenerator.js +++ b/src/babel/transformation/transformers/other/regenerator.js @@ -38,9 +38,9 @@ function convertNodePath(path) { keysAlongPath.push(path.key); if (parentNode !== path.container) { - var found = Object.keys(parentNode).some(containerKey => { - if (parentNode[containerKey] === path.container) { - keysAlongPath.push(containerKey); + var found = Object.keys(parentNode).some(listKey => { + if (parentNode[listKey] === path.container) { + keysAlongPath.push(listKey); return true; } }); diff --git a/src/babel/traversal/context.js b/src/babel/traversal/context.js index 58bc94d13e..3706f1905c 100644 --- a/src/babel/traversal/context.js +++ b/src/babel/traversal/context.js @@ -26,19 +26,19 @@ export default class TraversalContext { return false; } - create(node, obj, key, containerKey) { + create(node, obj, key, listKey) { var path = NodePath.get({ parentPath: this.parentPath, parent: node, container: obj, key: key, - containerKey: containerKey + listKey }); path.unshiftContext(this); return path; } - visitMultiple(container, parent, containerKey) { + visitMultiple(container, parent, listKey) { // nothing to traverse! if (container.length === 0) return false; @@ -51,7 +51,7 @@ export default class TraversalContext { for (let key = 0; key < container.length; key++) { var self = container[key]; if (self && this.shouldVisit(self)) { - queue.push(this.create(parent, container, key, containerKey)); + queue.push(this.create(parent, container, key, listKey)); } } diff --git a/src/babel/traversal/path/ancestry.js b/src/babel/traversal/path/ancestry.js index 22d335200e..b2a0036db3 100644 --- a/src/babel/traversal/path/ancestry.js +++ b/src/babel/traversal/path/ancestry.js @@ -1,3 +1,6 @@ +import * as t from "../../types"; +import NodePath from "./index"; + /** * Description */ @@ -28,16 +31,118 @@ export function getStatementParent() { * Description */ -export function getAncestry() { - var ancestry = []; +export function getEarliestCommonAncestorFrom(paths: Array): NodePath { + return this.getDeepestCommonAncestorFrom(paths, function (deepest, i, ancestries) { + var earliest; + var keys = t.VISITOR_KEYS[deepest.type]; - var path = this.parentPath; - while (path) { - ancestry.push(path.node); - path = path.parentPath; + for (var ancestry of (ancestries: Array)) { + var path = ancestry[i - 0]; + + if (!earliest) { + earliest = path; + continue; + } + + // handle containers + if (path.listKey && earliest.listKey === path.listKey) { + // we're in the same container so check + if (path.key < earliest.key) { + earliest = path; + continue; + } + } + + // handle keys + var earliestKeyIndex = keys.indexOf(earliest.parentKey); + var currentKeyIndex = keys.indexOf(path.parentKey); + if (earliestKeyIndex > currentKeyIndex) { + // key appears after + earliest = path; + } + } + + return earliest; + }); +} + +/** + * Get the earliest path in the tree where the provided `paths` intersect. + * + * TODO: Possible optimisation target. + */ + +export function getDeepestCommonAncestorFrom(paths: Array, filter?: Function): NodePath { + if (!paths.length) { + return this; } - return ancestry; + if (paths.length === 1) { + return paths[0]; + } + + // minimum depth of the tree so we know the highest node + var minDepth = Infinity; + + // last common ancestor + var lastCommonIndex, lastCommon; + + // get the ancestors of the path, breaking when the parent exceeds ourselves + var ancestries = paths.map((path) => { + var ancestry = []; + + do { + ancestry.unshift(path); + } while((path = path.parentPath) && path !== this); + + // save min depth to avoid going too far in + if (ancestry.length < minDepth) { + minDepth = ancestry.length; + } + + return ancestry; + }); + + // get the first ancestry so we have a seed to assess all other ancestries with + var first = ancestries[0]; + + // check ancestor equality + depthLoop: for (var i = 0; i < minDepth; i++) { + var shouldMatch = first[i]; + + for (var ancestry of (ancestries: Array)) { + if (ancestry[i] !== shouldMatch) { + // we've hit a snag + break depthLoop; + } + } + + lastCommonIndex = i; + lastCommon = shouldMatch; + } + + if (lastCommon) { + if (filter) { + return filter(lastCommon, lastCommonIndex, ancestries); + } else { + return lastCommon; + } + } else { + throw new Error("Couldn't find intersection"); + } +} + +/** + * Description + */ + +export function getAncestry() { + var path = this; + var paths = []; + do { + paths.push(path); + } while(path = path.parentPath); + return paths; } /** diff --git a/src/babel/traversal/path/context.js b/src/babel/traversal/path/context.js index 6790e1f54d..d6697d9510 100644 --- a/src/babel/traversal/path/context.js +++ b/src/babel/traversal/path/context.js @@ -148,7 +148,7 @@ export function resync() { if (this.removed) return; this._resyncParent(); - this._resyncContainer(); + this._resyncList(); this._resyncKey(); //this._resyncRemoved(); } @@ -184,12 +184,12 @@ export function _resyncKey() { this.key = null; } -export function _resyncContainer() { - var containerKey = this.containerKey; - var parentPath = this.parentPath; - if (!containerKey || !parentPath) return; +export function _resyncList() { + var listKey = this.listKey; + var parentPath = this.parentPath; + if (!listKey || !parentPath) return; - var newContainer = parentPath.node[containerKey]; + var newContainer = parentPath.node[listKey]; if (this.container === newContainer) return; // container is out of sync. this is likely the result of it being reassigned @@ -229,9 +229,10 @@ export function unshiftContext(context) { * Description */ -export function setup(parentPath, container, containerKey, key) { - this.containerKey = containerKey; - this.container = container; +export function setup(parentPath, container, listKey, key) { + this.listKey = listKey; + this.parentKey = listKey || key; + this.container = container; this.parentPath = parentPath || this.parentPath; this.setKey(key); diff --git a/src/babel/traversal/path/family.js b/src/babel/traversal/path/family.js index 4558abfaf9..8e91a40128 100644 --- a/src/babel/traversal/path/family.js +++ b/src/babel/traversal/path/family.js @@ -71,7 +71,7 @@ export function getSibling(key) { parentPath: this.parentPath, parent: this.parent, container: this.container, - containerKey: this.containerKey, + listKey: this.listKey, key: key }); } @@ -101,7 +101,7 @@ export function _getKey(key) { // requested a container so give them all the paths return container.map((_, i) => { return NodePath.get({ - containerKey: key, + listKey: key, parentPath: this, parent: node, container: container, diff --git a/src/babel/traversal/path/index.js b/src/babel/traversal/path/index.js index be693d8b47..f7199e5e43 100644 --- a/src/babel/traversal/path/index.js +++ b/src/babel/traversal/path/index.js @@ -20,11 +20,11 @@ export default class NodePath { this.parentPath = null; this.context = null; this.container = null; - this.containerKey = null; + this.listKey = null; + this.parentKey = null; this.key = null; this.node = null; this.scope = null; - this.breakOnScopePaths = null; this.type = null; this.typeAnnotation = null; } @@ -33,7 +33,7 @@ export default class NodePath { * Description */ - static get({ hub, parentPath, parent, container, containerKey, key }) { + static get({ hub, parentPath, parent, container, listKey, key }) { if (!hub && parentPath) { hub = parentPath.hub; } @@ -55,7 +55,7 @@ export default class NodePath { paths.push(path); } - path.setup(parentPath, container, containerKey, key); + path.setup(parentPath, container, listKey, key); return path; } diff --git a/src/babel/traversal/path/introspection.js b/src/babel/traversal/path/introspection.js index 173bded39c..33875b1013 100644 --- a/src/babel/traversal/path/introspection.js +++ b/src/babel/traversal/path/introspection.js @@ -260,10 +260,10 @@ export function _guessExecutionStatusRelativeTo(target) { return "function"; } - var targetPaths = getAncestry(target); + var targetPaths = target.getAncestry(); //if (targetPaths.indexOf(this) >= 0) return "after"; - var selfPaths = getAncestry(this); + var selfPaths = this.getAncestry(); // get ancestor where the branches intersect var commonPath; @@ -289,7 +289,7 @@ export function _guessExecutionStatusRelativeTo(target) { } // container list so let's see which one is after the other - if (targetRelationship.containerKey && targetRelationship.container === selfRelationship.container) { + if (targetRelationship.listKey && targetRelationship.container === selfRelationship.container) { return targetRelationship.key > selfRelationship.key ? "before" : "after"; } @@ -299,14 +299,6 @@ export function _guessExecutionStatusRelativeTo(target) { return targetKeyPosition > selfKeyPosition ? "before" : "after"; } -function getAncestry(path) { - var paths = []; - do { - paths.push(path); - } while(path = path.parentPath); - return paths; -} - /** * Resolve a "pointer" `NodePath` to it's absolute path. */ diff --git a/src/babel/traversal/path/lib/removal-hooks.js b/src/babel/traversal/path/lib/removal-hooks.js index 3fa89b8ab3..2da9519ea7 100644 --- a/src/babel/traversal/path/lib/removal-hooks.js +++ b/src/babel/traversal/path/lib/removal-hooks.js @@ -53,7 +53,7 @@ export var post = [ // var NODE; // remove an entire declaration if there are no declarators left - removeParent = removeParent || (self.containerKey === "declarations" && parent.isVariableDeclaration() && parent.node.declarations.length === 0); + removeParent = removeParent || (self.listKey === "declarations" && parent.isVariableDeclaration() && parent.node.declarations.length === 0); // NODE; // remove the entire expression statement if there's no expression diff --git a/src/babel/traversal/path/modification.js b/src/babel/traversal/path/modification.js index 9594ae4389..0275d7a2bb 100644 --- a/src/babel/traversal/path/modification.js +++ b/src/babel/traversal/path/modification.js @@ -42,7 +42,7 @@ export function _containerInsert(from, nodes) { this.container.splice(to, 0, node); if (this.context) { - var path = this.context.create(this.parent, this.container, to, this.containerKey); + var path = this.context.create(this.parent, this.container, to, this.listKey); paths.push(path); this.queueNode(path); } else { @@ -50,7 +50,7 @@ export function _containerInsert(from, nodes) { parentPath: this, parent: node, container: this.container, - containerKey: this.containerKey, + listKey: this.listKey, key: to })); } @@ -154,7 +154,7 @@ export function _verifyNodeList(nodes) { * Description */ -export function unshiftContainer(containerKey, nodes) { +export function unshiftContainer(listKey, nodes) { this._assertUnremoved(); nodes = this._verifyNodeList(nodes); @@ -162,12 +162,12 @@ export function unshiftContainer(containerKey, nodes) { // get the first path and insert our nodes before it, if it doesn't exist then it // doesn't matter, our nodes will be inserted anyway - var container = this.node[containerKey]; + var container = this.node[listKey]; var path = NodePath.get({ parentPath: this, parent: this.node, container: container, - containerKey, + listKey, key: 0 }); @@ -178,7 +178,7 @@ export function unshiftContainer(containerKey, nodes) { * Description */ -export function pushContainer(containerKey, nodes) { +export function pushContainer(listKey, nodes) { this._assertUnremoved(); nodes = this._verifyNodeList(nodes); @@ -186,13 +186,13 @@ export function pushContainer(containerKey, nodes) { // get an invisible path that represents the last node + 1 and replace it with our // nodes, effectively inlining it - var container = this.node[containerKey]; + var container = this.node[listKey]; var i = container.length; var path = NodePath.get({ parentPath: this, parent: this.node, container: container, - containerKey, + listKey, key: i }); diff --git a/src/babel/traversal/scope/index.js b/src/babel/traversal/scope/index.js index 2a69c855ca..eb0d4acad4 100644 --- a/src/babel/traversal/scope/index.js +++ b/src/babel/traversal/scope/index.js @@ -390,7 +390,8 @@ export default class Scope { var binding = scope.bindings[name]; console.log(" -", name, { constant: binding.constant, - references: binding.references + references: binding.references, + kind: binding.kind }); } } while(scope = scope.parent); diff --git a/test/core/fixtures/transformation/es6.parameters.rest/arrow-functions/actual.js b/test/core/fixtures/transformation/es6.parameters.rest/arrow-functions/actual.js index 9319e6f479..43378f3344 100644 --- a/test/core/fixtures/transformation/es6.parameters.rest/arrow-functions/actual.js +++ b/test/core/fixtures/transformation/es6.parameters.rest/arrow-functions/actual.js @@ -12,7 +12,6 @@ var somefun = function () { let somefg = (c, d, e, f, ...args3) => { var _a = args3[0]; }; - var _c = arguments[1]; var _d = args1[1]; }; let get1stArg = (...args) => args[0]; diff --git a/test/core/fixtures/transformation/es6.parameters.rest/arrow-functions/expected.js b/test/core/fixtures/transformation/es6.parameters.rest/arrow-functions/expected.js index 0ad069e01e..af24e56f78 100644 --- a/test/core/fixtures/transformation/es6.parameters.rest/arrow-functions/expected.js +++ b/test/core/fixtures/transformation/es6.parameters.rest/arrow-functions/expected.js @@ -6,22 +6,15 @@ var concat = function concat() { }; var somefun = function somefun() { - var _arguments = arguments; - var get2ndArg = function get2ndArg(a, b) { - for (var _len = arguments.length, args1 = Array(_len > 2 ? _len - 2 : 0), _key = 2; _key < _len; _key++) { - args1[_key - 2] = arguments[_key]; - } - - var _b = args1[0]; + var _b = arguments[2]; var somef = function somef(x, y, z) { var _a = arguments[3]; }; var somefg = function somefg(c, d, e, f) { var _a = arguments[4]; }; - var _c = _arguments[1]; - var _d = args1[1]; + var _d = arguments[3]; }; var get1stArg = function get1stArg() { return arguments[0]; @@ -29,8 +22,8 @@ var somefun = function somefun() { }; function demo1() { - for (var _len2 = arguments.length, args = Array(_len2), _key2 = 0; _key2 < _len2; _key2++) { - args[_key2] = arguments[_key2]; + for (var _len = arguments.length, args = Array(_len), _key = 0; _key < _len; _key++) { + args[_key] = arguments[_key]; } return function (i) { diff --git a/test/core/fixtures/transformation/es6.parameters.rest/deepest-common-ancestor-earliest-child/actual.js b/test/core/fixtures/transformation/es6.parameters.rest/deepest-common-ancestor-earliest-child/actual.js new file mode 100644 index 0000000000..0d983ffb7c --- /dev/null +++ b/test/core/fixtures/transformation/es6.parameters.rest/deepest-common-ancestor-earliest-child/actual.js @@ -0,0 +1,32 @@ +// single referenes +function r(...rest){ + if (noNeedToWork) return 0; + return rest; +} + +// multiple references +function r(...rest){ + if (noNeedToWork) return 0; + + rest; + rest; +} + +// multiple nested references +function r(...rest){ + if (noNeedToWork) return 0; + + if (true) { + return rest; + } else { + return rest; + } +} + +// nested reference with root reference +function r(...rest){ + if (noNeedToWork) return 0; + + if (lol) rest; + rest; +} diff --git a/test/core/fixtures/transformation/es6.parameters.rest/deepest-common-ancestor-earliest-child/expected.js b/test/core/fixtures/transformation/es6.parameters.rest/deepest-common-ancestor-earliest-child/expected.js new file mode 100644 index 0000000000..f5f8ced03b --- /dev/null +++ b/test/core/fixtures/transformation/es6.parameters.rest/deepest-common-ancestor-earliest-child/expected.js @@ -0,0 +1,51 @@ +// single referenes +"use strict"; + +function r() { + if (noNeedToWork) return 0; + + for (var _len = arguments.length, rest = Array(_len), _key = 0; _key < _len; _key++) { + rest[_key] = arguments[_key]; + } + + return rest; +} + +// multiple references +function r() { + if (noNeedToWork) return 0; + + for (var _len2 = arguments.length, rest = Array(_len2), _key2 = 0; _key2 < _len2; _key2++) { + rest[_key2] = arguments[_key2]; + } + + rest; + rest; +} + +// multiple nested references +function r() { + if (noNeedToWork) return 0; + + for (var _len3 = arguments.length, rest = Array(_len3), _key3 = 0; _key3 < _len3; _key3++) { + rest[_key3] = arguments[_key3]; + } + + if (true) { + return rest; + } else { + return rest; + } +} + +// nested reference with root reference +function r() { + if (noNeedToWork) return 0; + + for (var _len4 = arguments.length, rest = Array(_len4), _key4 = 0; _key4 < _len4; _key4++) { + rest[_key4] = arguments[_key4]; + } + + if (lol) rest; + rest; +} diff --git a/test/core/fixtures/transformation/es6.parameters.rest/deopt/actual.js b/test/core/fixtures/transformation/es6.parameters.rest/member-expression-deoptimisation/actual.js similarity index 100% rename from test/core/fixtures/transformation/es6.parameters.rest/deopt/actual.js rename to test/core/fixtures/transformation/es6.parameters.rest/member-expression-deoptimisation/actual.js diff --git a/test/core/fixtures/transformation/es6.parameters.rest/deopt/expected.js b/test/core/fixtures/transformation/es6.parameters.rest/member-expression-deoptimisation/expected.js similarity index 99% rename from test/core/fixtures/transformation/es6.parameters.rest/deopt/expected.js rename to test/core/fixtures/transformation/es6.parameters.rest/member-expression-deoptimisation/expected.js index eeef013489..b95a32286a 100644 --- a/test/core/fixtures/transformation/es6.parameters.rest/deopt/expected.js +++ b/test/core/fixtures/transformation/es6.parameters.rest/member-expression-deoptimisation/expected.js @@ -47,10 +47,11 @@ var a = function a(foo) { }; var b = function b(foo) { + var join = "join"; + for (var _len6 = arguments.length, bar = Array(_len6 > 1 ? _len6 - 1 : 0), _key6 = 1; _key6 < _len6; _key6++) { bar[_key6 - 1] = arguments[_key6]; } - var join = "join"; return bar[join]; -}; \ No newline at end of file +}; diff --git a/test/core/fixtures/transformation/es6.parameters.rest/single/actual.js b/test/core/fixtures/transformation/es6.parameters.rest/member-expression-optimisation/actual.js similarity index 100% rename from test/core/fixtures/transformation/es6.parameters.rest/single/actual.js rename to test/core/fixtures/transformation/es6.parameters.rest/member-expression-optimisation/actual.js diff --git a/test/core/fixtures/transformation/es6.parameters.rest/single/expected.js b/test/core/fixtures/transformation/es6.parameters.rest/member-expression-optimisation/expected.js similarity index 100% rename from test/core/fixtures/transformation/es6.parameters.rest/single/expected.js rename to test/core/fixtures/transformation/es6.parameters.rest/member-expression-optimisation/expected.js diff --git a/test/core/fixtures/transformation/es6.parameters.rest/spread-optimisation/actual.js b/test/core/fixtures/transformation/es6.parameters.rest/spread-optimisation/actual.js new file mode 100644 index 0000000000..a57ae130ab --- /dev/null +++ b/test/core/fixtures/transformation/es6.parameters.rest/spread-optimisation/actual.js @@ -0,0 +1,15 @@ +// optimisation + +function foo(...bar) { + foo(...bar); +} + +// deoptimisation + +function foo(a, ...b) { + foo(...b); +} + +function foo(...b) { + foo(1, ...b); +} diff --git a/test/core/fixtures/transformation/es6.parameters.rest/spread-optimisation/expected.js b/test/core/fixtures/transformation/es6.parameters.rest/spread-optimisation/expected.js new file mode 100644 index 0000000000..398292bd9a --- /dev/null +++ b/test/core/fixtures/transformation/es6.parameters.rest/spread-optimisation/expected.js @@ -0,0 +1,25 @@ +// optimisation + +"use strict"; + +function foo() { + foo.apply(undefined, arguments); +} + +// deoptimisation + +function foo(a) { + for (var _len = arguments.length, b = Array(_len > 1 ? _len - 1 : 0), _key = 1; _key < _len; _key++) { + b[_key - 1] = arguments[_key]; + } + + foo.apply(undefined, b); +} + +function foo() { + for (var _len2 = arguments.length, b = Array(_len2), _key2 = 0; _key2 < _len2; _key2++) { + b[_key2] = arguments[_key2]; + } + + foo.apply(undefined, [1].concat(b)); +}