Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer functions enclosed in all contexts. #2666

Merged
merged 1 commit into from
Mar 15, 2017
+377 −436
Merged

Defer functions enclosed in all contexts. #2666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/Parser/Hash.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,16 @@ struct Ident
return m_pidRefStack;
}

PidRefStack *GetTopRef(uint maxBlockId) const
{
PidRefStack *ref;
for (ref = m_pidRefStack; ref && (uint)ref->id > maxBlockId; ref = ref->prev)
{
; // nothing
}
return ref;
}

void SetTopRef(PidRefStack *ref)
{
m_pidRefStack = ref;
Expand Down
274 changes: 122 additions & 152 deletions lib/Parser/Parse.cpp
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 8 additions & 4 deletions lib/Parser/Parse.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@

#include "ParseFlags.h"

namespace Js
{
class ScopeInfo;
};

// Operator precedence levels
enum
{
Expand Down Expand Up @@ -369,7 +374,6 @@ class Parser
bool m_inDeferredNestedFunc; // true if parsing a function in deferred mode, nested within the current node
bool m_isInBackground;
bool m_reparsingLambdaParams;
bool m_inFIB;

// This bool is used for deferring the shorthand initializer error ( {x = 1}) - as it is allowed in the destructuring grammar.
bool m_hasDeferredShorthandInitError;
Expand Down Expand Up @@ -976,8 +980,8 @@ class Parser
void RemovePrevPidRef(IdentPtr pid, PidRefStack *lastRef);
void SetPidRefsInScopeDynamic(IdentPtr pid, int blockId);

void RestoreScopeInfo(Js::ParseableFunctionInfo* functionBody);
void FinishScopeInfo(Js::ParseableFunctionInfo* functionBody);
void RestoreScopeInfo(Js::ScopeInfo * scopeInfo);
void FinishScopeInfo(Js::ScopeInfo * scopeInfo);

BOOL PnodeLabelNoAST(IdentToken* pToken, LabelId* pLabelIdList);
LabelId* CreateLabelId(IdentToken* pToken);
Expand Down Expand Up @@ -1011,7 +1015,7 @@ class Parser
}

template <class Fn>
void VisitFunctionsInScope(ParseNodePtr pnodeScopeList, Fn fn);
void FinishFunctionsInScope(ParseNodePtr pnodeScopeList, Fn fn);
void FinishDeferredFunction(ParseNodePtr pnodeScopeList);

/***********************************************************************
Expand Down
4 changes: 0 additions & 4 deletions lib/Parser/ptree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,6 @@ struct PnFnc
RestorePoint *pRestorePoint;
DeferredFunctionStub *deferredStub;
bool canBeDeferred;
bool fibPreventsDeferral;

static const int32 MaxStackClosureAST = 800000;

Expand Down Expand Up @@ -286,7 +285,6 @@ struct PnFnc
{
fncFlags = kFunctionNone;
canBeDeferred = false;
fibPreventsDeferral = false;
}

void SetAsmjsMode(bool set = true) { SetFlags(kFunctionAsmjsMode, set); }
Expand Down Expand Up @@ -323,7 +321,6 @@ struct PnFnc
void SetIsDefaultModuleExport(bool set = true) { SetFlags(kFunctionIsDefaultModuleExport, set); }
void SetNestedFuncEscapes(bool set = true) { nestedFuncEscapes = set; }
void SetCanBeDeferred(bool set = true) { canBeDeferred = set; }
void SetFIBPreventsDeferral(bool set = true) { fibPreventsDeferral = set; }

bool CallsEval() const { return HasFlags(kFunctionCallsEval); }
bool ChildCallsEval() const { return HasFlags(kFunctionChildCallsEval); }
Expand Down Expand Up @@ -362,7 +359,6 @@ struct PnFnc
bool IsDefaultModuleExport() const { return HasFlags(kFunctionIsDefaultModuleExport); }
bool NestedFuncEscapes() const { return nestedFuncEscapes; }
bool CanBeDeferred() const { return canBeDeferred; }
bool FIBPreventsDeferral() const { return fibPreventsDeferral; }

size_t LengthInBytes()
{
Expand Down
28 changes: 6 additions & 22 deletions lib/Runtime/ByteCode/ByteCodeEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2993,6 +2993,11 @@ void ByteCodeGenerator::EmitOneFunction(ParseNode *pnode)
deferParseFunction->SetReportedInParamsCount(funcInfo->inArgsCount);
}

if (deferParseFunction->IsDeferred() || deferParseFunction->CanBeDeferred())
{
Js::ScopeInfo::SaveEnclosingScopeInfo(this, funcInfo);
}

if (funcInfo->root->sxFnc.pnodeBody == nullptr)
{
if (!PHASE_OFF1(Js::SkipNestedDeferredPhase))
Expand Down Expand Up @@ -3658,13 +3663,7 @@ void ByteCodeGenerator::EmitScopeList(ParseNode *pnode, ParseNode *breakOnBodySc
}
this->StartEmitFunction(pnode);

// Persist outer func scope info if nested func is deferred
if (CONFIG_FLAG(DeferNested))
{
FuncInfo* parentFunc = TopFuncInfo();
Js::ScopeInfo::SaveScopeInfoForDeferParse(this, parentFunc, funcInfo);
PushFuncInfo(_u("StartEmitFunction"), funcInfo);
}
PushFuncInfo(_u("StartEmitFunction"), funcInfo);

if (paramScope && !paramScope->GetCanMergeWithBodyScope())
{
Expand Down Expand Up @@ -3842,21 +3841,6 @@ void ByteCodeGenerator::StartEmitFunction(ParseNode *pnodeFnc)
// Only set the environment depth if it's truly known (i.e., not in eval or event handler).
funcInfo->GetParsedFunctionBody()->SetEnvDepth(this->envDepth);
}

if (pnodeFnc->sxFnc.FIBPreventsDeferral())
{
for (Scope *scope = this->currentScope; scope; scope = scope->GetEnclosingScope())
{
if (scope->GetScopeType() != ScopeType_FunctionBody &&
scope->GetScopeType() != ScopeType_Global &&
scope->GetScopeType() != ScopeType_GlobalEvalBlock &&
scope->GetMustInstantiate())
{
funcInfo->byteCodeFunction->SetAttributes((Js::FunctionInfo::Attributes)(funcInfo->byteCodeFunction->GetAttributes() & ~Js::FunctionInfo::Attributes::CanDefer));
break;
}
}
}
}

if (funcInfo->GetCallsEval())
Expand Down
149 changes: 92 additions & 57 deletions lib/Runtime/ByteCode/ByteCodeGenerator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -968,84 +968,101 @@ Js::RegSlot ByteCodeGenerator::EnregisterStringTemplateCallsiteConstant(ParseNod
//
// Restore all outer func scope info when reparsing a deferred func.
//
void ByteCodeGenerator::RestoreScopeInfo(Js::ParseableFunctionInfo* functionBody)
void ByteCodeGenerator::RestoreScopeInfo(Js::ScopeInfo *scopeInfo, FuncInfo * func)
{
if (functionBody && functionBody->GetScopeInfo())
if (scopeInfo)
{
PROBE_STACK(scriptContext, Js::Constants::MinStackByteCodeVisitor);

Js::ScopeInfo* scopeInfo = functionBody->GetScopeInfo();
RestoreScopeInfo(scopeInfo->GetParent()); // Recursively restore outer func scope info
Js::ParseableFunctionInfo * pfi = scopeInfo->GetFunctionInfo()->GetParseableFunctionInfo();
bool newFunc = (func == nullptr || func->byteCodeFunction != pfi);

Js::ScopeInfo* paramScopeInfo = scopeInfo->GetParamScopeInfo();
Scope* paramScope = nullptr;
if (paramScopeInfo != nullptr)
if (newFunc)
{
paramScope = paramScopeInfo->GetScope();
Assert(paramScope);
if (!paramScopeInfo->GetCanMergeWithBodyScope())
{
paramScope->SetCannotMergeWithBodyScope();
}
// We need the funcInfo before continuing the restoration of the param scope, so wait for the funcInfo to be created.
}

Scope* bodyScope = scopeInfo->GetScope();

Assert(bodyScope);
bodyScope->SetHasOwnLocalInClosure(scopeInfo->GetHasOwnLocalInClosure());

FuncInfo* func = Anew(alloc, FuncInfo, functionBody->GetDisplayName(), alloc, paramScope, bodyScope, nullptr, functionBody);

if (bodyScope->GetScopeType() == ScopeType_GlobalEvalBlock)
{
func->bodyScope = this->currentScope;
func = Anew(alloc, FuncInfo, pfi->GetDisplayName(), alloc, nullptr, nullptr, nullptr, pfi);
newFunc = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry -- can you explain what you mean?

}
PushFuncInfo(_u("RestoreScopeInfo"), func);

if (!functionBody->DoStackNestedFunc())
{
func->hasEscapedUseNestedFunc = true;
}
// Recursively restore enclosing scope info so outermost scopes/funcs are pushed first.
this->RestoreScopeInfo(scopeInfo->GetParentScopeInfo(), func);
this->RestoreOneScope(scopeInfo, func);

Js::ScopeInfo* funcExprScopeInfo = scopeInfo->GetFuncExprScopeInfo();
if (funcExprScopeInfo)
if (newFunc)
{
Scope* funcExprScope = funcExprScopeInfo->GetScope();
Assert(funcExprScope);
funcExprScope->SetFunc(func);
func->SetFuncExprScope(funcExprScope);
funcExprScopeInfo->GetScopeInfo(nullptr, this, func, funcExprScope);
}

// Restore the param scope after the function expression scope
if (paramScope != nullptr)
{
paramScope->SetFunc(func);
paramScopeInfo->GetScopeInfo(nullptr, this, func, paramScope);
PushFuncInfo(_u("RestoreScopeInfo"), func);
if (!pfi->DoStackNestedFunc())
{
func->hasEscapedUseNestedFunc = true;
}
}
scopeInfo->GetScopeInfo(nullptr, this, func, bodyScope);
}
else
{
Assert(this->TopFuncInfo() == nullptr);
// funcBody is glo
Assert(currentScope == nullptr);
currentScope = Anew(alloc, Scope, alloc, ScopeType_Global);
globalScope = currentScope;

FuncInfo *func = Anew(alloc, FuncInfo, Js::Constants::GlobalFunction,
alloc, nullptr, currentScope, nullptr, functionBody);
PushFuncInfo(_u("RestoreScopeInfo"), func);
if (func == nullptr || !func->byteCodeFunction->GetIsGlobalFunc())
{
func = Anew(alloc, FuncInfo, Js::Constants::GlobalFunction,
alloc, nullptr, nullptr/*currentScope*/, nullptr, nullptr/*functionBody*/);
PushFuncInfo(_u("RestoreScopeInfo"), func);
}
func->SetBodyScope(currentScope);
}
}

