From ac7459cb400125c952255da46ec5b2bb98d5d711 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 13 Apr 2020 16:42:54 +0200 Subject: [PATCH] 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 | 29 +++++++++++-- Libraries/LibJS/AST.h | 24 ++++++++--- Libraries/LibJS/Interpreter.cpp | 17 +++++--- Libraries/LibJS/Parser.cpp | 62 ++++++++++++++++++++++++---- Libraries/LibJS/Parser.h | 5 +++ Libraries/LibJS/Tests/let-scoping.js | 21 ++++++++++ Libraries/LibJS/Tests/var-scoping.js | 21 ++++++++++ 7 files changed, 158 insertions(+), 21 deletions(-) create mode 100644 Libraries/LibJS/Tests/let-scoping.js create mode 100644 Libraries/LibJS/Tests/var-scoping.js diff --git a/Libraries/LibJS/AST.cpp b/Libraries/LibJS/AST.cpp index 15974de8b9..67fab0a34c 100644 --- a/Libraries/LibJS/AST.cpp +++ b/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()) - child.dump(indent + 1); + if (!m_variables.is_empty()) { + 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 variables) +{ + m_variables.append(move(variables)); +} + } diff --git a/Libraries/LibJS/AST.h b/Libraries/LibJS/AST.h index 01e15a6832..de935fba91 100644 --- a/Libraries/LibJS/AST.h +++ b/Libraries/LibJS/AST.h @@ -38,6 +38,8 @@ namespace JS { +class VariableDeclaration; + template static inline NonnullRefPtr 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); + const NonnullRefPtrVector& variables() const { return m_variables; } + protected: ScopeNode() {} private: virtual bool is_scope_node() const final { return true; } NonnullRefPtrVector m_children; + NonnullRefPtrVector 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& parameters() const { return m_parameters; }; protected: - FunctionNode(const FlyString& name, NonnullRefPtr body, Vector parameters = {}) + FunctionNode(const FlyString& name, NonnullRefPtr body, Vector parameters, NonnullRefPtrVector 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& variables() const { return m_variables; } + private: FlyString m_name; NonnullRefPtr m_body; const Vector m_parameters; + NonnullRefPtrVector 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 body, Vector parameters = {}) - : FunctionNode(name, move(body), move(parameters)) + FunctionDeclaration(const FlyString& name, NonnullRefPtr body, Vector parameters, NonnullRefPtrVector variables) + : 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 body, Vector parameters = {}) - : FunctionNode(name, move(body), move(parameters)) + FunctionExpression(const FlyString& name, NonnullRefPtr body, Vector parameters, NonnullRefPtrVector variables) + : 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& declarations() const { return m_declarations; } + private: virtual const char* class_name() const override { return "VariableDeclaration"; } diff --git a/Libraries/LibJS/Interpreter.cpp b/Libraries/LibJS/Interpreter.cpp index 756855b499..5bd6404252 100644 --- a/Libraries/LibJS/Interpreter.cpp +++ b/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 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; } diff --git a/Libraries/LibJS/Parser.cpp b/Libraries/LibJS/Parser.cpp index 57538ab644..cb4e8d3992 100644 --- a/Libraries/LibJS/Parser.cpp +++ b/Libraries/LibJS/Parser.cpp @@ -26,11 +26,41 @@ #include "Parser.h" #include +#include #include #include 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()); + if (m_mask & Let) + m_parser.m_parser_state.m_let_scopes.append(NonnullRefPtrVector()); + } + + ~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 g_operator_precedence; Parser::ParserState::ParserState(Lexer lexer) : m_lexer(move(lexer)) @@ -170,6 +200,7 @@ Associativity Parser::operator_associativity(TokenType type) const NonnullRefPtr 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 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 Parser::parse_statement() RefPtr Parser::try_parse_arrow_function_expression(bool expect_parens) { save_state(); + m_parser_state.m_var_scopes.append(NonnullRefPtrVector()); + + ArmedScopeGuard state_rollback_guard = [&] { + m_parser_state.m_var_scopes.take_last(); + load_state(); + }; Vector parameters; bool parse_failed = false; @@ -264,10 +304,8 @@ RefPtr Parser::try_parse_arrow_function_expression(bool expe } } - if (parse_failed) { - load_state(); + if (parse_failed) return nullptr; - } auto function_body_result = [this]() -> RefPtr { if (match(TokenType::CurlyOpen)) { @@ -291,11 +329,11 @@ RefPtr 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("", move(body), move(parameters)); + return create_ast_node("", move(body), move(parameters), m_parser_state.m_var_scopes.take_last()); } - load_state(); return nullptr; } @@ -616,6 +654,7 @@ NonnullRefPtr Parser::parse_return_statement() NonnullRefPtr Parser::parse_block_statement() { + ScopePusher scope(*this, ScopePusher::Let); auto block = create_ast_node(); consume(TokenType::CurlyOpen); while (!done() && !match(TokenType::CurlyClose)) { @@ -629,12 +668,15 @@ NonnullRefPtr Parser::parse_block_statement() } } consume(TokenType::CurlyClose); + block->add_variables(m_parser_state.m_let_scopes.last()); return block; } template NonnullRefPtr Parser::parse_function_node() { + ScopePusher scope(*this, ScopePusher::Var); + consume(TokenType::Function); String name; if (FunctionNodeType::must_have_name()) { @@ -655,7 +697,8 @@ NonnullRefPtr Parser::parse_function_node() } consume(TokenType::ParenClose); auto body = parse_block_statement(); - return create_ast_node(name, move(body), move(parameters)); + body->add_variables(m_parser_state.m_var_scopes.last()); + return create_ast_node(name, move(body), move(parameters), NonnullRefPtrVector()); } NonnullRefPtr Parser::parse_variable_declaration() @@ -694,7 +737,12 @@ NonnullRefPtr Parser::parse_variable_declaration() } break; } - return create_ast_node(declaration_kind, move(declarations)); + auto declaration = create_ast_node(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 Parser::parse_throw_statement() diff --git a/Libraries/LibJS/Parser.h b/Libraries/LibJS/Parser.h index 10d6607073..d4d52a361b 100644 --- a/Libraries/LibJS/Parser.h +++ b/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> m_var_scopes; + Vector> m_let_scopes; explicit ParserState(Lexer); }; ParserState m_parser_state; Optional m_saved_state; + }; } diff --git a/Libraries/LibJS/Tests/let-scoping.js b/Libraries/LibJS/Tests/let-scoping.js new file mode 100644 index 0000000000..8bdeb9f04b --- /dev/null +++ b/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); +} diff --git a/Libraries/LibJS/Tests/var-scoping.js b/Libraries/LibJS/Tests/var-scoping.js new file mode 100644 index 0000000000..65ef361c97 --- /dev/null +++ b/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); +}