[1.8>1.9] [MERGE #4673 @boingoing] OS#14115684: Function-in-block sem…

…antics don't work for defer parse Merge pull request #4673 from boingoing:DeferFunctionInBlock In below code repro, we do not create var decl binding for `foo` inside the `nest` function in defer-parse mode. Due to this, we leave the reference to `foo` in `nest` on the pid ref stack and bind it to the formal `foo` parameter from the `test` function. This leads us to think formal `foo` is captured and try to put it into a scope slot. However, formal `foo` is not actually captured so no scope slot is allocated. When we actually generate bytecode for `test`, we hit a fail fast. ```js function test(foo) { function nest() { { function foo() { console.log('pass'); } } foo(); } nest(); } test(()=>console.log('fail')); ``` Fix is to create the var binding for `foo` at the function declaration for `foo` even in defer-parse mode. Fixes: https://microsoft.visualstudio.com/web/wi.aspx?id=14115684
chakra-core · Feb 13, 2018 · c6c0ff7 · c6c0ff7
2 parents db6afe6 + 2d349e1
commit c6c0ff7
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 13 deletions.
diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp
@@ -1631,7 +1631,7 @@ ParseNodePtr Parser::ParseBlock(LabelId* pLabelId)
         && outerBlockInfo->pnodeBlock->sxBlock.scope != nullptr
         && outerBlockInfo->pnodeBlock->sxBlock.scope->GetScopeType() == ScopeType_CatchParamPattern)
     {
-        // If we are parsing the catch block then destructured params can have let declrations. Let's add them to the new block.
+        // If we are parsing the catch block then destructured params can have let declarations. Let's add them to the new block.
         for (ParseNodePtr pnode = m_currentBlockInfo->pBlockInfoOuter->pnodeBlock->sxBlock.pnodeLexVars; pnode; pnode = pnode->sxVar.pnodeNext)
         {
             PidRefStack* ref = PushPidRef(pnode->sxVar.sym->GetPid());
@@ -1658,7 +1658,6 @@ ParseNodePtr Parser::ParseBlock(LabelId* pLabelId)
 
     ChkCurTok(tkRCurly, ERRnoRcurly);
 
-
     return pnodeBlock;
 }
 
@@ -5086,8 +5085,9 @@ ParseNodePtr Parser::ParseFncDecl(ushort flags, LPCOLESTR pNameHint, const bool
     pnodeFnc->sxFnc.SetIsClassConstructor((flags & fFncClassConstructor) != 0);
     pnodeFnc->sxFnc.SetIsBaseClassConstructor((flags & fFncBaseClassConstructor) != 0);
 
+    IdentPtr pFncNamePid = nullptr;
     bool needScanRCurly = true;
-    bool result = ParseFncDeclHelper<buildAST>(pnodeFnc, pNameHint, flags, &funcHasName, fUnaryOrParen, noStmtContext, &needScanRCurly, fModule);
+    bool result = ParseFncDeclHelper<buildAST>(pnodeFnc, pNameHint, flags, &funcHasName, fUnaryOrParen, noStmtContext, &needScanRCurly, fModule, &pFncNamePid);
     if (!result)
     {
         Assert(!pnodeFncBlockScope);
@@ -5170,9 +5170,10 @@ ParseNodePtr Parser::ParseFncDecl(ushort flags, LPCOLESTR pNameHint, const bool
 
     m_scopeCountNoAst = scopeCountNoAstSave;
 
-    if (buildAST && fDeclaration && !IsStrictMode())
+    if (fDeclaration && !IsStrictMode())
     {
-        if (pnodeFnc->sxFnc.pnodeName != nullptr && pnodeFnc->sxFnc.pnodeName->nop == knopVarDecl &&
+        if (pFncNamePid != nullptr &&
+            GetCurrentBlock() &&
             GetCurrentBlock()->sxBlock.blockType == PnodeBlockType::Regular)
         {
             // Add a function-scoped VarDecl with the same name as the function for
@@ -5181,9 +5182,9 @@ ParseNodePtr Parser::ParseFncDecl(ushort flags, LPCOLESTR pNameHint, const bool
             // level and we accomplish this by having each block scoped function
             // declaration assign to both the block scoped "let" binding, as well
             // as the function scoped "var" binding.
-            ParseNodePtr vardecl = CreateVarDeclNode(pnodeFnc->sxFnc.pnodeName->sxVar.pid, STVariable, false, nullptr, false);
+            ParseNodePtr vardecl = CreateVarDeclNode(pFncNamePid, STVariable, false, nullptr, false);
             vardecl->sxVar.isBlockScopeFncDeclVar = true;
-            if (vardecl->sxVar.sym->GetIsFormal())
+            if (GetCurrentFunctionNode() && vardecl->sxVar.sym->GetIsFormal())
             {
                 GetCurrentFunctionNode()->sxFnc.SetHasAnyWriteToFormals(true);
             }
@@ -5266,7 +5267,7 @@ void Parser::AppendFunctionToScopeList(bool fDeclaration, ParseNodePtr pnodeFnc)
 Parse a function definition.
 ***************************************************************************/
 template<bool buildAST>
-bool Parser::ParseFncDeclHelper(ParseNodePtr pnodeFnc, LPCOLESTR pNameHint, ushort flags, bool *pHasName, bool fUnaryOrParen, bool noStmtContext, bool *pNeedScanRCurly, bool skipFormals)
+bool Parser::ParseFncDeclHelper(ParseNodePtr pnodeFnc, LPCOLESTR pNameHint, ushort flags, bool *pHasName, bool fUnaryOrParen, bool noStmtContext, bool *pNeedScanRCurly, bool skipFormals, IdentPtr* pFncNamePid)
 {
     ParseNodePtr pnodeFncParent = GetCurrentFunctionNode();
     // is the following correct? When buildAST is false, m_currentNodeDeferredFunc can be nullptr on transition to deferred parse from non-deferred
@@ -5282,6 +5283,7 @@ bool Parser::ParseFncDeclHelper(ParseNodePtr pnodeFnc, LPCOLESTR pNameHint, usho
     StmtNest *pstmtSave;
     ParseNodePtr *lastNodeRef = nullptr;
     bool fFunctionInBlock = false;
+
     if (buildAST)
     {
         fFunctionInBlock = GetCurrentBlockInfo() != GetCurrentFunctionBlockInfo() &&
@@ -5309,7 +5311,7 @@ bool Parser::ParseFncDeclHelper(ParseNodePtr pnodeFnc, LPCOLESTR pNameHint, usho
         this->UpdateCurrentNodeFunc<buildAST>(pnodeFnc, fLambda);
     }
 
-    *pHasName = !fLambda && !fModule && this->ParseFncNames<buildAST>(pnodeFnc, pnodeFncSave, flags, &lastNodeRef);
+    *pHasName = !fLambda && !fModule && this->ParseFncNames<buildAST>(pnodeFnc, pnodeFncSave, flags, &lastNodeRef, pFncNamePid);
 
     if (fDeclaration)
     {
@@ -6409,7 +6411,7 @@ void Parser::ParseNestedDeferredFunc(ParseNodePtr pnodeFnc, bool fLambda, bool *
 }
 
 template<bool buildAST>
-bool Parser::ParseFncNames(ParseNodePtr pnodeFnc, ParseNodePtr pnodeFncParent, ushort flags, ParseNodePtr **pLastNodeRef)
+bool Parser::ParseFncNames(ParseNodePtr pnodeFnc, ParseNodePtr pnodeFncParent, ushort flags, ParseNodePtr **pLastNodeRef, IdentPtr* pFncNamePid)
 {
     BOOL fDeclaration = flags & fFncDeclaration;
     BOOL fIsAsync = flags & fFncAsync;
@@ -6499,7 +6501,6 @@ bool Parser::ParseFncNames(ParseNodePtr pnodeFnc, ParseNodePtr pnodeFncParent, u
 
     ichMinNames = m_pscan->IchMinTok();
 
-
     Assert(m_token.tk == tkID || (m_token.tk == tkYIELD && !fDeclaration));
 
     if (IsStrictMode())
@@ -6517,6 +6518,11 @@ bool Parser::ParseFncNames(ParseNodePtr pnodeFnc, ParseNodePtr pnodeFncParent, u
     pnodeT->ichMin = ichMinBase;
     pnodeT->ichLim = ichLimBase;
 
+    if (pFncNamePid != nullptr)
+    {
+        *pFncNamePid = pidBase;
+    }
+
     if (fDeclaration &&
         pnodeFncParent &&
         pnodeFncParent->sxFnc.pnodeName &&

diff --git a/lib/Parser/Parse.h b/lib/Parser/Parse.h
@@ -795,9 +795,9 @@ class Parser
     template<bool buildAST> void ParseComputedName(ParseNodePtr* ppnodeName, LPCOLESTR* ppNameHint, LPCOLESTR* ppFullNameHint = nullptr, uint32 *pNameLength = nullptr, uint32 *pShortNameOffset = nullptr);
     template<bool buildAST> ParseNodePtr ParseMemberGetSet(OpCode nop, LPCOLESTR* ppNameHint);
     template<bool buildAST> ParseNodePtr ParseFncDecl(ushort flags, LPCOLESTR pNameHint = NULL, const bool needsPIDOnRCurlyScan = false, bool resetParsingSuperRestrictionState = true, bool fUnaryOrParen = false);
-    template<bool buildAST> bool ParseFncNames(ParseNodePtr pnodeFnc, ParseNodePtr pnodeFncParent, ushort flags, ParseNodePtr **pLastNodeRef);
+    template<bool buildAST> bool ParseFncNames(ParseNodePtr pnodeFnc, ParseNodePtr pnodeFncParent, ushort flags, ParseNodePtr **pLastNodeRef, IdentPtr* pFncNamePid = nullptr);
     template<bool buildAST> void ParseFncFormals(ParseNodePtr pnodeFnc, ParseNodePtr pnodeParentFnc, ushort flags, bool isTopLevelDeferredFunc = false);
-    template<bool buildAST> bool ParseFncDeclHelper(ParseNodePtr pnodeFnc, LPCOLESTR pNameHint, ushort flags, bool *pHasName, bool fUnaryOrParen, bool noStmtContext, bool *pNeedScanRCurly, bool skipFormals = false);
+    template<bool buildAST> bool ParseFncDeclHelper(ParseNodePtr pnodeFnc, LPCOLESTR pNameHint, ushort flags, bool *pHasName, bool fUnaryOrParen, bool noStmtContext, bool *pNeedScanRCurly, bool skipFormals = false, IdentPtr* pFncNamePid = nullptr);
     template<bool buildAST> void ParseExpressionLambdaBody(ParseNodePtr pnodeFnc);
     template<bool buildAST> void UpdateCurrentNodeFunc(ParseNodePtr pnodeFnc, bool fLambda);
     bool FncDeclAllowedWithoutContext(ushort flags);

diff --git a/test/Bugs/bug_OS14115684.js b/test/Bugs/bug_OS14115684.js
@@ -0,0 +1,17 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+function test(foo) {
+    function nest() {
+        {
+            function foo() {
+                console.log('pass');
+            }
+        }
+        foo();
+    }
+    nest();
+}
+test(()=>console.log('fail'));
diff --git a/test/Bugs/rlexe.xml b/test/Bugs/rlexe.xml
@@ -445,4 +445,11 @@
       <files>bug14057294.js</files>
     </default>
   </test>
+  <test>
+    <default>
+      <files>bug_OS14115684.js</files>
+      <tags>BugFix</tags>
+      <compile-flags>-forceundodefer</compile-flags>
+    </default>
+  </test>
 </regress-exe>