From d6d3eb38887985cdd0e74ce24b1ae59bb4422584 Mon Sep 17 00:00:00 2001 From: Tooru Fujisawa Date: Sat, 22 Nov 2014 11:54:42 +0900 Subject: [PATCH] Bug 1018628 - Part 1: Support default parameter for destructuring. r=jorendorff --- js/src/frontend/BytecodeEmitter.cpp | 146 ++++++++++-------- js/src/frontend/BytecodeEmitter.h | 2 +- js/src/frontend/Parser.cpp | 81 +++++----- .../arguments/defaults-bound-to-function.js | 8 + .../arguments/defaults-destructuring-array.js | 17 ++ ...faults-destructuring-expression-closure.js | 19 +++ ...aults-destructuring-function-expression.js | 9 ++ .../arguments/defaults-destructuring-mixed.js | 29 ++++ .../defaults-destructuring-object.js | 27 ++++ .../defaults-destructuring-with-rest.js | 26 ++++ .../arguments/defaults-invalid-syntax.js | 6 - .../arguments/destructuring-after-defaults.js | 28 ++++ .../arguments/destructuring-with-rest.js | 21 +++ js/src/jsreflect.cpp | 58 +++---- .../js1_8_5/reflect-parse/declarations.js | 6 + 15 files changed, 329 insertions(+), 154 deletions(-) create mode 100644 js/src/jit-test/tests/arguments/defaults-destructuring-array.js create mode 100644 js/src/jit-test/tests/arguments/defaults-destructuring-expression-closure.js create mode 100644 js/src/jit-test/tests/arguments/defaults-destructuring-function-expression.js create mode 100644 js/src/jit-test/tests/arguments/defaults-destructuring-mixed.js create mode 100644 js/src/jit-test/tests/arguments/defaults-destructuring-object.js create mode 100644 js/src/jit-test/tests/arguments/defaults-destructuring-with-rest.js create mode 100644 js/src/jit-test/tests/arguments/destructuring-after-defaults.js create mode 100644 js/src/jit-test/tests/arguments/destructuring-with-rest.js diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 35bd6e2f34e..dc21245dcb8 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -7003,35 +7003,45 @@ BytecodeEmitter::emitUnary(ParseNode* pn) } bool -BytecodeEmitter::emitDefaults(ParseNode* pn) +BytecodeEmitter::emitDefaultsAndDestructuring(ParseNode* pn, ParseNode* pndestruct) { MOZ_ASSERT(pn->isKind(PNK_ARGSBODY)); + uint32_t i = 0; ParseNode* pnlast = pn->last(); + ParseNode* destruct = pndestruct ? pndestruct->pn_head : nullptr; for (ParseNode* arg = pn->pn_head; arg != pnlast; arg = arg->pn_next) { - if (!(arg->pn_dflags & PND_DEFAULT)) - continue; - if (!bindNameToSlot(arg)) - return false; - if (!emitVarOp(arg, JSOP_GETARG)) - return false; - if (!emit1(JSOP_UNDEFINED)) - return false; - if (!emit1(JSOP_STRICTEQ)) - return false; - // Emit source note to enable ion compilation. - if (!newSrcNote(SRC_IF)) - return false; - ptrdiff_t jump; - if (!emitJump(JSOP_IFEQ, 0, &jump)) - return false; - if (!emitTree(arg->expr())) - return false; - if (!emitVarOp(arg, JSOP_SETARG)) - return false; - if (!emit1(JSOP_POP)) - return false; - SET_JUMP_OFFSET(code(jump), offset() - jump); + if (arg->pn_dflags & PND_DEFAULT) { + if (!bindNameToSlot(arg)) + return false; + if (!emitVarOp(arg, JSOP_GETARG)) + return false; + if (!emit1(JSOP_UNDEFINED)) + return false; + if (!emit1(JSOP_STRICTEQ)) + return false; + // Emit source note to enable ion compilation. + if (!newSrcNote(SRC_IF)) + return false; + ptrdiff_t jump; + if (!emitJump(JSOP_IFEQ, 0, &jump)) + return false; + if (!emitTree(arg->expr())) + return false; + if (!emitVarOp(arg, JSOP_SETARG)) + return false; + if (!emit1(JSOP_POP)) + return false; + SET_JUMP_OFFSET(code(jump), offset() - jump); + } + if (destruct && destruct->pn_head->pn_right->frameSlot() == i) { + if (!emitTree(destruct)) + return false; + if (!emit1(JSOP_POP)) + return false; + destruct = destruct->pn_next; + } + ++i; } return true; @@ -7207,62 +7217,62 @@ BytecodeEmitter::emitTree(ParseNode* pn) ParseNode* pnlast = pn->last(); // Carefully emit everything in the right order: - // 1. Destructuring - // 2. Defaults - // 3. Functions + // 1. Defaults and Destructuring for each argument + // 2. Functions ParseNode* pnchild = pnlast->pn_head; - if (pnlast->pn_xflags & PNX_DESTRUCT) { - // Assign the destructuring arguments before defining any functions, - // see bug 419662. + ParseNode* pndestruct = nullptr; + bool hasDestructuring = pnlast->pn_xflags & PNX_DESTRUCT; + if (hasDestructuring) { MOZ_ASSERT(pnchild->isKind(PNK_SEMI)); - MOZ_ASSERT(pnchild->pn_kid->isKind(PNK_VAR) || pnchild->pn_kid->isKind(PNK_GLOBALCONST)); - if (!emitTree(pnchild)) - return false; + MOZ_ASSERT(pnchild->pn_kid->isKind(PNK_SEQ)); +#ifdef DEBUG + for (ParseNode* pnvar = pnchild->pn_kid->pn_head; pnvar; pnvar = pnvar->pn_next) + MOZ_ASSERT(pnvar->isKind(PNK_VAR) || pnvar->isKind(PNK_GLOBALCONST)); +#endif + pndestruct = pnchild->pn_head; pnchild = pnchild->pn_next; } bool hasDefaults = sc->asFunctionBox()->hasDefaults(); - if (hasDefaults) { - ParseNode* rest = nullptr; - bool restIsDefn = false; - if (fun->hasRest()) { - MOZ_ASSERT(!sc->asFunctionBox()->argumentsHasLocalBinding()); + ParseNode* rest = nullptr; + bool restIsDefn = false; + if (fun->hasRest() && hasDefaults) { + MOZ_ASSERT(!sc->asFunctionBox()->argumentsHasLocalBinding()); - // Defaults with a rest parameter need special handling. The - // rest parameter needs to be undefined while defaults are being - // processed. To do this, we create the rest argument and let it - // sit on the stack while processing defaults. The rest - // parameter's slot is set to undefined for the course of - // default processing. - rest = pn->pn_head; - while (rest->pn_next != pnlast) - rest = rest->pn_next; - restIsDefn = rest->isDefn(); - if (!emit1(JSOP_REST)) - return false; - checkTypeSet(JSOP_REST); - - // Only set the rest parameter if it's not aliased by a nested - // function in the body. - if (restIsDefn) { - if (!emit1(JSOP_UNDEFINED)) - return false; - if (!bindNameToSlot(rest)) - return false; - if (!emitVarOp(rest, JSOP_SETARG)) - return false; - if (!emit1(JSOP_POP)) - return false; - } - } - if (!emitDefaults(pn)) + // Defaults with a rest parameter need special handling. The + // rest parameter needs to be undefined while defaults are being + // processed. To do this, we create the rest argument and let it + // sit on the stack while processing defaults. The rest + // parameter's slot is set to undefined for the course of + // default processing. + rest = pn->pn_head; + while (rest->pn_next != pnlast) + rest = rest->pn_next; + restIsDefn = rest->isDefn(); + if (!emit1(JSOP_REST)) return false; - if (fun->hasRest()) { - if (restIsDefn && !emitVarOp(rest, JSOP_SETARG)) + checkTypeSet(JSOP_REST); + + // Only set the rest parameter if it's not aliased by a nested + // function in the body. + if (restIsDefn) { + if (!emit1(JSOP_UNDEFINED)) + return false; + if (!bindNameToSlot(rest)) + return false; + if (!emitVarOp(rest, JSOP_SETARG)) return false; if (!emit1(JSOP_POP)) return false; } } + if (!emitDefaultsAndDestructuring(pn, pndestruct)) + return false; + if (fun->hasRest() && hasDefaults) { + if (restIsDefn && !emitVarOp(rest, JSOP_SETARG)) + return false; + if (!emit1(JSOP_POP)) + return false; + } for (ParseNode* pn2 = pn->pn_head; pn2 != pnlast; pn2 = pn2->pn_next) { // Only bind the parameter if it's not aliased by a nested function // in the body. diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 98e93905bcd..e1fcbbecf38 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -569,7 +569,7 @@ struct BytecodeEmitter bool emitBreak(PropertyName* label); bool emitContinue(PropertyName* label); - bool emitDefaults(ParseNode* pn); + bool emitDefaultsAndDestructuring(ParseNode* pn, ParseNode* pndestruct); bool emitLexicalInitialization(ParseNode* pn, JSOp globalDefOp); bool pushInitialConstants(JSOp op, unsigned n); diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index 1c80fe4aaf7..d2f81d62cc8 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -1650,11 +1650,6 @@ Parser::functionArguments(YieldHandling yieldHandling, FunctionSyn return false; } - if (hasDefaults) { - report(ParseError, false, null(), JSMSG_NONDEFAULT_FORMAL_AFTER_DEFAULT); - return false; - } - funbox->hasDestructuringArgs = true; /* @@ -1682,16 +1677,22 @@ Parser::functionArguments(YieldHandling yieldHandling, FunctionSyn if (!rhs) return false; + handler.addFunctionArgument(funcpn, rhs); if (!pc->define(tokenStream, name, rhs, Definition::ARG)) return false; Node item = handler.newBinary(PNK_ASSIGN, lhs, rhs); if (!item) return false; + + Node vars = handler.newDeclarationList(PNK_VAR, item); + if (!vars) + return false; + if (list) { - handler.addList(list, item); + handler.addList(list, vars); } else { - list = handler.newDeclarationList(PNK_VAR, item); + list = handler.newList(PNK_SEQ, vars); if (!list) return false; *listp = list; @@ -1743,39 +1744,6 @@ Parser::functionArguments(YieldHandling yieldHandling, FunctionSyn RootedPropertyName name(context, tokenStream.currentName()); if (!defineArg(funcpn, name, disallowDuplicateArgs, &duplicatedArg)) return false; - - bool matched; - if (!tokenStream.matchToken(&matched, TOK_ASSIGN)) - return false; - if (matched) { - // A default argument without parentheses would look like: - // a = expr => body, but both operators are right-associative, so - // that would have been parsed as a = (expr => body) instead. - // Therefore it's impossible to get here with parenFreeArrow. - MOZ_ASSERT(!parenFreeArrow); - - if (*hasRest) { - report(ParseError, false, null(), JSMSG_REST_WITH_DEFAULT); - return false; - } - disallowDuplicateArgs = true; - if (duplicatedArg) { - report(ParseError, false, duplicatedArg, JSMSG_BAD_DUP_ARGS); - return false; - } - if (!hasDefaults) { - hasDefaults = true; - - // The Function.length property is the number of formals - // before the first default argument. - funbox->length = pc->numArgs() - 1; - } - Node def_expr = assignExprWithoutYield(yieldHandling, JSMSG_YIELD_IN_DEFAULT); - if (!def_expr) - return false; - handler.setLastFunctionArgumentDefault(funcpn, def_expr); - } - break; } @@ -1784,10 +1752,41 @@ Parser::functionArguments(YieldHandling yieldHandling, FunctionSyn return false; } + bool matched; + if (!tokenStream.matchToken(&matched, TOK_ASSIGN)) + return false; + if (matched) { + // A default argument without parentheses would look like: + // a = expr => body, but both operators are right-associative, so + // that would have been parsed as a = (expr => body) instead. + // Therefore it's impossible to get here with parenFreeArrow. + MOZ_ASSERT(!parenFreeArrow); + + if (*hasRest) { + report(ParseError, false, null(), JSMSG_REST_WITH_DEFAULT); + return false; + } + disallowDuplicateArgs = true; + if (duplicatedArg) { + report(ParseError, false, duplicatedArg, JSMSG_BAD_DUP_ARGS); + return false; + } + if (!hasDefaults) { + hasDefaults = true; + + // The Function.length property is the number of formals + // before the first default argument. + funbox->length = pc->numArgs() - 1; + } + Node def_expr = assignExprWithoutYield(yieldHandling, JSMSG_YIELD_IN_DEFAULT); + if (!def_expr) + return false; + handler.setLastFunctionArgumentDefault(funcpn, def_expr); + } + if (parenFreeArrow || kind == Setter) break; - bool matched; if (!tokenStream.matchToken(&matched, TOK_COMMA)) return false; if (!matched) diff --git a/js/src/jit-test/tests/arguments/defaults-bound-to-function.js b/js/src/jit-test/tests/arguments/defaults-bound-to-function.js index 7a7d71e4a86..899aa3fc2c5 100644 --- a/js/src/jit-test/tests/arguments/defaults-bound-to-function.js +++ b/js/src/jit-test/tests/arguments/defaults-bound-to-function.js @@ -32,3 +32,11 @@ function l(a=8, b=a) { function a() { return 42; } } assertEq(l(), 8); +function m([a, b]=[1, 2], c=a) { + function a() { return 42; } + assertEq(typeof a, "function"); + assertEq(a(), 42); + assertEq(b, 2); + assertEq(c, 1); +} +m(); diff --git a/js/src/jit-test/tests/arguments/defaults-destructuring-array.js b/js/src/jit-test/tests/arguments/defaults-destructuring-array.js new file mode 100644 index 00000000000..e3994a87951 --- /dev/null +++ b/js/src/jit-test/tests/arguments/defaults-destructuring-array.js @@ -0,0 +1,17 @@ +function f1(a, bIs, [b]=[3]) { + assertEq(a, 1); + assertEq(b, bIs); +} +assertEq(f1.length, 2); +f1(1, 3); +f1(1, 42, [42]); +f1(1, 3, undefined); + +function f2(a, bIs, [b]=[]) { + assertEq(a, 1); + assertEq(b, bIs); +} +assertEq(f2.length, 2); +f2(1, undefined); +f2(1, 42, [42]); +f2(1, undefined, undefined); diff --git a/js/src/jit-test/tests/arguments/defaults-destructuring-expression-closure.js b/js/src/jit-test/tests/arguments/defaults-destructuring-expression-closure.js new file mode 100644 index 00000000000..018da227380 --- /dev/null +++ b/js/src/jit-test/tests/arguments/defaults-destructuring-expression-closure.js @@ -0,0 +1,19 @@ +function f1(a, bIs, cIs, dIs, {b}={b: 3}, c=4, [d]=[5]) ( + assertEq(a, 1), + assertEq(b, bIs), + assertEq(c, cIs), + assertEq(d, dIs) +); +assertEq(f1.length, 4); +f1(1, 3, 4, 5); +f1(1, 42, 43, 44, {b: 42}, 43, [44]); + +let f2 = (a, bIs, cIs, dIs, {b}={b: 3}, c=4, [d]=[5]) => ( + assertEq(a, 1), + assertEq(b, bIs), + assertEq(c, cIs), + assertEq(d, dIs) +); +assertEq(f2.length, 4); +f2(1, 3, 4, 5); +f2(1, 42, 43, 44, {b: 42}, 43, [44]); diff --git a/js/src/jit-test/tests/arguments/defaults-destructuring-function-expression.js b/js/src/jit-test/tests/arguments/defaults-destructuring-function-expression.js new file mode 100644 index 00000000000..9517a5c4d49 --- /dev/null +++ b/js/src/jit-test/tests/arguments/defaults-destructuring-function-expression.js @@ -0,0 +1,9 @@ +let f = function(a, bIs, cIs, dIs, {b}={b: 3}, c=4, [d]=[5]) { + assertEq(a, 1); + assertEq(b, bIs); + assertEq(c, cIs); + assertEq(d, dIs); +}; +assertEq(f.length, 4); +f(1, 3, 4, 5); +f(1, 42, 43, 44, {b: 42}, 43, [44]); diff --git a/js/src/jit-test/tests/arguments/defaults-destructuring-mixed.js b/js/src/jit-test/tests/arguments/defaults-destructuring-mixed.js new file mode 100644 index 00000000000..74cb00f1f26 --- /dev/null +++ b/js/src/jit-test/tests/arguments/defaults-destructuring-mixed.js @@ -0,0 +1,29 @@ +function f1(a, bIs, cIs, dIs, b=3, {c}={c: 4}, [d]=[5]) { + assertEq(a, 1); + assertEq(b, bIs); + assertEq(c, cIs); + assertEq(d, dIs); +} +assertEq(f1.length, 4); +f1(1, 3, 4, 5); +f1(1, 42, 4, 5, 42); +f1(1, 42, 43, 5, 42, {c: 43}); +f1(1, 42, 43, 44, 42, {c: 43}, [44]); +f1(1, 3, 4, 5, undefined); +f1(1, 42, 4, 5, 42, undefined); +f1(1, 3, 42, 5, undefined, {c: 42}); +f1(1, 3, 4, 42, undefined, undefined, [42]); + +function f2(a, bIs, cIs, dIs, eIs, {b}={b: 3}, [c]=[b], d=c, {ee: e}={ee: d}) { + assertEq(a, 1); + assertEq(b, bIs); + assertEq(c, cIs); + assertEq(d, dIs); + assertEq(e, eIs); +} +assertEq(f2.length, 5); +f2(1, 3, 3, 3, 3); +f2(1, 42, 42, 42, 42, {b: 42}); +f2(1, 42, 43, 43, 43, {b: 42}, [43]); +f2(1, 42, 43, 44, 44, {b: 42}, [43], 44); +f2(1, 42, 43, 44, 45, {b: 42}, [43], 44, {ee: 45}); diff --git a/js/src/jit-test/tests/arguments/defaults-destructuring-object.js b/js/src/jit-test/tests/arguments/defaults-destructuring-object.js new file mode 100644 index 00000000000..0ca970a1447 --- /dev/null +++ b/js/src/jit-test/tests/arguments/defaults-destructuring-object.js @@ -0,0 +1,27 @@ +function f1(a, bIs, cIs, {b}={b: 3}, {cc: c}={cc: 4}) { + assertEq(a, 1); + assertEq(b, bIs); + assertEq(c, cIs); +} +assertEq(f1.length, 3); +f1(1, 3, 4); +f1(1, 42, 4, {b: 42}); +f1(1, 42, 4, {b: 42}, undefined); +f1(1, 42, 43, {b: 42}, {cc: 43}); +f1(1, 3, 4, undefined); +f1(1, 3, 4, undefined, undefined); +f1(1, 3, 43, undefined, {cc: 43}); + +function f2(a, bIs, cIs, {b}={}, {cc: c}={}) { + assertEq(a, 1); + assertEq(b, bIs); + assertEq(c, cIs); +} +assertEq(f2.length, 3); +f2(1, undefined, undefined); +f2(1, 42, undefined, {b: 42}); +f2(1, 42, undefined, {b: 42}, undefined); +f2(1, 42, 43, {b: 42}, {cc: 43}); +f2(1, undefined, undefined, undefined); +f2(1, undefined, undefined, undefined, undefined); +f2(1, undefined, 43, undefined, {cc: 43}); diff --git a/js/src/jit-test/tests/arguments/defaults-destructuring-with-rest.js b/js/src/jit-test/tests/arguments/defaults-destructuring-with-rest.js new file mode 100644 index 00000000000..0f35d6ba30b --- /dev/null +++ b/js/src/jit-test/tests/arguments/defaults-destructuring-with-rest.js @@ -0,0 +1,26 @@ +load(libdir + "asserts.js"); +load(libdir + "eqArrayHelper.js"); + +function f1(a, bIs, [b]=[3], ...rest) { + assertEq(a, 1); + assertEq(bIs, b); + assertEqArray(rest, []); +} +assertEq(f1.length, 2); +f1(1, 3); +f1(1, 42, [42]); + +function f2([a]=[rest], ...rest) { + assertEq(a, undefined); +} +f2(); + +function f3([a]=[rest], ...rest) { + assertEq(a, 1); + assertEqArray(rest, [2, 3, 4]); +} +f3([1], 2, 3, 4); + +function f4([a]=rest, ...rest) { +} +assertThrowsInstanceOf(f4, TypeError); diff --git a/js/src/jit-test/tests/arguments/defaults-invalid-syntax.js b/js/src/jit-test/tests/arguments/defaults-invalid-syntax.js index 4ee54796de0..6acf0955c11 100644 --- a/js/src/jit-test/tests/arguments/defaults-invalid-syntax.js +++ b/js/src/jit-test/tests/arguments/defaults-invalid-syntax.js @@ -3,12 +3,6 @@ load(libdir + "asserts.js"); assertThrowsInstanceOf(function () { eval("function f(...rest=23) {}"); }, SyntaxError); -assertThrowsInstanceOf(function () { - eval("function f([a]=4) {}"); -}, SyntaxError); -assertThrowsInstanceOf(function () { - eval("function f(a=4, [b]) {}"); -}, SyntaxError); assertThrowsInstanceOf(function () { eval("function f(a=yield 24) {}"); }, SyntaxError); diff --git a/js/src/jit-test/tests/arguments/destructuring-after-defaults.js b/js/src/jit-test/tests/arguments/destructuring-after-defaults.js new file mode 100644 index 00000000000..9cc31bf7610 --- /dev/null +++ b/js/src/jit-test/tests/arguments/destructuring-after-defaults.js @@ -0,0 +1,28 @@ +load(libdir + "asserts.js"); + +function f1(a, bIs, cIs, dIs, b=1, [c], {d}) { + assertEq(a, 1); + assertEq(b, bIs); + assertEq(c, cIs); + assertEq(d, dIs); +} +assertEq(f1.length, 4); +f1(1, 1, 42, 43, undefined, [42], {d: 43}); +f1(1, 42, 43, 44, 42, [43], {d: 44}); +assertThrowsInstanceOf(function () { + f1(1, 1, 1, 1); +}, TypeError); + +function f2(a=(assertEq(a, undefined), assertEq(b, undefined), + assertEq(c, undefined), assertEq(d, undefined), 1), + [b], + c=(assertEq(a, 1), assertEq(b, 42), + assertEq(c, undefined), assertEq(d, undefined), 2), + {d}) { + assertEq(a, 1); + assertEq(b, 42); + assertEq(c, 2); + assertEq(d, 43); +} +assertEq(f2.length, 0); +f2(undefined, [42], undefined, {d: 43}); diff --git a/js/src/jit-test/tests/arguments/destructuring-with-rest.js b/js/src/jit-test/tests/arguments/destructuring-with-rest.js new file mode 100644 index 00000000000..81e69ce54df --- /dev/null +++ b/js/src/jit-test/tests/arguments/destructuring-with-rest.js @@ -0,0 +1,21 @@ +load(libdir + "eqArrayHelper.js"); + +function f1(a, bIs, [b], ...rest) { + assertEq(a, 1); + assertEq(bIs, b); + assertEqArray(rest, []); +} +assertEq(f1.length, 3); +f1(1, 3, [3]); +f1(1, 42, [42]); + +function f2([a], ...rest) { + assertEq(a, undefined); +} +f2([]); + +function f3([a], ...rest) { + assertEq(a, 1); + assertEqArray(rest, [2, 3, 4]); +} +f3([1], 2, 3, 4); diff --git a/js/src/jsreflect.cpp b/js/src/jsreflect.cpp index 6004de8628b..910bac9af30 100644 --- a/js/src/jsreflect.cpp +++ b/js/src/jsreflect.cpp @@ -3443,7 +3443,7 @@ ASTSerializer::functionArgsAndBody(ParseNode* pn, NodeVector& args, NodeVector& pndestruct = head->pn_kid; LOCAL_ASSERT(pndestruct); - LOCAL_ASSERT(pndestruct->isKind(PNK_VAR)); + LOCAL_ASSERT(pndestruct->isKind(PNK_SEQ)); } else { pndestruct = nullptr; } @@ -3489,8 +3489,10 @@ ASTSerializer::functionArgs(ParseNode* pn, ParseNode* pnargs, ParseNode* pndestr ParseNode* pnbody, NodeVector& args, NodeVector& defaults, MutableHandleValue rest) { + if (!pnargs) + return true; + uint32_t i = 0; - ParseNode* arg = pnargs ? pnargs->pn_head : nullptr; ParseNode* destruct = pndestruct ? pndestruct->pn_head : nullptr; RootedValue node(cx); bool defaultsNull = true; @@ -3502,51 +3504,31 @@ ASTSerializer::functionArgs(ParseNode* pn, ParseNode* pnargs, ParseNode* pndestr * Arguments are found in potentially two different places: 1) the * argsbody sequence (which ends with the body node), or 2) a * destructuring initialization at the beginning of the body. Loop - * |arg| through the argsbody and |destruct| through the initial - * destructuring assignments, stopping only when we've exhausted - * both. + * |arg| through the argsbody, and use |destruct| if it has same index. */ - while ((arg && arg != pnbody) || destruct) { - if (destruct && destruct->pn_right->frameSlot() == i) { - if (!pattern(destruct->pn_left, &node) || - !args.append(node) || !defaults.append(NullValue())) - { + for (ParseNode* arg = pnargs->pn_head; arg && arg != pnbody; arg = arg->pn_next) { + MOZ_ASSERT(arg->isKind(PNK_NAME)); + if (destruct && destruct->pn_head->pn_right->frameSlot() == i) { + if (!pattern(destruct->pn_head->pn_left, &node) || !args.append(node)) return false; - } destruct = destruct->pn_next; - } else if (arg && arg != pnbody) { - /* - * We don't check that arg->frameSlot() == i since we - * can't call that method if the arg def has been turned - * into a use, e.g.: - * - * function(a) { function a() { } } - * - * There's no other way to ask a non-destructuring arg its - * index in the formals list, so we rely on the ability to - * ask destructuring args their index above. - */ - MOZ_ASSERT(arg->isKind(PNK_NAME) || arg->isKind(PNK_ASSIGN)); - ParseNode* argName = arg->isKind(PNK_NAME) ? arg : arg->pn_left; - if (!identifier(argName, &node)) + } else { + if (!identifier(arg, &node)) return false; if (rest.isUndefined() && arg->pn_next == pnbody) rest.setObject(node.toObject()); else if (!args.append(node)) return false; - if (arg->pn_dflags & PND_DEFAULT) { - defaultsNull = false; - ParseNode* expr = arg->expr(); - RootedValue def(cx); - if (!expression(expr, &def) || !defaults.append(def)) - return false; - } else { - if (!defaults.append(NullValue())) - return false; - } - arg = arg->pn_next; + } + if (arg->pn_dflags & PND_DEFAULT) { + defaultsNull = false; + ParseNode* expr = arg->expr(); + RootedValue def(cx); + if (!expression(expr, &def) || !defaults.append(def)) + return false; } else { - LOCAL_NOT_REACHED("missing function argument"); + if (!defaults.append(NullValue())) + return false; } ++i; } diff --git a/js/src/tests/js1_8_5/reflect-parse/declarations.js b/js/src/tests/js1_8_5/reflect-parse/declarations.js index 9e875d4eeef..86f524fd394 100644 --- a/js/src/tests/js1_8_5/reflect-parse/declarations.js +++ b/js/src/tests/js1_8_5/reflect-parse/declarations.js @@ -32,6 +32,12 @@ assertDecl("function foo(a=(function () {})) { function a() {} }", funDecl(ident("foo"), [ident("a")], blockStmt([funDecl(ident("a"), [], blockStmt([]))]), [funExpr(null, [], blockStmt([]))])); +// Bug 1018628: default paremeter for destructuring +assertDecl("function f(a=1, [x,y]=[2,3]) { }", + funDecl(ident("f"), + [ident("a"), arrPatt([ident("x"), ident("y")])], + blockStmt([]), + [lit(1), arrExpr([lit(2), lit(3)])])); // Bug 591437: rebound args have their defs turned into uses assertDecl("function f(a) { function a() { } }",