LibJS: Hoist variable declarations to the nearest relevant scope

"var" declarations are hoisted to the nearest function scope, while
"let" and "const" are hoisted to the nearest block scope.

This is done by the parser, which keeps two scope stacks, one stack
for the current var scope and one for the current let/const scope.

When the interpreter enters a scope, we walk all of the declarations
and insert them into the variable environment.

We don't support the temporal dead zone for let/const yet.

Libraries/LibJS/AST.cpp

@@ -412,8 +412,18 @@ void ASTNode::dump(int indent) const
 void ScopeNode::dump(int indent) const
 {
     ASTNode::dump(indent);
-    for (auto& child : children())
+    if (!m_variables.is_empty()) {
-        child.dump(indent + 1);
+        print_indent(indent + 1);
+        printf("(Variables)\n");
+        for (auto& variable : m_variables)
+            variable.dump(indent + 2);
+    }
+    if (!m_children.is_empty()) {
+        print_indent(indent + 1);
+        printf("(Children)\n");
+        for (auto& child : children())
+            child.dump(indent + 2);
+    }
 }
 
 void BinaryExpression::dump(int indent) const
@@ -578,7 +588,15 @@ void FunctionNode::dump(int indent, const char* class_name) const
 
     print_indent(indent);
     printf("%s '%s(%s)'\n", class_name, name().characters(), parameters_builder.build().characters());
-    body().dump(indent + 1);
+    if (!m_variables.is_empty()) {
+        print_indent(indent + 1);
+        printf("(Variables)\n");
+    }
+    for (auto& variable : m_variables)
+        variable.dump(indent + 2);
+    print_indent(indent + 1);
+    printf("(Body)\n");
+    body().dump(indent + 2);
 }
 
 void FunctionDeclaration::dump(int indent) const
@@ -1163,4 +1181,9 @@ Value SequenceExpression::execute(Interpreter& interpreter) const
     return last_value;
 }
 
+void ScopeNode::add_variables(NonnullRefPtrVector<VariableDeclaration> variables)
+{
+    m_variables.append(move(variables));
+}
+
 }

Libraries/LibJS/AST.h

@@ -38,6 +38,8 @@
 
 namespace JS {
 
+class VariableDeclaration;
+
 template<class T, class... Args>
 static inline NonnullRefPtr<T>
 create_ast_node(Args&&... args)
@@ -54,6 +56,7 @@ public:
     virtual bool is_identifier() const { return false; }
     virtual bool is_member_expression() const { return false; }
     virtual bool is_scope_node() const { return false; }
+    virtual bool is_program() const { return false; }
     virtual bool is_variable_declaration() const { return false; }
     virtual bool is_new_expression() const { return false; }
 
@@ -105,12 +108,16 @@ public:
     virtual Value execute(Interpreter&) const override;
     virtual void dump(int indent) const override;
 
+    void add_variables(NonnullRefPtrVector<VariableDeclaration>);
+    const NonnullRefPtrVector<VariableDeclaration>& variables() const { return m_variables; }
+
 protected:
     ScopeNode() {}
 
 private:
     virtual bool is_scope_node() const final { return true; }
     NonnullRefPtrVector<Statement> m_children;
+    NonnullRefPtrVector<VariableDeclaration> m_variables;
 };
 
 class Program : public ScopeNode {
@@ -118,6 +125,7 @@ public:
     Program() {}
 
 private:
+    virtual bool is_program() const override { return true; }
     virtual const char* class_name() const override { return "Program"; }
 };
 
@@ -142,19 +150,23 @@ public:
     const Vector<FlyString>& parameters() const { return m_parameters; };
 
 protected:
-    FunctionNode(const FlyString& name, NonnullRefPtr<Statement> body, Vector<FlyString> parameters = {})
+    FunctionNode(const FlyString& name, NonnullRefPtr<Statement> body, Vector<FlyString> parameters, NonnullRefPtrVector<VariableDeclaration> variables)
         : m_name(name)
         , m_body(move(body))
         , m_parameters(move(parameters))
+        , m_variables(move(variables))
     {
     }
 
     void dump(int indent, const char* class_name) const;
 
+    const NonnullRefPtrVector<VariableDeclaration>& variables() const { return m_variables; }
+
 private:
     FlyString m_name;
     NonnullRefPtr<Statement> m_body;
     const Vector<FlyString> m_parameters;
+    NonnullRefPtrVector<VariableDeclaration> m_variables;
 };
 
 class FunctionDeclaration final
@@ -163,8 +175,8 @@ class FunctionDeclaration final
 public:
     static bool must_have_name() { return true; }
 
-    FunctionDeclaration(const FlyString& name, NonnullRefPtr<Statement> body, Vector<FlyString> parameters = {})
+    FunctionDeclaration(const FlyString& name, NonnullRefPtr<Statement> body, Vector<FlyString> parameters, NonnullRefPtrVector<VariableDeclaration> variables)
-        : FunctionNode(name, move(body), move(parameters))
+        : FunctionNode(name, move(body), move(parameters), move(variables))
     {
     }
 
