Fix function hoisting behaviour (non-strict mode)

include/hermes/Sema/SemContext.h

@@ -435,6 +435,21 @@ class SemContext {
   /// Set the "declaration decl" of the specified identifier node.
   void setDeclarationDecl(ESTree::IdentifierNode *node, Decl *decl);
 
+  /// Set a promoted "declaration decl" for the specified identifier node.
+  void setPromotedDecl(ESTree::IdentifierNode *node, Decl *decl) {
+    promotedFunctionDecls_[node] = decl;
+  }
+
+  /// \return a global "declaration decl" associated with a promoted function
+  /// identifier if one exists, else nullptr.
+  Decl *getPromotedDecl(ESTree::IdentifierNode *node) {
+    if (auto it = promotedFunctionDecls_.find(node);
+        it != promotedFunctionDecls_.end()) {
+      return it->second;
+    }
+    return nullptr;
+  }
+
   /// Set the "declaration decl" and the "expression decl" of the identifier
   /// node to the same value.
   void setBothDecl(ESTree::IdentifierNode *node, Decl *decl) {
@@ -490,6 +505,14 @@ class SemContext {
   /// "expression decl" are both set and are not the same value.
   llvh::DenseMap<ESTree::IdentifierNode *, Decl *>
       sideIdentifierDeclarationDecl_{};
+
+  /// This side table is used to associate a "declaration decl" with an
+  /// ESTree::IdentifierNode in scenarios where a scoped function
+  /// is promoted to the global scope.
+  /// In promotedFunctionDecls_ we store the scoped declaration, while in
+  /// sideIdentifierDeclarationDecl_ the global-like declaration
+  llvh::DenseMap<ESTree::IdentifierNode *, Decl *>
+      promotedFunctionDecls_{};
 };
 
 class SemContextDumper {

lib/IRGen/ESTreeIRGen-func.cpp

@@ -75,6 +75,7 @@ void ESTreeIRGen::genFunctionDeclaration(
   }
 
   Value *funcStorage = resolveIdentifier(id);
+  Value *funcStoragePromoted = resolveIdentifierPromoted(id);
   assert(funcStorage && "Function declaration storage must have been resolved");
 
   auto *newFuncParentScope = curFunction()->curScope->getVariableScope();
@@ -92,6 +93,8 @@ void ESTreeIRGen::genFunctionDeclaration(
       Builder.createCreateFunctionInst(curFunction()->curScope, newFunc);
 
   emitStore(newClosure, funcStorage, true);
+  if (funcStoragePromoted)
+    emitStore(newClosure, funcStoragePromoted, true);
 }
 
 Value *ESTreeIRGen::genFunctionExpression(

lib/IRGen/ESTreeIRGen.h

@@ -1168,6 +1168,13 @@ class ESTreeIRGen {
     return getDeclData(getIDDecl(id));
   }
 
+  /// Resolve the identifier node associated with the promoted function
+  /// to the corresponding global variable in expression context.
+  Value *resolveIdentifierPromoted(ESTree::IdentifierNode *id) {
+    auto declPromoted = getIDDeclPromoted(id);
+    return declPromoted ? getDeclData(declPromoted) : nullptr;
+  }
+
   /// Cast the node to ESTree::IdentifierNode and resolve it to an expression
   /// decl.
   Value *resolveIdentifierFromID(ESTree::Node *id) {
@@ -1429,6 +1436,13 @@ class ESTreeIRGen {
     return decl;
   }
 
+  /// Return the global Decl associated with the promoted function identifier.
+  sema::Decl *getIDDeclPromoted(ESTree::IdentifierNode *id) {
+    assert(id && "IdentifierNode cannot be null");
+    sema::Decl *decl = semCtx_.getPromotedDecl(id);
+    return decl;
+  }
+
   /// Set the customData field of the declaration with the specified value.
   static void setDeclData(sema::Decl *decl, Value *value) {
     assert(decl);

lib/Sema/SemanticResolver.cpp

@@ -1829,6 +1829,16 @@ void SemanticResolver::validateAndDeclareIdentifier(
     }
   }
 
+  // if the identifier is promoted and we already have a (global) declaration
+  // for it, then we need to create and assign a scoped declaration
+  if (semCtx_.getDeclarationDecl(ident) &&
+      functionContext()->promotedFuncDecls.count(ident->_name)) {
+    decl = semCtx_.newDeclInScope(ident->_name, kind, curScope_);
+    bindingTable_.put(ident->_name, Binding{decl, ident});
+    semCtx_.setPromotedDecl(ident, decl);
+    return;
+  }
+
   // Create new decl.
   if (!decl) {
     if (Decl::isKindGlobal(kind))

test/IRGen/function-promotion.js

@@ -37,7 +37,7 @@ function foo() {
 // CHECK-NEXT:       ReturnInst %6: any
 // CHECK-NEXT:function_end
 
-// CHECK:scope %VS1 [x: any, y: any, z: any, x#1: any]
+// CHECK:scope %VS1 [x: any, y: any, z: any, x#1: any, y#1: any, z#1: any]
 
 // CHECK:function foo(): any
 // CHECK-NEXT:%BB0:
@@ -51,13 +51,15 @@ function foo() {
 // CHECK-NEXT:       StoreFrameInst %1: environment, %6: object, [%VS1.x#1]: any
 // CHECK-NEXT:  %8 = CreateFunctionInst (:object) %1: environment, %y(): functionCode
 // CHECK-NEXT:       StoreFrameInst %1: environment, %8: object, [%VS1.y]: any
-// CHECK-NEXT:  %10 = CreateFunctionInst (:object) %1: environment, %z(): functionCode
-// CHECK-NEXT:        StoreFrameInst %1: environment, %10: object, [%VS1.z]: any
-// CHECK-NEXT:  %12 = TryLoadGlobalPropertyInst (:any) globalObject: object, "print": string
-// CHECK-NEXT:  %13 = LoadFrameInst (:any) %1: environment, [%VS1.x]: any
-// CHECK-NEXT:  %14 = LoadFrameInst (:any) %1: environment, [%VS1.y]: any
-// CHECK-NEXT:  %15 = LoadFrameInst (:any) %1: environment, [%VS1.z]: any
-// CHECK-NEXT:  %16 = CallInst (:any) %12: any, empty: any, false: boolean, empty: any, undefined: undefined, undefined: undefined, %13: any, %14: any, %15: any
+// CHECK-NEXT:        StoreFrameInst %1: environment, %8: object, [%VS1.y#1]: any
+// CHECK-NEXT:  %11 = CreateFunctionInst (:object) %1: environment, %z(): functionCode
+// CHECK-NEXT:        StoreFrameInst %1: environment, %11: object, [%VS1.z]: any
+// CHECK-NEXT:        StoreFrameInst %1: environment, %11: object, [%VS1.z#1]: any
+// CHECK-NEXT:  %14 = TryLoadGlobalPropertyInst (:any) globalObject: object, "print": string
+// CHECK-NEXT:  %15 = LoadFrameInst (:any) %1: environment, [%VS1.x]: any
+// CHECK-NEXT:  %16 = LoadFrameInst (:any) %1: environment, [%VS1.y]: any
+// CHECK-NEXT:  %17 = LoadFrameInst (:any) %1: environment, [%VS1.z]: any
+// CHECK-NEXT:  %18 = CallInst (:any) %14: any, empty: any, false: boolean, empty: any, undefined: undefined, undefined: undefined, %15: any, %16: any, %17: any
 // CHECK-NEXT:        ReturnInst undefined: undefined
 // CHECK-NEXT:function_end
 

test/IRGen/scoped-func-init.js

@@ -38,7 +38,7 @@ function foo() {
 // CHKLOOSE-NEXT:       ReturnInst %8: any
 // CHKLOOSE-NEXT:function_end
 
-// CHKLOOSE:scope %VS1 [f: any]
+// CHKLOOSE:scope %VS1 [f: any, f#1: any]
 
 // CHKLOOSE:function foo(): any
 // CHKLOOSE-NEXT:%BB0:
@@ -49,6 +49,7 @@ function foo() {
 // CHKLOOSE-NEXT:       StorePropertyLooseInst %3: any, globalObject: object, "init": string
 // CHKLOOSE-NEXT:  %5 = CreateFunctionInst (:object) %1: environment, %f(): functionCode
 // CHKLOOSE-NEXT:       StoreFrameInst %1: environment, %5: object, [%VS1.f]: any
+// CHKLOOSE-NEXT:       StoreFrameInst %1: environment, %5: object, [%VS1.f#1]: any
 // CHKLOOSE-NEXT:       ReturnInst undefined: undefined
 // CHKLOOSE-NEXT:function_end
 

test/Sema/regress-function-promotion-decl.js

@@ -28,11 +28,12 @@ function func() {
 // CHECK-NEXT:            Decl %d.2 'inner' Var
 // CHECK-NEXT:            Decl %d.3 'arguments' Var Arguments
 // CHECK-NEXT:            Scope %s.3
-// CHECK-NEXT:                Decl %d.4 'foo' Let
+// CHECK-NEXT:                Decl %d.4 'inner' ScopedFunction
+// CHECK-NEXT:                Decl %d.5 'foo' Let
 // CHECK-NEXT:                hoistedFunction inner
 // CHECK-NEXT:        Func loose
 // CHECK-NEXT:            Scope %s.4
-// CHECK-NEXT:                Decl %d.5 'arguments' Var Arguments
+// CHECK-NEXT:                Decl %d.6 'arguments' Var Arguments
 
 // CHECK:Program Scope %s.1
 // CHECK-NEXT:    FunctionDeclaration
@@ -45,4 +46,4 @@ function func() {
 // CHECK-NEXT:                VariableDeclaration
 // CHECK-NEXT:                    VariableDeclarator
 // CHECK-NEXT:                        NumericLiteral
-// CHECK-NEXT:                        Id 'foo' [D:E:%d.4 'foo']
+// CHECK-NEXT:                        Id 'foo' [D:E:%d.5 'foo']

test/hermes/promoted-function-redeclaration.js

@@ -0,0 +1,33 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// RUN: %hermes %s | %FileCheck --match-full-lines %s
+
+{
+    function promotedFunction(endRecursion) {
+        if(endRecursion) {
+            promotedFunction(true);
+            return;
+        }
+        print ('SUCCESS');
+    }
+
+    callback = promotedFunction;
+}
+
+{
+    function promotedFunction(endRecursion) {
+        if(endRecursion) {
+            promotedFunction(true);
+            return;
+        }
+        print ('FAIL');
+    }
+}
+
+callback(false);
+//CHECK: SUCCESS