Skip to content

Commit 293a30b

Browse files
Kevin SmithKevin Smith
Kevin Smith
authored and
Kevin Smith
committed
Fix patterns in rest formal parameters.
Currently, when a pattern is present as a rest parameter the parser incorrectly interprets the pattern as a non-rest parameter. This change allows patterns to appear in the AST for rest parameters. Fixes #5550 Fixes #5551
1 parent bcb97d1 commit 293a30b

File tree

7 files changed

+321
-42
lines changed

7 files changed

+321
-42
lines changed

‎lib/Parser/FormalsUtil.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,5 +38,10 @@ void MapFormalsFromPattern(ParseNodeFnc *pnodeFunc, Fn fn)
3838
Parser::MapBindIdentifier(pnode->AsParseNodeParamPattern()->pnode1, fn);
3939
}
4040
}
41-
}
4241

42+
ParseNodePtr rest = pnodeFunc->pnodeRest;
43+
if (rest != nullptr && rest->nop == knopParamPattern)
44+
{
45+
Parser::MapBindIdentifier(rest->AsParseNodeParamPattern()->pnode1, fn);
46+
}
47+
}

‎lib/Parser/Parse.cpp

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6797,12 +6797,20 @@ void Parser::ParseFncFormals(ParseNodeFnc * pnodeFnc, ParseNodeFnc * pnodeParent
67976797
for (Js::ArgSlot argPos = 0; ; ++argPos)
67986798
{
67996799
bool isBindingPattern = false;
6800+
68006801
if (m_scriptContext->GetConfig()->IsES6RestEnabled() && m_token.tk == tkEllipsis)
68016802
{
6803+
if (flags & fFncOneArg)
6804+
{
6805+
// The parameter of a setter cannot be a rest parameter.
6806+
Error(ERRUnexpectedEllipsis);
6807+
}
6808+
68026809
// Possible rest parameter
68036810
this->GetScanner()->Scan();
68046811
seenRestParameter = true;
68056812
}
6813+
68066814
if (m_token.tk != tkID)
68076815
{
68086816
if (IsES6DestructuringEnabled() && IsPossiblePatternStart())
@@ -6855,10 +6863,20 @@ void Parser::ParseFncFormals(ParseNodeFnc * pnodeFnc, ParseNodeFnc * pnodeParent
68556863
Assert(pnodePattern);
68566864
paramPattern = CreateParamPatternNode(pnodePattern);
68576865
}
6858-
// Linking the current formal parameter (which is pattern parameter) with other formals.
6859-
*m_ppnodeVar = paramPattern;
6860-
paramPattern->pnodeNext = nullptr;
6861-
m_ppnodeVar = &paramPattern->pnodeNext;
6866+
6867+
if (seenRestParameter)
6868+
{
6869+
Assert(pnodeFnc->pnodeRest == nullptr);
6870+
pnodeFnc->pnodeRest = paramPattern;
6871+
}
6872+
else
6873+
{
6874+
// Linking the current formal parameter (which is pattern parameter)
6875+
// with other formals.
6876+
*m_ppnodeVar = paramPattern;
6877+
paramPattern->pnodeNext = nullptr;
6878+
m_ppnodeVar = &paramPattern->pnodeNext;
6879+
}
68626880
}
68636881

68646882
isBindingPattern = true;
@@ -6880,22 +6898,21 @@ void Parser::ParseFncFormals(ParseNodeFnc * pnodeFnc, ParseNodeFnc * pnodeParent
68806898
if (seenRestParameter)
68816899
{
68826900
this->GetCurrentFunctionNode()->SetHasNonSimpleParameterList();
6883-
if (flags & fFncOneArg)
6884-
{
6885-
// The parameter of a setter cannot be a rest parameter.
6886-
Error(ERRUnexpectedEllipsis);
6887-
}
68886901
pnodeT = CreateDeclNode(knopVarDecl, pid, STFormal, false);
68896902
pnodeT->sym->SetIsNonSimpleParameter(true);
68906903
if (buildAST)
68916904
{
68926905
// When only validating formals, we won't have a function node.
6906+
Assert(pnodeFnc->pnodeRest == nullptr);
68936907
pnodeFnc->pnodeRest = pnodeT;
68946908
if (!isNonSimpleParameterList)
68956909
{
68966910
// This is the first non-simple parameter we've seen. We need to go back
68976911
// and set the Symbols of all previous parameters.
6898-
MapFormalsWithoutRest(m_currentNodeFunc, [&](ParseNodePtr pnodeArg) { pnodeArg->AsParseNodeVar()->sym->SetIsNonSimpleParameter(true); });
6912+
MapFormalsWithoutRest(m_currentNodeFunc, [](ParseNodePtr pnodeArg)
6913+
{
6914+
pnodeArg->AsParseNodeVar()->sym->SetIsNonSimpleParameter(true);
6915+
});
68996916
}
69006917
}
69016918

@@ -6923,11 +6940,6 @@ void Parser::ParseFncFormals(ParseNodeFnc * pnodeFnc, ParseNodeFnc * pnodeParent
69236940

69246941
this->GetScanner()->Scan();
69256942

6926-
if (seenRestParameter && m_token.tk != tkRParen && m_token.tk != tkAsg)
6927-
{
6928-
Error(ERRRestLastArg);
6929-
}
6930-
69316943
if (m_token.tk == tkAsg && m_scriptContext->GetConfig()->IsES6DefaultArgsEnabled())
69326944
{
69336945
if (seenRestParameter && m_scriptContext->GetConfig()->IsES6RestEnabled())
@@ -7018,6 +7030,11 @@ void Parser::ParseFncFormals(ParseNodeFnc * pnodeFnc, ParseNodeFnc * pnodeParent
70187030

70197031
this->GetScanner()->Scan();
70207032

7033+
if (seenRestParameter)
7034+
{
7035+
Error(ERRRestLastArg);
7036+
}
7037+
70217038
if (m_token.tk == tkRParen && m_scriptContext->GetConfig()->IsES7TrailingCommaEnabled())
70227039
{
70237040
break;

‎lib/Parser/ptree.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ class ParseNodeFnc : public ParseNode
509509
ParseNodePtr pnodeParams;
510510
ParseNodePtr pnodeVars;
511511
ParseNodePtr pnodeBody;
512-
ParseNodeVar * pnodeRest;
512+
ParseNodePtr pnodeRest;
513513

514514
FuncInfo *funcInfo; // function information gathered during byte code generation
515515
Scope *scope;

‎lib/Runtime/ByteCode/ByteCodeEmitter.cpp

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1592,8 +1592,6 @@ void ByteCodeGenerator::EmitScopeObjectInit(FuncInfo *funcInfo)
15921592
Js::PropertyIdArray *propIds = funcInfo->GetParsedFunctionBody()->AllocatePropertyIdArrayForFormals(extraAlloc, slotCount, Js::ActivationObjectEx::ExtraSlotCount());
15931593

15941594
ParseNodeFnc *pnodeFnc = funcInfo->root;
1595-
ParseNode *pnode;
1596-
Symbol *sym;
15971595

15981596
if (funcInfo->GetFuncExprNameReference() && pnodeFnc->GetFuncSymbol()->GetScope() == funcInfo->GetBodyScope())
15991597
{
@@ -1633,10 +1631,15 @@ void ByteCodeGenerator::EmitScopeObjectInit(FuncInfo *funcInfo)
16331631
};
16341632
MapFormalsWithoutRest(pnodeFnc, initArg);
16351633

1636-
// If the rest is in the slot - we need to keep that slot.
1637-
if (pnodeFnc->pnodeRest != nullptr && pnodeFnc->pnodeRest->sym->IsInSlot(this, funcInfo))
1634+
ParseNodePtr rest = pnodeFnc->pnodeRest;
1635+
if (rest != nullptr && rest->IsVarLetOrConst())
16381636
{
1639-
Symbol::SaveToPropIdArray(pnodeFnc->pnodeRest->sym, propIds, this);
1637+
// If the rest is in the slot - we need to keep that slot.
1638+
Symbol *sym = rest->AsParseNodeVar()->sym;
1639+
if (sym->IsInSlot(this, funcInfo))
1640+
{
1641+
Symbol::SaveToPropIdArray(sym, propIds, this);
1642+
}
16401643
}
16411644
}
16421645
else
@@ -1661,7 +1664,7 @@ void ByteCodeGenerator::EmitScopeObjectInit(FuncInfo *funcInfo)
16611664
{
16621665
if (pnodeName->AsParseNodeBin()->pnode1->nop == knopVarDecl)
16631666
{
1664-
sym = pnodeName->AsParseNodeBin()->pnode1->AsParseNodeVar()->sym;
1667+
Symbol *sym = pnodeName->AsParseNodeBin()->pnode1->AsParseNodeVar()->sym;
16651668
if (sym)
16661669
{
16671670
Symbol::SaveToPropIdArray(sym, propIds, this, &firstFuncSlot);
@@ -1671,7 +1674,7 @@ void ByteCodeGenerator::EmitScopeObjectInit(FuncInfo *funcInfo)
16711674
}
16721675
if (pnodeName->nop == knopVarDecl)
16731676
{
1674-
sym = pnodeName->AsParseNodeVar()->sym;
1677+
Symbol *sym = pnodeName->AsParseNodeVar()->sym;
16751678
if (sym)
16761679
{
16771680
Symbol::SaveToPropIdArray(sym, propIds, this, &firstFuncSlot);
@@ -1685,6 +1688,8 @@ void ByteCodeGenerator::EmitScopeObjectInit(FuncInfo *funcInfo)
16851688

16861689
if (currentScope->GetScopeType() != ScopeType_Parameter)
16871690
{
1691+
ParseNode *pnode;
1692+
Symbol *sym;
16881693
for (pnode = pnodeFnc->pnodeVars; pnode; pnode = pnode->AsParseNodeVar()->pnodeNext)
16891694
{
16901695
sym = pnode->AsParseNodeVar()->sym;
@@ -2423,10 +2428,12 @@ void ByteCodeGenerator::HomeArguments(FuncInfo *funcInfo)
24232428
// Transfer formal parameters to their home locations on the local frame.
24242429
if (funcInfo->GetHasArguments())
24252430
{
2426-
if (funcInfo->root->pnodeRest != nullptr)
2431+
ParseNodePtr rest = funcInfo->root->pnodeRest;
2432+
if (rest != nullptr && rest->IsVarLetOrConst())
24272433
{
24282434
// Since we don't have to iterate over arguments here, we'll trust the location to be correct.
2429-
EmitLoadFormalIntoRegister(funcInfo->root->pnodeRest, funcInfo->root->pnodeRest->sym->GetLocation() + 1, funcInfo);
2435+
Symbol* sym = rest->AsParseNodeVar()->sym;
2436+
EmitLoadFormalIntoRegister(rest, sym->GetLocation() + 1, funcInfo);
24302437
}
24312438

24322439
// The arguments object creation helper does this work for us.
@@ -2709,8 +2716,7 @@ void ByteCodeGenerator::EmitDefaultArgs(FuncInfo *funcInfo, ParseNodeFnc *pnodeF
27092716
m_writer.RecordCrossFrameEntryExitRecord(/* isEnterBlock = */ true);
27102717
m_writer.Br(Js::OpCode::TryCatch, catchLabel);
27112718

2712-
// Rest cannot have a default argument, so we ignore it.
2713-
MapFormalsWithoutRest(pnodeFnc, emitDefaultArg);
2719+
MapFormals(pnodeFnc, emitDefaultArg);
27142720

27152721
m_writer.RecordCrossFrameEntryExitRecord(/* isEnterBlock = */ false);
27162722
m_writer.Empty(Js::OpCode::Leave);
@@ -2746,8 +2752,7 @@ void ByteCodeGenerator::EmitDefaultArgs(FuncInfo *funcInfo, ParseNodeFnc *pnodeF
27462752
}
27472753
else
27482754
{
2749-
// Rest cannot have a default argument, so we ignore it.
2750-
MapFormalsWithoutRest(pnodeFnc, emitDefaultArg);
2755+
MapFormals(pnodeFnc, emitDefaultArg);
27512756
}
27522757

27532758
if (m_writer.GetCurrentOffset() > beginOffset)
@@ -3074,12 +3079,6 @@ void ByteCodeGenerator::EmitOneFunction(ParseNodeFnc *pnodeFnc)
30743079
MapFormalsWithoutRest(pnodeFnc, [&](ParseNode *pnodeArg) { EmitPropStore(pnodeArg->AsParseNodeVar()->sym->GetLocation(), pnodeArg->AsParseNodeVar()->sym, pnodeArg->AsParseNodeVar()->pid, funcInfo); });
30753080
}
30763081

3077-
// Rest needs to trigger use before declaration until all default args have been processed.
3078-
if (pnodeFnc->pnodeRest != nullptr)
3079-
{
3080-
pnodeFnc->pnodeRest->sym->SetNeedDeclaration(false);
3081-
}
3082-
30833082
Js::RegSlot formalsUpperBound = Js::Constants::NoRegister; // Needed for tracking the last RegSlot in the param scope
30843083
if (!funcInfo->IsBodyAndParamScopeMerged())
30853084
{

‎test/es6/destructuring_bugs.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,23 +249,23 @@ var tests = [
249249
{
250250
name: "Rest as pattern at param with arguments/eval at function body",
251251
body: function () {
252-
(function ([a, b], c, ...{rest1, rest2}) {
252+
(function ([a, b], c, ...{'0': rest1, '1': rest2}) {
253253
eval("");
254254
assert.areEqual(rest1, 4);
255255
assert.areEqual(rest2, 5);
256256
assert.areEqual(c, 3);
257257
assert.areEqual(arguments[1], 3);
258-
})([1, 2], 3, {rest1:4, rest2:5});
258+
})([1, 2], 3, 4, 5);
259259

260-
(function ([a, b], c, ...{rest1, rest2}) {
260+
(function ([a, b], c, ...{'0': rest1, '1': rest2}) {
261261
(function () {
262262
assert.areEqual(rest1, 4);
263263
assert.areEqual(rest2, 5);
264264
assert.areEqual(a+b, 3);
265265
})();
266266
eval("");
267267
assert.areEqual(arguments[0], [1, 2]);
268-
})([1, 2], 3, {rest1:4, rest2:5});
268+
})([1, 2], 3, 4, 5);
269269
}
270270
},
271271
{

0 commit comments

Comments
 (0)