Skip to content

Commit

Permalink
[MERGE #5380 @boingoing] OS#17895855 - Concise-body lambda function f…
Browse files Browse the repository at this point in the history
…ollowed by in keyword is not handled correctly

Merge pull request #5380 from boingoing:lambda_concise_body_in_expression

At the moment we parse this as a postfix operator, it should be part of the lambda body except for when we're parsing a for..in loop header.

Fixes:
https://microsoft.visualstudio.com/OS/_workitems/edit/17895855
  • Loading branch information
boingoing committed Jun 28, 2018
2 parents efe444d + f3dd30d commit 6d3f131
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 31 deletions.
44 changes: 22 additions & 22 deletions lib/Parser/Parse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4852,7 +4852,7 @@ BOOL Parser::IsDeferredFnc()
}

template<bool buildAST>
ParseNode * Parser::ParseFncDeclCheckScope(ushort flags, bool resetParsingSuperRestrictionState)
ParseNode * Parser::ParseFncDeclCheckScope(ushort flags, bool resetParsingSuperRestrictionState, bool fAllowIn)
{
ParseNodeBlock * pnodeFncBlockScope = nullptr;
ParseNodePtr *ppnodeScopeSave = nullptr;
Expand Down Expand Up @@ -4880,7 +4880,7 @@ ParseNode * Parser::ParseFncDeclCheckScope(ushort flags, bool resetParsingSuperR
}
}

ParseNodeFnc * pnodeFnc = ParseFncDeclInternal<buildAST>(flags, nullptr, /* needsPIDOnRCurlyScan */ false, resetParsingSuperRestrictionState, /* fUnaryOrParen */ false, noStmtContext);
ParseNodeFnc * pnodeFnc = ParseFncDeclInternal<buildAST>(flags, nullptr, /* needsPIDOnRCurlyScan */ false, resetParsingSuperRestrictionState, /* fUnaryOrParen */ false, noStmtContext, fAllowIn);

