[MERGE #5042 @VSadov] Top function declarations in script/eval should conflict with global let/const

Merge pull request #5042 from VSadov:fix4953

For the purpose of declaration conflicts

```js
function f(){}
```
is the same as
```js
var f = function(){};
```
and likewise should conflict with global properties of the same name, including global let/const.
In a case if function declaration is added via a script, a dynamic check should be emitted.

Missing such check causes asserts when setting up callsite cache slots or incorrect behavior otherwise.

Fixes:#4953
Fixes:#3275

lib/Runtime/ByteCode/ByteCodeEmitter.cpp

@@ -5594,6 +5594,57 @@ void ByteCodeGenerator::EnsureNoRedeclarations(ParseNodeBlock *pnodeBlock, FuncI
         });
     }
 
+    auto emitRedeclCheck = [this](Symbol * sym, FuncInfo * funcInfo)
+    {
+        Js::PropertyId propertyId = sym->EnsurePosition(this);
+
+        if (this->flags & fscrEval)
+        {
+            if (!funcInfo->byteCodeFunction->GetIsStrictMode())
+            {
+                this->m_writer.ScopedProperty(Js::OpCode::ScopedEnsureNoRedeclFld, ByteCodeGenerator::RootObjectRegister,
+                    funcInfo->FindOrAddReferencedPropertyId(propertyId));
+            }
+        }
+        else
+        {
+            this->m_writer.ElementRootU(Js::OpCode::EnsureNoRootRedeclFld, funcInfo->FindOrAddReferencedPropertyId(propertyId));
+        }
+    };
+
+    // scan for function declarations
+    // these behave like "var" declarations
+    for (ParseNodePtr pnode = pnodeBlock->pnodeScopes; pnode;)
+    {
+        switch (pnode->nop) {
+
+        case knopFncDecl:
+            if (pnode->AsParseNodeFnc()->IsDeclaration())
+            {
+                emitRedeclCheck(pnode->AsParseNodeFnc()->pnodeName->sym, funcInfo);
+            }
+
+            pnode = pnode->AsParseNodeFnc()->pnodeNext;
+            break;
+
+        case knopBlock:
+            pnode = pnode->AsParseNodeBlock()->pnodeNext;
+            break;
+
+        case knopCatch:
+            pnode = pnode->AsParseNodeCatch()->pnodeNext;
+            break;
+
+        case knopWith:
+            pnode = pnode->AsParseNodeWith()->pnodeNext;
+            break;
+
+        default:
+            Assert(UNREACHED);
+        }
+    }
+
+    // scan for var declarations
     for (ParseNode *pnode = funcInfo->root->pnodeVars; pnode; pnode = pnode->AsParseNodeVar()->pnodeNext)
     {
         Symbol* sym = pnode->AsParseNodeVar()->sym;
@@ -5618,20 +5669,7 @@ void ByteCodeGenerator::EnsureNoRedeclarations(ParseNodeBlock *pnodeBlock, FuncI
 
         if (sym->GetSymbolType() == STVariable)
         {
-            Js::PropertyId propertyId = sym->EnsurePosition(this);
-
-            if (this->flags & fscrEval)
-            {
-                if (!funcInfo->byteCodeFunction->GetIsStrictMode())
-                {
-                    this->m_writer.ScopedProperty(Js::OpCode::ScopedEnsureNoRedeclFld, ByteCodeGenerator::RootObjectRegister,
-                        funcInfo->FindOrAddReferencedPropertyId(propertyId));
-                }
-            }
-            else
-            {
-                this->m_writer.ElementRootU(Js::OpCode::EnsureNoRootRedeclFld, funcInfo->FindOrAddReferencedPropertyId(propertyId));
-            }
+            emitRedeclCheck(sym, funcInfo);
         }
     }
 }

test/LetConst/funcDeclConflict.js

