Skip to content

Commit 6c4ce8f

Browse files
boingoingboingoing
committed
[MERGE #6193 @boingoing] Correctly handle named function expressions when saving ScopeInfo
Merge pull request #6193 from boingoing:fix_paramscopecapturesbodydecl_2 Previous fix for a similar issue #6157 didn't handle the case of a function expression in which the param and function expression scopes are merged with the body scope. In that case, the function defined in the param scope needs to have access to the body scope of the enclosing function.
2 parents 9f72ec4 + 0779c3e commit 6c4ce8f

File tree

8 files changed

+180
-30
lines changed

8 files changed

+180
-30
lines changed

‎lib/Parser/Parse.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1973,6 +1973,20 @@ void Parser::BindPidRefsInScope(IdentPtr pid, Symbol *sym, int blockId, uint max
19731973
}
19741974
}
19751975

1976+
if (m_currentNodeFunc && m_currentNodeFunc->pnodeName && pid == m_currentNodeFunc->pnodeName->pid && !m_currentNodeFunc->IsDeclaration() && m_currentNodeFunc->IsBodyAndParamScopeMerged())
1977+
{
1978+
Scope* funcExprScope = m_currentNodeFunc->scope;
1979+
Assert(funcExprScope->GetScopeType() == ScopeType_FuncExpr);
1980+
1981+
ParseNodeBlock* bodyScope = m_currentNodeFunc->pnodeBodyScope;
1982+
Assert(bodyScope->blockType == PnodeBlockType::Function);
1983+
1984+
if (ref->GetScopeId() < bodyScope->blockId && ref->GetScopeId() > blockId)
1985+
{
1986+
funcExprScope->SetIsObject();
1987+
}
1988+
}
1989+
19761990
if (ref->GetScopeId() == blockId)
19771991
{
19781992
break;
@@ -5378,9 +5392,14 @@ ParseNodeFnc * Parser::ParseFncDeclInternal(ushort flags, LPCOLESTR pNameHint, c
53785392
pnodeFnc->SetIsModule(fModule);
53795393
pnodeFnc->SetIsClassConstructor((flags & fFncClassConstructor) != 0);
53805394
pnodeFnc->SetIsBaseClassConstructor((flags & fFncBaseClassConstructor) != 0);
5381-
pnodeFnc->SetIsDeclaredInParamScope(this->m_currentScope && this->m_currentScope->GetScopeType() == ScopeType_Parameter);
53825395
pnodeFnc->SetHomeObjLocation(Js::Constants::NoRegister);
53835396

5397+
if (this->m_currentScope && this->m_currentScope->GetScopeType() == ScopeType_Parameter)
5398+
{
5399+
pnodeFnc->SetIsDeclaredInParamScope();
5400+
this->m_currentScope->SetHasNestedParamFunc();
5401+
}
5402+
53845403
IdentPtr pFncNamePid = nullptr;
53855404
bool needScanRCurly = true;
53865405
ParseFncDeclHelper<buildAST>(pnodeFnc, pNameHint, flags, fUnaryOrParen, noStmtContext, &needScanRCurly, fModule, &pFncNamePid, fAllowIn);

‎lib/Runtime/ByteCode/ByteCodeEmitter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3481,8 +3481,6 @@ void ByteCodeGenerator::EmitScopeList(ParseNode *pnode, ParseNode *breakOnBodySc
34813481
}
34823482
this->StartEmitFunction(pnode->AsParseNodeFnc());
34833483

3484-
PushFuncInfo(_u("StartEmitFunction"), funcInfo);
3485-
34863484
if (!funcInfo->IsBodyAndParamScopeMerged())
34873485
{
34883486
this->EmitScopeList(pnode->AsParseNodeFnc()->pnodeBodyScope->pnodeScopes);
@@ -3886,6 +3884,8 @@ void ByteCodeGenerator::StartEmitFunction(ParseNodeFnc *pnodeFnc)
38863884
}
38873885
}
38883886

3887+
PushFuncInfo(_u("StartEmitFunction"), funcInfo);
3888+
38893889
if (!funcInfo->IsBodyAndParamScopeMerged())
38903890
{
38913891
ParseNodeBlock * paramBlock = pnodeFnc->pnodeScopes;

‎lib/Runtime/ByteCode/ByteCodeGenerator.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1840,7 +1840,7 @@ FuncInfo *ByteCodeGenerator::FindEnclosingNonLambda()
18401840
return nullptr;
18411841
}
18421842

1843-
FuncInfo* GetParentFuncInfo(FuncInfo* child)
1843+
FuncInfo* ByteCodeGenerator::GetParentFuncInfo(FuncInfo* child)
18441844
{
18451845
for (Scope* scope = child->GetBodyScope(); scope; scope = scope->GetEnclosingScope())
18461846
{
@@ -1853,6 +1853,19 @@ FuncInfo* GetParentFuncInfo(FuncInfo* child)
18531853
return nullptr;
18541854
}
18551855

1856+
FuncInfo* ByteCodeGenerator::GetEnclosingFuncInfo()
1857+
{
1858+
FuncInfo* top = this->funcInfoStack->Pop();
1859+
1860+
Assert(!this->funcInfoStack->Empty());
1861+
1862+
FuncInfo* second = this->funcInfoStack->Top();
1863+
1864+
this->funcInfoStack->Push(top);
1865+
1866+
return second;
1867+
}
1868+
18561869
bool ByteCodeGenerator::CanStackNestedFunc(FuncInfo * funcInfo, bool trace)
18571870
{
18581871
#if ENABLE_DEBUG_CONFIG_OPTIONS
@@ -2634,7 +2647,7 @@ void AssignFuncSymRegister(ParseNodeFnc * pnodeFnc, ByteCodeGenerator * byteCode
26342647
Assert(byteCodeGenerator->GetCurrentScope()->GetFunc() == sym->GetScope()->GetFunc());
26352648
if (byteCodeGenerator->GetCurrentScope()->GetFunc() != sym->GetScope()->GetFunc())
26362649
{
2637-
Assert(GetParentFuncInfo(byteCodeGenerator->GetCurrentScope()->GetFunc()) == sym->GetScope()->GetFunc());
2650+
Assert(ByteCodeGenerator::GetParentFuncInfo(byteCodeGenerator->GetCurrentScope()->GetFunc()) == sym->GetScope()->GetFunc());
26382651
sym->GetScope()->SetMustInstantiate(true);
26392652
byteCodeGenerator->ProcessCapturedSym(sym);
26402653
sym->GetScope()->GetFunc()->SetHasLocalInClosure(true);

‎lib/Runtime/ByteCode/ByteCodeGenerator.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,8 @@ class ByteCodeGenerator
402402
void PopulateFormalsScope(uint beginOffset, FuncInfo *funcInfo, ParseNodeFnc *pnodeFnc);
403403
void InsertPropertyToDebuggerScope(FuncInfo* funcInfo, Js::DebuggerScope* debuggerScope, Symbol* sym);
404404
FuncInfo *FindEnclosingNonLambda();
405+
static FuncInfo* GetParentFuncInfo(FuncInfo* child);
406+
FuncInfo* GetEnclosingFuncInfo();
405407

406408
bool CanStackNestedFunc(FuncInfo * funcInfo, bool trace = false);
407409
void CheckDeferParseHasMaybeEscapedNestedFunc();

‎lib/Runtime/ByteCode/Scope.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class Scope
4141
BYTE canMergeWithBodyScope : 1;
4242
BYTE hasLocalInClosure : 1;
4343
BYTE isBlockInLoop : 1;
44+
BYTE hasNestedParamFunc : 1;
4445
public:
4546
#if DBG
4647
BYTE isRestored : 1;
@@ -60,6 +61,7 @@ class Scope
6061
canMergeWithBodyScope(true),
6162
hasLocalInClosure(false),
6263
isBlockInLoop(false),
64+
hasNestedParamFunc(false),
6365
location(Js::Constants::NoRegister),
6466
m_symList(nullptr),
6567
m_count(0),
@@ -252,6 +254,9 @@ class Scope
252254
void SetIsBlockInLoop(bool is = true) { isBlockInLoop = is; }
253255
bool IsBlockInLoop() const { return isBlockInLoop; }
254256

257+
void SetHasNestedParamFunc(bool is = true) { hasNestedParamFunc = is; }
258+
bool GetHasNestedParamFunc() const { return hasNestedParamFunc; }
259+
255260
bool HasInnerScopeIndex() const { return innerScopeIndex != (uint)-1; }
256261
uint GetInnerScopeIndex() const { return innerScopeIndex; }
257262
void SetInnerScopeIndex(uint index) { innerScopeIndex = index; }

‎lib/Runtime/ByteCode/ScopeInfo.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,17 @@ namespace Js
112112
ScopeInfo * ScopeInfo::SaveScopeInfo(ByteCodeGenerator* byteCodeGenerator, Scope * scope, ScriptContext * scriptContext)
113113
{
114114
// Advance past scopes that will be excluded from the closure environment. (But note that we always want the body scope.)
115-
while (scope && (!scope->GetMustInstantiate() && scope != scope->GetFunc()->GetBodyScope()))
115+
while (scope)
116116
{
117+
FuncInfo* func = scope->GetFunc();
118+
119+
if (scope->GetMustInstantiate() ||
120+
func->GetBodyScope() == scope ||
121+
(func->GetParamScope() == scope && func->IsBodyAndParamScopeMerged() && scope->GetHasNestedParamFunc()))
122+
{
123+
break;
124+
}
125+
117126
scope = scope->GetEnclosingScope();
118127
}
119128

@@ -162,13 +171,12 @@ namespace Js
162171
Scope* currentScope = byteCodeGenerator->GetCurrentScope();
163172
Assert(currentScope->GetFunc() == funcInfo);
164173

165-
// Disable the below temporarily because of bad interaction with named function expressions
166-
#if 0
167-
if (funcInfo->root->IsDeclaredInParamScope()) {
174+
if (funcInfo->root->IsDeclaredInParamScope())
175+
{
168176
Assert(currentScope->GetScopeType() == ScopeType_FunctionBody);
169177
Assert(currentScope->GetEnclosingScope());
170178

171-
FuncInfo* func = currentScope->GetEnclosingScope()->GetFunc();
179+
FuncInfo* func = byteCodeGenerator->GetEnclosingFuncInfo();
172180
Assert(func);
173181

174182
if (func->IsBodyAndParamScopeMerged())
@@ -178,7 +186,6 @@ namespace Js
178186
Assert(!currentScope->GetMustInstantiate());
179187
}
180188
}
181-
#endif
182189

183190
while (currentScope->GetFunc() == funcInfo)
184191
{

‎test/Bugs/bug_OS18926499.js

Lines changed: 123 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,130 @@
33
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
44
//-------------------------------------------------------------------------------------------------------
55

6-
function foo(a = (()=>+x)()) {
7-
function bar() { eval(''); }
8-
var x;
9-
}
6+
let throwingFunctions = [{
7+
msg: "Split-scope parent, param-scope child capturing symbol from parent body scope",
8+
body: function foo(a = (()=>+x)()) {
9+
function bar() { eval(''); }
10+
var x;
11+
}
12+
},
13+
{
14+
msg: "Split-scope parent, param-scope child capturing symbol from parent body scope",
15+
body: function foo(a = (()=>+x)()) {
16+
eval('');
17+
var x;
18+
}
19+
},
20+
{
21+
msg: "Merged-scope parent, param-scope child capturing symbol from parent body scope",
22+
body: function foo(a = () => +x) {
23+
var x = 1;
24+
return a();
25+
}
26+
},
27+
{
28+
msg: "Merged-scope parent, param-scope child capturing symbol from parent body scope",
29+
body: function foo(a = (()=>+x)()) {
30+
var x;
31+
}
32+
},
33+
{
34+
msg: "Func expr parent, param-scope func expr child capturing symbol from parent body scope",
35+
body: foo3 = function foo3a(a = (function foo3b() { return +x; })()) {
36+
var x = 123;
37+
}
38+
},
39+
{
40+
msg: "Func expr parent, param-scope func expr child capturing symbol from parent body scope",
41+
body: foo3 = function foo3a(a = (function foo3b() { return +x; })()) {
42+
eval('');
43+
var x = 123;
44+
}
45+
},
46+
{
47+
msg: "Func expr parent, param-scope func expr child capturing symbol from parent body scope",
48+
body: foo3 = function foo3a(a = (function foo3b() { return +x; })()) {
49+
function bar() { eval(''); }
50+
var x = 123;
51+
}
52+
},
53+
{
54+
msg: "Param-scope func expr child with nested func expr capturing symbol from parent body scope",
55+
body: foo5 = function foo5a(a = (function(){(function(b = 123) { +x; })()})()) {
56+
function bar() { eval(''); }
57+
var x;
58+
}
59+
},
60+
{
61+
msg: "Multiple nested func expr, inner param-scope function capturing outer func expr name",
62+
body: foo3 = function foo3a(a = (function foo3b(b = (function foo3c(c = (function foo3d() { +x; })()){})()){})()){ var x;}
63+
}];
1064

11-
try {
12-
foo();
13-
console.log('fail');
14-
} catch {
15-
console.log("pass");
16-
}
65+
let nonThrowingFunctions = [{
66+
msg: "Func expr parent, param-scope func expr child capturing parent func expr name",
67+
body: foo3 = function foo3a(a = (function foo3b() { +foo3a; })()) {
68+
return +a;
69+
}
70+
},
71+
{
72+
msg: "Func expr parent, param-scope func expr child capturing own func expr name",
73+
body: foo3 = function foo3a(a = (function foo3b() { +foo3b; })()) {
74+
return +a;
75+
}
76+
},
77+
{
78+
msg: "Func expr parent, param-scope func expr child capturing expression name hint",
79+
body: foo3 = function foo3a(a = (function foo3b() { +foo3; })()) {
80+
return +a;
81+
}
82+
},
83+
{
84+
msg: "Func expr parent, param-scope func expr child capturing parent argument name",
85+
body: foo3 = function foo3a(b = 123, a = (function foo3b() { return +b; })()) {
86+
if (123 !== a) throw 123;
87+
}
88+
},
89+
{
90+
msg: "Func expr parent, param-scope func expr child capturing symbol from parent body scope",
91+
body: foo3 = function foo3a(b = 123, a = (function foo3b() { return +b; })()) {
92+
if (123 !== a) throw 123;
93+
}
94+
},
95+
{
96+
msg: "Multiple nested func expr, inner param-scope function capturing outer func expr name",
97+
body: foo3 = function foo3a(a = (function foo3b(b = (function foo3c(c = (function foo3d() { foo3d; })()){})()){})()){}
98+
},
99+
{
100+
msg: "Multiple nested func expr, inner param-scope function capturing outer func expr name",
101+
body: foo3 = function foo3a(a = (function foo3b(b = (function foo3c(c = (function foo3d() { foo3c; })()){})()){})()){}
102+
},
103+
{
104+
msg: "Multiple nested func expr, inner param-scope function capturing outer func expr name",
105+
body: foo3 = function foo3a(a = (function foo3b(b = (function foo3c(c = (function foo3d() { foo3b; })()){})()){})()){}
106+
},
107+
{
108+
msg: "Multiple nested func expr, inner param-scope function capturing outer func expr name",
109+
body: foo3 = function foo3a(a = (function foo3b(b = (function foo3c(c = (function foo3d() { foo3a; })()){})()){})()){}
110+
},
111+
{
112+
msg: "Multiple nested func expr, inner param-scope function capturing outer func expr name",
113+
body: foo3 = function foo3a(a = (function foo3b(b = (function foo3c(c = (function foo3d() { foo3; })()){})()){})()){}
114+
}];
17115

18-
function foo2(a = () => x) { var x = 1; return a(); }
116+
for (let fn of throwingFunctions) {
117+
try {
118+
fn.body();
119+
console.log(`fail: ${fn.msg}`);
120+
} catch {
121+
console.log("pass");
122+
}
123+
}
19124

20-
try {
21-
foo2()
22-
console.log('fail');
23-
} catch {
24-
console.log('pass');
125+
for (let fn of nonThrowingFunctions) {
126+
try {
127+
fn.body();
128+
console.log("pass");
129+
} catch {
130+
console.log(`fail: ${fn.msg}`);
131+
}
25132
}

‎test/Bugs/rlexe.xml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -582,15 +582,12 @@
582582
<compile-flags>-args summary -endargs</compile-flags>
583583
</default>
584584
</test>
585-
<!--
586-
Re-enable after fixing ScopeInfo::SaveEnclosingScopeInfo to handle named function expressions
587585
<test>
588586
<default>
589587
<files>bug_OS18926499.js</files>
590588
<compile-flags>-force:deferparse</compile-flags>
591589
</default>
592590
</test>
593-
-->
594591
<test>
595592
<default>
596593
<files>Bug19767482.js</files>

0 commit comments

Comments
 (0)