[MERGE #5545 @boingoing] Support deferred stubs for functions declare…

…d in parameter scope Merge pull request #5545 from boingoing:deferredstubsforparameterscope We previously skipped creating deferred function stubs for functions declared in the parameter scope. This throws-off the order of nested functions under functions with children defined in the parameter scope. We worked-around this by not using deferred stubs in parents with default arguments. Add support for creating deferred stubs and using them when parsing function parameter lists.
chakra-core · Jul 27, 2018 · b975dfa · b975dfa
2 parents 9bd7d4b + 2f88043
commit b975dfa
Show file tree

Hide file tree

Showing 4 changed files with 138 additions and 57 deletions.
diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp
@@ -5405,10 +5405,7 @@ void Parser::ParseFncDeclHelper(ParseNodeFnc * pnodeFnc, LPCOLESTR pNameHint, us
                 m_reparsingLambdaParams = true;
             }
 
-            DeferredFunctionStub* savedDeferredStub = m_currDeferredStub;
-            m_currDeferredStub = nullptr;
             this->ParseFncFormals<buildAST>(pnodeFnc, pnodeFncParent, flags, isTopLevelDeferredFunc);
-            m_currDeferredStub = savedDeferredStub;
 
             m_reparsingLambdaParams = fLambdaParamsSave;
         }
@@ -5822,7 +5819,7 @@ void Parser::ParseTopLevelDeferredFunc(ParseNodeFnc * pnodeFnc, ParseNodeFnc * p
     {
         ParseExpressionLambdaBody<false>(pnodeFnc, fAllowIn);
     }
-    else if (pnodeFncParent != nullptr && m_currDeferredStub != nullptr && !pnodeFncParent->HasDefaultArguments())
+    else if (pnodeFncParent != nullptr && m_currDeferredStub != nullptr)
     {
         // We've already parsed this function body for syntax errors on the initial parse of the script.
         // We have information that allows us to skip it, so do so.
@@ -13953,43 +13950,13 @@ bool Parser::IsCreatingStateCache()
         && CONFIG_FLAG(ParserStateCache));
 }
 