@@ -0,0 +1,179 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+this.WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
+
+function f1_f(){};
+var f2_v = 123;
+function f3_f(){};
+var f4_v = 123;
+
+let f5_l = 123;
+const f6_c = 123;
+
+var ex = "0";
+
+// Top level function in script does not conflict with another top level function
+WScript.LoadScript("function f1_f(){};");
+
+// Top level function in script does not conflict with same-named var
+WScript.LoadScript("function f2_v(){};");
+
+// Top level function in eval does not conflict with another top level function
+eval("function f3_f(){};");
+
+// Top level function in eval does not conflict with same-named var
+eval("function f4_v(){};");
+
+// Top level function in script conflicts with top level let
+try { WScript.LoadScript("function f5_l(){};"); } catch (e) { ex = e.message }
+
+assert.areEqual("Let/Const redeclaration", ex);
+ex = "1";
+
+// Top level function in script conflicts with top level const
+try { WScript.LoadScript("function f6_c(){};"); } catch (e) { ex = e.message }
+
+assert.areEqual("Let/Const redeclaration", ex);
+ex = "2";
+
+// Top level function in eval conflicts with top level let
+try { eval("function f5_l(){};") } catch (e) { ex = e.message}
+
+assert.areEqual("Let/Const redeclaration", ex);
+ex = "3";
+
+// Top level function in eval conflicts with top level const
+try { eval("function f6_c(){};") } catch (e) { ex = e.message }
+
+assert.areEqual("Let/Const redeclaration", ex);
+ex = "4";
+
+(function ff() {
+    if (true) {
+        let fo5_l = 123;
+
+        if (true) {
+            // TODO: this is blocked by https://github.com/Microsoft/ChakraCore/issues/5070
+            //
+            // Top level function in eval conflicts with outer function level let
+            try { eval("function fo5_l(){};") } catch (e) { ex = e.message }
+
+            assert.areEqual("4", ex);
+            ex = "5";
+        }
+    }
+
+    if (true) {
+        // Top level function in eval conflicts with outer function level const
+        try { eval("function fo6_c(){};") } catch (e) { ex = e.message }
+    }
+
+    assert.areEqual("Let/Const redeclaration", ex);
+    ex = "6";
+
+    const fo6_c = 123;
+})();
+
+(function ffs() {
+    'use strict';
+
+    let fos5_l = 123;
+
+    // Top level function in eval conflicts with outer function level let (strict)
+    eval("function fos5_l(){};");
+
+    if (true) {
+        // Top level function in eval conflicts with outer function level const (strict)
+        eval("function fos6_c(){};");
+    }
+
+    const fos6_c = 123;
+})();
+
+
+(function ffl() {
+    let fo5_l_sl = 123;
+
+    // Top level function in eval conflicts with outer function level let
+    try { eval("function fo5_l_sl(){};") } catch (e) { ex = e.message }
+
+    assert.areEqual("Let/Const redeclaration", ex);
+    ex = "7";
+
+    // Top level function in eval conflicts with outer function level const
+    try { eval("function fo6_c_sl(){};") } catch (e) { ex = e.message }
+
+    assert.areEqual("Let/Const redeclaration", ex);
+    ex = "8";
+
+    const fo6_c_sl = 123;
+})();
+
+(function ffsl() {
+    'use strict';
+
+    let fos5_l = 123;
+
+    // Top level function in eval conflicts with outer function level let (strict)
+    WScript.LoadScript("function fos5_l_sl(){};");
+
+    // Top level function in eval conflicts with outer function level const (strict)
+    WScript.LoadScript("function fos6_c_sl(){};");
+
+    const fos6_c_sl = 123;
+})();
+
+(function ffn() {
+    let fo5_l_nf = 123;
+
+    // Top level function in "new Function" does not conflict with outer function level let
+    f = (new Function("return function fo5_l_nf(){};"))();
+
+    // Top level function in "new Function" does not conflict with outer function level const
+    f = (new Function("return function fo6_c_nf(){};"))();
+
+    assert.areEqual("function fo6_c_nf(){}", f.toString());
+
+    const fo6_c_nf = 123;
+})();
+
+// Top level function in eval does not conflict with top level const, in a class (since strict is assumed)    
+class C1
+{                
+    static M()
+    {
+        eval("function f6_c(){};");
+    }
+}
+
+C1.M();
+
+// Top level function in eval does not conflict with class level get      
+class C2
+{                
+    get f7_l() {return "q";};
+
+    static M()
+    {
+        eval("function f7_l(){};");
+    }
+}
+
+C2.M();
+
+WScript.RegisterModuleSource("mod0.js", `
+    import 'mod1.js';
+    let qwer = 12;
+`);
+
+WScript.RegisterModuleSource("mod1.js",`
+    // no redeclaration here since modules are not introducing global names.
+    export function qwer(){};
+`);
+
+WScript.LoadScriptFile("mod0.js", "module");
+
+WScript.Echo("PASS");

test/LetConst/rlexe.xml