void ByteCodeGenerator::RestoreOneScope(Js::ScopeInfo * scopeInfo, FuncInfo * func)
{
TRACE_BYTECODE(_u("\nRestore ScopeInfo: %s #symbols: %d %s\n"),
func->name, scopeInfo->GetSymbolCount(), scopeInfo->IsObject() ? _u("isObject") : _u(""));

Scope * scope = scopeInfo->GetScope();

scope->SetFunc(func);

switch (scope->GetScopeType())
{
case ScopeType_Parameter:
if (!scopeInfo->GetCanMergeWithBodyScope())
{
scope->SetCannotMergeWithBodyScope();
}
Assert(func->GetParamScope() == nullptr);
func->SetParamScope(scope);
break;

case ScopeType_FuncExpr:
Assert(func->GetFuncExprScope() == nullptr);
func->SetFuncExprScope(scope);
break;

case ScopeType_FunctionBody:
case ScopeType_GlobalEvalBlock:
Assert(func->GetBodyScope() == nullptr || (func->GetBodyScope()->GetScopeType() == ScopeType_Global && scope->GetScopeType() == ScopeType_GlobalEvalBlock));
func->SetBodyScope(scope);
func->SetHasCachedScope(scopeInfo->IsCached());
break;
}

Assert(!scopeInfo->IsCached() || scope == func->GetBodyScope());

// scopeInfo->scope was created/saved during parsing.
// We no longer need it by now.
// Clear it to avoid GC false positive (arena memory later used by GC).
scopeInfo->SetScope(nullptr);
this->PushScope(scope);
}