-DeferredFunctionStub * Parser::BuildDeferredStubTree(ParseNodeFnc *pnodeFnc, Recycler *recycler)
+uint Parser::BuildDeferredStubTreeHelper(ParseNodeBlock* pnodeBlock, DeferredFunctionStub* deferredStubs, uint currentStubIndex, uint deferredStubCount, Recycler *recycler)
 {
-    Assert(CONFIG_FLAG(ParserStateCache));
-
-    uint nestedCount = pnodeFnc->nestedCount;
-    if (nestedCount == 0)
-    {
-        return nullptr;
-    }
+    Assert(pnodeBlock != nullptr
+        && (pnodeBlock->blockType == PnodeBlockType::Function
+            || pnodeBlock->blockType == PnodeBlockType::Parameter));
 
-    if (pnodeFnc->deferredStub)
-    {
-        return pnodeFnc->deferredStub;
-    }
-
-    DeferredFunctionStub* deferredStubs = RecyclerNewArray(recycler, DeferredFunctionStub, nestedCount);
-    uint i = 0;
-    ParseNodeBlock* pnodeBlock = pnodeFnc->pnodeBodyScope;
-    ParseNodePtr pnodeChild = nullptr;
-
-    if (pnodeFnc->nop == knopProg)
-    {
-        Assert(pnodeFnc->pnodeBodyScope == nullptr
-            && pnodeFnc->pnodeScopes != nullptr
-            && pnodeFnc->pnodeScopes->blockType == PnodeBlockType::Global);
-
-        pnodeBlock = pnodeFnc->pnodeScopes;
-        pnodeChild = pnodeFnc->pnodeScopes->pnodeScopes;
-    }
-    else
-    {
-        Assert(pnodeBlock != nullptr
-            && (pnodeBlock->blockType == PnodeBlockType::Function
-                || pnodeBlock->blockType == PnodeBlockType::Parameter));
-
-        pnodeChild = pnodeBlock->pnodeScopes;
-    }
+    ParseNodePtr pnodeChild = pnodeBlock->pnodeScopes;
 
     while (pnodeChild != nullptr)
     {
@@ -14004,37 +13971,56 @@ DeferredFunctionStub * Parser::BuildDeferredStubTree(ParseNodeFnc *pnodeFnc, Rec
         }
 
         ParseNodeFnc* pnodeFncChild = pnodeChild->AsParseNodeFnc();
-        AnalysisAssertOrFailFast(i < nestedCount);
-
-        if (pnodeFncChild->pnodeBody != nullptr)
-        {
-            // Anomalous case of a non-deferred function nested within a deferred one.
-            // Work around by discarding the stub tree.
-            return nullptr;
-        }
+        AnalysisAssertOrFailFast(currentStubIndex < deferredStubCount);
+        Assert(pnodeFncChild->pnodeBody == nullptr);
 
         if (pnodeFncChild->IsGeneratedDefault())
         {
-            ++i;
+            ++currentStubIndex;
             pnodeChild = pnodeFncChild->pnodeNext;
             continue;
         }
 
-        deferredStubs[i].fncFlags = pnodeFncChild->fncFlags;
-        deferredStubs[i].nestedCount = pnodeFncChild->nestedCount;
-        deferredStubs[i].restorePoint = *pnodeFncChild->pRestorePoint;
-        deferredStubs[i].deferredStubs = BuildDeferredStubTree(pnodeFncChild, recycler);
-        deferredStubs[i].ichMin = pnodeChild->ichMin;
+        deferredStubs[currentStubIndex].fncFlags = pnodeFncChild->fncFlags;
+        deferredStubs[currentStubIndex].nestedCount = pnodeFncChild->nestedCount;
+        deferredStubs[currentStubIndex].restorePoint = *pnodeFncChild->pRestorePoint;
+        deferredStubs[currentStubIndex].deferredStubs = BuildDeferredStubTree(pnodeFncChild, recycler);
+        deferredStubs[currentStubIndex].ichMin = pnodeChild->ichMin;
 
         // Save the set of captured names onto the deferred stub.
         // Since this set is allocated in the Parser arena, we'll have to convert these
         // into indices in a string table which will survive when the parser goes away.
-        deferredStubs[i].capturedNamePointers = pnodeFncChild->GetCapturedNames();
+        deferredStubs[currentStubIndex].capturedNamePointers = pnodeFncChild->GetCapturedNames();
 
-        ++i;
+        ++currentStubIndex;
         pnodeChild = pnodeFncChild->pnodeNext;
     }
 
+    return currentStubIndex;
+}
+
+DeferredFunctionStub * Parser::BuildDeferredStubTree(ParseNodeFnc *pnodeFnc, Recycler *recycler)
+{
+    Assert(CONFIG_FLAG(ParserStateCache));
+
+    uint nestedCount = pnodeFnc->nestedCount;
+    if (nestedCount == 0)
+    {
+        return nullptr;
+    }
+
+    if (pnodeFnc->deferredStub)
+    {
+        return pnodeFnc->deferredStub;
+    }
+
+    DeferredFunctionStub* deferredStubs = RecyclerNewArray(recycler, DeferredFunctionStub, nestedCount);
+
+    uint currentStubIndex = BuildDeferredStubTreeHelper(pnodeFnc->pnodeScopes, deferredStubs, 0, nestedCount, recycler);
+    currentStubIndex = BuildDeferredStubTreeHelper(pnodeFnc->pnodeBodyScope, deferredStubs, currentStubIndex, nestedCount, recycler);
+
+    Assert(currentStubIndex == nestedCount);
+
     pnodeFnc->deferredStub = deferredStubs;
     return deferredStubs;
 }