@@ -180,8 +192,8 @@ class FunctionExpression final : public Expression
 public:
     static bool must_have_name() { return false; }
 
-    FunctionExpression(const FlyString& name, NonnullRefPtr<Statement> body, Vector<FlyString> parameters = {})
+    FunctionExpression(const FlyString& name, NonnullRefPtr<Statement> body, Vector<FlyString> parameters, NonnullRefPtrVector<VariableDeclaration> variables)
-        : FunctionNode(name, move(body), move(parameters))
+        : FunctionNode(name, move(body), move(parameters), move(variables))
     {
     }
 
@@ -641,6 +653,8 @@ public:
     virtual Value execute(Interpreter&) const override;
     virtual void dump(int indent) const override;
 
+    const NonnullRefPtrVector<VariableDeclarator>& declarations() const { return m_declarations; }
+
 private:
     virtual const char* class_name() const override { return "VariableDeclaration"; }
 

Libraries/LibJS/Interpreter.cpp

@@ -92,9 +92,20 @@ Value Interpreter::run(const Statement& statement, ArgumentVector arguments, Sco
 void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector arguments, ScopeType scope_type)
 {
     HashMap<FlyString, Variable> scope_variables_with_declaration_kind;
+
+    for (auto& declaration : scope_node.variables()) {
+        for (auto& declarator : declaration.declarations()) {
+            if (scope_node.is_program())
+                global_object().put(declarator.id().string(), js_undefined());
+            else
+                scope_variables_with_declaration_kind.set(declarator.id().string(), { js_undefined(), declaration.declaration_kind() });
+        }
+    }
+
     for (auto& argument : arguments) {
         scope_variables_with_declaration_kind.set(argument.name, { argument.value, DeclarationKind::Var });
     }
+
     m_scope_stack.append({ scope_type, scope_node, move(scope_variables_with_declaration_kind) });
 }
 
@@ -118,9 +129,6 @@ void Interpreter::declare_variable(const FlyString& name, DeclarationKind declar
         for (ssize_t i = m_scope_stack.size() - 1; i >= 0; --i) {
             auto& scope = m_scope_stack.at(i);
             if (scope.type == ScopeType::Function) {
-                if (scope.variables.get(name).has_value() && scope.variables.get(name).value().declaration_kind != DeclarationKind::Var)
-                    ASSERT_NOT_REACHED();
-
                 scope.variables.set(move(name), { js_undefined(), declaration_kind });
                 return;
             }
@@ -130,9 +138,6 @@ void Interpreter::declare_variable(const FlyString& name, DeclarationKind declar
         break;
     case DeclarationKind::Let:
     case DeclarationKind::Const:
-        if (m_scope_stack.last().variables.get(name).has_value())
-            ASSERT_NOT_REACHED();
-
         m_scope_stack.last().variables.set(move(name), { js_undefined(), declaration_kind });
         break;
     }

Libraries/LibJS/Parser.cpp

@@ -26,11 +26,41 @@
 
 #include "Parser.h"
 #include <AK/HashMap.h>
+#include <AK/ScopeGuard.h>
 #include <AK/StdLibExtras.h>
 #include <stdio.h>
 
 namespace JS {
 
+class ScopePusher {
+public:
+    enum Type {
+        Var = 1,
+        Let = 2,
+    };
+
+    ScopePusher(Parser& parser, unsigned mask)
+        : m_parser(parser)
+        , m_mask(mask)
+    {
+        if (m_mask & Var)
+            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>());
+    }
+
+    ~ScopePusher()
+    {
+        if (m_mask & Var)
+            m_parser.m_parser_state.m_var_scopes.take_last();
+        if (m_mask & Let)
+            m_parser.m_parser_state.m_let_scopes.take_last();
+    }
+
+    Parser& m_parser;
+    unsigned m_mask { 0 };
+};
+
 static HashMap<TokenType, int> g_operator_precedence;
 Parser::ParserState::ParserState(Lexer lexer)
     : m_lexer(move(lexer))
@@ -170,6 +200,7 @@ Associativity Parser::operator_associativity(TokenType type) const
 
 NonnullRefPtr<Program> Parser::parse_program()
 {
+    ScopePusher scope(*this, ScopePusher::Var | ScopePusher::Let);
     auto program = adopt(*new Program);
     while (!done()) {
         if (match(TokenType::Semicolon)) {
@@ -181,6 +212,9 @@ NonnullRefPtr<Program> Parser::parse_program()
             consume();
         }
     }
+    ASSERT(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());
     return program;
 }
 
@@ -230,6 +264,12 @@ NonnullRefPtr<Statement> Parser::parse_statement()
 RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expect_parens)
 {
     save_state();
+    m_parser_state.m_var_scopes.append(NonnullRefPtrVector<VariableDeclaration>());
+
+    ArmedScopeGuard state_rollback_guard = [&] {
+        m_parser_state.m_var_scopes.take_last();
+        load_state();
+    };
 
     Vector<FlyString> parameters;
     bool parse_failed = false;
@@ -264,10 +304,8 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
         }
     }
 
