From 1d203808596071b0891459dc4c9aadea4d9c4d6c Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 22 Jun 2021 15:42:44 +0200 Subject: [PATCH] LibJS: Split the per-call-frame environment into lexical and variable To better follow the spec, we need to distinguish between the current execution context's lexical environment and variable environment. This patch moves us to having two record pointers, although both of them point at the same environment records for now. --- Userland/Libraries/LibJS/AST.cpp | 12 +++++----- .../Libraries/LibJS/Bytecode/Interpreter.cpp | 2 +- Userland/Libraries/LibJS/Bytecode/Op.cpp | 7 +++--- Userland/Libraries/LibJS/Interpreter.cpp | 22 ++++++++++------- Userland/Libraries/LibJS/Interpreter.h | 2 +- Userland/Libraries/LibJS/Parser.cpp | 2 +- .../LibJS/Runtime/AbstractOperations.cpp | 2 +- .../Runtime/GeneratorFunctionConstructor.cpp | 2 +- .../LibJS/Runtime/GeneratorObject.cpp | 2 +- .../Libraries/LibJS/Runtime/GlobalObject.cpp | 2 +- .../LibJS/Runtime/ScriptFunction.cpp | 4 ++-- Userland/Libraries/LibJS/Runtime/VM.cpp | 24 ++++++++++--------- Userland/Libraries/LibJS/Runtime/VM.h | 10 +++++--- 13 files changed, 52 insertions(+), 41 deletions(-) diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 54322dad44..57e5c92ee4 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -102,7 +102,7 @@ Value FunctionDeclaration::execute(Interpreter& interpreter, GlobalObject&) cons Value FunctionExpression::execute(Interpreter& interpreter, GlobalObject& global_object) const { InterpreterNodeScope node_scope { interpreter, *this }; - return ScriptFunction::create(global_object, name(), body(), parameters(), function_length(), interpreter.current_environment_record(), kind(), is_strict_mode() || interpreter.vm().in_strict_mode(), is_arrow_function()); + return ScriptFunction::create(global_object, name(), body(), parameters(), function_length(), interpreter.lexical_environment(), kind(), is_strict_mode() || interpreter.vm().in_strict_mode(), is_arrow_function()); } Value ExpressionStatement::execute(Interpreter& interpreter, GlobalObject& global_object) const @@ -303,8 +303,8 @@ Value WithStatement::execute(Interpreter& interpreter, GlobalObject& global_obje VERIFY(object); - auto* object_environment_record = new_object_environment(*object, true, interpreter.vm().call_frame().environment_record); - TemporaryChange scope_change(interpreter.vm().call_frame().environment_record, object_environment_record); + auto* object_environment_record = new_object_environment(*object, true, interpreter.vm().call_frame().lexical_environment); + TemporaryChange scope_change(interpreter.vm().call_frame().lexical_environment, object_environment_record); return interpreter.execute_statement(global_object, m_body).value_or(js_undefined()); } @@ -850,7 +850,7 @@ Value ClassDeclaration::execute(Interpreter& interpreter, GlobalObject& global_o if (interpreter.exception()) return {}; - interpreter.current_environment_record()->put_into_environment_record(m_class_expression->name(), { class_constructor, DeclarationKind::Let }); + interpreter.lexical_environment()->put_into_environment_record(m_class_expression->name(), { class_constructor, DeclarationKind::Let }); return {}; } @@ -2052,8 +2052,8 @@ Value TryStatement::execute(Interpreter& interpreter, GlobalObject& global_objec HashMap parameters; parameters.set(m_handler->parameter(), Variable { exception->value(), DeclarationKind::Var }); - auto* catch_scope = interpreter.heap().allocate(global_object, move(parameters), interpreter.vm().call_frame().environment_record); - TemporaryChange scope_change(interpreter.vm().call_frame().environment_record, catch_scope); + auto* catch_scope = interpreter.heap().allocate(global_object, move(parameters), interpreter.vm().call_frame().lexical_environment); + TemporaryChange scope_change(interpreter.vm().call_frame().lexical_environment, catch_scope); result = interpreter.execute_statement(global_object, m_handler->body()); } } diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp index dfa96f32c1..0a9d77240c 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -48,7 +48,7 @@ Value Interpreter::run(Executable const& executable, BasicBlock const* entry_poi global_call_frame.this_value = &global_object(); static FlyString global_execution_context_name = "(*BC* global execution context)"; global_call_frame.function_name = global_execution_context_name; - global_call_frame.environment_record = &global_object(); + global_call_frame.lexical_environment = &global_object(); VERIFY(!vm().exception()); // FIXME: How do we know if we're in strict mode? Maybe the Bytecode::Block should know this? // global_call_frame.is_strict_mode = ???; diff --git a/Userland/Libraries/LibJS/Bytecode/Op.cpp b/Userland/Libraries/LibJS/Bytecode/Op.cpp index 23cef0d119..6e610433fd 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Op.cpp @@ -312,7 +312,7 @@ void Call::execute_impl(Bytecode::Interpreter& interpreter) const void NewFunction::execute_impl(Bytecode::Interpreter& interpreter) const { auto& vm = interpreter.vm(); - interpreter.accumulator() = ScriptFunction::create(interpreter.global_object(), m_function_node.name(), m_function_node.body(), m_function_node.parameters(), m_function_node.function_length(), vm.current_environment_record(), m_function_node.kind(), m_function_node.is_strict_mode(), m_function_node.is_arrow_function()); + interpreter.accumulator() = ScriptFunction::create(interpreter.global_object(), m_function_node.name(), m_function_node.body(), m_function_node.parameters(), m_function_node.function_length(), vm.lexical_environment(), m_function_node.kind(), m_function_node.is_strict_mode(), m_function_node.is_arrow_function()); } void Return::execute_impl(Bytecode::Interpreter& interpreter) const @@ -386,8 +386,9 @@ void PushDeclarativeEnvironmentRecord::execute_impl(Bytecode::Interpreter& inter HashMap resolved_variables; for (auto& it : m_variables) resolved_variables.set(interpreter.current_executable().get_string(it.key), it.value); - auto* environment_record = interpreter.vm().heap().allocate(interpreter.global_object(), move(resolved_variables), interpreter.vm().current_environment_record()); - interpreter.vm().call_frame().environment_record = environment_record; + auto* environment_record = interpreter.vm().heap().allocate(interpreter.global_object(), move(resolved_variables), interpreter.vm().lexical_environment()); + interpreter.vm().call_frame().lexical_environment = environment_record; + interpreter.vm().call_frame().variable_environment = environment_record; } void Yield::execute_impl(Bytecode::Interpreter& interpreter) const diff --git a/Userland/Libraries/LibJS/Interpreter.cpp b/Userland/Libraries/LibJS/Interpreter.cpp index 8779ad23f7..df7f4ec42c 100644 --- a/Userland/Libraries/LibJS/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Interpreter.cpp @@ -50,7 +50,8 @@ void Interpreter::run(GlobalObject& global_object, const Program& program) global_call_frame.this_value = &global_object; static FlyString global_execution_context_name = "(global execution context)"; global_call_frame.function_name = global_execution_context_name; - global_call_frame.environment_record = &global_object; + global_call_frame.lexical_environment = &global_object; + global_call_frame.variable_environment = &global_object; VERIFY(!vm.exception()); global_call_frame.is_strict_mode = program.is_strict_mode(); vm.push_call_frame(global_call_frame, global_object); @@ -83,7 +84,7 @@ void Interpreter::enter_scope(const ScopeNode& scope_node, ScopeType scope_type, { ScopeGuard guard([&] { for (auto& declaration : scope_node.functions()) { - auto* function = ScriptFunction::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), current_environment_record(), declaration.kind(), declaration.is_strict_mode()); + auto* function = ScriptFunction::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), lexical_environment(), declaration.kind(), declaration.is_strict_mode()); vm().set_variable(declaration.name(), function, global_object); } }); @@ -91,7 +92,7 @@ void Interpreter::enter_scope(const ScopeNode& scope_node, ScopeType scope_type, if (scope_type == ScopeType::Function) { push_scope({ scope_type, scope_node, false }); for (auto& declaration : scope_node.functions()) - current_environment_record()->put_into_environment_record(declaration.name(), { js_undefined(), DeclarationKind::Var }); + lexical_environment()->put_into_environment_record(declaration.name(), { js_undefined(), DeclarationKind::Var }); return; } @@ -131,8 +132,9 @@ void Interpreter::enter_scope(const ScopeNode& scope_node, ScopeType scope_type, bool pushed_environment_record = false; if (!scope_variables_with_declaration_kind.is_empty()) { - auto* environment_record = heap().allocate(global_object, move(scope_variables_with_declaration_kind), current_environment_record()); - vm().call_frame().environment_record = environment_record; + auto* environment_record = heap().allocate(global_object, move(scope_variables_with_declaration_kind), lexical_environment()); + vm().call_frame().lexical_environment = environment_record; + vm().call_frame().variable_environment = environment_record; pushed_environment_record = true; } @@ -143,8 +145,10 @@ void Interpreter::exit_scope(const ScopeNode& scope_node) { while (!m_scope_stack.is_empty()) { auto popped_scope = m_scope_stack.take_last(); - if (popped_scope.pushed_environment) - vm().call_frame().environment_record = vm().call_frame().environment_record->outer_environment(); + if (popped_scope.pushed_environment) { + vm().call_frame().lexical_environment = vm().call_frame().lexical_environment->outer_environment(); + vm().call_frame().variable_environment = vm().call_frame().variable_environment->outer_environment(); + } if (popped_scope.scope_node.ptr() == &scope_node) break; } @@ -195,8 +199,8 @@ Value Interpreter::execute_statement(GlobalObject& global_object, const Statemen FunctionEnvironmentRecord* Interpreter::current_function_environment_record() { - VERIFY(is(vm().call_frame().environment_record)); - return static_cast(vm().call_frame().environment_record); + VERIFY(is(vm().call_frame().lexical_environment)); + return static_cast(vm().call_frame().lexical_environment); } } diff --git a/Userland/Libraries/LibJS/Interpreter.h b/Userland/Libraries/LibJS/Interpreter.h index 182699cd45..a342fc8b10 100644 --- a/Userland/Libraries/LibJS/Interpreter.h +++ b/Userland/Libraries/LibJS/Interpreter.h @@ -56,7 +56,7 @@ public: ALWAYS_INLINE Heap& heap() { return vm().heap(); } ALWAYS_INLINE Exception* exception() { return vm().exception(); } - EnvironmentRecord* current_environment_record() { return vm().current_environment_record(); } + EnvironmentRecord* lexical_environment() { return vm().lexical_environment(); } FunctionEnvironmentRecord* current_function_environment_record(); diff --git a/Userland/Libraries/LibJS/Parser.cpp b/Userland/Libraries/LibJS/Parser.cpp index 02ef9a026b..3a3da65e36 100644 --- a/Userland/Libraries/LibJS/Parser.cpp +++ b/Userland/Libraries/LibJS/Parser.cpp @@ -258,7 +258,7 @@ NonnullRefPtr Parser::parse_program() program->add_variables(m_state.let_scopes.last()); program->add_functions(m_state.function_scopes.last()); } else { - syntax_error("Unclosed environment_record"); + syntax_error("Unclosed lexical_environment"); } program->source_range().end = position(); return program; diff --git a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp index 6fbfb754c4..21e23f295e 100644 --- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp +++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp @@ -181,7 +181,7 @@ ObjectEnvironmentRecord* new_object_environment(Object& object, bool is_with_env EnvironmentRecord& get_this_environment(VM& vm) { // FIXME: Should be the *lexical* environment. - for (auto* env = vm.current_environment_record(); env; env = env->outer_environment()) { + for (auto* env = vm.lexical_environment(); env; env = env->outer_environment()) { if (env->has_this_binding()) return *env; } diff --git a/Userland/Libraries/LibJS/Runtime/GeneratorFunctionConstructor.cpp b/Userland/Libraries/LibJS/Runtime/GeneratorFunctionConstructor.cpp index b2d5264d94..a1e0b02314 100644 --- a/Userland/Libraries/LibJS/Runtime/GeneratorFunctionConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/GeneratorFunctionConstructor.cpp @@ -62,7 +62,7 @@ Value GeneratorFunctionConstructor::construct(Function& new_target) block.dump(executable); } - return ScriptFunction::create(global_object(), function->name(), function->body(), function->parameters(), function->function_length(), vm().current_environment_record(), FunctionKind::Generator, function->is_strict_mode(), false); + return ScriptFunction::create(global_object(), function->name(), function->body(), function->parameters(), function->function_length(), vm().lexical_environment(), FunctionKind::Generator, function->is_strict_mode(), false); } } diff --git a/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp b/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp index 69b2674261..3da30afd6b 100644 --- a/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp @@ -107,7 +107,7 @@ Value GeneratorObject::next_impl(VM& vm, GlobalObject& global_object, Optionalrun(*m_generating_function->bytecode_executable(), next_block); diff --git a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp index 173d23d91d..e05a861c8f 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp @@ -362,7 +362,7 @@ JS_DEFINE_NATIVE_FUNCTION(GlobalObject::eval) } auto& caller_frame = vm.call_stack().at(vm.call_stack().size() - 2); - TemporaryChange scope_change(vm.call_frame().environment_record, caller_frame->environment_record); + TemporaryChange scope_change(vm.call_frame().lexical_environment, caller_frame->lexical_environment); auto& interpreter = vm.interpreter(); return interpreter.execute_statement(global_object, program).value_or(js_undefined()); diff --git a/Userland/Libraries/LibJS/Runtime/ScriptFunction.cpp b/Userland/Libraries/LibJS/Runtime/ScriptFunction.cpp index 47cd3e8f3b..9f657e8add 100644 --- a/Userland/Libraries/LibJS/Runtime/ScriptFunction.cpp +++ b/Userland/Libraries/LibJS/Runtime/ScriptFunction.cpp @@ -166,7 +166,7 @@ Value ScriptFunction::execute_function_body() if (i >= call_frame_args.size()) call_frame_args.resize(i + 1); call_frame_args[i] = argument_value; - vm.assign(param, argument_value, global_object(), true, vm.current_environment_record()); + vm.assign(param, argument_value, global_object(), true, vm.lexical_environment()); }); if (vm.exception()) @@ -191,7 +191,7 @@ Value ScriptFunction::execute_function_body() if (m_kind != FunctionKind::Generator) return result; - return GeneratorObject::create(global_object(), result, this, vm.call_frame().environment_record, bytecode_interpreter->snapshot_frame()); + return GeneratorObject::create(global_object(), result, this, vm.call_frame().lexical_environment, bytecode_interpreter->snapshot_frame()); } else { VERIFY(m_kind != FunctionKind::Generator); OwnPtr local_interpreter; diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp index a313801049..238db115f9 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.cpp +++ b/Userland/Libraries/LibJS/Runtime/VM.cpp @@ -107,7 +107,7 @@ void VM::gather_roots(HashTable& roots) if (argument.is_cell()) roots.set(&argument.as_cell()); } - roots.set(call_frame->environment_record); + roots.set(call_frame->lexical_environment); } #define __JS_ENUMERATE(SymbolName, snake_name) \ @@ -137,7 +137,7 @@ void VM::set_variable(const FlyString& name, Value value, GlobalObject& global_o { Optional possible_match; if (!specific_scope && m_call_stack.size()) { - for (auto* environment_record = current_environment_record(); environment_record; environment_record = environment_record->outer_environment()) { + for (auto* environment_record = lexical_environment(); environment_record; environment_record = environment_record->outer_environment()) { possible_match = environment_record->get_from_environment_record(name); if (possible_match.has_value()) { specific_scope = environment_record; @@ -169,7 +169,7 @@ bool VM::delete_variable(FlyString const& name) EnvironmentRecord* specific_scope = nullptr; Optional possible_match; if (!m_call_stack.is_empty()) { - for (auto* environment_record = current_environment_record(); environment_record; environment_record = environment_record->outer_environment()) { + for (auto* environment_record = lexical_environment(); environment_record; environment_record = environment_record->outer_environment()) { possible_match = environment_record->get_from_environment_record(name); if (possible_match.has_value()) { specific_scope = environment_record; @@ -365,7 +365,7 @@ Value VM::get_variable(const FlyString& name, GlobalObject& global_object) // a function parameter, or by a local var declaration, we use that. // Otherwise, we return a lazily constructed Array with all the argument values. // FIXME: Do something much more spec-compliant. - auto possible_match = current_environment_record()->get_from_environment_record(name); + auto possible_match = lexical_environment()->get_from_environment_record(name); if (possible_match.has_value()) return possible_match.value().value; if (!call_frame().arguments_object) { @@ -378,7 +378,7 @@ Value VM::get_variable(const FlyString& name, GlobalObject& global_object) return call_frame().arguments_object; } - for (auto* environment_record = current_environment_record(); environment_record; environment_record = environment_record->outer_environment()) { + for (auto* environment_record = lexical_environment(); environment_record; environment_record = environment_record->outer_environment()) { auto possible_match = environment_record->get_from_environment_record(name); if (exception()) return {}; @@ -395,7 +395,7 @@ Value VM::get_variable(const FlyString& name, GlobalObject& global_object) Reference VM::get_reference(const FlyString& name) { if (m_call_stack.size()) { - for (auto* environment_record = current_environment_record(); environment_record; environment_record = environment_record->outer_environment()) { + for (auto* environment_record = lexical_environment(); environment_record; environment_record = environment_record->outer_environment()) { if (is(environment_record)) break; auto possible_match = environment_record->get_from_environment_record(name); @@ -427,7 +427,8 @@ Value VM::construct(Function& function, Function& new_target, Optionalset_new_target(&new_target); @@ -462,8 +463,8 @@ Value VM::construct(Function& function, Function& new_target, Optional(current_environment_record())); - static_cast(current_environment_record())->replace_this_binding(result); + VERIFY(is(lexical_environment())); + static_cast(lexical_environment())->replace_this_binding(result); } auto prototype = new_target.get(names.prototype); if (exception()) @@ -525,7 +526,8 @@ Value VM::call_internal(Function& function, Value this_value, Optionalthis_binding_status() == FunctionEnvironmentRecord::ThisBindingStatus::Uninitialized); @@ -612,7 +614,7 @@ void VM::dump_backtrace() const void VM::dump_environment_record_chain() const { - for (auto* environment_record = current_environment_record(); environment_record; environment_record = environment_record->outer_environment()) { + for (auto* environment_record = lexical_environment(); environment_record; environment_record = environment_record->outer_environment()) { dbgln("+> {} ({:p})", environment_record->class_name(), environment_record); if (is(*environment_record)) { auto& declarative_environment_record = static_cast(*environment_record); diff --git a/Userland/Libraries/LibJS/Runtime/VM.h b/Userland/Libraries/LibJS/Runtime/VM.h index c095541f56..58a21210b3 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.h +++ b/Userland/Libraries/LibJS/Runtime/VM.h @@ -49,7 +49,8 @@ struct CallFrame { Value this_value; Vector arguments; Array* arguments_object { nullptr }; - EnvironmentRecord* environment_record { nullptr }; + EnvironmentRecord* lexical_environment { nullptr }; + EnvironmentRecord* variable_environment { nullptr }; bool is_strict_mode { false }; }; @@ -122,8 +123,11 @@ public: const Vector& call_stack() const { return m_call_stack; } Vector& call_stack() { return m_call_stack; } - EnvironmentRecord const* current_environment_record() const { return call_frame().environment_record; } - EnvironmentRecord* current_environment_record() { return call_frame().environment_record; } + EnvironmentRecord const* lexical_environment() const { return call_frame().lexical_environment; } + EnvironmentRecord* lexical_environment() { return call_frame().lexical_environment; } + + EnvironmentRecord const* variable_environment() const { return call_frame().variable_environment; } + EnvironmentRecord* variable_environment() { return call_frame().variable_environment; } bool in_strict_mode() const;