diff --git a/lib/Parser/Parse.h b/lib/Parser/Parse.h
@@ -272,6 +272,8 @@ class Parser
         SourceContextInfo * sourceContextInfo, Js::ParseableFunctionInfo* functionInfo);
 
 protected:
+    static uint BuildDeferredStubTreeHelper(ParseNodeBlock* pnodeBlock, DeferredFunctionStub* deferredStubs, uint currentStubIndex, uint deferredStubCount, Recycler *recycler);
+
     HRESULT ParseSourceInternal(
         __out ParseNodeProg ** parseTree, LPCUTF8 pszSrc, size_t offsetInBytes,
         size_t lengthInCodePoints, charcount_t offsetInChars, bool isUtf8,

diff --git a/test/Bugs/deferredStubBugs.js b/test/Bugs/deferredStubBugs.js
@@ -0,0 +1,86 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+let pass = 'pass';
+let fail = 'fail';
+
+function func4(a = 123) {
+    function v8() {
+        function v9() {
+            return v9;
+        }
+        return v9();
+    }
+    return v8();
+}
+func4();
+
+
+var func5 = (a = 123) => (function v6() {
+    function v7() {
+        return v7;
+    }
+    return v7();
+})()
+func5();
+
+function func6(a = v => { console.log('pass'); }, b = v => { return a; }) {
+    function c() {
+        return b();
+    }
+    return c();
+}
+func6()();
+
+function func7(a, b = function() { return pass; }, c) {
+    function func8(d, e = function() { return b; }, f) {
+        return e;
+    }
+    return func8();
+}
+console.log(func7()()());
+
+var func9 = (a, b = () => pass, c) => {
+    var func10 = (d, e = () => b, f) => {
+        return e;
+    }
+    return func10();
+}
+console.log(func9()()());
+
+var func11 = (a, b = () => { return pass; }, c) => {
+    var func12 = (d, e = () => { return b; }, f) => {
+        return e;
+    }
+    return func12();
+}
+console.log(func11()()());
+
+function func13(a = (b = () => pass, c = () => fail) => b()) {
+    return a();
+}
+console.log(func13());
+
+function func14(a = (b = () => { return fail; }, c = () => { return pass; }) => { return c(); }) {
+    return a();
+}
+console.log(func14());
+
+function func15(a = class A { meth() { return pass } static meth2() { return fail; } }, b = v => fail, c = (v) => { return fail }, d = fail) {
+    return new a().meth();
+}
+console.log(func15());
+function func16(a = class A { meth() { return fail } static meth2() { return pass; } }, b = v => fail, c = (v) => { return fail }, d = fail) {
+    return a.meth2();
+}
+console.log(func16());
+function func17(a = class A { meth() { return fail } static meth2() { return fail; } }, b = v => pass, c = (v) => { return fail }, d = fail) {
+    return b();
+}
+console.log(func17());
+function func18(a = class A { meth() { return fail } static meth2() { return fail; } }, b = v => fail, c = (v) => { return pass }, d = fail) {
+    return c();
+}
+console.log(func18());
diff --git a/test/Bugs/rlexe.xml b/test/Bugs/rlexe.xml
@@ -488,7 +488,7 @@
     <default>
       <files>bug17785360.js</files>
     </default>
-  </test> 
+  </test>
   <test>
     <default>
       <files>bug_OS17530048.js</files>
@@ -510,7 +510,7 @@
     <default>
       <files>OS_17745531.js</files>
     </default>
-  </test>   
+  </test>
   <test>
     <default>
       <files>SuperUndoDeferIssue.js</files>
@@ -523,4 +523,11 @@
       <compile-flags>-force:cachedScope</compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>deferredStubBugs.js</files>
+      <tags>exclude_jshost</tags>
+      <compile-flags>-force:deferparse -parserstatecache -useparserstatecache</compile-flags>
+    </default>
+  </test>
 </regress-exe>