Merge pull request #3375 from jmm/parameters-rest-optimization-errors

Fix errors in parameters rest transformation [T7138]

packages/babel-plugin-transform-es2015-parameters/src/rest.js

@@ -58,33 +58,59 @@ let memberExpressionOptimisationVisitor = {
       state.deopted = true;
     } else {
       let {parentPath} = path;
-      let grandparentPath = parentPath.parentPath;
 
-      // ex: args[0]
-      if (
-        parentPath.isMemberExpression({ computed: true, object: node }) &&
+      // ex: `args[0]`
+      // ex: `args.whatever`
+      if (parentPath.isMemberExpression({ object: node })) {
+        let grandparentPath = parentPath.parentPath;
 
-        // ex: `args[0] = "whatever"`
-        !(
-          grandparentPath.isAssignmentExpression() &&
-          parentPath.node === grandparentPath.node.left
-        )
-      ) {
-        // 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;
-        }
-      }
+        let argsOptEligible = !state.deopted && !(
+          // ex: `args[0] = "whatever"`
+          (
+            grandparentPath.isAssignmentExpression() &&
+            parentPath.node === grandparentPath.node.left
+          ) ||
 
-      // 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: `[args[0]] = ["whatever"]`
+          grandparentPath.isLVal() ||
+
+          // 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;
+          }
         }
       }
 

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;
+}

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;
+}

packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/actual.js

@@ -44,3 +44,51 @@ 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]();
+}
+
+// In addition to swap() above because at some point someone tried checking
+// grandparent path for isArrayExpression() to deopt.
+function arrayDestructure (...rest) {
+  [rest[0]] = x;
+}
+
+function forOf (...rest) {
+  for (rest[0] of this);
+}
+
+function postfixIncrement (...rest) {
+  rest[0]++;
+}
+
+function postfixDecrement (...rest) {
+  rest[0]--;
+}

packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js

@@ -77,3 +77,120 @@ function x() {
 
   rest[0] = 0;
 }
+
+function swap() {
+  for (var _len9 = arguments.length, rest = Array(_len9), _key9 = 0; _key9 < _len9; _key9++) {
+    rest[_key9] = arguments[_key9];
+  }
+
+  var _ref = [rest[1], rest[0]];
+  rest[0] = _ref[0];
+  rest[1] = _ref[1];
+}
+
+function forIn() {
+  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]);
+  }
+}
+
+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]();
+}
+
+// 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];
+}
+
+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;
+      }
+    }
+  }
+}
+
+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]--;
+}