Skip to content

Commit 8fc4c15

Browse files
aneeshdkaneeshdk
committed
Fixing an issue with class definition in the param scope
When visiting the pnodescopes I had a check to avoid visiting the body scope as part of the param scope. The condition I was using for the check does not seem to cover all scenarios. This changelist fixes that issue.
1 parent 48295e3 commit 8fc4c15

4 files changed

Lines changed: 54 additions & 8 deletions

File tree

‎lib/Runtime/ByteCode/ByteCodeEmitter.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3536,7 +3536,7 @@ void ByteCodeGenerator::MapReferencedPropertyIds(FuncInfo * funcInfo)
35363536
#endif
35373537
}
35383538

3539-
void ByteCodeGenerator::EmitScopeList(ParseNode *pnode, bool breakOnNonFunc)
3539+
void ByteCodeGenerator::EmitScopeList(ParseNode *pnode, ParseNode *breakOnBodyScopeNode)
35403540
{
35413541
while (pnode)
35423542
{
@@ -3581,7 +3581,7 @@ void ByteCodeGenerator::EmitScopeList(ParseNode *pnode, bool breakOnNonFunc)
35813581

35823582
// While emitting the functions we have to stop when we see the body scope block.
35833583
// Otherwise functions defined in the body scope will not be able to get the right references.
3584-
this->EmitScopeList(paramBlock->sxBlock.pnodeScopes, true);
3584+
this->EmitScopeList(paramBlock->sxBlock.pnodeScopes, pnode->sxFnc.pnodeBodyScope);
35853585
Assert(this->GetCurrentScope() == paramScope);
35863586
}
35873587

@@ -3641,7 +3641,7 @@ void ByteCodeGenerator::EmitScopeList(ParseNode *pnode, bool breakOnNonFunc)
36413641
break;
36423642
}
36433643

3644-
if (breakOnNonFunc && pnode && pnode->nop != knopFncDecl)
3644+
if (breakOnBodyScopeNode != nullptr && breakOnBodyScopeNode == pnode)
36453645
{
36463646
break;
36473647
}

‎lib/Runtime/ByteCode/ByteCodeGenerator.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3024,7 +3024,7 @@ void AddFunctionsToScope(ParseNodePtr scope, ByteCodeGenerator * byteCodeGenerat
30243024

30253025
template <class PrefixFn, class PostfixFn>
30263026
void VisitNestedScopes(ParseNode* pnodeScopeList, ParseNode* pnodeParent, ByteCodeGenerator* byteCodeGenerator,
3027-
PrefixFn prefix, PostfixFn postfix, uint *pIndex, bool breakOnNonFnc = false)
3027+
PrefixFn prefix, PostfixFn postfix, uint *pIndex, bool breakOnBodyScope = false)
30283028
{
30293029
// Visit all scopes nested in this scope before visiting this function's statements. This way we have all the
30303030
// attributes of all the inner functions before we assign registers within this function.
@@ -3251,7 +3251,7 @@ void VisitNestedScopes(ParseNode* pnodeScopeList, ParseNode* pnodeParent, ByteCo
32513251
return;
32523252
}
32533253

3254-
if (breakOnNonFnc && pnodeScope->nop != knopFncDecl)
3254+
if (breakOnBodyScope && pnodeScope == pnodeParent->sxFnc.pnodeBodyScope)
32553255
{
32563256
break;
32573257
}

‎lib/Runtime/ByteCode/ByteCodeGenerator.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ class ByteCodeGenerator
284284

285285
void DefineLabels(FuncInfo *funcInfo);
286286
void EmitProgram(ParseNode *pnodeProg);
287-
void EmitScopeList(ParseNode *pnode, bool breakOnNonFunc = false);
287+
void EmitScopeList(ParseNode *pnode, ParseNode *breakOnBodyScopeNode = nullptr);
288288
void EmitDefaultArgs(FuncInfo *funcInfo, ParseNode *pnode);
289289
void EmitOneFunction(ParseNode *pnode);
290290
void EmitGlobalFncDeclInit(Js::RegSlot rhsLocation, Js::PropertyId propertyId, FuncInfo * funcInfo);

‎test/es6/default-splitscope.js‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ var tests = [
250250
}
251251
},
252252
{
253-
name: "Split parameter scope in class methods",
253+
name: "Split parameter scope and class",
254254
body: function () {
255255
class c {
256256
f(a = 10, d, b = function () { return a; }, c) {
@@ -315,7 +315,53 @@ var tests = [
315315
return y;
316316
}
317317
}
318-
assert.areEqual(10, (new c4()).f({})(), "The method defined as the default destructured value of the parameter should capture the formal from the param scope");
318+
assert.areEqual(10, (new c4()).f({})(), "The method defined as the default destructured value of the parameter should capture the formal from the param scope");
319+
320+
function f3(a = 10, d, b = (function () { return a; }, class { method1() { return a; } }), c) {
321+
var a = 20;
322+
assert.areEqual(10, (new b()).method1(), "Class method defined within the param scope should capture the formal from the param scope");
323+
return b;
324+
}
325+
result = f3();
326+
assert.areEqual(10, (new result()).method1(), "Methods defined in a class defined, after another function definition, in the param scope should capture the formals form that param scope itself");
327+
328+
function f4(a = 10, d, b = (function () { return a; }, class {}, class { method1() { return a; } }), c) {
329+
var a = 20;
330+
return b;
331+
}
332+
result = f4();
333+
assert.areEqual(10, (new result()).method1(), "Methods defined in a class defined, after another class definition, in the param scope should capture the formals form that param scope itself");
334+
335+
function f5(a = 10, d, b = (function () { return a; }, class {}, function () {}, class { method1() { return a; } }), c) {
336+
var a = 20;
337+
return b;
338+
}
339+
result = f5();
340+
assert.areEqual(10, (new result()).method1(), "Methods defined in a class defined, after a function and class, in the param scope should capture the formals form that param scope itself");
341+
342+
function f6(a = 10, d, b = (function () { return a; }, class {}, function (a, b = () => a) {}, class { method1() { return a; } }), c) {
343+
var a = 20;
344+
return b;
345+
}
346+
result = f6();
347+
assert.areEqual(10, (new result()).method1(), "Methods defined in a class defined, after a split scope function, in the param scope should capture the formals form that param scope itself");
348+
349+
function f7(a = 10, d, b = (function () { return a; }, class c1 { method1() { return a; } }), c) {
350+
var a = 20;
351+
assert.areEqual(10, (new b()).method1(), "Class method defined within the param scope should capture the formal from the param scope");
352+
return b;
353+
}
354+
result = f7();
355+
assert.areEqual(10, (new result()).method1(), "Methods defined in a class with name defined, after another function definition, in the param scope should capture the formals form that param scope itself");
356+
357+
function f8(a = 10, d, b = class c1 { method1() { return a; } }, c = (function () { return a; }, class c2 extends b { method2() { return a * a; } })) {
358+
var a = 20;
359+
assert.areEqual(10, (new b()).method1(), "Class method defined within the param scope should capture the formal from the param scope");
360+
return c;
361+
}
362+
result = f8();
363+
assert.areEqual(10, (new result()).method1(), "Methods defined in a class extending another class defined, after another function definition, in the param scope should capture the formals form that param scope itself");
364+
assert.areEqual(100, (new result()).method2(), "Method in the derived class returns the right value");
319365
}
320366
},
321367
{

0 commit comments

Comments
 (0)