@@ -391,4 +391,9 @@
       <files>shadowedsetter.js</files>
     </default>
   </test>
+  <test>
+    <default>
+      <files>funcDeclConflict.js</files>
+    </default>
+  </test>
 </regress-exe>

test/es6/blockscope-functionbinding.baseline

@@ -160,27 +160,27 @@ function j() { }
 const j
 function k() { }
 var k
-function l(one) { }
-function l(one) { }
-function l(one) { }
-function l(two) { }
-function l(two) { }
+ReferenceError: Let/Const redeclaration
+let l
+let l
+ReferenceError: Let/Const redeclaration
+inner let l
 outer let l
-TypeError: Assignment to const
+ReferenceError: Let/Const redeclaration
 const m
 const m
-TypeError: Assignment to const
+ReferenceError: Let/Const redeclaration
 inner const m
 const m
-function m(three) { }
-function m(three) { }
+ReferenceError: Let/Const redeclaration
+inner let m
 const m
 function n() { }
 function n() { }
 function n() { }
-ReferenceError: Use before declaration
+ReferenceError: Let/Const redeclaration
 let o
-TypeError: Assignment to const
+ReferenceError: Let/Const redeclaration
 const p
 function q() { }
 var q
@@ -239,32 +239,34 @@ function glo_t19_j() { }
 const glo_t19_j
 function glo_t19_k() { }
 var glo_t19_k
-function glo_t19_l(one) { }
-function glo_t19_l(one) { }
-function glo_t19_l(one) { }
+ReferenceError: Let/Const redeclaration
+let glo_t19_l
+let glo_t19_l
 undefined
-function glo_t19_l(two) { }
-function glo_t19_l(two) { }
+ReferenceError: Let/Const redeclaration
+let declaredLater
+ReferenceError: Let/Const redeclaration
+inner let glo_t19_l
 outer let glo_t19_l
 undefined
-TypeError: Assignment to const
+ReferenceError: Let/Const redeclaration
 const glo_t19_m
 const glo_t19_m
 undefined
-TypeError: Assignment to const
+ReferenceError: Let/Const redeclaration
 inner const m
 const glo_t19_m
 undefined
-function glo_t19_m(three) { }
-function glo_t19_m(three) { }
+ReferenceError: Let/Const redeclaration
+inner let m
 const glo_t19_m
 undefined
 function glo_t19_n() { }
 function glo_t19_n() { }
 function glo_t19_n() { }
-ReferenceError: Use before declaration
+ReferenceError: Let/Const redeclaration
 let glo_t19_o
-TypeError: Assignment to const
+ReferenceError: Let/Const redeclaration
 const glo_t19_p
 function glo_t19_q() { }
 var glo_t19_q

test/es6/blockscope-functionbinding.js

@@ -454,7 +454,7 @@ print('\nTest 19: function declaration var binding should be ignored when same n
     (function () {
         let l = 'let l';
         {
-            eval("function l(one) { }; print(l);");
+            try { eval("function l(one) { }; print(l);"); } catch (e) { print(e); }
             print(l);
         }
         print(l);
@@ -462,7 +462,7 @@ print('\nTest 19: function declaration var binding should be ignored when same n
         l = 'outer let l';
         {
             let l = 'inner let l';
-            eval("function l(two) { }; print(l);");
+            try { eval("function l(two) { }; print(l);"); } catch (e) { print(e); }
             print(l);
         }
         print(l);
@@ -819,16 +819,22 @@ print(glo_t19_k);
 // dynamic via eval
 let glo_t19_l = 'let glo_t19_l';
 {
-    eval("function glo_t19_l(one) { }; print(glo_t19_l);");
+    try { eval("function glo_t19_l(one) { }; print(glo_t19_l);"); } catch (e) { print(e); }
     print(glo_t19_l);
 }
 print(glo_t19_l);
 print(this.glo_t19_l);
 
+{
+    try { eval("function declaredLater(one) { }; print(declaredLater);"); } catch (e) { print(e); }
+}
+let declaredLater = 'let declaredLater';
+print(declaredLater);
+
 glo_t19_l = 'outer let glo_t19_l';
 {
     let glo_t19_l = 'inner let glo_t19_l';
-    eval("function glo_t19_l(two) { }; print(glo_t19_l);");
+    try { eval("function glo_t19_l(two) { }; print(glo_t19_l);"); } catch (e) { print(e); }
     print(glo_t19_l);
 }
 print(glo_t19_l);