Skip to content

Commit 3c28308

Browse files
committed
This change set implements param scope and the body scope as parallel scopes.
The previous logic of having param scope as an inner scope to the body scope does not work well in eval cases. This changelist only addresses function definitions in the param scope. I will be working on the eval cases next. This works in interpreter mode right now. Backend changes are not done yet. Two new bytecodes were added. BeginBodyScope marks the beginning of the body scope where a new closure object is created for the body scope. Until then closure created for the param scope is used. After BeginBodyScope theparam scope one is not used other than copying the initial value for the body scope symbols from the param scope symbols. This required the creation of another new bytecode LdParamSlot. This also include some refactoring required for enabling these.
1 parent 2905975 commit 3c28308

22 files changed

Lines changed: 1345 additions & 867 deletions

lib/Parser/Parse.cpp

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4615,15 +4615,17 @@ bool Parser::ParseFncDeclHelper(ParseNodePtr pnodeFnc, ParseNodePtr pnodeFncPare
46154615
pstmtSave = m_pstmtCur;
46164616
SetCurrentStatement(nullptr);
46174617

4618-
// Function definition is inside the parent function's parameter scope
4618+
// Function definition is inside the parent function's parameter scope
46194619
bool isEnclosedInParamScope = this->m_currentScope->GetScopeType() == ScopeType_Parameter;
46204620

4621-
if (this->m_currentScope->GetScopeType() == ScopeType_FuncExpr)
4621+
if (this->m_currentScope->GetScopeType() == ScopeType_FuncExpr || this->m_currentScope->GetScopeType() == ScopeType_Block)
46224622
{
4623-
// Or this is a function expression enclosed in a parameter scope
4623+
// Or this is a function expression or class enclosed in a parameter scope
46244624
isEnclosedInParamScope = this->m_currentScope->GetEnclosingScope() && this->m_currentScope->GetEnclosingScope()->GetScopeType() == ScopeType_Parameter;
46254625
}
46264626

4627+
Assert(!isEnclosedInParamScope || pnodeFncParent->sxFnc.HasNonSimpleParameterList());
4628+
46274629
RestorePoint beginFormals;
46284630
m_pscan->Capture(&beginFormals);
46294631
BOOL fWasAlreadyStrictMode = IsStrictMode();
@@ -4859,44 +4861,52 @@ bool Parser::ParseFncDeclHelper(ParseNodePtr pnodeFnc, ParseNodePtr pnodeFncPare
48594861
pnodeFnc->sxFnc.pnodeVars = nullptr;
48604862
m_ppnodeVar = &pnodeFnc->sxFnc.pnodeVars;
48614863

4862-
// We can't merge the param scope and body scope any more as the nested methods may be capturing params.
48634864
if (pnodeFnc->sxFnc.HasNonSimpleParameterList() && !fAsync)
48644865
{
48654866
Scope* paramScope = pnodeFnc->sxFnc.pnodeScopes->sxBlock.scope;
4867+
Assert(paramScope != nullptr);
48664868

4867-
paramScope->ForEachSymbolUntil([this, paramScope](Symbol* sym) {
4868-
if (sym->GetPid()->GetTopRef()->sym == nullptr)
4869+
if (pnodeFnc->sxFnc.CallsEval() || pnodeFnc->sxFnc.ChildCallsEval())
4870+
{
4871+
if (!m_scriptContext->GetConfig()->IsES6DefaultArgsSplitScopeEnabled())
48694872
{
4870-
if (m_scriptContext->GetConfig()->IsES6DefaultArgsSplitScopeEnabled())
4873+
Error(ERREvalNotSupportedInParamScope);
4874+
}
4875+
}
4876+
else
4877+
{
4878+
paramScope->ForEachSymbolUntil([this, paramScope](Symbol* sym) {
4879+
if (sym->GetPid()->GetTopRef()->sym == nullptr)
48714880
{
4872-
// One of the symbol has non local reference. Mark the param scope as we can't merge it with body scope.
4873-
paramScope->SetCannotMergeWithBodyScope();
4874-
return true;
4881+
if (m_scriptContext->GetConfig()->IsES6DefaultArgsSplitScopeEnabled())
4882+
{
4883+
// One of the symbol has non local reference. Mark the param scope as we can't merge it with body scope.
4884+
paramScope->SetCannotMergeWithBodyScope();
4885+
return true;
4886+
}
4887+
else
4888+
{
4889+
Error(ERRFuncRefFormalNotSupportedInParamScope);
4890+
}
48754891
}
48764892
else
48774893
{
4878-
Error(ERRFuncRefFormalNotSupportedInParamScope);
4894+
// If no non-local references are there then the top of the ref stack should point to the same symbol.
4895+
Assert(sym->GetPid()->GetTopRef()->sym == sym);
48794896
}
4880-
}
4881-
else
4882-
{
4883-
Assert(sym->GetPid()->GetTopRef()->sym == sym);
4884-
}
4885-
return false;
4886-
});
4887-
4888-
if (!m_scriptContext->GetConfig()->IsES6DefaultArgsSplitScopeEnabled() && (pnodeFnc->sxFnc.CallsEval() || pnodeFnc->sxFnc.ChildCallsEval()))
4889-
{
4890-
Error(ERREvalNotSupportedInParamScope);
4897+
return false;
4898+
});
48914899
}
48924900

48934901
if (!paramScope->GetCanMergeWithBodyScope())
48944902
{
48954903
OUTPUT_TRACE_DEBUGONLY(Js::ParsePhase, L"The param and body scope of the function %s cannot be merged\n", pnodeFnc->sxFnc.pnodeName ? pnodeFnc->sxFnc.pnodeName->sxVar.pid->Psz() : L"Anonymous function");
4896-
// Now add a new symbol reference for each formal in the param scope to the body scope.
4904+
// Add a new symbol reference for each formal in the param scope to the body scope.
48974905
paramScope->ForEachSymbol([this](Symbol* param) {
48984906
OUTPUT_TRACE_DEBUGONLY(Js::ParsePhase, L"Creating a duplicate symbol for the parameter %s in the body scope\n", param->GetPid()->Psz());
4899-
this->CreateVarDeclNode(param->GetPid(), param->GetSymbolType(), false, nullptr, false);
4907+
ParseNodePtr paramNode = this->CreateVarDeclNode(param->GetPid(), STVariable, false, nullptr, false);
4908+
Assert(paramNode && paramNode->sxVar.sym->GetScope()->GetScopeType() == ScopeType_FunctionBody);
4909+
paramNode->sxVar.sym->SetHasInit(true);
49004910
});
49014911
}
49024912
}

lib/Runtime/Base/FunctionBody.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,9 +413,9 @@ namespace Js
413413
FunctionBody::FunctionBody(ScriptContext* scriptContext, const wchar_t* displayName, uint displayNameLength, uint displayShortNameOffset, uint nestedCount,
414414
Utf8SourceInfo* utf8SourceInfo, uint uFunctionNumber, uint uScriptId,
415415
Js::LocalFunctionId functionId, Js::PropertyRecordList* boundPropertyRecords, Attributes attributes
416-
#ifdef PERF_COUNTERS
416+
#ifdef PERF_COUNTERS
417417
, bool isDeserializedFunction
418-
#endif
418+
#endif
419419
) :
420420
ParseableFunctionInfo(scriptContext->CurrentThunk, nestedCount, functionId, utf8SourceInfo, scriptContext, uFunctionNumber, displayName, displayNameLength, displayShortNameOffset, attributes, boundPropertyRecords),
421421
m_uScriptId(uScriptId),
@@ -458,6 +458,7 @@ namespace Js
458458
m_CallsEval(false),
459459
m_ChildCallsEval(false),
460460
m_hasReferenceableBuiltInArguments(false),
461+
m_isParamAndBodyScopeMerged(true),
461462
m_firstFunctionObject(true),
462463
m_inlineCachesOnFunctionObject(false),
463464
m_hasDoneAllNonLocalReferenced(false),
@@ -1028,6 +1029,7 @@ namespace Js
10281029
CopyDeferParseField(scopeObjectSize);
10291030
#endif
10301031
CopyDeferParseField(scopeSlotArraySize);
1032+
CopyDeferParseField(paramScopeSlotArraySize);
10311033
other->SetCachedSourceString(this->GetCachedSourceString());
10321034
other->SetDeferredStubs(this->GetDeferredStubs());
10331035
CopyDeferParseField(m_isAsmjsMode);
@@ -1129,6 +1131,7 @@ namespace Js
11291131
m_displayNameLength(0),
11301132
m_displayShortNameOffset(0),
11311133
scopeSlotArraySize(0),
1134+
paramScopeSlotArraySize(0),
11321135
m_reparsed(false),
11331136
m_isAsmJsFunction(false)
11341137
#if DBG
@@ -3674,6 +3677,7 @@ namespace Js
36743677
newFunctionInfo->m_isPublicLibraryCode = this->m_isPublicLibraryCode;
36753678

36763679
newFunctionInfo->scopeSlotArraySize = this->scopeSlotArraySize;
3680+
newFunctionInfo->paramScopeSlotArraySize = this->paramScopeSlotArraySize;
36773681

36783682
this->ForEachNestedFunc([&](FunctionProxy* proxy, uint32 index)
36793683
{

lib/Runtime/Base/FunctionBody.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1945,6 +1945,7 @@ namespace Js
19451945
bool m_ChildCallsEval : 1;
19461946
bool m_CallsEval : 1;
19471947
bool m_hasReferenceableBuiltInArguments : 1;
1948+
bool m_isParamAndBodyScopeMerged : 1;
19481949

19491950
// Used in the debug purpose. This is to avoid setting all locals to non-local-referenced, multiple times for each child function.
19501951
bool m_hasDoneAllNonLocalReferenced : 1;
@@ -2567,10 +2568,16 @@ namespace Js
25672568
!PHASE_OFF(Js::PolymorphicInlineFixedMethodsPhase, this) && !PHASE_OFF(Js::PolymorphicInlineFixedMethodsPhase, topFunctionBody);
25682569
}
25692570

2570-
Js::PropertyId * GetPropertyIdsForScopeSlotArray() const { return static_cast<Js::PropertyId *>(this->GetAuxPtr(AuxPointerType::PropertyIdsForScopeSlotArray)); }
2571-
void SetPropertyIdsForScopeSlotArray(Js::PropertyId * propertyIdsForScopeSlotArray, uint scopeSlotCount)
2571+
void SetScopeSlotArraySizes(uint scopeSlotCount, uint scopeSlotCountForParamScope)
25722572
{
25732573
this->scopeSlotArraySize = scopeSlotCount;
2574+
this->paramScopeSlotArraySize = scopeSlotCountForParamScope;
2575+
}
2576+
2577+
Js::PropertyId * GetPropertyIdsForScopeSlotArray() const { return static_cast<Js::PropertyId *>(this->GetAuxPtr(AuxPointerType::PropertyIdsForScopeSlotArray)); }
2578+
void SetPropertyIdsForScopeSlotArray(Js::PropertyId * propertyIdsForScopeSlotArray, uint scopeSlotCount, uint scopeSlotCountForParamScope = 0)
2579+
{
2580+
SetScopeSlotArraySizes(scopeSlotCount, scopeSlotCountForParamScope);
25742581
this->SetAuxPtr(AuxPointerType::PropertyIdsForScopeSlotArray, propertyIdsForScopeSlotArray);
25752582
}
25762583

@@ -2945,6 +2952,9 @@ namespace Js
29452952
bool HasReferenceableBuiltInArguments() const { return m_hasReferenceableBuiltInArguments; }
29462953
void SetHasReferenceableBuiltInArguments(bool value) { m_hasReferenceableBuiltInArguments = value; }
29472954

2955+
bool IsParamAndBodyScopeMerged() const { return m_isParamAndBodyScopeMerged; }
2956+
void SetParamAndBodyScopeNotMerged() { m_isParamAndBodyScopeMerged = false; }
2957+
29482958
// Used for the debug purpose. This is to avoid setting all locals to non-local-referenced, multiple time for each child function.
29492959
bool HasDoneAllNonLocalReferenced() const { return m_hasDoneAllNonLocalReferenced; }
29502960
void SetHasDoneAllNonLocalReferenced(bool set) { m_hasDoneAllNonLocalReferenced = set; }

lib/Runtime/ByteCode/ByteCodeDumper.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,7 @@ namespace Js
832832
Output::Print(L" [%d] = R%d ",data->SlotIndex, data->Value);
833833
break;
834834
case OpCode::LdLocalSlot:
835+
case OpCode::LdParamSlot:
835836
case OpCode::LdEnvObj:
836837
case OpCode::LdLocalObjSlot:
837838
Output::Print(L" R%d = [%d] ",data->Value, data->SlotIndex);

0 commit comments

Comments
 (0)