FuncInfo * ByteCodeGenerator::StartBindGlobalStatements(ParseNode *pnode)
{
if (parentScopeInfo && parentScopeInfo->GetParent() && (!parentScopeInfo->GetParent()->GetIsGlobalFunc() || parentScopeInfo->GetParent()->IsEval()))
if (parentScopeInfo)
{
Assert(CONFIG_FLAG(DeferNested));
trackEnvDepth = true;
RestoreScopeInfo(parentScopeInfo->GetParent());
RestoreScopeInfo(parentScopeInfo, nullptr);
trackEnvDepth = false;
// "currentScope" is the parentFunc scope. This ensures the deferred func declaration
// symbol will bind to the func declaration symbol already available in parentFunc scope.
Expand Down Expand Up @@ -1211,7 +1228,7 @@ FuncInfo * ByteCodeGenerator::StartBindFunction(const char16 *name, uint nameLen
if (parsedFunctionBody->GetScopeInfo())
{
// Propagate flags from the (real) parent function.
Js::ParseableFunctionInfo *parent = parsedFunctionBody->GetScopeInfo()->GetParent();
Js::ParseableFunctionInfo *parent = parsedFunctionBody->GetScopeInfo()->GetParseableFunctionInfo();
if (parent)
{
if (parent->GetHasOrParentHasArguments())
Expand Down Expand Up @@ -1632,7 +1649,8 @@ Symbol * ByteCodeGenerator::FindSymbol(Symbol **symRef, IdentPtr pid, bool forRe

bool didTransferToFncVarSym = false;

if (!PHASE_OFF(Js::OptimizeBlockScopePhase, top->byteCodeFunction) &&
#pragma prefast(suppress:6237, "The right hand side condition does not have any side effects.")
if (PHASE_ON(Js::OptimizeBlockScopePhase, top->byteCodeFunction) &&
sym->GetIsBlockVar() &&
!sym->GetScope()->IsBlockInLoop() &&
sym->GetSymbolType() == STFunction)
Expand Down Expand Up @@ -1755,7 +1773,7 @@ Symbol * ByteCodeGenerator::AddSymbolToScope(Scope *scope, const char16 *key, in
// on such compiles, so we essentially have to migrate the symbol to the new scope.
// We check fscrEvalCode, not fscrEval, because the same thing can happen in indirect eval,
// when fscrEval is not set.
Assert(scope->GetScopeType() == ScopeType_Global);
Assert(scope->GetScopeType() == ScopeType_Global || scope->GetScopeType() == ScopeType_GlobalEvalBlock);
scope->AddNewSymbol(sym);
}

Expand Down Expand Up @@ -1995,7 +2013,7 @@ void ByteCodeGenerator::Generate(__in ParseNode *pnode, uint32 grfscr, __in Byte

void ByteCodeGenerator::CheckDeferParseHasMaybeEscapedNestedFunc()
{
if (!this->parentScopeInfo || (this->parentScopeInfo->GetParent() && this->parentScopeInfo->GetParent()->GetIsGlobalFunc()))
if (!this->parentScopeInfo)
{
return;
}
Expand Down Expand Up @@ -2023,7 +2041,7 @@ void ByteCodeGenerator::CheckDeferParseHasMaybeEscapedNestedFunc()
else
{
// We have to wait until it is parsed before we populate the stack nested func parent.
FuncInfo * parentFunc = top->GetBodyScope()->GetEnclosingFunc();
FuncInfo * parentFunc = top->GetParamScope() ? top->GetParamScope()->GetEnclosingFunc() : top->GetBodyScope()->GetEnclosingFunc();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

top->GetParamScope()->GetEnclosingFunc() : top->GetBodyScope()->GetEnclosingFunc() [](start = 55, length = 82)

Aren't these two the same?

if (!parentFunc->IsGlobalFunction())
{
Assert(parentFunc->byteCodeFunction != rootFuncBody);
Expand All @@ -2040,6 +2058,11 @@ void ByteCodeGenerator::CheckDeferParseHasMaybeEscapedNestedFunc()
FuncInfo * funcInfo = i.Data();
Assert(funcInfo->IsRestored());
Js::ParseableFunctionInfo * parseableFunctionInfo = funcInfo->byteCodeFunction;
if (parseableFunctionInfo == nullptr)
{
Assert(funcInfo->GetBodyScope() && funcInfo->GetBodyScope()->GetScopeType() == ScopeType_Global);
return;
}
bool didStackNestedFunc = parseableFunctionInfo->DoStackNestedFunc();
if (!didStackNestedFunc)
{
Expand Down Expand Up @@ -3247,6 +3270,7 @@ void AddFunctionsToScope(ParseNodePtr scope, ByteCodeGenerator * byteCodeGenerat
sym->GetScope() != sym->GetScope()->GetFunc()->GetParamScope())
{
sym->SetIsBlockVar(true);
sym->SetHasRealBlockVarRef(true);
}
}
});
Expand Down Expand Up @@ -3629,15 +3653,21 @@ void PostVisitBlock(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator)
return;
}

Scope *scope = pnode->sxBlock.scope;

if (pnode->sxBlock.GetCallsEval() || pnode->sxBlock.GetChildCallsEval() || (byteCodeGenerator->GetFlags() & (fscrEval | fscrImplicitThis | fscrImplicitParents)))
{
Scope *scope = pnode->sxBlock.scope;
bool scopeIsEmpty = scope->IsEmpty();
scope->SetIsObject();
scope->SetCapturesAll(true);
scope->SetMustInstantiate(!scopeIsEmpty);
}

if (scope->GetHasOwnLocalInClosure())
{
byteCodeGenerator->ProcessScopeWithCapturedSym(scope);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

byteCodeGenerator->ProcessScopeWithCapturedSym(scope); [](start = 8, length = 54)

Why do we have to do this here? Doesn't this happen when we find the captured symbol itself? Or is it to cover the case where the scope is recreated from the ScopeInfo so we have to mark it again? If that is the case should we do this while Restoring the scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been doing this when we find the capturing reference to the symbol. Now that reference may be deferred, so byte code gen won't see it. It would be great to go through and remove the calls at the points where they're no longer needed, but I'm not doing that cleanup as part of this change.

}

byteCodeGenerator->PopScope();
byteCodeGenerator->PopBlock();

Expand Down Expand Up @@ -3686,6 +3716,11 @@ void PreVisitCatch(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator)

void PostVisitCatch(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator)
{
Scope *scope = pnode->sxCatch.scope;
if (scope->GetHasOwnLocalInClosure())
{
byteCodeGenerator->ProcessScopeWithCapturedSym(scope);
}
byteCodeGenerator->EndBindCatch();
}

Expand Down
Loading