From 2691c656390f8cb699867f57cc2b762e1645d9f9 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Sat, 9 Oct 2021 19:00:06 +0100 Subject: [PATCH] LibJS: Convert create_immutable_binding() to ThrowCompletionOr Also add spec step comments to it while we're here. --- Userland/Libraries/LibJS/AST.cpp | 13 ++++++------- .../Libraries/LibJS/Runtime/AbstractOperations.cpp | 2 +- .../LibJS/Runtime/DeclarativeEnvironment.cpp | 8 +++++++- .../LibJS/Runtime/DeclarativeEnvironment.h | 2 +- .../LibJS/Runtime/ECMAScriptFunctionObject.cpp | 4 ++-- Userland/Libraries/LibJS/Runtime/Environment.h | 2 +- .../Libraries/LibJS/Runtime/GlobalEnvironment.cpp | 14 ++++++++------ .../Libraries/LibJS/Runtime/GlobalEnvironment.h | 2 +- .../Libraries/LibJS/Runtime/ObjectEnvironment.cpp | 2 +- .../Libraries/LibJS/Runtime/ObjectEnvironment.h | 2 +- 10 files changed, 29 insertions(+), 22 deletions(-) diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 4f8897981b..b1a059607e 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -192,7 +192,7 @@ Value FunctionExpression::instantiate_ordinary_function_expression(Interpreter& if (has_own_name) { VERIFY(scope); scope = new_declarative_environment(*scope); - scope->create_immutable_binding(global_object, name(), false); + MUST(scope->create_immutable_binding(global_object, name(), false)); } auto closure = ECMAScriptFunctionObject::create(global_object, used_name, body(), parameters(), function_length(), scope, kind(), is_strict_mode(), might_need_arguments_object(), contains_direct_call_to_eval(), is_arrow_function()); @@ -543,7 +543,7 @@ Value ForStatement::execute(Interpreter& interpreter, GlobalObject& global_objec auto& declaration = static_cast(*m_init); declaration.for_each_bound_name([&](auto const& name) { if (declaration.declaration_kind() == DeclarationKind::Const) { - loop_environment->create_immutable_binding(global_object, name, true); + MUST(loop_environment->create_immutable_binding(global_object, name, true)); } else { MUST(loop_environment->create_mutable_binding(global_object, name, false)); let_declarations.append(name); @@ -683,7 +683,7 @@ struct ForInOfHeadState { auto& for_declaration = static_cast(*expression_lhs); for_declaration.for_each_bound_name([&](auto const& name) { if (for_declaration.declaration_kind() == DeclarationKind::Const) - iteration_environment->create_immutable_binding(global_object, name, false); + MUST(iteration_environment->create_immutable_binding(global_object, name, false)); else MUST(iteration_environment->create_mutable_binding(global_object, name, true)); }); @@ -1210,7 +1210,7 @@ ThrowCompletionOr ClassExpression::class_definition_evaluation(Interprete VERIFY(environment); auto* class_scope = new_declarative_environment(*environment); if (!binding_name.is_null()) - class_scope->create_immutable_binding(global_object, binding_name, true); + MUST(class_scope->create_immutable_binding(global_object, binding_name, true)); ArmedScopeGuard restore_environment = [&] { vm.running_execution_context().lexical_environment = environment; @@ -3188,7 +3188,7 @@ void ScopeNode::block_declaration_instantiation(GlobalObject& global_object, Env auto is_constant_declaration = declaration.is_constant_declaration(); declaration.for_each_bound_name([&](auto const& name) { if (is_constant_declaration) { - environment->create_immutable_binding(global_object, name, true); + MUST(environment->create_immutable_binding(global_object, name, true)); } else { if (!MUST(environment->has_binding(name))) MUST(environment->create_mutable_binding(global_object, name, false)); @@ -3327,10 +3327,9 @@ ThrowCompletionOr Program::global_declaration_instantiation(Interpreter& i for_each_lexically_scoped_declaration([&](Declaration const& declaration) { declaration.for_each_bound_name([&](auto const& name) { if (declaration.is_constant_declaration()) - global_environment.create_immutable_binding(global_object, name, true); + (void)global_environment.create_immutable_binding(global_object, name, true); else (void)global_environment.create_mutable_binding(global_object, name, false); - if (interpreter.exception()) return IterationDecision::Break; return IterationDecision::Continue; diff --git a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp index c2b3e1a43d..63a513254f 100644 --- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp +++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp @@ -678,7 +678,7 @@ ThrowCompletionOr eval_declaration_instantiation(VM& vm, GlobalObject& glo program.for_each_lexically_scoped_declaration([&](Declaration const& declaration) { declaration.for_each_bound_name([&](auto const& name) { if (declaration.is_constant_declaration()) - lexical_environment->create_immutable_binding(global_object, name, true); + (void)lexical_environment->create_immutable_binding(global_object, name, true); else (void)lexical_environment->create_mutable_binding(global_object, name, false); if (vm.exception()) diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp index ad5d91e930..0aa77544e3 100644 --- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp +++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp @@ -66,8 +66,9 @@ ThrowCompletionOr DeclarativeEnvironment::create_mutable_binding(GlobalObj } // 9.1.1.1.3 CreateImmutableBinding ( N, S ), https://tc39.es/ecma262/#sec-declarative-environment-records-createimmutablebinding-n-s -void DeclarativeEnvironment::create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) +ThrowCompletionOr DeclarativeEnvironment::create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) { + // 2. Create an immutable binding in envRec for N and record that it is uninitialized. If S is true, record that the newly created binding is a strict binding. m_bindings.append(Binding { .value = {}, .strict = strict, @@ -76,7 +77,12 @@ void DeclarativeEnvironment::create_immutable_binding(GlobalObject&, FlyString c .initialized = false, }); auto result = m_names.set(name, m_bindings.size() - 1); + + // 1. Assert: envRec does not already have a binding for N. VERIFY(result == AK::HashSetResult::InsertedNewEntry); + + // 3. Return NormalCompletion(empty). + return {}; } // 9.1.1.1.4 InitializeBinding ( N, V ), https://tc39.es/ecma262/#sec-declarative-environment-records-initializebinding-n-v diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h index bf850633d7..51413459ea 100644 --- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h +++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h @@ -24,7 +24,7 @@ public: virtual ThrowCompletionOr has_binding(FlyString const& name, Optional* = nullptr) const override; virtual ThrowCompletionOr create_mutable_binding(GlobalObject&, FlyString const& name, bool can_be_deleted) override; - virtual void create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override; + virtual ThrowCompletionOr create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override; virtual void initialize_binding(GlobalObject&, FlyString const& name, Value) override; virtual void set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override; virtual Value get_binding_value(GlobalObject&, FlyString const& name, bool strict) override; diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp index 3e91172b79..065b6046e2 100644 --- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp @@ -374,7 +374,7 @@ ThrowCompletionOr ECMAScriptFunctionObject::function_declaration_instantia arguments_object = create_mapped_arguments_object(global_object(), *this, formal_parameters(), vm.running_execution_context().arguments, *environment); if (is_strict_mode()) - environment->create_immutable_binding(global_object(), vm.names.arguments.as_string(), false); + MUST(environment->create_immutable_binding(global_object(), vm.names.arguments.as_string(), false)); else MUST(environment->create_mutable_binding(global_object(), vm.names.arguments.as_string(), false)); @@ -524,7 +524,7 @@ ThrowCompletionOr ECMAScriptFunctionObject::function_declaration_instantia scope_body->for_each_lexically_scoped_declaration([&](Declaration const& declaration) { declaration.for_each_bound_name([&](auto const& name) { if (declaration.is_constant_declaration()) - lex_environment->create_immutable_binding(global_object(), name, true); + MUST(lex_environment->create_immutable_binding(global_object(), name, true)); else MUST(lex_environment->create_mutable_binding(global_object(), name, false)); return IterationDecision::Continue; diff --git a/Userland/Libraries/LibJS/Runtime/Environment.h b/Userland/Libraries/LibJS/Runtime/Environment.h index 3312cd949f..5b1552ba58 100644 --- a/Userland/Libraries/LibJS/Runtime/Environment.h +++ b/Userland/Libraries/LibJS/Runtime/Environment.h @@ -35,7 +35,7 @@ public: virtual ThrowCompletionOr has_binding([[maybe_unused]] FlyString const& name, [[maybe_unused]] Optional* out_index = nullptr) const { return false; } virtual ThrowCompletionOr create_mutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, [[maybe_unused]] bool can_be_deleted) { return {}; } - virtual void create_immutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, [[maybe_unused]] bool strict) { } + virtual ThrowCompletionOr create_immutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, [[maybe_unused]] bool strict) { return {}; } virtual void initialize_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, Value) { } virtual void set_mutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, Value, [[maybe_unused]] bool strict) { } virtual Value get_binding_value(GlobalObject&, [[maybe_unused]] FlyString const& name, [[maybe_unused]] bool strict) { return {}; } diff --git a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp index 1d43ec6c6a..c6e5043180 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp +++ b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp @@ -64,13 +64,15 @@ ThrowCompletionOr GlobalEnvironment::create_mutable_binding(GlobalObject& } // 9.1.1.4.3 CreateImmutableBinding ( N, S ), https://tc39.es/ecma262/#sec-global-environment-records-createimmutablebinding-n-s -void GlobalEnvironment::create_immutable_binding(GlobalObject& global_object, FlyString const& name, bool strict) +ThrowCompletionOr GlobalEnvironment::create_immutable_binding(GlobalObject& global_object, FlyString const& name, bool strict) { - if (MUST(m_declarative_record->has_binding(name))) { - global_object.vm().throw_exception(global_object, ErrorType::FixmeAddAnErrorString); - return; - } - m_declarative_record->create_immutable_binding(global_object, name, strict); + // 1. Let DclRec be envRec.[[DeclarativeRecord]]. + // 2. If DclRec.HasBinding(N) is true, throw a TypeError exception. + if (MUST(m_declarative_record->has_binding(name))) + return vm().throw_completion(global_object, ErrorType::FixmeAddAnErrorString); + + // 3. Return DclRec.CreateImmutableBinding(N, S). + return m_declarative_record->create_immutable_binding(global_object, name, strict); } // 9.1.1.4.4 InitializeBinding ( N, V ), https://tc39.es/ecma262/#sec-global-environment-records-initializebinding-n-v diff --git a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h index fa96cdcbe1..2b47f1ac67 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h +++ b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h @@ -21,7 +21,7 @@ public: virtual ThrowCompletionOr has_binding(FlyString const& name, Optional* = nullptr) const override; virtual ThrowCompletionOr create_mutable_binding(GlobalObject&, FlyString const& name, bool can_be_deleted) override; - virtual void create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override; + virtual ThrowCompletionOr create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override; virtual void initialize_binding(GlobalObject&, FlyString const& name, Value) override; virtual void set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override; virtual Value get_binding_value(GlobalObject&, FlyString const& name, bool strict) override; diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp index 80f4230ae4..8561991dd2 100644 --- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp +++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp @@ -69,7 +69,7 @@ ThrowCompletionOr ObjectEnvironment::create_mutable_binding(GlobalObject&, } // 9.1.1.2.3 CreateImmutableBinding ( N, S ), https://tc39.es/ecma262/#sec-object-environment-records-createimmutablebinding-n-s -void ObjectEnvironment::create_immutable_binding(GlobalObject&, FlyString const&, bool) +ThrowCompletionOr ObjectEnvironment::create_immutable_binding(GlobalObject&, FlyString const&, bool) { // "The CreateImmutableBinding concrete method of an object Environment Record is never used within this specification." VERIFY_NOT_REACHED(); diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h index ea4d4c4ab8..54d685e666 100644 --- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h +++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h @@ -22,7 +22,7 @@ public: virtual ThrowCompletionOr has_binding(FlyString const& name, Optional* = nullptr) const override; virtual ThrowCompletionOr create_mutable_binding(GlobalObject&, FlyString const& name, bool can_be_deleted) override; - virtual void create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override; + virtual ThrowCompletionOr create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override; virtual void initialize_binding(GlobalObject&, FlyString const& name, Value) override; virtual void set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override; virtual Value get_binding_value(GlobalObject&, FlyString const& name, bool strict) override;