From a82bf11e2af4406918f2c8d451642a96f5fe9147 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 2 Feb 2016 16:20:27 -0500 Subject: [PATCH 1/8] Add rest deoptimization fixtures. (Failing.) Commit message edited by @jmm. --- .../actual.js | 30 +++++++++++++++++++ .../expected.js | 18 +++++++++++ 2 files changed, 48 insertions(+) diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js index fb6ba1b013..d641bdb1a1 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js @@ -44,3 +44,33 @@ var b = function (foo, baz, ...bar) { function x (...rest) { rest[0] = 0; } + +function swap (...rest) { + [rest[0], rest[1]] = [rest[1], rest[0]]; +} + +function forIn (...rest) { + for (rest[0] in this) { + foo(rest[0]); + } +} + +function inc (...rest) { + ++rest[0]; +} + +function dec (...rest) { + --rest[0]; +} + +function del (...rest) { + delete rest[0]; +} + +function method (...rest) { + rest[0](); +} + +function newExp (...rest) { + new rest[0](); +} diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js index d23fd1278f..caa3943beb 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js @@ -77,3 +77,21 @@ function x() { rest[0] = 0; } + +function swap() { + for (var _len9 = arguments.length, rest = Array(_len9), _key9 = 0; _key9 < _len9; _key9++) { + rest[_key9] = arguments[_key9]; + } + + [rest[0], rest[1]] = [rest[1], rest[0]]; +} + +function x() { + for (var _len10 = arguments.length, rest = Array(_len10), _key10 = 0; _key10 < _len10; _key10++) { + rest[_key10] = arguments[_key10]; + } + + for (rest[0] in this) { + foo(rest[0]); + } +} From 0be3a58dd226556a9d9b8e78235b3ae59cdee092 Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Wed, 10 Feb 2016 17:15:42 -0500 Subject: [PATCH 2/8] Add expected fixtures for new actuals. (Failing.) --- .../expected.js | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js index caa3943beb..f1f31ed400 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js @@ -95,3 +95,43 @@ function x() { foo(rest[0]); } } + +function inc() { + for (var _len11 = arguments.length, rest = Array(_len11), _key11 = 0; _key11 < _len11; _key11++) { + rest[_key11] = arguments[_key11]; + } + + ++rest[0]; +} + +function dec() { + for (var _len12 = arguments.length, rest = Array(_len12), _key12 = 0; _key12 < _len12; _key12++) { + rest[_key12] = arguments[_key12]; + } + + --rest[0]; +} + +function del() { + for (var _len13 = arguments.length, rest = Array(_len13), _key13 = 0; _key13 < _len13; _key13++) { + rest[_key13] = arguments[_key13]; + } + + delete rest[0]; +} + +function method() { + for (var _len14 = arguments.length, rest = Array(_len14), _key14 = 0; _key14 < _len14; _key14++) { + rest[_key14] = arguments[_key14]; + } + + rest[0](); +} + +function newExp() { + for (var _len15 = arguments.length, rest = Array(_len15), _key15 = 0; _key15 < _len15; _key15++) { + rest[_key15] = arguments[_key15]; + } + + new rest[0](); +} From 6ca07974c92fa7f63a19736122921c5b31d15816 Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Fri, 19 Feb 2016 09:18:40 -0500 Subject: [PATCH 3/8] Add array destruct w/o `[rest[0]]` on RHS fixture. (Failing.) --- .../rest-member-expression-deoptimisation/actual.js | 6 ++++++ .../expected.js | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js index d641bdb1a1..e2dd499687 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js @@ -74,3 +74,9 @@ function method (...rest) { function newExp (...rest) { new rest[0](); } + +// In addition to swap() above because at some point someone tried checking +// grandparent path for isArrayExpression() to deopt. +function arrayDestructure (...rest) { + [rest[0]] = x; +} diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js index f1f31ed400..f8a3e5d0eb 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js @@ -135,3 +135,15 @@ function newExp() { new rest[0](); } + +// In addition to swap() above because at some point someone tried checking +// grandparent path for isArrayExpression() to deopt. +function arrayDestructure() { + for (var _len16 = arguments.length, rest = Array(_len16), _key16 = 0; _key16 < _len16; _key16++) { + rest[_key16] = arguments[_key16]; + } + + var _x = babelHelpers.slicedToArray(x, 1); + + rest[0] = _x[0]; +} From 183fbab967cce7d6fc7ad61c26673928caef9322 Mon Sep 17 00:00:00 2001 From: Victor Felder Date: Thu, 11 Feb 2016 00:08:56 +0100 Subject: [PATCH 4/8] Fix some rest optimization errors. Commit message edited by @jmm. --- .../babel-plugin-transform-es2015-parameters/src/rest.js | 9 ++++++++- .../rest-member-expression-deoptimisation/expected.js | 6 ++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/src/rest.js b/packages/babel-plugin-transform-es2015-parameters/src/rest.js index bda2cb0557..265996da7d 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/rest.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/rest.js @@ -60,6 +60,12 @@ let memberExpressionOptimisationVisitor = { let {parentPath} = path; let grandparentPath = parentPath.parentPath; + // ex: [rest[0]] = [rest[1]] + if (grandparentPath.isLVal()) { + state.deopted = true; + return; + } + // ex: args[0] if ( parentPath.isMemberExpression({ computed: true, object: node }) && @@ -68,7 +74,8 @@ let memberExpressionOptimisationVisitor = { !( grandparentPath.isAssignmentExpression() && parentPath.node === grandparentPath.node.left - ) + ) && + !grandparentPath.isForInStatement() ) { // if we know that this member expression is referencing a number then // we can safely optimise it diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js index f8a3e5d0eb..2354ea3aee 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js @@ -83,10 +83,12 @@ function swap() { rest[_key9] = arguments[_key9]; } - [rest[0], rest[1]] = [rest[1], rest[0]]; + var _ref = [rest[1], rest[0]]; + rest[0] = _ref[0]; + rest[1] = _ref[1]; } -function x() { +function forIn() { for (var _len10 = arguments.length, rest = Array(_len10), _key10 = 0; _key10 < _len10; _key10++) { rest[_key10] = arguments[_key10]; } From 1c304965e7f5e4049ed60ea72469061ab59edccf Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Mon, 22 Feb 2016 18:13:18 -0500 Subject: [PATCH 5/8] Add rest loop position optimization fixture. (Failing.) With destructuring assignment to an element. This makes the function ineligible for `arguments` optimization, while remaining eligible for loop position optimization. --- .../actual.js | 8 +++++++- .../expected.js | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-deepest-common-ancestor-earliest-child/actual.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-deepest-common-ancestor-earliest-child/actual.js index 3f3606cb0a..646ce0290e 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-deepest-common-ancestor-earliest-child/actual.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-deepest-common-ancestor-earliest-child/actual.js @@ -1,4 +1,4 @@ -// single referenes +// single reference function r(...rest){ if (noNeedToWork) return 0; return rest; @@ -66,3 +66,9 @@ function runQueue(queue, ...args) { } } } + +function r(...rest){ + if (noNeedToWork) return 0; + [rest[0]] = x; + return rest; +} diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-deepest-common-ancestor-earliest-child/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-deepest-common-ancestor-earliest-child/expected.js index 320886f014..3876f086d8 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-deepest-common-ancestor-earliest-child/expected.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-deepest-common-ancestor-earliest-child/expected.js @@ -1,4 +1,4 @@ -// single referenes +// single reference function r() { if (noNeedToWork) return 0; @@ -99,3 +99,19 @@ function runQueue(queue) { } } } + +function r() { + if (noNeedToWork) return 0; + + for (var _len9 = arguments.length, rest = Array(_len9), _key9 = 0; _key9 < _len9; _key9++) { + rest[_key9] = arguments[_key9]; + } + + var _x = x; + + var _x2 = babelHelpers.slicedToArray(_x, 1); + + rest[0] = _x2[0]; + + return rest; +} From 5f98d22b6d7ea83578b54d3e262d9ea0749ed9fa Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Tue, 16 Feb 2016 19:09:05 -0500 Subject: [PATCH 6/8] Add for-of fixture. (Failing.) --- .../actual.js | 4 +++ .../expected.js | 29 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js index e2dd499687..96ad5983f7 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js @@ -80,3 +80,7 @@ function newExp (...rest) { function arrayDestructure (...rest) { [rest[0]] = x; } + +function forOf (...rest) { + for (rest[0] of this); +} diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js index 2354ea3aee..9f3765fe91 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js @@ -149,3 +149,32 @@ function arrayDestructure() { rest[0] = _x[0]; } + +function forOf() { + for (var _len17 = arguments.length, rest = Array(_len17), _key17 = 0; _key17 < _len17; _key17++) { + rest[_key17] = arguments[_key17]; + } + + var _iteratorNormalCompletion = true; + var _didIteratorError = false; + var _iteratorError = undefined; + + try { + for (var _iterator = this[Symbol.iterator](), _step; !(_iteratorNormalCompletion = (_step = _iterator.next()).done); _iteratorNormalCompletion = true) { + rest[0] = _step.value; + } + } catch (err) { + _didIteratorError = true; + _iteratorError = err; + } finally { + try { + if (!_iteratorNormalCompletion && _iterator.return) { + _iterator.return(); + } + } finally { + if (_didIteratorError) { + throw _iteratorError; + } + } + } +} From 8419be1afcb34662ae9bff612c63b54b68de1296 Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Tue, 23 Feb 2016 08:55:59 -0500 Subject: [PATCH 7/8] Add postfix update expression fixtures. (Failing.) --- .../actual.js | 8 ++++++++ .../expected.js | 16 ++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js index 96ad5983f7..a974c8d1d2 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js @@ -84,3 +84,11 @@ function arrayDestructure (...rest) { function forOf (...rest) { for (rest[0] of this); } + +function postfixIncrement (...rest) { + rest[0]++; +} + +function postfixDecrement (...rest) { + rest[0]--; +} diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js index 9f3765fe91..7a07b92415 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js @@ -178,3 +178,19 @@ function forOf() { } } } + +function postfixIncrement() { + for (var _len18 = arguments.length, rest = Array(_len18), _key18 = 0; _key18 < _len18; _key18++) { + rest[_key18] = arguments[_key18]; + } + + rest[0]++; +} + +function postfixDecrement() { + for (var _len19 = arguments.length, rest = Array(_len19), _key19 = 0; _key19 < _len19; _key19++) { + rest[_key19] = arguments[_key19]; + } + + rest[0]--; +} From 49e30f49bcaaa3490bc346270ce5b97c136b11ad Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Thu, 25 Feb 2016 18:42:52 -0500 Subject: [PATCH 8/8] Fix rest optimization errors. --- .../src/rest.js | 79 ++++++++++++------- 1 file changed, 49 insertions(+), 30 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/src/rest.js b/packages/babel-plugin-transform-es2015-parameters/src/rest.js index 265996da7d..5522c36310 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/rest.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/rest.js @@ -58,40 +58,59 @@ let memberExpressionOptimisationVisitor = { state.deopted = true; } else { let {parentPath} = path; - let grandparentPath = parentPath.parentPath; - // ex: [rest[0]] = [rest[1]] - if (grandparentPath.isLVal()) { - state.deopted = true; - return; - } + // ex: `args[0]` + // ex: `args.whatever` + if (parentPath.isMemberExpression({ object: node })) { + let grandparentPath = parentPath.parentPath; - // ex: args[0] - if ( - parentPath.isMemberExpression({ computed: true, object: node }) && + let argsOptEligible = !state.deopted && !( + // ex: `args[0] = "whatever"` + ( + grandparentPath.isAssignmentExpression() && + parentPath.node === grandparentPath.node.left + ) || - // ex: `args[0] = "whatever"` - !( - grandparentPath.isAssignmentExpression() && - parentPath.node === grandparentPath.node.left - ) && - !grandparentPath.isForInStatement() - ) { - // if we know that this member expression is referencing a number then - // we can safely optimise it - let prop = parentPath.get("property"); - if (prop.isBaseType("number")) { - state.candidates.push({cause: "indexGetter", path}); - return; - } - } + // ex: `[args[0]] = ["whatever"]` + grandparentPath.isLVal() || - // ex: args.length - if (parentPath.isMemberExpression({ computed: false, object: node })) { - let prop = parentPath.get("property"); - if (prop.node.name === "length") { - state.candidates.push({cause: "lengthGetter", path}); - return; + // ex: `for (rest[0] in this)` + // ex: `for (rest[0] of this)` + grandparentPath.isForXStatement() || + + // ex: `++args[0]` + // ex: `args[0]--` + grandparentPath.isUpdateExpression() || + + // ex: `delete args[0]` + grandparentPath.isUnaryExpression({ operator: "delete" }) || + + // ex: `args[0]()` + // ex: `new args[0]()` + // ex: `new args[0]` + ( + ( + grandparentPath.isCallExpression() || + grandparentPath.isNewExpression() + ) && + parentPath.node === grandparentPath.node.callee + ) + ); + + if (argsOptEligible) { + if (parentPath.node.computed) { + // if we know that this member expression is referencing a number then + // we can safely optimise it + if (parentPath.get("property").isBaseType("number")) { + state.candidates.push({cause: "indexGetter", path}); + return; + } + } + // args.length + else if (parentPath.node.property.name === "length") { + state.candidates.push({cause: "lengthGetter", path}); + return; + } } }