[MERGE chakra-core#3643 @pleath] Clean up several unused/ineffectual flags in FE structures.

pleath · pleath · commit f8307d223aa2 · 2017-09-05T17:35:58.000-07:00
Merge pull request chakra-core#3643 from pleath:pidcleanup Delete code that exposed more redeferral candidates in RS2 but is not needed now. Stop creating a node and scope to hold the name of a function expression if we're parsing an arrow function.
diff --git a/lib/Parser/Hash.h b/lib/Parser/Hash.h
@@ -31,19 +31,6 @@ enum
     fidKwdRsvd      = 0x0001,     // the keyword is a reserved word
     fidKwdFutRsvd   = 0x0002,     // a future reserved word, but only in strict mode
 
-    // Flags to identify tracked aliases of "eval"
-    fidEval         = 0x0008,
-    // Flags to identify tracked aliases of "let"
-    fidLetOrConst   = 0x0010,     // ID has previously been used in a block-scoped declaration
-
-    // This flag is used by the Parser CountDcls and FillDcls methods.
-    // CountDcls sets the bit as it walks through the var decls so that
-    // it can skip duplicates. FillDcls clears the bit as it walks through
-    // again to skip duplicates.
-    fidGlobalDcl    = 0x2000,
-
-    fidUsed         = 0x4000,  // name referenced by source code
-
     fidModuleExport = 0x8000    // name is module export
 };
 
@@ -314,12 +301,6 @@ struct Ident
     Js::PropertyId GetPropertyId() const { return m_propertyId; }
     void SetPropertyId(Js::PropertyId id) { m_propertyId = id; }
 
-    void SetIsEval() { m_grfid |= fidEval; }
-    BOOL GetIsEval() const { return m_grfid & fidEval; }
-
-    void SetIsLetOrConst() { m_grfid |= fidLetOrConst; }
-    BOOL GetIsLetOrConst() const { return m_grfid & fidLetOrConst; }
-
     void SetIsModuleExport() { m_grfid |= fidModuleExport; }
     BOOL GetIsModuleExport() const { return m_grfid & fidModuleExport; }
 
diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp
@@ -730,31 +730,25 @@ ParseNodePtr Parser::CreateNodeT(charcount_t ichMin,charcount_t ichLim)
     return pnode;
 }
 
-ParseNodePtr Parser::CreateDeclNode(OpCode nop, IdentPtr pid, SymbolType symbolType, bool errorOnRedecl, bool *isRedecl)
+ParseNodePtr Parser::CreateDeclNode(OpCode nop, IdentPtr pid, SymbolType symbolType, bool errorOnRedecl)
 {
     ParseNodePtr pnode = CreateNode(nop);
 
     pnode->sxVar.InitDeclNode(pid, NULL);
 
     if (symbolType != STUnknown)
     {
-        pnode->sxVar.sym = AddDeclForPid(pnode, pid, symbolType, errorOnRedecl, isRedecl);
+        pnode->sxVar.sym = AddDeclForPid(pnode, pid, symbolType, errorOnRedecl);
     }
 
     return pnode;
 }
 
