Fix module export symbols should not be marked as having non-local references

When we do BindPidRefsInScope, we walk the PidRef stack and check to see
if any ref is non-local. Global and module export symbols should never be
marked as non-local.

Problem is that not all refs in the PidRef stack are necessarily marked to
indicate that the symbol is a module export. If a symbol which is an
export has a non-local ref which does not indicate it is a module export,
we will erroneously set the hasNonLocalReference flag on that symbol.

Here's an example of how this can happen:
function foo() { };
export { foo };

In this case, the top ref to symbol foo does not say that foo is a module
export so we might set the hasNonLocalReference flag on symbol foo when we
look at the top ref. The next ref in the PidRef stack would indicate that
foo is a symbol but by then we've already marked it.

A fix is to move the module export flag out of PidRef and into the Ident
itself. Then we can check to see if the pid is a module export before we
walk the PidRef stack in BindPidRefsInScope.

Author: boingoing

lib/Parser/Hash.h

@@ -27,23 +27,24 @@ ULONG CaseInsensitiveComputeHash(LPCOLESTR posz);
 
 enum
 {
-    fidNil        =     0x0000,
-    fidKwdRsvd    = 0x0001,     // the keyword is a reserved word
-    fidKwdFutRsvd = 0x0002,     // a future reserved word, but only in strict mode
+    fidNil          = 0x0000,
+    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,
+    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,
+    fidGlobalDcl    = 0x2000,
 
-    fidUsed       =    0x4000  // name referenced by source code
+    fidUsed         = 0x4000,  // name referenced by source code
 
+    fidModuleExport = 0x8000    // name is module export
 };
 
 struct BlockIdsStack
@@ -84,8 +85,6 @@ struct PidRefStack
     bool IsAssignment() const { return isAsg; }
     bool IsDynamicBinding() const { return isDynamic; }
     void SetDynamicBinding()  { isDynamic = true; }
-    bool IsModuleExport() const { return isModuleExport; }
-    void SetModuleExport()    { isModuleExport = true; }
 
     Symbol **GetSymRef()
     {
@@ -283,6 +282,9 @@ struct Ident
 
     void SetIsLetOrConst() { m_grfid |= fidLetOrConst; }
     BOOL GetIsLetOrConst() const { return m_grfid & fidLetOrConst; }
+
+    void SetIsModuleExport() { m_grfid |= fidModuleExport; }
+    BOOL GetIsModuleExport() const { return m_grfid & fidModuleExport; }
 };
 
 

lib/Parser/Parse.cpp

@@ -1368,15 +1368,6 @@ ParseNodePtr Parser::CreateModuleImportDeclNode(IdentPtr localName)
     return declNode;
 }
 
-void Parser::MarkIdentifierReferenceIsModuleExport(IdentPtr localName)
-{
-    PidRefStack* pidRef = this->PushPidRef(localName);
-
-    Assert(pidRef != nullptr);
-
-    pidRef->SetModuleExport();
-}
-
 ParseNodePtr Parser::CreateVarDeclNode(IdentPtr pid, SymbolType symbolType, bool autoArgumentsObject, ParseNodePtr pnodeFnc, bool errorOnRedecl)
 {
     ParseNodePtr pnode = CreateDeclNode(knopVarDecl, pid, symbolType, errorOnRedecl);
@@ -1679,6 +1670,11 @@ void Parser::BindPidRefsInScope(IdentPtr pid, Symbol *sym, int blockId, uint max
     Js::LocalFunctionId funcId = GetCurrentFunctionNode()->sxFnc.functionId;
     Assert(sym);
 
+    if (pid->GetIsModuleExport())
+    {
+        sym->SetIsModuleExportStorage(true);
+    }
+
     for (ref = pid->GetTopRef(); ref && ref->GetScopeId() >= blockId; ref = nextRef)
     {
         // Fix up sym* on PID ref.
@@ -1702,11 +1698,6 @@ void Parser::BindPidRefsInScope(IdentPtr pid, Symbol *sym, int blockId, uint max
             }
         }
 
-        if (ref->IsModuleExport())
-        {
-            sym->SetIsModuleExportStorage(true);
-        }
-
         if (ref->GetFuncScopeId() != funcId && !sym->GetIsGlobal() && !sym->GetIsModuleExportStorage())
         {
             Assert(ref->GetFuncScopeId() > funcId);
@@ -2129,7 +2120,7 @@ void Parser::ParseNamedImportOrExportClause(ModuleImportOrExportEntryList* impor
             }
             else
             {
-                MarkIdentifierReferenceIsModuleExport(identifierName);
+                identifierName->SetIsModuleExport();
                 AddModuleImportOrExportEntry(importOrExportEntryList, nullptr, identifierName, identifierAs, nullptr);
             }
         }

test/es6/module-functionality.js

@@ -291,6 +291,17 @@ var tests = [
             WScript.LoadModule(functionBody, 'samethread');
         }
     },
+    {
+        name: "Nested function in module function body which captures exported symbol doesn't create empty frame object",
+        body: function() { 
+            let functionBody = 
+                `function foo() { };
+                export { foo };
+                function bar() { foo(); };`;
+                
+            WScript.LoadModule(functionBody, 'samethread');
+        }
+    },
 ];
 
 testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });