Skip to content

Commit

Permalink
Fix error in rest parameter length optimization (#3573)
Browse files Browse the repository at this point in the history
If there aren鈥檛 enough arguments to get to the offset index, we would
return an negative length.
  • Loading branch information
jridgewell authored and hzoo committed Jul 13, 2016
1 parent 57ef3ea commit 823ffbd
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 20 deletions.
27 changes: 12 additions & 15 deletions packages/babel-plugin-transform-es2015-parameters/src/rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@ let buildRest = template(`
}
`);

let loadRest = template(`
let restIndex = template(`
ARGUMENTS.length <= INDEX ? undefined : ARGUMENTS[INDEX]
`);

let restLength = template(`
ARGUMENTS.length <= OFFSET ? 0 : ARGUMENTS.length - OFFSET
`);

let memberExpressionOptimisationVisitor = {
Scope(path, state) {
// check if this scope has a local binding that will shadow the rest parameter
Expand Down Expand Up @@ -155,21 +159,18 @@ function optimiseIndexGetter(path, argsId, offset) {
index = t.binaryExpression("+", path.parent.property, t.numericLiteral(offset));
}

path.parentPath.replaceWith(loadRest({
path.parentPath.replaceWith(restIndex({
ARGUMENTS: argsId,
INDEX: index,
}));
}

function optimiseLengthGetter(path, argsLengthExpression, argsId, offset) {
function optimiseLengthGetter(path, argsId, offset) {
if (offset) {
path.parentPath.replaceWith(
t.binaryExpression(
"-",
argsLengthExpression,
t.numericLiteral(offset),
)
);
path.parentPath.replaceWith(restLength({
ARGUMENTS: argsId,
OFFSET: t.numericLiteral(offset),
}));
} else {
path.replaceWith(argsId);
}
Expand All @@ -183,10 +184,6 @@ export let visitor = {
let rest = node.params.pop().argument;

let argsId = t.identifier("arguments");
let argsLengthExpression = t.memberExpression(
argsId,
t.identifier("length"),
);

// otherwise `arguments` will be remapped in arrow functions
argsId._shadowedFunctionLiteral = path;
Expand Down Expand Up @@ -230,7 +227,7 @@ export let visitor = {
optimiseIndexGetter(path, argsId, state.offset);
break;
case "lengthGetter":
optimiseLengthGetter(path, argsLengthExpression, argsId, state.offset);
optimiseLengthGetter(path, argsId, state.offset);
break;
default:
path.replaceWith(argsId);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
var length = function (a, b, ...items) {
return items.length;
};

assert.equal(length(), 0);
assert.equal(length(1), 0);
assert.equal(length(1, 2), 0);
assert.equal(length(1, 2, 3), 1);
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
var t = function (f) {
arguments.length <= 1 ? undefined : arguments[1];
arguments.length <= arguments.length - 1 - 1 + 1 ? undefined : arguments[arguments.length - 1 - 1 + 1];
arguments.length <= (arguments.length <= 1 ? 0 : arguments.length - 1) - 1 + 1 ? undefined : arguments[(arguments.length <= 1 ? 0 : arguments.length - 1) - 1 + 1];
};

function t(f) {
Expand All @@ -11,4 +11,4 @@ function t(f) {
items;
items[0];
items[items.length - 1];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ var b = function () {
};

var b = function (foo) {
return (arguments.length - 1) * 2;
return (arguments.length <= 1 ? 0 : arguments.length - 1) * 2;
};

var b = function (foo, baz) {
return arguments.length - 2;
return arguments.length <= 2 ? 0 : arguments.length - 2;
};

function x() {
Expand Down Expand Up @@ -193,4 +193,4 @@ function postfixDecrement() {
}

rest[0]--;
}
}

0 comments on commit 823ffbd

Please sign in to comment.