LibJS: Hoist function declarations

This patch adds function declaration hoisting. The mechanism
is similar to var hoisting. Hoisted function declarations are to be put
before the hoisted var declarations, hence they have to be treated
separately.

Libraries/LibJS/AST.cpp

@@ -67,10 +67,8 @@ Value ScopeNode::execute(Interpreter& interpreter) const
     return interpreter.run(*this);
 }
 
-Value FunctionDeclaration::execute(Interpreter& interpreter) const
+Value FunctionDeclaration::execute(Interpreter&) const
 {
-    auto* function = ScriptFunction::create(interpreter.global_object(), name(), body(), parameters(), function_length(), interpreter.current_environment());
-    interpreter.set_variable(name(), function);
     return js_undefined();
 }
 
@@ -1765,4 +1763,9 @@ void ScopeNode::add_variables(NonnullRefPtrVector<VariableDeclaration> variables
     m_variables.append(move(variables));
 }
 
+void ScopeNode::add_functions(NonnullRefPtrVector<FunctionDeclaration> functions)
+{
+    m_functions.append(move(functions));
+}
+
 }

Libraries/LibJS/AST.h

@@ -40,6 +40,7 @@
 namespace JS {
 
 class VariableDeclaration;
+class FunctionDeclaration;
 
 template<class T, class... Args>
 static inline NonnullRefPtr<T>
@@ -124,8 +125,9 @@ public:
     virtual void dump(int indent) const override;
 
     void add_variables(NonnullRefPtrVector<VariableDeclaration>);
+    void add_functions(NonnullRefPtrVector<FunctionDeclaration>);
     const NonnullRefPtrVector<VariableDeclaration>& variables() const { return m_variables; }
-
+    const NonnullRefPtrVector<FunctionDeclaration>& functions() const { return m_functions; }
     bool in_strict_mode() const { return m_strict_mode; }
     void set_strict_mode() { m_strict_mode = true; }
 
@@ -136,6 +138,7 @@ private:
     virtual bool is_scope_node() const final { return true; }
     NonnullRefPtrVector<Statement> m_children;
     NonnullRefPtrVector<VariableDeclaration> m_variables;
+    NonnullRefPtrVector<FunctionDeclaration> m_functions;
     bool m_strict_mode { false };
 };
 
@@ -175,6 +178,7 @@ public:
     const FlyString& name() const { return m_name; }
     const Statement& body() const { return *m_body; }
     const Vector<Parameter>& parameters() const { return m_parameters; };
+    i32 function_length() const { return m_function_length; }
 
 protected:
     FunctionNode(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables)
@@ -189,7 +193,6 @@ protected:
     void dump(int indent, const char* class_name) const;
 
     const NonnullRefPtrVector<VariableDeclaration>& variables() const { return m_variables; }
-    i32 function_length() const { return m_function_length; }
 
 private:
     FlyString m_name;

Libraries/LibJS/Interpreter.cpp

@@ -35,6 +35,7 @@
 #include <LibJS/Runtime/NativeFunction.h>
 #include <LibJS/Runtime/Object.h>
 #include <LibJS/Runtime/Reference.h>
+#include <LibJS/Runtime/ScriptFunction.h>
 #include <LibJS/Runtime/Shape.h>
 #include <LibJS/Runtime/SymbolObject.h>
 #include <LibJS/Runtime/Value.h>
@@ -91,6 +92,11 @@ Value Interpreter::run(const Statement& statement, ArgumentVector arguments, Sco
 
 void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector arguments, ScopeType scope_type)
 {
+    for (auto& declaration : scope_node.functions()) {
+        auto* function = ScriptFunction::create(global_object(), declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), current_environment());
+        set_variable(declaration.name(), function);
+    }
+
     if (scope_type == ScopeType::Function) {
         m_scope_stack.append({ scope_type, scope_node, false });
         return;

Libraries/LibJS/Parser.cpp

@@ -37,6 +37,7 @@ public:
     enum Type {
         Var = 1,
         Let = 2,
+        Function = 3,
     };
 
     ScopePusher(Parser& parser, unsigned mask)
@@ -47,6 +48,8 @@ public:
             m_parser.m_parser_state.m_var_scopes.append(NonnullRefPtrVector<VariableDeclaration>());
         if (m_mask & Let)
             m_parser.m_parser_state.m_let_scopes.append(NonnullRefPtrVector<VariableDeclaration>());
+        if (m_mask & Function)
+            m_parser.m_parser_state.m_function_scopes.append(NonnullRefPtrVector<FunctionDeclaration>());
     }
 
     ~ScopePusher()
@@ -55,6 +58,8 @@ public:
             m_parser.m_parser_state.m_var_scopes.take_last();
         if (m_mask & Let)
             m_parser.m_parser_state.m_let_scopes.take_last();
+        if (m_mask & Function)
+            m_parser.m_parser_state.m_function_scopes.take_last();
     }
 
     Parser& m_parser;
