From 3faeabf1dc1a8e0377b46741480a21435d7e6dcd Mon Sep 17 00:00:00 2001
From: Linus Groh <mail@linusgroh.de>
Date: Tue, 6 Jul 2021 12:21:12 +0100
Subject: [PATCH] Revert "LibJS: Don't hoist functions under certain
 circumstances"

This reverts commit 3411d50737725b382ae526e91a8bbd60656c3323.

It was causing LeakSanitizer on CI to fail, possibly due to a circular
reference.
---
 Userland/Libraries/LibJS/AST.cpp    |  4 +--
 Userland/Libraries/LibJS/AST.h      |  2 +-
 Userland/Libraries/LibJS/Parser.cpp | 39 +++--------------------------
 Userland/Libraries/LibJS/Parser.h   |  8 +-----
 4 files changed, 8 insertions(+), 45 deletions(-)

diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp
index db0485caf0..23b16fb21b 100644
--- a/Userland/Libraries/LibJS/AST.cpp
+++ b/Userland/Libraries/LibJS/AST.cpp
@@ -2377,9 +2377,9 @@ void ScopeNode::add_functions(NonnullRefPtrVector<FunctionDeclaration> functions
     m_functions.extend(move(functions));
 }
 
-void ScopeNode::add_hoisted_function(NonnullRefPtr<FunctionDeclaration> hoisted_function)
+void ScopeNode::add_hoisted_functions(NonnullRefPtrVector<FunctionDeclaration> hoisted_functions)
 {
-    m_hoisted_functions.append(hoisted_function);
+    m_hoisted_functions.extend(move(hoisted_functions));
 }
 
 }
diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h
index 7b94fe38af..f6bc2639de 100644
--- a/Userland/Libraries/LibJS/AST.h
+++ b/Userland/Libraries/LibJS/AST.h
@@ -145,7 +145,7 @@ public:
 
     void add_variables(NonnullRefPtrVector<VariableDeclaration>);
     void add_functions(NonnullRefPtrVector<FunctionDeclaration>);
-    void add_hoisted_function(NonnullRefPtr<FunctionDeclaration>);
+    void add_hoisted_functions(NonnullRefPtrVector<FunctionDeclaration>);
     NonnullRefPtrVector<VariableDeclaration> const& variables() const { return m_variables; }
     NonnullRefPtrVector<FunctionDeclaration> const& functions() const { return m_functions; }
     NonnullRefPtrVector<FunctionDeclaration> const& hoisted_functions() const { return m_hoisted_functions; }
diff --git a/Userland/Libraries/LibJS/Parser.cpp b/Userland/Libraries/LibJS/Parser.cpp
index 21dae61d20..bbfceebeec 100644
--- a/Userland/Libraries/LibJS/Parser.cpp
+++ b/Userland/Libraries/LibJS/Parser.cpp
@@ -64,24 +64,7 @@ public:
             scope_node->add_variables(m_parser.m_state.let_scopes.last());
 
         scope_node->add_functions(m_parser.m_state.current_scope->function_declarations);
-
-        for (auto& hoistable_function : m_parser.m_state.current_scope->hoisted_function_declarations) {
-            if (is_hoistable(hoistable_function)) {
-                scope_node->add_hoisted_function(hoistable_function.declaration);
-            }
-        }
-    }
-
-    static bool is_hoistable(Parser::Scope::HoistableDeclaration& declaration)
-    {
-        auto& name = declaration.declaration->name();
-        // See if we find any conflicting lexical declaration on the way up
-        for (RefPtr<Parser::Scope> scope = declaration.scope; !scope.is_null(); scope = scope->parent) {
-            if (scope->lexical_declarations.contains(name)) {
-                return false;
-            }
-        }
-        return true;
+        scope_node->add_hoisted_functions(m_parser.m_state.current_scope->hoisted_function_declarations);
     }
 
     Parser& m_parser;
@@ -321,8 +304,7 @@ NonnullRefPtr<Declaration> Parser::parse_declaration()
     case TokenType::Function: {
         auto declaration = parse_function_node<FunctionDeclaration>();
         m_state.current_scope->function_declarations.append(declaration);
-        auto hoisting_target = m_state.current_scope->get_current_function_scope();
-        hoisting_target->hoisted_function_declarations.append({ declaration, *m_state.current_scope });
+        m_state.current_scope->get_current_function_scope()->hoisted_function_declarations.append(declaration);
         return declaration;
     }
     case TokenType::Let:
@@ -1778,23 +1760,10 @@ NonnullRefPtr<VariableDeclaration> Parser::parse_variable_declaration(bool for_l
         consume_or_insert_semicolon();
 
     auto declaration = create_ast_node<VariableDeclaration>({ m_state.current_token.filename(), rule_start.position(), position() }, declaration_kind, move(declarations));
-    if (declaration_kind == DeclarationKind::Var) {
+    if (declaration_kind == DeclarationKind::Var)
         m_state.var_scopes.last().append(declaration);
-    } else {
+    else
         m_state.let_scopes.last().append(declaration);
-
-        for (auto& declarator : declaration->declarations()) {
-            declarator.target().visit(
-                [&](const NonnullRefPtr<Identifier>& id) {
-                    m_state.current_scope->lexical_declarations.set(id->string());
-                },
-                [&](const NonnullRefPtr<BindingPattern>& binding) {
-                    binding->for_each_bound_name([&](const auto& name) {
-                        m_state.current_scope->lexical_declarations.set(name);
-                    });
-                });
-        }
-    }
     return declaration;
 }
 
diff --git a/Userland/Libraries/LibJS/Parser.h b/Userland/Libraries/LibJS/Parser.h
index cef0813585..0e231a3dff 100644
--- a/Userland/Libraries/LibJS/Parser.h
+++ b/Userland/Libraries/LibJS/Parser.h
@@ -201,18 +201,12 @@ private:
             Function,
             Block,
         };
-        struct HoistableDeclaration {
-            NonnullRefPtr<FunctionDeclaration> declaration;
-            NonnullRefPtr<Scope> scope; // where it is actually declared
-        };
 
         Type type;
         RefPtr<Scope> parent;
 
         NonnullRefPtrVector<FunctionDeclaration> function_declarations;
-        Vector<HoistableDeclaration> hoisted_function_declarations;
-
-        HashTable<FlyString> lexical_declarations;
+        NonnullRefPtrVector<FunctionDeclaration> hoisted_function_declarations;
 
         explicit Scope(Type, RefPtr<Scope>);
         RefPtr<Scope> get_current_function_scope();