if (pnodeFncBlockScope)
{
Expand All @@ -4899,14 +4899,14 @@ ParseNode * Parser::ParseFncDeclCheckScope(ushort flags, bool resetParsingSuperR
}

template<bool buildAST>
ParseNodeFnc * Parser::ParseFncDeclNoCheckScope(ushort flags, LPCOLESTR pNameHint, const bool needsPIDOnRCurlyScan, bool resetParsingSuperRestrictionState, bool fUnaryOrParen)
ParseNodeFnc * Parser::ParseFncDeclNoCheckScope(ushort flags, LPCOLESTR pNameHint, const bool needsPIDOnRCurlyScan, bool resetParsingSuperRestrictionState, bool fUnaryOrParen, bool fAllowIn)
{
Assert((flags & fFncDeclaration) == 0);
return ParseFncDeclInternal<buildAST>(flags, pNameHint, needsPIDOnRCurlyScan, resetParsingSuperRestrictionState, fUnaryOrParen, /* noStmtContext */ false);
return ParseFncDeclInternal<buildAST>(flags, pNameHint, needsPIDOnRCurlyScan, resetParsingSuperRestrictionState, fUnaryOrParen, /* noStmtContext */ false, fAllowIn);
}

template<bool buildAST>
ParseNodeFnc * Parser::ParseFncDeclInternal(ushort flags, LPCOLESTR pNameHint, const bool needsPIDOnRCurlyScan, bool resetParsingSuperRestrictionState, bool fUnaryOrParen, bool noStmtContext)
ParseNodeFnc * Parser::ParseFncDeclInternal(ushort flags, LPCOLESTR pNameHint, const bool needsPIDOnRCurlyScan, bool resetParsingSuperRestrictionState, bool fUnaryOrParen, bool noStmtContext, bool fAllowIn)
{
AutoParsingSuperRestrictionStateRestorer restorer(this);
if (resetParsingSuperRestrictionState)
Expand Down Expand Up @@ -4987,7 +4987,7 @@ ParseNodeFnc * Parser::ParseFncDeclInternal(ushort flags, LPCOLESTR pNameHint, c

IdentPtr pFncNamePid = nullptr;
bool needScanRCurly = true;
ParseFncDeclHelper<buildAST>(pnodeFnc, pNameHint, flags, fUnaryOrParen, noStmtContext, &needScanRCurly, fModule, &pFncNamePid);
ParseFncDeclHelper<buildAST>(pnodeFnc, pNameHint, flags, fUnaryOrParen, noStmtContext, &needScanRCurly, fModule, &pFncNamePid, fAllowIn);
AddNestedCapturedNames(pnodeFnc);

AnalysisAssert(pnodeFnc);
Expand Down Expand Up @@ -5159,7 +5159,7 @@ void Parser::AppendFunctionToScopeList(bool fDeclaration, ParseNodeFnc * pnodeFn
Parse a function definition.
***************************************************************************/
template<bool buildAST>
void Parser::ParseFncDeclHelper(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, ushort flags, bool fUnaryOrParen, bool noStmtContext, bool *pNeedScanRCurly, bool skipFormals, IdentPtr* pFncNamePid)
void Parser::ParseFncDeclHelper(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, ushort flags, bool fUnaryOrParen, bool noStmtContext, bool *pNeedScanRCurly, bool skipFormals, IdentPtr* pFncNamePid, bool fAllowIn)
{
Assert(pnodeFnc);
ParseNodeFnc * pnodeFncParent = GetCurrentFunctionNode();
Expand Down Expand Up @@ -5559,7 +5559,7 @@ void Parser::ParseFncDeclHelper(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, us
{
fDeferred = true;

this->ParseTopLevelDeferredFunc(pnodeFnc, pnodeFncSave, pNameHint, fLambda, pNeedScanRCurly);
this->ParseTopLevelDeferredFunc(pnodeFnc, pnodeFncSave, pNameHint, fLambda, pNeedScanRCurly, fAllowIn);
}
else
{
Expand Down Expand Up @@ -5594,13 +5594,13 @@ void Parser::ParseFncDeclHelper(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, us
m_currDeferredStubCount = childStub->nestedCount;
m_currDeferredStub = childStub->deferredStubs;
}
this->FinishFncDecl(pnodeFnc, pNameHint, fLambda, skipFormals);
this->FinishFncDecl(pnodeFnc, pNameHint, fLambda, skipFormals, fAllowIn);
m_currDeferredStub = savedStub;
m_currDeferredStubCount = savedStubCount;
}
else
{
this->ParseNestedDeferredFunc(pnodeFnc, fLambda, pNeedScanRCurly, &strictModeTurnedOn);
this->ParseNestedDeferredFunc(pnodeFnc, fLambda, pNeedScanRCurly, &strictModeTurnedOn, fAllowIn);
}
}

Expand Down Expand Up @@ -5789,7 +5789,7 @@ void Parser::UpdateCurrentNodeFunc(ParseNodeFnc * pnodeFnc, bool fLambda)
}
}

void Parser::ParseTopLevelDeferredFunc(ParseNodeFnc * pnodeFnc, ParseNodeFnc * pnodeFncParent, LPCOLESTR pNameHint, bool fLambda, bool *pNeedScanRCurly)
void Parser::ParseTopLevelDeferredFunc(ParseNodeFnc * pnodeFnc, ParseNodeFnc * pnodeFncParent, LPCOLESTR pNameHint, bool fLambda, bool *pNeedScanRCurly, bool fAllowIn)
{
// Parse a function body that is a transition point from building AST to doing fast syntax check.

Expand Down Expand Up @@ -5826,7 +5826,7 @@ void Parser::ParseTopLevelDeferredFunc(ParseNodeFnc * pnodeFnc, ParseNodeFnc * p
// Their more-complicated text extents won't match the deferred-stub and the single expression should be fast to scan, anyway.
if (fLambda && !*pNeedScanRCurly)
{
ParseExpressionLambdaBody<false>(pnodeFnc);
ParseExpressionLambdaBody<false>(pnodeFnc, fAllowIn);
}
else if (pnodeFncParent != nullptr && m_currDeferredStub != nullptr && !pnodeFncParent->HasDefaultArguments())
{
Expand Down Expand Up @@ -6220,15 +6220,15 @@ ParseNodeFnc * Parser::CreateDummyFuncNode(bool fDeclaration)
return pnodeFnc;
}

void Parser::ParseNestedDeferredFunc(ParseNodeFnc * pnodeFnc, bool fLambda, bool *pNeedScanRCurly, bool *pStrictModeTurnedOn)
void Parser::ParseNestedDeferredFunc(ParseNodeFnc * pnodeFnc, bool fLambda, bool *pNeedScanRCurly, bool *pStrictModeTurnedOn, bool fAllowIn)
{
// Parse a function nested inside another deferred function.

size_t lengthBeforeBody = this->GetSourceLength();

if (m_token.tk != tkLCurly && fLambda)
{
ParseExpressionLambdaBody<false>(pnodeFnc);
ParseExpressionLambdaBody<false>(pnodeFnc, fAllowIn);
*pNeedScanRCurly = false;
}
else
Expand Down Expand Up @@ -6855,7 +6855,7 @@ ParseNodeFnc * Parser::GenerateEmptyConstructor(bool extends)
}

template<bool buildAST>
void Parser::ParseExpressionLambdaBody(ParseNodeFnc * pnodeLambda)
void Parser::ParseExpressionLambdaBody(ParseNodeFnc * pnodeLambda, bool fAllowIn)
{
ParseNodePtr *lastNodeRef = nullptr;

Expand All @@ -6876,7 +6876,7 @@ void Parser::ParseExpressionLambdaBody(ParseNodeFnc * pnodeLambda)
// The scanner needs to create a pid in the case of a string constant token immediately following the lambda body expression.
// Otherwise, we'll save null for the string constant pid which will AV during ByteCode generation.
BYTE fScanDeferredFlagsSave = this->GetScanner()->SetDeferredParse(FALSE);
ParseNodePtr result = ParseExpr<buildAST>(koplAsg, nullptr, TRUE, FALSE, nullptr, nullptr, nullptr, &token, false, nullptr, &lastRParen);
ParseNodePtr result = ParseExpr<buildAST>(koplAsg, nullptr, fAllowIn, FALSE, nullptr, nullptr, nullptr, &token, false, nullptr, &lastRParen);
this->GetScanner()->SetDeferredParseFlags(fScanDeferredFlagsSave);

this->MarkEscapingRef(result, &token);
Expand Down Expand Up @@ -6974,7 +6974,7 @@ void Parser::CheckStrictFormalParameters()
Assert(m_token.tk == tkRParen);
}

void Parser::FinishFncNode(ParseNodeFnc * pnodeFnc)
void Parser::FinishFncNode(ParseNodeFnc * pnodeFnc, bool fAllowIn)
{
AnalysisAssert(pnodeFnc);

Expand Down Expand Up @@ -7141,7 +7141,7 @@ void Parser::FinishFncNode(ParseNodeFnc * pnodeFnc)
const charcount_t ichLim = pnodeFnc->ichLim;
const size_t cbLim = pnodeFnc->cbLim;

this->FinishFncDecl(pnodeFnc, NULL, fLambda);
this->FinishFncDecl(pnodeFnc, NULL, fLambda, /* skipCurlyBraces */ false, fAllowIn);

#if DBG
// The pnode extent may not match the original extent.
Expand Down Expand Up @@ -7174,7 +7174,7 @@ void Parser::FinishFncNode(ParseNodeFnc * pnodeFnc)
this->GetScanner()->SetAwaitIsKeywordRegion(fPreviousAwaitIsKeyword);
}

void Parser::FinishFncDecl(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, bool fLambda, bool skipCurlyBraces)
void Parser::FinishFncDecl(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, bool fLambda, bool skipCurlyBraces, bool fAllowIn)
{
LPCOLESTR name = NULL;
JS_ETW(int32 startAstSize = *m_pCurrentAstSize);
Expand All @@ -7196,7 +7196,7 @@ void Parser::FinishFncDecl(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, bool fL

if (fLambda && m_token.tk != tkLCurly)
{
ParseExpressionLambdaBody<true>(pnodeFnc);
ParseExpressionLambdaBody<true>(pnodeFnc, fAllowIn);
}
else
{
Expand Down Expand Up @@ -8846,15 +8846,15 @@ ParseNodePtr Parser::ParseExpr(int oplMin,
this->GetScanner()->SeekTo(termStart);
}
}
pnode = ParseFncDeclNoCheckScope<buildAST>(flags, nullptr, /* needsPIDOnRCurlyScan = */false, /* resetParsingSuperRestrictionState = */false);
pnode = ParseFncDeclNoCheckScope<buildAST>(flags, nullptr, /* needsPIDOnRCurlyScan = */false, /* resetParsingSuperRestrictionState = */false, /* fUnaryOrParen = */ false, fAllowIn);
if (isAsyncMethod)
{
pnode->AsParseNodeFnc()->cbMin = iecpMin;
pnode->ichMin = ichMin;
}

// ArrowFunction/AsyncArrowFunction is part of AssignmentExpression, which should terminate the expression unless followed by a comma
if (m_token.tk != tkComma)
if (m_token.tk != tkComma && m_token.tk != tkIN)
{
if (!(IsTerminateToken()))
{
Expand Down
18 changes: 9 additions & 9 deletions lib/Parser/Parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -801,18 +801,18 @@ class Parser

template<bool buildAST> void ParseComputedName(ParseNodePtr* ppnodeName, LPCOLESTR* ppNameHint, LPCOLESTR* ppFullNameHint = nullptr, uint32 *pNameLength = nullptr, uint32 *pShortNameOffset = nullptr);
template<bool buildAST> ParseNodeBin * ParseMemberGetSet(OpCode nop, LPCOLESTR* ppNameHint);
template<bool buildAST> ParseNode * ParseFncDeclCheckScope(ushort flags, bool resetParsingSuperRestrictionState = true);
template<bool buildAST> ParseNodeFnc * ParseFncDeclNoCheckScope(ushort flags, LPCOLESTR pNameHint = nullptr, const bool needsPIDOnRCurlyScan = false, bool resetParsingSuperRestrictionState = true, bool fUnaryOrParen = false);
template<bool buildAST> ParseNodeFnc * ParseFncDeclInternal(ushort flags, LPCOLESTR pNameHint, const bool needsPIDOnRCurlyScan, bool resetParsingSuperRestrictionState, bool fUnaryOrParen, bool noStmtContext);
template<bool buildAST> ParseNode * ParseFncDeclCheckScope(ushort flags, bool resetParsingSuperRestrictionState = true, bool fAllowIn = true);
template<bool buildAST> ParseNodeFnc * ParseFncDeclNoCheckScope(ushort flags, LPCOLESTR pNameHint = nullptr, const bool needsPIDOnRCurlyScan = false, bool resetParsingSuperRestrictionState = true, bool fUnaryOrParen = false, bool fAllowIn = true);
template<bool buildAST> ParseNodeFnc * ParseFncDeclInternal(ushort flags, LPCOLESTR pNameHint, const bool needsPIDOnRCurlyScan, bool resetParsingSuperRestrictionState, bool fUnaryOrParen, bool noStmtContext, bool fAllowIn = true);
template<bool buildAST> void ParseFncName(ParseNodeFnc * pnodeFnc, ushort flags, IdentPtr* pFncNamePid = nullptr);
template<bool buildAST> void ParseFncFormals(ParseNodeFnc * pnodeFnc, ParseNodeFnc * pnodeParentFnc, ushort flags, bool isTopLevelDeferredFunc = false);
template<bool buildAST> void ParseFncDeclHelper(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, ushort flags, bool fUnaryOrParen, bool noStmtContext, bool *pNeedScanRCurly, bool skipFormals = false, IdentPtr* pFncNamePid = nullptr);
template<bool buildAST> void ParseExpressionLambdaBody(ParseNodeFnc * pnodeFnc);
template<bool buildAST> void ParseFncDeclHelper(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, ushort flags, bool fUnaryOrParen, bool noStmtContext, bool *pNeedScanRCurly, bool skipFormals = false, IdentPtr* pFncNamePid = nullptr, bool fAllowIn = true);
template<bool buildAST> void ParseExpressionLambdaBody(ParseNodeFnc * pnodeFnc, bool fAllowIn = true);
template<bool buildAST> void UpdateCurrentNodeFunc(ParseNodeFnc * pnodeFnc, bool fLambda);
bool FncDeclAllowedWithoutContext(ushort flags);
void FinishFncDecl(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, bool fLambda, bool skipCurlyBraces = false);
void ParseTopLevelDeferredFunc(ParseNodeFnc * pnodeFnc, ParseNodeFnc * pnodeFncParent, LPCOLESTR pNameHint, bool fLambda, bool *pNeedScanRCurly = nullptr);
void ParseNestedDeferredFunc(ParseNodeFnc * pnodeFnc, bool fLambda, bool *pNeedScanRCurly, bool *pStrictModeTurnedOn);
void FinishFncDecl(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, bool fLambda, bool skipCurlyBraces = false, bool fAllowIn = true);
void ParseTopLevelDeferredFunc(ParseNodeFnc * pnodeFnc, ParseNodeFnc * pnodeFncParent, LPCOLESTR pNameHint, bool fLambda, bool *pNeedScanRCurly = nullptr, bool fAllowIn = true);
void ParseNestedDeferredFunc(ParseNodeFnc * pnodeFnc, bool fLambda, bool *pNeedScanRCurly, bool *pStrictModeTurnedOn, bool fAllowIn = true);
void CheckStrictFormalParameters();
ParseNodeVar * AddArgumentsNodeToVars(ParseNodeFnc * pnodeFnc);
ParseNodeVar * InsertVarAtBeginning(ParseNodeFnc * pnodeFnc, IdentPtr pid);
Expand Down Expand Up @@ -845,7 +845,7 @@ class Parser
LPCOLESTR AppendNameHints(LPCOLESTR leftStr, uint32 leftLen, LPCOLESTR rightStr, uint32 rightLen, uint32 *pNameLength, uint32 *pShortNameOffset, bool ignoreAddDotWithSpace = false, bool wrapInBrackets = false);
WCHAR * AllocateStringOfLength(ULONG length);

void FinishFncNode(ParseNodeFnc * pnodeFnc);
void FinishFncNode(ParseNodeFnc * pnodeFnc, bool fAllowIn = true);

template<bool buildAST> bool ParseOptionalExpr(
ParseNodePtr* pnode,
Expand Down
47 changes: 47 additions & 0 deletions test/es6/bug_OS17895855.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------

WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");

var tests = [
{
name: "Concise-body lambda function containing in expression",
body: function () {
var l = a => '0' in [123]
assert.areEqual("a => '0' in [123]", l.toString(), "consise-body lambda containing in expression");
assert.isTrue(l(), "in expression can be the concise-body lambda body");
}
},
{
name: "Concise-body lambda function as var decl initializer in a for..in loop",
body: function () {
for (var a = () => 'pass' in []) {
assert.fail("Should not enter for loop since [] has no properties");
}
assert.areEqual('pass', a(), "var decl from for loop should have initialized a");

for (var a2 = () => 'pass' in [123]) {
assert.areEqual('0', a2, "Should enter the for loop with property '0'");
}
assert.areEqual('0', a2, "var decl from for loop should have been assigned to during iteration");
}
},
{
name: "Concise-body lambda function as var decl initializer in a for..in..in loop",
body: function () {
for (var b = () => 'pass' in [] in []) {
assert.fail("Should not enter for loop");
}
assert.areEqual('pass', b(), "var decl from for loop should still have initial value");

for (var b2 = () => 'pass' in '0' in [123]) {
assert.fail("var decl initialization turns into var b2 = () => 'pass' in true which should not enter this loop");
}
assert.areEqual('pass', b2(), "var decl was not overriden inside the for loop");
}
},
];

testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });
6 changes: 6 additions & 0 deletions test/es6/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1486,4 +1486,10 @@
<compile-flags>-args summary -endargs</compile-flags>
</default>
</test>
<test>
<default>
<files>bug_OS17895855.js</files>
<compile-flags>-args summary -endargs</compile-flags>
</default>
</test>
</regress-exe>

0 comments on commit 6d3f131

Please sign in to comment.