@@ -204,7 +209,7 @@ Associativity Parser::operator_associativity(TokenType type) const
 
 NonnullRefPtr<Program> Parser::parse_program()
 {
-    ScopePusher scope(*this, ScopePusher::Var | ScopePusher::Let);
+    ScopePusher scope(*this, ScopePusher::Var | ScopePusher::Let | ScopePusher::Function);
     auto program = adopt(*new Program);
 
     bool first = true;
@@ -228,6 +233,7 @@ NonnullRefPtr<Program> Parser::parse_program()
     if (m_parser_state.m_var_scopes.size() == 1) {
         program->add_variables(m_parser_state.m_var_scopes.last());
         program->add_variables(m_parser_state.m_let_scopes.last());
+        program->add_functions(m_parser_state.m_function_scopes.last());
     } else {
         syntax_error("Unclosed scope");
     }
@@ -238,8 +244,11 @@ NonnullRefPtr<Statement> Parser::parse_statement()
 {
     auto statement = [this]() -> NonnullRefPtr<Statement> {
     switch (m_parser_state.m_current_token.type()) {
-    case TokenType::Function:
-        return parse_function_node<FunctionDeclaration>();
+    case TokenType::Function: {
+        auto declaration = parse_function_node<FunctionDeclaration>();
+        m_parser_state.m_function_scopes.last().append(declaration);
+        return declaration;
+    }
     case TokenType::CurlyOpen:
         return parse_block_statement();
     case TokenType::Return:
@@ -590,8 +599,7 @@ NonnullRefPtr<ObjectExpression> Parser::parse_object_expression()
                 syntax_error(
                     "Expected '(' for object getter or setter property",
                     m_parser_state.m_current_token.line_number(),
-                    m_parser_state.m_current_token.line_column()
-                );
+                    m_parser_state.m_current_token.line_column());
                 skip_to_next_property();
                 continue;
             }
@@ -606,8 +614,7 @@ NonnullRefPtr<ObjectExpression> Parser::parse_object_expression()
                 syntax_error(
                     "Object getter property must have no arguments",
                     m_parser_state.m_current_token.line_number(),
-                    m_parser_state.m_current_token.line_column()
-                );
+                    m_parser_state.m_current_token.line_column());
                 skip_to_next_property();
                 continue;
             }
@@ -615,8 +622,7 @@ NonnullRefPtr<ObjectExpression> Parser::parse_object_expression()
                 syntax_error(
                     "Object setter property must have one argument",
                     m_parser_state.m_current_token.line_number(),
-                    m_parser_state.m_current_token.line_column()
-                );
+                    m_parser_state.m_current_token.line_column());
                 skip_to_next_property();
                 continue;
             }
@@ -1059,13 +1065,14 @@ NonnullRefPtr<BlockStatement> Parser::parse_block_statement()
     m_parser_state.m_strict_mode = initial_strict_mode_state;
     consume(TokenType::CurlyClose);
     block->add_variables(m_parser_state.m_let_scopes.last());
+    block->add_functions(m_parser_state.m_function_scopes.last());
     return block;
 }
 
 template<typename FunctionNodeType>
 NonnullRefPtr<FunctionNodeType> Parser::parse_function_node(bool check_for_function_and_name)
 {
-    ScopePusher scope(*this, ScopePusher::Var);
+    ScopePusher scope(*this, ScopePusher::Var | ScopePusher::Function);
 
     if (check_for_function_and_name)
         consume(TokenType::Function);
@@ -1109,6 +1116,7 @@ NonnullRefPtr<FunctionNodeType> Parser::parse_function_node(bool check_for_funct
 
     auto body = parse_block_statement();
     body->add_variables(m_parser_state.m_var_scopes.last());
+    body->add_functions(m_parser_state.m_function_scopes.last());
     return create_ast_node<FunctionNodeType>(name, move(body), move(parameters), function_length, NonnullRefPtrVector<VariableDeclaration>());
 }
 

Libraries/LibJS/Parser.h

@@ -147,6 +147,7 @@ private:
         Vector<Error> m_errors;
         Vector<NonnullRefPtrVector<VariableDeclaration>> m_var_scopes;
         Vector<NonnullRefPtrVector<VariableDeclaration>> m_let_scopes;
+        Vector<NonnullRefPtrVector<FunctionDeclaration>> m_function_scopes;
         UseStrictDirectiveState m_use_strict_directive { UseStrictDirectiveState::None };
         bool m_strict_mode { false };
 

Libraries/LibJS/Tests/function-hoisting.js

@@ -0,0 +1,37 @@
+load("test-common.js");
+
+try {
+    var callHoisted = hoisted();
+    function hoisted() {
+        return true;
+    }
+    assert(hoisted() === true);
+    assert(callHoisted === true);
+
+    {
+        var callScopedHoisted = scopedHoisted();
+        function scopedHoisted() {
+            return "foo";
+        }
+        assert(scopedHoisted() === "foo");
+        assert(callScopedHoisted === "foo");
+    }
+    assert(scopedHoisted() === "foo");
+    assert(callScopedHoisted === "foo");
+
+    const test = () => {
+        var iife = (function () {
+            return declaredLater();
+        })();
+        function declaredLater() {
+            return "yay";
+        }
+        return iife;
+    };
+    assert(typeof declaredLater === "undefined");
+    assert(test() === "yay");
+
+    console.log("PASS");
+} catch (e) {
+    console.log("FAIL: " + e);
+}