-    if (parse_failed) {
+    if (parse_failed)
-        load_state();
         return nullptr;
-    }
 
     auto function_body_result = [this]() -> RefPtr<BlockStatement> {
         if (match(TokenType::CurlyOpen)) {
@@ -291,11 +329,11 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
     }();
 
     if (!function_body_result.is_null()) {
+        state_rollback_guard.disarm();
         auto body = function_body_result.release_nonnull();
-        return create_ast_node<FunctionExpression>("", move(body), move(parameters));
+        return create_ast_node<FunctionExpression>("", move(body), move(parameters), m_parser_state.m_var_scopes.take_last());
     }
 
-    load_state();
     return nullptr;
 }
 
@@ -616,6 +654,7 @@ NonnullRefPtr<ReturnStatement> Parser::parse_return_statement()
 
 NonnullRefPtr<BlockStatement> Parser::parse_block_statement()
 {
+    ScopePusher scope(*this, ScopePusher::Let);
     auto block = create_ast_node<BlockStatement>();
     consume(TokenType::CurlyOpen);
     while (!done() && !match(TokenType::CurlyClose)) {
@@ -629,12 +668,15 @@ NonnullRefPtr<BlockStatement> Parser::parse_block_statement()
         }
     }
     consume(TokenType::CurlyClose);
+    block->add_variables(m_parser_state.m_let_scopes.last());
     return block;
 }
 
 template<typename FunctionNodeType>
 NonnullRefPtr<FunctionNodeType> Parser::parse_function_node()
 {
+    ScopePusher scope(*this, ScopePusher::Var);
+
     consume(TokenType::Function);
     String name;
     if (FunctionNodeType::must_have_name()) {
@@ -655,7 +697,8 @@ NonnullRefPtr<FunctionNodeType> Parser::parse_function_node()
     }
     consume(TokenType::ParenClose);
     auto body = parse_block_statement();
-    return create_ast_node<FunctionNodeType>(name, move(body), move(parameters));
+    body->add_variables(m_parser_state.m_var_scopes.last());
+    return create_ast_node<FunctionNodeType>(name, move(body), move(parameters), NonnullRefPtrVector<VariableDeclaration>());
 }
 
 NonnullRefPtr<VariableDeclaration> Parser::parse_variable_declaration()
@@ -694,7 +737,12 @@ NonnullRefPtr<VariableDeclaration> Parser::parse_variable_declaration()
         }
         break;
     }
-    return create_ast_node<VariableDeclaration>(declaration_kind, move(declarations));
+    auto declaration = create_ast_node<VariableDeclaration>(declaration_kind, move(declarations));
+    if (declaration->declaration_kind() == DeclarationKind::Var)
+        m_parser_state.m_var_scopes.last().append(declaration);
+    else
+        m_parser_state.m_let_scopes.last().append(declaration);
+    return declaration;
 }
 
 NonnullRefPtr<ThrowStatement> Parser::parse_throw_statement()

Libraries/LibJS/Parser.h

@@ -75,6 +75,8 @@ public:
     bool has_errors() const { return m_parser_state.m_lexer.has_errors() || m_parser_state.m_has_errors; }
 
 private:
+    friend class ScopePusher;
+
     int operator_precedence(TokenType) const;
     Associativity operator_associativity(TokenType) const;
     bool match_expression() const;
@@ -94,11 +96,14 @@ private:
         Lexer m_lexer;
         Token m_current_token;
         bool m_has_errors = false;
+        Vector<NonnullRefPtrVector<VariableDeclaration>> m_var_scopes;
+        Vector<NonnullRefPtrVector<VariableDeclaration>> m_let_scopes;
 
         explicit ParserState(Lexer);
     };
 
     ParserState m_parser_state;
     Optional<ParserState> m_saved_state;
+
 };
 }

Libraries/LibJS/Tests/let-scoping.js

@@ -0,0 +1,21 @@
+try {
+
+    let i = 1;
+    {
+        let i = 3;
+        assert(i === 3);
+    }
+
+    assert(i === 1);
+
+    {
+        const i = 2;
+        assert(i === 2);
+    }
+
+    assert(i === 1);
+
+    console.log("PASS");
+} catch (e) {
+    console.log("FAIL: " + e);
+}

Libraries/LibJS/Tests/var-scoping.js

@@ -0,0 +1,21 @@
+try {
+    function foo() {
+        i = 3;
+        assert(i === 3);
+        var i;
+    }
+
+    foo();
+
+    var caught_exception;
+    try {
+        j = i;
+    } catch (e) {
+        caught_exception = e;
+    }
+    assert(caught_exception !== undefined);
+
+    console.log("PASS");
+} catch (e) {
+    console.log("FAIL: " + e);
+}