-Symbol* Parser::AddDeclForPid(ParseNodePtr pnode, IdentPtr pid, SymbolType symbolType, bool errorOnRedecl, bool *isRedecl)
+Symbol* Parser::AddDeclForPid(ParseNodePtr pnode, IdentPtr pid, SymbolType symbolType, bool errorOnRedecl)
 {
     Assert(pnode->IsVarLetOrConst());
 
     PidRefStack *refForUse = nullptr, *refForDecl = nullptr;
-
-    if (isRedecl)
-    {
-        *isRedecl = false;
-    }
-
     BlockInfoStack *blockInfo;
     bool fBlockScope = false;
     if (pnode->nop != knopVarDecl || symbolType == STFunction)
@@ -813,10 +807,6 @@ Symbol* Parser::AddDeclForPid(ParseNodePtr pnode, IdentPtr pid, SymbolType symbo
     Symbol *sym = refForDecl->GetSym();
     if (sym != nullptr)
     {
-        if (isRedecl)
-        {
-            *isRedecl = true;
-        }
         // Multiple declarations in the same scope. 3 possibilities: error, existing one wins, new one wins.
         switch (pnode->nop)
         {
@@ -1385,9 +1375,9 @@ ParseNodePtr Parser::CreateModuleImportDeclNode(IdentPtr localName)
     return declNode;
 }
 
-ParseNodePtr Parser::CreateVarDeclNode(IdentPtr pid, SymbolType symbolType, bool autoArgumentsObject, ParseNodePtr pnodeFnc, bool errorOnRedecl, bool *isRedecl)
+ParseNodePtr Parser::CreateVarDeclNode(IdentPtr pid, SymbolType symbolType, bool autoArgumentsObject, ParseNodePtr pnodeFnc, bool errorOnRedecl)
 {
-    ParseNodePtr pnode = CreateDeclNode(knopVarDecl, pid, symbolType, errorOnRedecl, isRedecl);
+    ParseNodePtr pnode = CreateDeclNode(knopVarDecl, pid, symbolType, errorOnRedecl);
 
     // Append the variable to the end of the current variable list.
     AssertMem(m_ppnodeVar);
@@ -1413,7 +1403,6 @@ ParseNodePtr Parser::CreateBlockScopedDeclNode(IdentPtr pid, OpCode nodeType)
     if (nullptr != pid)
     {
         AssertMem(pid);
-        pid->SetIsLetOrConst();
         AddVarDeclToBlock(pnode);
         CheckPidIsValid(pid);
     }
@@ -4855,13 +4844,8 @@ 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.
-            bool isRedecl = false;
-            ParseNodePtr vardecl = CreateVarDeclNode(pnodeFnc->sxFnc.pnodeName->sxVar.pid, STVariable, false, nullptr, false, &isRedecl);
+            ParseNodePtr vardecl = CreateVarDeclNode(pnodeFnc->sxFnc.pnodeName->sxVar.pid, STVariable, false, nullptr, false);
             vardecl->sxVar.isBlockScopeFncDeclVar = true;
-            if (isRedecl)
-            {
-                vardecl->sxVar.sym->SetHasBlockFncVarRedecl();
-            }
         }
     }
 
@@ -4972,8 +4956,11 @@ bool Parser::ParseFncDeclHelper(ParseNodePtr pnodeFnc, LPCOLESTR pNameHint, usho
     Scope *fncExprScope = nullptr;
     if (!fDeclaration)
     {
-        pnodeFncExprScope = StartParseBlock<buildAST>(PnodeBlockType::Function, ScopeType_FuncExpr);
-        fncExprScope = pnodeFncExprScope->sxBlock.scope;
+        if (!fLambda)
+        {
+            pnodeFncExprScope = StartParseBlock<buildAST>(PnodeBlockType::Function, ScopeType_FuncExpr);
+            fncExprScope = pnodeFncExprScope->sxBlock.scope;
+        }
 
         // Function expression: push the new function onto the stack now so that the name (if any) will be
         // local to the new function.
diff --git a/lib/Parser/Parse.h b/lib/Parser/Parse.h
@@ -271,8 +271,8 @@ class Parser
     ParseNodePtr CreateTempRef(ParseNode* tempNode);
 
     ParseNodePtr CreateNode(OpCode nop) { return CreateNode(nop, m_pscan? m_pscan->IchMinTok() : 0); }
-    ParseNodePtr CreateDeclNode(OpCode nop, IdentPtr pid, SymbolType symbolType, bool errorOnRedecl = true, bool *isRedecl = nullptr);
-    Symbol*      AddDeclForPid(ParseNodePtr pnode, IdentPtr pid, SymbolType symbolType, bool errorOnRedecl, bool *isRedecl = nullptr);
+    ParseNodePtr CreateDeclNode(OpCode nop, IdentPtr pid, SymbolType symbolType, bool errorOnRedecl = true);
+    Symbol*      AddDeclForPid(ParseNodePtr pnode, IdentPtr pid, SymbolType symbolType, bool errorOnRedecl);
     void         CheckRedeclarationErrorForBlockId(IdentPtr pid, int blockId);
     ParseNodePtr CreateNameNode(IdentPtr pid)
     {
@@ -325,7 +325,7 @@ class Parser
     void CheckPidIsValid(IdentPtr pid, bool autoArgumentsObject = false);
     void AddVarDeclToBlock(ParseNode *pnode);
     // Add a var declaration. Only use while parsing. Assumes m_ppnodeVar is pointing to the right place already
-    ParseNodePtr CreateVarDeclNode(IdentPtr pid, SymbolType symbolType, bool autoArgumentsObject = false, ParseNodePtr pnodeFnc = NULL, bool checkReDecl = true, bool *isRedecl = nullptr);
+    ParseNodePtr CreateVarDeclNode(IdentPtr pid, SymbolType symbolType, bool autoArgumentsObject = false, ParseNodePtr pnodeFnc = NULL, bool checkReDecl = true);
     // Add a var declaration, during parse tree rewriting. Will setup m_ppnodeVar for the given pnodeFnc
     ParseNodePtr AddVarDeclNode(IdentPtr pid, ParseNodePtr pnodeFnc);
     // Add a 'const' or 'let' declaration.
diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp
@@ -1182,7 +1182,7 @@ void EmitAssignmentToFuncName(ParseNode *pnodeFnc, ByteCodeGenerator *byteCodeGe
             {
                 byteCodeGenerator->EmitPropStore(pnodeFnc->location, sym, nullptr, funcInfoParent);
             }
-            else if (!sym->GetIsBlockVar() || sym->HasRealBlockVarRef() || sym->GetScope()->GetIsObject())
+            else
             {
                 byteCodeGenerator->EmitLocalPropInit(pnodeFnc->location, sym, funcInfoParent);
             }
diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp
@@ -1648,64 +1648,6 @@ Symbol * ByteCodeGenerator::FindSymbol(Symbol **symRef, IdentPtr pid, bool forRe
             }
         }
 
-        bool didTransferToFncVarSym = false;
-
-        #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)
-        {
-            // Try to use the var-scoped function binding in place of the lexically scoped one.
-            // This can be done if neither binding is explicitly assigned to, if there's no ambiguity in the binding
-            // (with/eval), and if the function is not declared in a loop. (Loops are problematic, because as the loop
-            // iterates different instances can be captured. If we always capture the var-scoped binding, then we
-            // always get the latest instance, when we should get the instance belonging to the iteration that captured it.)
-            if (sym->GetHasNonLocalReference())
-            {
-                if (!scope)
-                {
-                    Js::PropertyId i;
-                    scope = FindScopeForSym(symScope, nullptr, &i, top);
-                }
-                if (scope == symScope && !scope->GetIsObject())
-                {
-                    Symbol *fncVarSym = sym->GetFuncScopeVarSym();
-                    if (fncVarSym &&
-                        !fncVarSym->HasBlockFncVarRedecl() &&
-                        sym->GetAssignmentState() == NotAssigned &&
-                        fncVarSym->GetAssignmentState() == NotAssigned)
-                    {
-                        // Make sure no dynamic scope intrudes between the two bindings.
-                        bool foundDynamicScope = false;
-                        for (Scope *tmpScope = symScope->GetEnclosingScope(); tmpScope != fncVarSym->GetScope(); tmpScope = tmpScope->GetEnclosingScope())
-                        {
-                            Assert(tmpScope);
-                            if (tmpScope->GetIsDynamic())
-                            {
-                                foundDynamicScope = true;
-                                break;
-                            }
-                        }
-                        if (!foundDynamicScope)
-                        {
-                            didTransferToFncVarSym = true;
-                            sym = fncVarSym;
-                            symScope = sym->GetScope();
-                            if (nonLocalRef)
-                            {
-                                sym->SetHasNonLocalReference();
-                            }
-                        }
-                    }
-                }
-            }
-        }
-        if (!didTransferToFncVarSym)
-        {
-            sym->SetHasRealBlockVarRef();
-        }
-
         // This may not be a non-local reference, but the symbol may still be accessed non-locally. ('with', e.g.)
         // In that case, make sure we still process the symbol and its scope for closure capture.
         if (nonLocalRef || sym->GetHasNonLocalReference())
@@ -3242,7 +3184,6 @@ void AddFunctionsToScope(ParseNodePtr scope, ByteCodeGenerator * byteCodeGenerat
                 sym->GetScope() != sym->GetScope()->GetFunc()->GetParamScope())
             {
                 sym->SetIsBlockVar(true);
-                sym->SetHasRealBlockVarRef(true);
             }
         }
     });
diff --git a/lib/Runtime/ByteCode/Symbol.h b/lib/Runtime/ByteCode/Symbol.h
@@ -40,8 +40,6 @@ class Symbol
     BYTE isGlobalCatch : 1;
     BYTE isCommittedToSlot : 1;
     BYTE hasNonCommittedReference : 1;
-    BYTE hasRealBlockVarRef : 1;
-    BYTE hasBlockFncVarRedecl : 1;
     BYTE hasVisitedCapturingFunc : 1;
     BYTE isTrackedForDebugger : 1; // Whether the sym is tracked for debugger scope. This is fine because a sym can only be added to (not more than) one scope.
     BYTE isModuleExportStorage : 1; // If true, this symbol should be stored in the global scope export storage array.
@@ -75,8 +73,6 @@ class Symbol
         isGlobalCatch(false),
         isCommittedToSlot(false),
         hasNonCommittedReference(false),
-        hasRealBlockVarRef(false),
-        hasBlockFncVarRedecl(false),
         hasVisitedCapturingFunc(false),
         isTrackedForDebugger(false),
         isNonSimpleParameter(false),
@@ -316,26 +312,6 @@ class Symbol
         isUsed = is;
     }
 
-    bool HasRealBlockVarRef() const
-    {
-        return hasRealBlockVarRef;
-    }
-
-    void SetHasRealBlockVarRef(bool has = true)
-    {
-        hasRealBlockVarRef = has;
-    }
-
-    bool HasBlockFncVarRedecl() const
-    {
-        return hasBlockFncVarRedecl;
-    }
-
-    void SetHasBlockFncVarRedecl(bool has = true)
-    {
-        hasBlockFncVarRedecl = has;
-    }
-
     AssignmentState GetAssignmentState() const
     {
         return assignmentState;

Original file line number	Diff line number	Diff line change
`@@ -1182,7 +1182,7 @@ void EmitAssignmentToFuncName(ParseNode pnodeFnc, ByteCodeGenerator byteCodeGe`
`1182`	`1182`	`{`
`1183`	`1183`	`byteCodeGenerator->EmitPropStore(pnodeFnc->location, sym, nullptr, funcInfoParent);`
`1184`	`1184`	`}`
`1185`		`- else if (!sym->GetIsBlockVar() \|\| sym->HasRealBlockVarRef() \|\| sym->GetScope()->GetIsObject())`
	`1185`	`+ else`
`1186`	`1186`	`{`
`1187`	`1187`	`byteCodeGenerator->EmitLocalPropInit(pnodeFnc->location, sym, funcInfoParent);`
`1188`	`1188`	`}`