[MERGE #2666 @pleath] Defer functions enclosed in all contexts.

Merge pull request #2666 from pleath:deferfib

Support deferral of functions enclosed in scopes other than function body and function expression scope -- in particular, ES6-style lexical scopes and parameter scopes. This requires changing ScopeInfo so that it is associated with a single scope rather than a function body. Instead of carrying per-function information for body/var scope, parameter scope, and function expression scope, ScopeInfo will describe one scope, with a value identifying the scope type, a pointer to the FunctionInfo that contains it, and a pointer to the enclosing ScopeInfo. A FunctionProxy will point to the ScopeInfo that is the nearest enclosing scope. At parse time, we will reconstitute the closure environment by walking the list of enclosing ScopeInfo's. The code that allowed redeferral to work around the context limitation of deferring parsing is deleted, and the OptimizeBlockScope feature is off by default, as it doesn't play well with this new logic and isn't required to make (re-)deferral effective. (We will want to revisit it, though, so I've left it there.)

Author: pleath

lib/Parser/Hash.h

@@ -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;

lib/Parser/Parse.cpp

@@ -6,6 +6,11 @@
 
 #include "ParseFlags.h"
 
+namespace Js
+{
+    class ScopeInfo;
+};
+
 // Operator precedence levels
 enum
 {
@@ -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;
@@ -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);
@@ -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);
 
     /***********************************************************************

lib/Parser/Parse.h

@@ -247,7 +247,6 @@ struct PnFnc
     RestorePoint *pRestorePoint;
     DeferredFunctionStub *deferredStub;
     bool canBeDeferred;
-    bool fibPreventsDeferral;
 
     static const int32 MaxStackClosureAST = 800000;
 
@@ -286,7 +285,6 @@ struct PnFnc
     {
         fncFlags = kFunctionNone;
         canBeDeferred = false;
-        fibPreventsDeferral = false;
     }
 
     void SetAsmjsMode(bool set = true) { SetFlags(kFunctionAsmjsMode, set); }
@@ -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); }
@@ -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()
     {

lib/Parser/ptree.h

@@ -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))
@@ -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())
                 {
@@ -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())

lib/Runtime/ByteCode/ByteCodeEmitter.cpp

@@ -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;
         }
-        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.
@@ -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())
@@ -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)
@@ -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);
     }
 
@@ -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;
     }
@@ -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();
         if (!parentFunc->IsGlobalFunction())
         {
             Assert(parentFunc->byteCodeFunction != rootFuncBody);
@@ -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)
         {
@@ -3247,6 +3270,7 @@ void AddFunctionsToScope(ParseNodePtr scope, ByteCodeGenerator * byteCodeGenerat
                 sym->GetScope() != sym->GetScope()->GetFunc()->GetParamScope())
             {
                 sym->SetIsBlockVar(true);
+                sym->SetHasRealBlockVarRef(true);
             }
         }
     });
@@ -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);
+    }
+
     byteCodeGenerator->PopScope();
     byteCodeGenerator->PopBlock();
 
@@ -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();
 }