LibJS: Update spec steps / links for the import-assertions proposal

This proposal has reached stage 4 and been merged into the main ECMA-262
spec. See:

4e3450e

Libraries/LibJS/AST.cpp

@@ -1898,10 +1898,10 @@ ModuleRequest::ModuleRequest(FlyString module_specifier_, Vector<ImportAttribute
     : module_specifier(move(module_specifier_))
     , attributes(move(attributes))
 {
-    // Perform step 10.e. from EvaluateImportCall, https://tc39.es/proposal-import-attributes/#sec-evaluate-import-call
+    // 13.3.10.2 EvaluateImportCall ( specifierExpression [ , optionsExpression ] ), https://tc39.es/ecma262/#sec-evaluate-import-call
-    // or step 2. from WithClauseToAttributes, https://tc39.es/proposal-import-attributes/#sec-with-clause-to-attributes
+    // 16.2.2.4 Static Semantics: WithClauseToAttributes, https://tc39.es/ecma262/#sec-withclausetoattributes
-    // e. / 2. Sort assertions by the code point order of the [[Key]] of each element.
+    // 2. Sort attributes according to the lexicographic order of their [[Key]] field, treating the value of each such
-    // NOTE: This sorting is observable only in that hosts are prohibited from distinguishing among assertions by the order they occur in.
+    //    field as a sequence of UTF-16 code unit values.
     quick_sort(this->attributes, [](ImportAttribute const& lhs, ImportAttribute const& rhs) {
         return lhs.key < rhs.key;
     });

Libraries/LibJS/CyclicModule.cpp

@@ -322,7 +322,7 @@ ThrowCompletionOr<u32> CyclicModule::inner_module_linking(VM& vm, Vector<Module*
 }
 
 // 16.2.1.5.3 Evaluate ( ), https://tc39.es/ecma262/#sec-moduleevaluation
-ThrowCompletionOr<Promise*> CyclicModule::evaluate(VM& vm)
+ThrowCompletionOr<GC::Ref<Promise>> CyclicModule::evaluate(VM& vm)
 {
     dbgln_if(JS_MODULE_DEBUG, "[JS MODULE] evaluate[{}](vm)", this);
     // 1. Assert: This call to Evaluate is not happening at the same time as another call to Evaluate within the surrounding agent.
@@ -336,7 +336,7 @@ ThrowCompletionOr<Promise*> CyclicModule::evaluate(VM& vm)
     //       In that case we first check if this module itself has a top level capability.
     //       See also: https://github.com/tc39/ecma262/issues/2823 .
     if (m_top_level_capability != nullptr)
-        return as<Promise>(m_top_level_capability->promise().ptr());
+        return as<Promise>(*m_top_level_capability->promise());
 
     // 3. If module.[[Status]] is either evaluating-async or evaluated, set module to module.[[CycleRoot]].
     if ((m_status == ModuleStatus::EvaluatingAsync || m_status == ModuleStatus::Evaluated) && m_cycle_root != this) {
@@ -350,7 +350,7 @@ ThrowCompletionOr<Promise*> CyclicModule::evaluate(VM& vm)
     // 4. If module.[[TopLevelCapability]] is not empty, then
     if (m_top_level_capability != nullptr) {
         // a. Return module.[[TopLevelCapability]].[[Promise]].
-        return as<Promise>(m_top_level_capability->promise().ptr());
+        return as<Promise>(*m_top_level_capability->promise());
     }
 
     // 5. Let stack be a new empty List.
@@ -416,7 +416,7 @@ ThrowCompletionOr<Promise*> CyclicModule::evaluate(VM& vm)
     }
 
     // 11. Return capability.[[Promise]].
-    return as<Promise>(m_top_level_capability->promise().ptr());
+    return as<Promise>(*m_top_level_capability->promise());
 }
 
 // 16.2.1.5.2.1 InnerModuleEvaluation ( module, stack, index ), https://tc39.es/ecma262/#sec-innermoduleevaluation
@@ -892,7 +892,7 @@ void continue_dynamic_import(GC::Ref<PromiseCapability> promise_capability, Thro
             auto namespace_ = module.get_module_namespace(vm);
 
             // ii. Perform ! Call(promiseCapability.[[Resolve]], undefined, « namespace »).
-            MUST(call(vm, *promise_capability->resolve(), js_undefined(), namespace_.value()));
+            MUST(call(vm, *promise_capability->resolve(), js_undefined(), namespace_));
 
             // iii. Return unused.
             return js_undefined();

Libraries/LibJS/CyclicModule.h

@@ -33,7 +33,7 @@ public:
     // Note: Do not call these methods directly unless you are HostResolveImportedModule.
     //       Badges cannot be used because other hosts must be able to call this (and it is called recursively)
     virtual ThrowCompletionOr<void> link(VM& vm) override final;
-    virtual ThrowCompletionOr<Promise*> evaluate(VM& vm) override final;
+    virtual ThrowCompletionOr<GC::Ref<Promise>> evaluate(VM& vm) override final;
 
     virtual PromiseCapability& load_requested_modules(GC::Ptr<GraphLoadingState::HostDefined>) override;
 

Libraries/LibJS/Module.cpp

@@ -52,7 +52,7 @@ ThrowCompletionOr<u32> Module::inner_module_linking(VM& vm, Vector<Module*>&, u3
 ThrowCompletionOr<u32> Module::inner_module_evaluation(VM& vm, Vector<Module*>&, u32 index)
 {
     // 1. If module is not a Cyclic Module Record, then
-    // a. Let promise be ! module.Evaluate().
+    // a. Let promise be module.Evaluate().
     auto promise = TRY(evaluate(vm));
 
     // b. Assert: promise.[[PromiseState]] is not pending.
@@ -122,26 +122,26 @@ void finish_loading_imported_module(ImportedModuleReferrer referrer, ModuleReque
 }
 
 // 16.2.1.10 GetModuleNamespace ( module ), https://tc39.es/ecma262/#sec-getmodulenamespace
-ThrowCompletionOr<Object*> Module::get_module_namespace(VM& vm)
+GC::Ref<Object> Module::get_module_namespace(VM& vm)
 {
-    // 1. Assert: If module is a Cyclic Module Record, then module.[[Status]] is not unlinked.
+    // 1. Assert: If module is a Cyclic Module Record, then module.[[Status]] is not NEW or UNLINKED.
     // FIXME: Spec bug: https://github.com/tc39/ecma262/issues/3114
 
     // 2. Let namespace be module.[[Namespace]].
-    auto* namespace_ = m_namespace.ptr();
+    auto namespace_ = m_namespace;
 
-    // 3. If namespace is empty, then
+    // 3. If namespace is EMPTY, then
     if (!namespace_) {
-        // a. Let exportedNames be ? module.GetExportedNames().
+        // a. Let exportedNames be module.GetExportedNames().
-        auto exported_names = TRY(get_exported_names(vm));
+        auto exported_names = get_exported_names(vm);
 
         // b. Let unambiguousNames be a new empty List.
         Vector<FlyString> unambiguous_names;
 
         // c. For each element name of exportedNames, do
         for (auto& name : exported_names) {
-            // i. Let resolution be ? module.ResolveExport(name).
+            // i. Let resolution be module.ResolveExport(name).
-            auto resolution = TRY(resolve_export(vm, name));
+            auto resolution = resolve_export(vm, name);
 
             // ii. If resolution is a ResolvedBinding Record, append name to unambiguousNames.
             if (resolution.is_valid())
@@ -150,16 +150,20 @@ ThrowCompletionOr<Object*> Module::get_module_namespace(VM& vm)
 
         // d. Set namespace to ModuleNamespaceCreate(module, unambiguousNames).
         namespace_ = module_namespace_create(unambiguous_names);
-        VERIFY(m_namespace);
-        // Note: This set the local variable 'namespace' and not the member variable which is done by ModuleNamespaceCreate
     }
 
     // 4. Return namespace.
-    return namespace_;
+    return *namespace_;
+}
+
+Vector<FlyString> Module::get_exported_names(VM& vm)
+{
+    HashTable<Module const*> export_star_set;
+    return get_exported_names(vm, export_star_set);
 }
 
 // 10.4.6.12 ModuleNamespaceCreate ( module, exports ), https://tc39.es/ecma262/#sec-modulenamespacecreate
-Object* Module::module_namespace_create(Vector<FlyString> unambiguous_names)
+GC::Ref<Object> Module::module_namespace_create(Vector<FlyString> unambiguous_names)
 {
     auto& realm = this->realm();
 
@@ -176,7 +180,7 @@ Object* Module::module_namespace_create(Vector<FlyString> unambiguous_names)
     auto module_namespace = realm.create<ModuleNamespaceObject>(realm, this, move(unambiguous_names));
 
     // 9. Set module.[[Namespace]] to M.
-    m_namespace = make_root(module_namespace);
+    m_namespace = module_namespace;
 
     // 10. Return M.
     return module_namespace;

Libraries/LibJS/Module.h

@@ -104,13 +104,15 @@ public:
 
     Script::HostDefined* host_defined() const { return m_host_defined; }
 
-    ThrowCompletionOr<Object*> get_module_namespace(VM& vm);
+    GC::Ref<Object> get_module_namespace(VM& vm);
 
     virtual ThrowCompletionOr<void> link(VM& vm) = 0;
-    virtual ThrowCompletionOr<Promise*> evaluate(VM& vm) = 0;
+    virtual ThrowCompletionOr<GC::Ref<Promise>> evaluate(VM& vm) = 0;
 
-    virtual ThrowCompletionOr<Vector<FlyString>> get_exported_names(VM& vm, Vector<Module*> export_star_set = {}) = 0;
+    Vector<FlyString> get_exported_names(VM& vm);
-    virtual ThrowCompletionOr<ResolvedBinding> resolve_export(VM& vm, FlyString const& export_name, Vector<ResolvedBinding> resolve_set = {}) = 0;
+    virtual Vector<FlyString> get_exported_names(VM& vm, HashTable<Module const*>& export_star_set) = 0;
+
+    virtual ResolvedBinding resolve_export(VM& vm, FlyString const& export_name, Vector<ResolvedBinding> resolve_set = {}) = 0;
 
     virtual ThrowCompletionOr<u32> inner_module_linking(VM& vm, Vector<Module*>& stack, u32 index);
     virtual ThrowCompletionOr<u32> inner_module_evaluation(VM& vm, Vector<Module*>& stack, u32 index);
@@ -128,7 +130,7 @@ protected:
     }
 
 private:
-    Object* module_namespace_create(Vector<FlyString> unambiguous_names);
+    GC::Ref<Object> module_namespace_create(Vector<FlyString> unambiguous_names);
 
     // These handles are only safe as long as the VM they live in is valid.
     // But evaluated modules SHOULD be stored in the VM so unless you intentionally

Libraries/LibJS/Parser.cpp

@@ -1292,15 +1292,14 @@ RefPtr<MetaProperty const> Parser::try_parse_import_meta_expression()
     return create_ast_node<MetaProperty>({ m_source_code, rule_start.position(), position() }, MetaProperty::Type::ImportMeta);
 }
 
+// https://tc39.es/ecma262/#prod-ImportCall
 NonnullRefPtr<ImportCall const> Parser::parse_import_call()
 {
     auto rule_start = push_start();
 
-    // We use the extended definition:
+    // ImportCall[Yield, Await] :
-    //  ImportCall[Yield, Await]:
+    //     import ( AssignmentExpression[+In, ?Yield, ?Await] ,opt )
-    //      import(AssignmentExpression[+In, ?Yield, ?Await] ,opt)
+    //     import ( AssignmentExpression[+In, ?Yield, ?Await] , AssignmentExpression[+In, ?Yield, ?Await] ,opt )
-    //      import(AssignmentExpression[+In, ?Yield, ?Await] ,AssignmentExpression[+In, ?Yield, ?Await] ,opt)
-    // From https://tc39.es/proposal-import-attributes/#sec-evaluate-import-call
 
     consume(TokenType::Import);
     consume(TokenType::ParenOpen);
@@ -4597,7 +4596,7 @@ FlyString Parser::consume_string_value()
     return value;
 }
 
-// WithClause, https://tc39.es/proposal-import-attributes/#prod-WithClause
+// https://tc39.es/ecma262/#prod-WithClause
 ModuleRequest Parser::parse_module_request()
 {
     // Does not include the 'from' since that is not always required.
@@ -4651,13 +4650,12 @@ ModuleRequest Parser::parse_module_request()
 
 static FlyString default_string_value = "default"_fly_string;
 
+// https://tc39.es/ecma262/#prod-ImportDeclaration
 NonnullRefPtr<ImportStatement const> Parser::parse_import_statement(Program& program)
 {
-    // We use the extended syntax which adds:
+    // ImportDeclaration :
-    //     ImportDeclaration:
+    //     import ImportClause FromClause WithClauseopt ;
-    //         import ImportClause FromClause WithClause[opt] ;
+    //     import ModuleSpecifier WithClauseopt ;
-    //         import ModuleSpecifier WithClause[opt] ;
-    // From: https://tc39.es/proposal-import-attributes/#prod-ImportDeclaration
 
     auto rule_start = push_start();
     if (program.type() != Program::Type::Module)
@@ -4811,12 +4809,17 @@ NonnullRefPtr<ImportStatement const> Parser::parse_import_statement(Program& pro
     return create_ast_node<ImportStatement>({ m_source_code, rule_start.position(), position() }, move(module_request), move(entries));
 }
 
+// https://tc39.es/ecma262/#prod-ExportDeclaration
 NonnullRefPtr<ExportStatement const> Parser::parse_export_statement(Program& program)
 {
-    // We use the extended syntax which adds:
+    // ExportDeclaration:
-    //  ExportDeclaration:
+    //     export ExportFromClause FromClause WithClauseopt ;
-    //      export ExportFromClause FromClause [no LineTerminator here] WithClause ;
+    //     export NamedExports ;
-    // From:  https://tc39.es/proposal-import-attributes/#sec-exports
+    //     export VariableStatement[~Yield, +Await]
+    //     export Declaration[~Yield, +Await]
+    //     export default HoistableDeclaration[~Yield, +Await, +Default]
+    //     export default ClassDeclaration[~Yield, +Await, +Default]
+    //     export default [lookahead ∉ { function, async [no LineTerminator here] function, class }] AssignmentExpression[+In, ~Yield, +Await] ;
 
     auto rule_start = push_start();
     if (program.type() != Program::Type::Module)

Libraries/LibJS/Runtime/AbstractOperations.cpp

@@ -1708,7 +1708,7 @@ Completion dispose_resources(VM& vm, DisposeCapability& dispose_capability, Comp
     return completion;
 }
 
-// https://tc39.es/proposal-import-attributes/#sec-AllImportAttributesSupported
+// 16.2.1.12 AllImportAttributesSupported ( attributes ), https://tc39.es/ecma262/#sec-AllImportAttributesSupported
 static bool all_import_attributes_supported(VM& vm, Vector<ImportAttribute> const& attributes)
 {
     // 1. Let supported be HostGetSupportedImportAttributes().
@@ -1725,8 +1725,8 @@ static bool all_import_attributes_supported(VM& vm, Vector<ImportAttribute> cons
     return true;
 }
 
-// 13.3.10.2 EvaluateImportCall ( specifierExpression [ , optionsExpression ] ), https://tc39.es/proposal-import-attributes/#sec-evaluate-import-call
+// 13.3.10.2 EvaluateImportCall ( specifierExpression [ , optionsExpression ] ), https://tc39.es/ecma262/#sec-evaluate-import-call
-ThrowCompletionOr<Value> perform_import_call(VM& vm, Value specifier, Value options_value)
+ThrowCompletionOr<Value> perform_import_call(VM& vm, Value specifier, Value options)
 {
     auto& realm = *vm.current_realm();
 
@@ -1744,49 +1744,57 @@ ThrowCompletionOr<Value> perform_import_call(VM& vm, Value specifier, Value opti
         return GC::Ref<CyclicModule> { as<CyclicModule>(*active_script_or_module.get<GC::Ref<Module>>()) };
     }();
 
+    // 3. Let specifierRef be ? Evaluation of specifierExpression.
+    // 4. Let specifier be ? GetValue(specifierRef).
+    // 5. If optionsExpression is present, then
+    //     a. Let optionsRef be ? Evaluation of optionsExpression.
+    //     b. Let options be ? GetValue(optionsRef).
+    // 6. Else,
+    //    a. Let options be undefined.
+
     // 7. Let promiseCapability be ! NewPromiseCapability(%Promise%).
     auto promise_capability = MUST(new_promise_capability(vm, realm.intrinsics().promise_constructor()));
 
     // 8. Let specifierString be Completion(ToString(specifier)).
     // 9. IfAbruptRejectPromise(specifierString, promiseCapability).
-    auto specifier_string = TRY_OR_REJECT_WITH_VALUE(vm, promise_capability, specifier.to_string(vm));
+    FlyString specifier_string = TRY_OR_REJECT(vm, promise_capability, specifier.to_string(vm));
 
     // 10. Let attributes be a new empty List.
     Vector<ImportAttribute> attributes;
 
     // 11. If options is not undefined, then
-    if (!options_value.is_undefined()) {
+    if (!options.is_undefined()) {
-        // a. If Type(options) is not Object,
+        // a. If options is not an Object, then
-        if (!options_value.is_object()) {
+        if (!options.is_object()) {
-            auto error = TypeError::create(realm, TRY_OR_THROW_OOM(vm, String::formatted(ErrorType::NotAnObject.message(), "ImportOptions")));
             // i. Perform ! Call(promiseCapability.[[Reject]], undefined, « a newly created TypeError object »).
+            auto error = TypeError::create(realm, MUST(String::formatted(ErrorType::NotAnObject.message(), "options"sv)));
             MUST(call(vm, *promise_capability->reject(), js_undefined(), error));
 
             // ii. Return promiseCapability.[[Promise]].
-            return Value { promise_capability->promise() };
+            return promise_capability->promise();
         }
 
         // b. Let attributesObj be Completion(Get(options, "with")).
         // c. IfAbruptRejectPromise(attributesObj, promiseCapability).
-        auto attributes_obj = TRY_OR_REJECT_WITH_VALUE(vm, promise_capability, options_value.get(vm, vm.names.with));
+        auto attributes_obj = TRY_OR_REJECT(vm, promise_capability, options.get(vm, vm.names.with));
 
         // d. If attributesObj is not undefined, then
         if (!attributes_obj.is_undefined()) {
-            // i. If Type(attributesObj) is not Object, then
+            // i. If attributesObj is not an Object, then
             if (!attributes_obj.is_object()) {
-                auto error = TypeError::create(realm, TRY_OR_THROW_OOM(vm, String::formatted(ErrorType::NotAnObject.message(), "ImportOptionsAssertions")));
                 // 1. Perform ! Call(promiseCapability.[[Reject]], undefined, « a newly created TypeError object »).
+                auto error = TypeError::create(realm, MUST(String::formatted(ErrorType::NotAnObject.message(), "with"sv)));
                 MUST(call(vm, *promise_capability->reject(), js_undefined(), error));
 
                 // 2. Return promiseCapability.[[Promise]].
-                return Value { promise_capability->promise() };
+                return promise_capability->promise();
             }
 
-            // ii. Let entries be Completion(EnumerableOwnProperties(attributesObj, key+value)).
+            // ii. Let entries be Completion(EnumerableOwnProperties(attributesObj, KEY+VALUE)).
             // iii. IfAbruptRejectPromise(entries, promiseCapability).
-            auto entries = TRY_OR_REJECT_WITH_VALUE(vm, promise_capability, attributes_obj.as_object().enumerable_own_property_names(Object::PropertyKind::KeyAndValue));
+            auto entries = TRY_OR_REJECT(vm, promise_capability, attributes_obj.as_object().enumerable_own_property_names(Object::PropertyKind::KeyAndValue));
 
-            // iv. For each entry of entries, do
+            // iv. For each element entry of entries, do
             for (auto const& entry : entries) {
                 // 1. Let key be ! Get(entry, "0").
                 auto key = MUST(entry.get(vm, PropertyKey(0)));
@@ -1794,46 +1802,48 @@ ThrowCompletionOr<Value> perform_import_call(VM& vm, Value specifier, Value opti
                 // 2. Let value be ! Get(entry, "1").
                 auto value = MUST(entry.get(vm, PropertyKey(1)));
 
-                // 3. If Type(value) is not String, then
+                // 3. If key is a String, then
-                if (!value.is_string()) {
+                if (key.is_string()) {
-                    auto error = TypeError::create(realm, TRY_OR_THROW_OOM(vm, String::formatted(ErrorType::NotAString.message(), "Import Assertion option value")));
+                    // a. If value is not a String, then
-                    // a. Perform ! Call(promiseCapability.[[Reject]], undefined, « a newly created TypeError object »).
+                    if (!value.is_string()) {
-                    MUST(call(vm, *promise_capability->reject(), js_undefined(), error));
+                        // i. Perform ! Call(promiseCapability.[[Reject]], undefined, « a newly created TypeError object »).
+                        auto error = TypeError::create(realm, MUST(String::formatted(ErrorType::NotAString.message(), "Import attribute value"sv)));
+                        MUST(call(vm, *promise_capability->reject(), js_undefined(), error));
 
-                    // b. Return promiseCapability.[[Promise]].
+                        // ii. Return promiseCapability.[[Promise]].
-                    return Value { promise_capability->promise() };
+                        return promise_capability->promise();
+                    }
+
+                    // b. Append the ImportAttribute Record { [[Key]]: key, [[Value]]: value } to attributes.
+                    attributes.empend(key.as_string().utf8_string(), value.as_string().utf8_string());
                 }
-
-                // 4. Append the ImportAttribute Record { [[Key]]: key, [[Value]]: value } to attributes.
-                attributes.empend(key.as_string().utf8_string(), value.as_string().utf8_string());
             }
         }
 
         // e. If AllImportAttributesSupported(attributes) is false, then
         if (!all_import_attributes_supported(vm, attributes)) {
-            auto error = TypeError::create(realm, TRY_OR_THROW_OOM(vm, String::formatted(ErrorType::NotAnObject.message(), "ImportOptionsAssertions")));
             // i. Perform ! Call(promiseCapability.[[Reject]], undefined, « a newly created TypeError object »).
+            auto error = TypeError::create(realm, MUST(String::formatted(ErrorType::ImportAttributeUnsupported.message())));
             MUST(call(vm, *promise_capability->reject(), js_undefined(), error));
 
             // ii. Return promiseCapability.[[Promise]].
-            return Value { promise_capability->promise() };
+            return promise_capability->promise();
         }
 
-        // f. Sort attributes according to the lexicographic order of their [[Key]] fields,
+        // f. Sort attributes according to the lexicographic order of their [[Key]] field, treating the value of each
-        //    treating the value of each such field as a sequence of UTF-16 code unit values.
+        //    such field as a sequence of UTF-16 code unit values. NOTE: This sorting is observable only in that hosts
-        //    NOTE: This sorting is observable only in that hosts are prohibited from
+        //    are prohibited from changing behaviour based on the order in which attributes are enumerated.
-        //    distinguishing among attributes by the order they occur in.
         // NOTE: This is done when constructing the ModuleRequest.
     }
 
     // 12. Let moduleRequest be a new ModuleRequest Record { [[Specifier]]: specifierString, [[Attributes]]: attributes }.
-    ModuleRequest request { specifier_string, attributes };
+    ModuleRequest request { move(specifier_string), move(attributes) };
 
-    // 13. Perform HostLoadImportedModule(referrer, moduleRequest, empty, promiseCapability).
+    // 13. Perform HostLoadImportedModule(referrer, moduleRequest, EMPTY, promiseCapability).
-    vm.host_load_imported_module(referrer, move(request), nullptr, promise_capability);
+    vm.host_load_imported_module(referrer, request, nullptr, promise_capability);
 
     // 13. Return promiseCapability.[[Promise]].
-    return Value { promise_capability->promise() };
+    return promise_capability->promise();
 }
 
 // 14.5.2.1 GetOptionsObject ( options ), https://tc39.es/proposal-temporal/#sec-getoptionsobject

Libraries/LibJS/Runtime/ErrorTypes.h

@@ -52,6 +52,7 @@
         "Legacy RegExp static property getter must be called with the RegExp constructor for the this value")                       \
     M(GetLegacyRegExpStaticPropertyValueEmpty, "Legacy RegExp static property getter value is empty")                               \
     M(GlobalEnvironmentAlreadyHasBinding, "Global environment already has binding '{}'")                                            \
+    M(ImportAttributeUnsupported, "Every import attribute is not supported")                                                        \
     M(IndexOutOfRange, "Index {} is out of range of array length {}")                                                               \
     M(InOperatorWithObject, "'in' operator must be used on an object")                                                              \
     M(InstanceOfOperatorBadPrototype, "'prototype' property of {} is not an object")                                                \

Libraries/LibJS/Runtime/ModuleNamespaceObject.cpp

@@ -155,8 +155,8 @@ ThrowCompletionOr<Value> ModuleNamespaceObject::internal_get(PropertyKey const&
         return js_undefined();
 
     // 4. Let m be O.[[Module]].
-    // 5. Let binding be ! m.ResolveExport(P).
+    // 5. Let binding be m.ResolveExport(P).
-    auto binding = MUST(m_module->resolve_export(vm, property_key.to_string()));
+    auto binding = m_module->resolve_export(vm, property_key.to_string());
 
     // 6. Assert: binding is a ResolvedBinding Record.
     VERIFY(binding.is_valid());
@@ -169,8 +169,8 @@ ThrowCompletionOr<Value> ModuleNamespaceObject::internal_get(PropertyKey const&
 
     // 9. If binding.[[BindingName]] is namespace, then
     if (binding.is_namespace()) {
-        // a. Return ? GetModuleNamespace(targetModule).
+        // a. Return GetModuleNamespace(targetModule)..
-        return TRY(target_module->get_module_namespace(vm));
+        return target_module->get_module_namespace(vm);
     }
 
     // 10. Let targetEnv be targetModule.[[Environment]].

Libraries/LibJS/Runtime/ModuleRequest.h

@@ -18,7 +18,7 @@ struct ModuleWithSpecifier {
     GC::Ref<Module> module; // [[Module]]
 };
 
-// https://tc39.es/proposal-import-attributes/#importattribute-record
+// https://tc39.es/ecma262/#importattribute-record
 struct ImportAttribute {
     String key;
     String value;
@@ -26,7 +26,7 @@ struct ImportAttribute {
     bool operator==(ImportAttribute const&) const = default;
 };
 
-// https://tc39.es/proposal-import-attributes/#modulerequest-record
+// https://tc39.es/ecma262/#modulerequest-record
 struct ModuleRequest {
     ModuleRequest() = default;
 

Libraries/LibJS/Runtime/PromiseCapability.h

@@ -64,26 +64,6 @@ private:
 #define TRY_OR_MUST_REJECT(vm, capability, expression) \
     __TRY_OR_REJECT(vm, capability, expression, MUST)
 
-// 27.2.1.1.1 IfAbruptRejectPromise ( value, capability ), https://tc39.es/ecma262/#sec-ifabruptrejectpromise
-#define TRY_OR_REJECT_WITH_VALUE(vm, capability, expression)                                                                     \
-    ({                                                                                                                           \
-        auto&& _temporary_try_or_reject_result = (expression);                                                                   \
-        /* 1. If value is an abrupt completion, then */                                                                          \
-        if (_temporary_try_or_reject_result.is_error()) {                                                                        \
-            /* a. Perform ? Call(capability.[[Reject]], undefined, « value.[[Value]] »). */                                      \
-            TRY(JS::call(vm, *(capability)->reject(), js_undefined(), _temporary_try_or_reject_result.release_error().value())); \
-                                                                                                                                 \
-            /* b. Return capability.[[Promise]]. */                                                                              \
-            return Value { (capability)->promise() };                                                                            \
-        }                                                                                                                        \
-                                                                                                                                 \
-        static_assert(!::AK::Detail::IsLvalueReference<decltype(_temporary_try_or_reject_result.release_value())>,               \
-            "Do not return a reference from a fallible expression");                                                             \
-                                                                                                                                 \
-        /* 2. Else if value is a Completion Record, set value to value.[[Value]]. */                                             \
-        _temporary_try_or_reject_result.release_value();                                                                         \
-    })
-
 // 27.2.1.5 NewPromiseCapability ( C ), https://tc39.es/ecma262/#sec-newpromisecapability
 ThrowCompletionOr<GC::Ref<PromiseCapability>> new_promise_capability(VM& vm, Value constructor);
 

Libraries/LibJS/Runtime/VM.cpp

@@ -563,7 +563,7 @@ ThrowCompletionOr<void> VM::link_and_eval_module(CyclicModule& module)
     if (evaluated_or_error.is_error())
         return evaluated_or_error.throw_completion();
 
-    auto* evaluated_value = evaluated_or_error.value();
+    auto evaluated_value = evaluated_or_error.value();
 
     run_queued_promise_jobs();
     VERIFY(m_promise_jobs.is_empty());

Libraries/LibJS/Runtime/VM.h

@@ -269,11 +269,7 @@ public:
 
     ScriptOrModule get_active_script_or_module() const;
 
-    // NOTE: The host defined implementation described in the web spec https://html.spec.whatwg.org/multipage/webappapis.html#hostloadimportedmodule
+    // 16.2.1.10 HostLoadImportedModule ( referrer, moduleRequest, hostDefined, payload ), https://tc39.es/ecma262/#sec-HostLoadImportedModule
-    //       currently references proposal-import-attributes.
-    //       Our implementation of this proposal is outdated however, as such we try to adapt the proposal and living standard
-    //       to match our implementation for now.
-    // 16.2.1.8 HostLoadImportedModule ( referrer, moduleRequest, hostDefined, payload ), https://tc39.es/proposal-import-attributes/#sec-HostLoadImportedModule
     Function<void(ImportedModuleReferrer, ModuleRequest const&, GC::Ptr<GraphLoadingState::HostDefined>, ImportedModulePayload)> host_load_imported_module;
 
     Function<HashMap<PropertyKey, Value>(SourceTextModule&)> host_get_import_meta_properties;

Libraries/LibJS/SourceTextModule.cpp

@@ -20,10 +20,10 @@ namespace JS {
 
 GC_DEFINE_ALLOCATOR(SourceTextModule);
 
-// 16.2.2.2 Static Semantics: WithClauseToAttributes, https://tc39.es/proposal-import-attributes/#sec-with-clause-to-attributes
+// 16.2.2.4 Static Semantics: WithClauseToAttributes, https://tc39.es/ecma262/#sec-withclausetoattributes
 static Vector<ImportAttribute> with_clause_to_assertions(Vector<ImportAttribute> const& source_attributes)
 {
-    // WithClause : AttributesKeyword { WithEntries , opt }
+    // WithClause : with { WithEntries ,opt }
     // 1. Let attributes be WithClauseToAttributes of WithEntries.
     Vector<ImportAttribute> attributes;
 
@@ -37,18 +37,20 @@ static Vector<ImportAttribute> with_clause_to_assertions(Vector<ImportAttribute>
         attributes.empend(attribute);
     }
 
-    // 2. Sort attributes according to the lexicographic order of their [[Key]] fields, treating the value of each such field as a sequence of UTF-16 code unit values. NOTE: This sorting is observable only in that hosts are prohibited from distinguishing among attributes by the order they occur in.
+    // 2. Sort attributes according to the lexicographic order of their [[Key]] field, treating the value of each such
-    // Note: The sorting is done in construction of the ModuleRequest object.
+    //    field as a sequence of UTF-16 code unit values. NOTE: This sorting is observable only in that hosts are
+    //    prohibited from changing behaviour based on the order in which attributes are enumerated.
+    // NOTE: The sorting is done in construction of the ModuleRequest object.
 
     // 3. Return attributes.
     return attributes;
 }
 
-// 16.2.1.3 Static Semantics: ModuleRequests, https://tc39.es/ecma262/#sec-static-semantics-modulerequests
+// 16.2.1.4 Static Semantics: ModuleRequests, https://tc39.es/ecma262/#sec-static-semantics-modulerequests
 static Vector<ModuleRequest> module_requests(Program& program)
 {
     // A List of all the ModuleSpecifier strings used by the module represented by this record to request the importation of a module.
-    // Note: The List is source text occurrence ordered!
+    // NOTE: The List is source text occurrence ordered!
     struct RequestedModuleAndSourceIndex {
         u32 source_offset { 0 };
         ModuleRequest const* module_request { nullptr };
@@ -56,44 +58,45 @@ static Vector<ModuleRequest> module_requests(Program& program)
 
     Vector<RequestedModuleAndSourceIndex> requested_modules_with_indices;
 
-    for (auto& import_statement : program.imports())
+    for (auto const& import_statement : program.imports())
         requested_modules_with_indices.empend(import_statement->start_offset(), &import_statement->module_request());
 
-    for (auto& export_statement : program.exports()) {
+    for (auto const& export_statement : program.exports()) {
-        for (auto& export_entry : export_statement->entries()) {
+        for (auto const& export_entry : export_statement->entries()) {
             if (!export_entry.is_module_request())
                 continue;
             requested_modules_with_indices.empend(export_statement->start_offset(), &export_statement->module_request());
         }
     }
 
-    // Note: The List is source code occurrence ordered. https://tc39.es/proposal-import-attributes/#table-cyclic-module-fields
+    // NOTE: The List is source code occurrence ordered. https://tc39.es/ecma262/#table-cyclic-module-fields
     quick_sort(requested_modules_with_indices, [&](RequestedModuleAndSourceIndex const& lhs, RequestedModuleAndSourceIndex const& rhs) {
         return lhs.source_offset < rhs.source_offset;
     });
 
     Vector<ModuleRequest> requested_modules_in_source_order;
     requested_modules_in_source_order.ensure_capacity(requested_modules_with_indices.size());
-    for (auto& module : requested_modules_with_indices) {
-        // 16.2.1.3 Static Semantics: ModuleRequests, https://tc39.es/proposal-import-attributes/#sec-static-semantics-modulerequests
-        if (module.module_request->attributes.is_empty()) {
-            //  ExportDeclaration : export ExportFromClause FromClause ;
-            //  ImportDeclaration : import ImportClause FromClause ;
 
-            // 2. Let specifier be SV of FromClause.
+    for (auto const& module : requested_modules_with_indices) {
-            // 3. Return a List whose sole element is the ModuleRequest Record { [[Specifer]]: specifier, [[Attributes]]: « » }.
+        if (module.module_request->attributes.is_empty()) {
+            // ImportDeclaration : import ImportClause FromClause ;
+            // ExportDeclaration : export ExportFromClause FromClause ;
+
+            // 1. Let specifier be SV of FromClause.
+            // 2. Return a List whose sole element is the ModuleRequest Record { [[Specifer]]: specifier, [[Attributes]]: « » }.
             requested_modules_in_source_order.empend(module.module_request->module_specifier);
         } else {
-            //  ExportDeclaration : export ExportFromClause FromClause WithClause ;
+            // ImportDeclaration : import ImportClause FromClause WithClause ;
-            //  ImportDeclaration : import ImportClause FromClause WithClause ;
+            // ExportDeclaration : export ExportFromClause FromClause WithClause ;
 
             // 1. Let specifier be the SV of FromClause.
             // 2. Let attributes be WithClauseToAttributes of WithClause.
             auto attributes = with_clause_to_assertions(module.module_request->attributes);
+
             // NOTE: We have to modify the attributes in place because else it might keep unsupported ones.
             const_cast<ModuleRequest*>(module.module_request)->attributes = move(attributes);
 
-            // 3. Return a List whose sole element is the ModuleRequest Record { [[Specifer]]: specifier, [[Attributes]]: attributes }.
+            // 3. Return a List whose sole element is the ModuleRequest Record { [[Specifier]]: specifier, [[Attributes]]: attributes }.
             requested_modules_in_source_order.empend(module.module_request->module_specifier, module.module_request->attributes);
         }
     }
@@ -125,7 +128,7 @@ void SourceTextModule::visit_edges(Cell::Visitor& visitor)
     m_execution_context->visit_edges(visitor);
 }
 
-// 16.2.1.6.1 ParseModule ( sourceText, realm, hostDefined ), https://tc39.es/ecma262/#sec-parsemodule
+// 16.2.1.7.1 ParseModule ( sourceText, realm, hostDefined ), https://tc39.es/ecma262/#sec-parsemodule
 Result<GC::Ref<SourceTextModule>, Vector<ParserError>> SourceTextModule::parse(StringView source_text, Realm& realm, StringView filename, Script::HostDefined* host_defined)
 {
     // 1. Let body be ParseText(sourceText, Module).
@@ -145,7 +148,7 @@ Result<GC::Ref<SourceTextModule>, Vector<ParserError>> SourceTextModule::parse(S
         import_entries.extend(import_statement->entries());
 
     // 5. Let importedBoundNames be ImportedLocalNames(importEntries).
-    // Note: Since we have to potentially extract the import entry we just use importEntries
+    // NOTE: Since we have to potentially extract the import entry we just use importEntries
     //       In the future it might be an optimization to have a set/map of string to speed up the search.
 
     // 6. Let indirectExportEntries be a new empty List.
@@ -157,13 +160,12 @@ Result<GC::Ref<SourceTextModule>, Vector<ParserError>> SourceTextModule::parse(S
     // 8. Let starExportEntries be a new empty List.
     Vector<ExportEntry> star_export_entries;
 
-    // Note: Not in the spec but makes it easier to find the default.
+    // NOTE: Not in the spec but makes it easier to find the default.
     RefPtr<ExportStatement const> default_export;
 
     // 9. Let exportEntries be ExportEntries of body.
     // 10. For each ExportEntry Record ee of exportEntries, do
     for (auto const& export_statement : body->exports()) {
-
         if (export_statement->is_default_export()) {
             VERIFY(!default_export);
             VERIFY(export_statement->entries().size() == 1);
@@ -181,7 +183,6 @@ Result<GC::Ref<SourceTextModule>, Vector<ParserError>> SourceTextModule::parse(S
         }
 
         for (auto const& export_entry : export_statement->entries()) {
-
             // Special case, export {} from "module" should add "module" to
             // required_modules but not any import or export so skip here.
             if (export_entry.kind == ExportEntry::Kind::EmptyNamedExport) {
@@ -207,7 +208,7 @@ Result<GC::Ref<SourceTextModule>, Vector<ParserError>> SourceTextModule::parse(S
                     // 1. Let ie be the element of importEntries whose [[LocalName]] is the same as ee.[[LocalName]].
                     auto& import_entry = *in_imported_bound_names;
 
-                    // 2. If ie.[[ImportName]] is namespace-object, then
+                    // 2. If ie.[[ImportName]] is NAMESPACE-OBJECT, then
                     if (import_entry.is_namespace()) {
                         // a. NOTE: This is a re-export of an imported module namespace object.
                         // b. Append ee to localExportEntries.
@@ -260,33 +261,34 @@ Result<GC::Ref<SourceTextModule>, Vector<ParserError>> SourceTextModule::parse(S
         move(default_export));
 }
 
-// 16.2.1.6.2 GetExportedNames ( [ exportStarSet ] ), https://tc39.es/ecma262/#sec-getexportednames
+// 16.2.1.7.2.1 GetExportedNames ( [ exportStarSet ] ), https://tc39.es/ecma262/#sec-getexportednames
-ThrowCompletionOr<Vector<FlyString>> SourceTextModule::get_exported_names(VM& vm, Vector<Module*> export_star_set)
+Vector<FlyString> SourceTextModule::get_exported_names(VM& vm, HashTable<Module const*>& export_star_set)
 {
     dbgln_if(JS_MODULE_DEBUG, "[JS MODULE] get_export_names of {}", filename());
-    // 1. Assert: module.[[Status]] is not new.
+
+    // 1. Assert: module.[[Status]] is not NEW.
     VERIFY(m_status != ModuleStatus::New);
 
     // 2. If exportStarSet is not present, set exportStarSet to a new empty List.
-    // Note: This is done by default argument
+    // NOTE: This is done by Module.
 
     // 3. If exportStarSet contains module, then
-    if (export_star_set.contains_slow(this)) {
+    if (export_star_set.contains(this)) {
         // a. Assert: We've reached the starting point of an export * circularity.
         // FIXME: How do we check that?
 
         // b. Return a new empty List.
-        return Vector<FlyString> {};
+        return {};
     }
 
     // 4. Append module to exportStarSet.
-    export_star_set.append(this);
+    export_star_set.set(this);
 
     // 5. Let exportedNames be a new empty List.
     Vector<FlyString> exported_names;
 
     // 6. For each ExportEntry Record e of module.[[LocalExportEntries]], do
-    for (auto& entry : m_local_export_entries) {
+    for (auto const& entry : m_local_export_entries) {
         // a. Assert: module provides the direct binding for this export.
         // FIXME: How do we check that?
 
@@ -298,7 +300,7 @@ ThrowCompletionOr<Vector<FlyString>> SourceTextModule::get_exported_names(VM& vm
     }
 
     // 7. For each ExportEntry Record e of module.[[IndirectExportEntries]], do
-    for (auto& entry : m_indirect_export_entries) {
+    for (auto const& entry : m_indirect_export_entries) {
         // a. a. Assert: module imports a specific binding for this export.
         // FIXME: How do we check that?
 
@@ -310,19 +312,19 @@ ThrowCompletionOr<Vector<FlyString>> SourceTextModule::get_exported_names(VM& vm
     }
 
     // 8. For each ExportEntry Record e of module.[[StarExportEntries]], do
-    for (auto& entry : m_star_export_entries) {
+    for (auto const& entry : m_star_export_entries) {
         // a. Assert: e.[[ModuleRequest]] is not null.
         // b. Let requestedModule be GetImportedModule(module, e.[[ModuleRequest]]).
         auto requested_module = get_imported_module(entry.module_request());
 
-        // c. Let starNames be ? requestedModule.GetExportedNames(exportStarSet).
+        // c. Let starNames be requestedModule.GetExportedNames(exportStarSet).
-        auto star_names = TRY(requested_module->get_exported_names(vm, export_star_set));
+        auto star_names = requested_module->get_exported_names(vm, export_star_set);
 
         // d. For each element n of starNames, do
-        for (auto& name : star_names) {
+        for (auto const& name : star_names) {
-            // i. If SameValue(n, "default") is false, then
+            // i. If n is not "default", then
             if (name != "default"sv) {
-                // 1. If n is not an element of exportedNames, then
+                // 1. If exportedNames does not contain n, then
                 if (!exported_names.contains_slow(name)) {
                     // a. Append n to exportedNames.
                     exported_names.empend(name);
@@ -335,14 +337,18 @@ ThrowCompletionOr<Vector<FlyString>> SourceTextModule::get_exported_names(VM& vm
     return exported_names;
 }
 
-// 16.2.1.6.4 InitializeEnvironment ( ), https://tc39.es/ecma262/#sec-source-text-module-record-initialize-environment
+// 16.2.1.7.3.1 InitializeEnvironment ( ), https://tc39.es/ecma262/#sec-source-text-module-record-initialize-environment
 ThrowCompletionOr<void> SourceTextModule::initialize_environment(VM& vm)
 {
     // 1. For each ExportEntry Record e of module.[[IndirectExportEntries]], do
-    for (auto& entry : m_indirect_export_entries) {
+    for (auto const& entry : m_indirect_export_entries) {
-        // a. Let resolution be ? module.ResolveExport(e.[[ExportName]]).
+        // a. Assert: e.[[ExportName]] is not null.
-        auto resolution = TRY(resolve_export(vm, entry.export_name.value()));
+        VERIFY(entry.export_name.has_value());
-        // b. If resolution is null or ambiguous, throw a SyntaxError exception.
+
+        // a. Let resolution be module.ResolveExport(e.[[ExportName]]).
+        auto resolution = resolve_export(vm, entry.export_name.value());
+
+        // b. If resolution is either null or AMBIGUOUS, throw a SyntaxError exception.
         if (!resolution.is_valid())
             return vm.throw_completion<SyntaxError>(ErrorType::InvalidOrAmbiguousExportEntry, entry.export_name);
 
@@ -351,29 +357,27 @@ ThrowCompletionOr<void> SourceTextModule::initialize_environment(VM& vm)
     }
 
     // 2. Assert: All named exports from module are resolvable.
-    // Note: We check all the indirect export entries above in step 1 and all
+    // NOTE: We check all the indirect export entries above in step 1 and all the local named exports are resolvable by construction.
-    // the local named exports are resolvable by construction.
 
     // 3. Let realm be module.[[Realm]].
     // 4. Assert: realm is not undefined.
-    // Note: This must be true because we use a reference.
+    auto& realm = this->realm();
 
     // 5. Let env be NewModuleEnvironment(realm.[[GlobalEnv]]).
-    auto environment = vm.heap().allocate<ModuleEnvironment>(&realm().global_environment());
+    auto environment = vm.heap().allocate<ModuleEnvironment>(&realm.global_environment());
 
     // 6. Set module.[[Environment]] to env.
     set_environment(environment);
 
     // 7. For each ImportEntry Record in of module.[[ImportEntries]], do
-    for (auto& import_entry : m_import_entries) {
+    for (auto const& import_entry : m_import_entries) {
         // a. Let importedModule be GetImportedModule(module, in.[[ModuleRequest]]).
         auto imported_module = get_imported_module(import_entry.module_request());
-        // b. NOTE: The above call cannot fail because imported module requests are a subset of module.[[RequestedModules]], and these have been resolved earlier in this algorithm.
 
-        // c. If in.[[ImportName]] is namespace-object, then
+        // b. If in.[[ImportName]] is NAMESPACE-OBJECT, then
         if (import_entry.is_namespace()) {
-            // i. Let namespace be ? GetModuleNamespace(importedModule).
+            // i. Let namespace be GetModuleNamespace(importedModule).
-            auto* namespace_ = TRY(imported_module->get_module_namespace(vm));
+            auto namespace_ = imported_module->get_module_namespace(vm);
 
             // ii. Perform ! env.CreateImmutableBinding(in.[[LocalName]], true).
             MUST(environment->create_immutable_binding(vm, import_entry.local_name, true));
@@ -381,19 +385,19 @@ ThrowCompletionOr<void> SourceTextModule::initialize_environment(VM& vm)
             // iii. Perform ! env.InitializeBinding(in.[[LocalName]], namespace, normal).
             MUST(environment->initialize_binding(vm, import_entry.local_name, namespace_, Environment::InitializeBindingHint::Normal));
         }
-        // d. Else,
+        // c. Else,
         else {
-            // i. Let resolution be ? importedModule.ResolveExport(in.[[ImportName]]).
+            // i. Let resolution be importedModule.ResolveExport(in.[[ImportName]]).
-            auto resolution = TRY(imported_module->resolve_export(vm, import_entry.import_name.value()));
+            auto resolution = imported_module->resolve_export(vm, import_entry.import_name.value());
 
-            // ii. If resolution is null or ambiguous, throw a SyntaxError exception.
+            // ii. If resolution is either null or AMBIGUOUS, throw a SyntaxError exception.
             if (!resolution.is_valid())
                 return vm.throw_completion<SyntaxError>(ErrorType::InvalidOrAmbiguousExportEntry, import_entry.import_name);
 
-            // iii. If resolution.[[BindingName]] is namespace, then
+            // iii. If resolution.[[BindingName]] is NAMESPACE, then
             if (resolution.is_namespace()) {
-                // 1. Let namespace be ? GetModuleNamespace(resolution.[[Module]]).
+                // 1. Let namespace be GetModuleNamespace(resolution.[[Module]]).
-                auto* namespace_ = TRY(resolution.module->get_module_namespace(vm));
+                auto namespace_ = resolution.module->get_module_namespace(vm);
 
                 // 2. Perform ! env.CreateImmutableBinding(in.[[LocalName]], true).
                 MUST(environment->create_immutable_binding(vm, import_entry.local_name, true));
@@ -410,15 +414,15 @@ ThrowCompletionOr<void> SourceTextModule::initialize_environment(VM& vm)
     }
 
     // 8. Let moduleContext be a new ECMAScript code execution context.
-    // Note: this has already been created during the construction of this object.
+    // NOTE: this has already been created during the construction of this object.
 
     // 9. Set the Function of moduleContext to null.
 
     // 10. Assert: module.[[Realm]] is not undefined.
-    // Note: This must be true because we use a reference.
+    // NOTE: This must be true because we use a reference.
 
     // 11. Set the Realm of moduleContext to module.[[Realm]].
-    m_execution_context->realm = &realm();
+    m_execution_context->realm = &this->realm();
 
     // 12. Set the ScriptOrModule of moduleContext to module.
     m_execution_context->script_or_module = GC::Ref<Module>(*this);
@@ -432,7 +436,7 @@ ThrowCompletionOr<void> SourceTextModule::initialize_environment(VM& vm)
     // 15. Set the PrivateEnvironment of moduleContext to null.
 
     // 16. Set module.[[Context]] to moduleContext.
-    // Note: We're already working on that one.
+    // NOTE: We're already working on that one.
 
     // 17. Push moduleContext onto the execution context stack; moduleContext is now the running execution context.
     TRY(vm.push_execution_context(*m_execution_context, {}));
@@ -440,7 +444,7 @@ ThrowCompletionOr<void> SourceTextModule::initialize_environment(VM& vm)
     // 18. Let code be module.[[ECMAScriptCode]].
 
     // 19. Let varDeclarations be the VarScopedDeclarations of code.
-    // Note: We just loop through them in step 21.
+    // NOTE: We just loop through them in step 21.
 
     // 20. Let declaredVarNames be a new empty List.
     Vector<FlyString> declared_var_names;
@@ -465,7 +469,7 @@ ThrowCompletionOr<void> SourceTextModule::initialize_environment(VM& vm)
     }));
 
     // 22. Let lexDeclarations be the LexicallyScopedDeclarations of code.
-    // Note: We only loop through them in step 24.
+    // NOTE: We only loop through them in step 24.
 
     // 23. Let privateEnv be null.
     PrivateEnvironment* private_environment = nullptr;
@@ -474,8 +478,6 @@ ThrowCompletionOr<void> SourceTextModule::initialize_environment(VM& vm)
     // NOTE: Due to the use of MUST in the callback, an exception should not result from `for_each_lexically_scoped_declaration`.
     MUST(m_ecmascript_code->for_each_lexically_scoped_declaration([&](Declaration const& declaration) {
         // a. For each element dn of the BoundNames of d, do
-        // NOTE: Due to the use of MUST with `create_immutable_binding`, `create_mutable_binding` and `initialize_binding` below,
-        //       an exception should not result from `for_each_bound_identifier`.
         MUST(declaration.for_each_bound_identifier([&](auto const& identifier) {
             auto const& name = identifier.string();
             // i. If IsConstantDeclaration of d is true, then
@@ -503,7 +505,7 @@ ThrowCompletionOr<void> SourceTextModule::initialize_environment(VM& vm)
                 auto function = ECMAScriptFunctionObject::create_from_function_node(
                     function_declaration,
                     function_name,
-                    realm(),
+                    realm,
                     environment,
                     private_environment);
 
@@ -513,23 +515,20 @@ ThrowCompletionOr<void> SourceTextModule::initialize_environment(VM& vm)
         }));
     }));
 
-    // Note: The default export name is also part of the local lexical declarations but
+    // NOTE: The default export name is also part of the local lexical declarations but instead of making that a special
-    //       instead of making that a special case in the parser we just check it here.
+    //       case in the parser we just check it here. This is only needed for things which are not declarations. For more
-    //       This is only needed for things which are not declarations.
+    //       info check Parser::parse_export_statement. Furthermore, that declaration is not constant. so we take 24.a.ii.
-    //       For more info check Parser::parse_export_statement.
-    //       Furthermore, that declaration is not constant. so we take 24.a.ii
     if (m_default_export) {
         VERIFY(m_default_export->has_statement());
 
-        auto const& statement = m_default_export->statement();
+        if (auto const& statement = m_default_export->statement(); !is<Declaration>(statement)) {
-        if (!is<Declaration>(statement)) {
             auto const& name = m_default_export->entries()[0].local_or_import_name;
             dbgln_if(JS_MODULE_DEBUG, "[JS MODULE] Adding default export to lexical declarations: local name: {}, Expression: {}", name, statement.class_name());
 
             // 1. Perform ! env.CreateMutableBinding(dn, false).
             MUST(environment->create_mutable_binding(vm, name.value(), false));
 
-            // Note: Since this is not a function declaration 24.a.iii never applies
+            // NOTE: Since this is not a function declaration 24.a.iii never applies
         }
     }
 
@@ -540,18 +539,18 @@ ThrowCompletionOr<void> SourceTextModule::initialize_environment(VM& vm)
     return {};
 }
 
-// 16.2.1.6.3 ResolveExport ( exportName [ , resolveSet ] ), https://tc39.es/ecma262/#sec-resolveexport
+// 16.2.1.7.2.2 ResolveExport ( exportName [ , resolveSet ] ), https://tc39.es/ecma262/#sec-resolveexport
-ThrowCompletionOr<ResolvedBinding> SourceTextModule::resolve_export(VM& vm, FlyString const& export_name, Vector<ResolvedBinding> resolve_set)
+ResolvedBinding SourceTextModule::resolve_export(VM& vm, FlyString const& export_name, Vector<ResolvedBinding> resolve_set)
 {
-    // 1. Assert: module.[[Status]] is not new.
+    // 1. Assert: module.[[Status]] is not NEW.
     VERIFY(m_status != ModuleStatus::New);
 
     // 2. If resolveSet is not present, set resolveSet to a new empty List.
-    // Note: This is done by the default argument.
+    // NOTE: This is done by the default argument.
 
     // 3. For each Record { [[Module]], [[ExportName]] } r of resolveSet, do
-    for (auto& [type, module, exported_name] : resolve_set) {
+    for (auto const& [type, module, exported_name] : resolve_set) {
-        // a. If module and r.[[Module]] are the same Module Record and SameValue(exportName, r.[[ExportName]]) is true, then
+        // a. If module and r.[[Module]] are the same Module Record and exportName is r.[[ExportName]], then
         if (module == this && exported_name == export_name) {
             // i. Assert: This is a circular import request.
 
@@ -564,8 +563,8 @@ ThrowCompletionOr<ResolvedBinding> SourceTextModule::resolve_export(VM& vm, FlyS
     resolve_set.append({ ResolvedBinding::Type::BindingName, this, export_name });
 
     // 5. For each ExportEntry Record e of module.[[LocalExportEntries]], do
-    for (auto& entry : m_local_export_entries) {
+    for (auto const& entry : m_local_export_entries) {
-        // a. If SameValue(exportName, e.[[ExportName]]) is true, then
+        // a. If e.[[ExportName]] is exportName, then
         if (export_name != entry.export_name)
             continue;
 
@@ -581,8 +580,8 @@ ThrowCompletionOr<ResolvedBinding> SourceTextModule::resolve_export(VM& vm, FlyS
     }
 
     // 5. For each ExportEntry Record e of module.[[IndirectExportEntries]], do
-    for (auto& entry : m_indirect_export_entries) {
+    for (auto const& entry : m_indirect_export_entries) {
-        // a. If SameValue(exportName, e.[[ExportName]]) is true, then
+        // a. If e.[[ExportName]] is exportName, then
         if (export_name != entry.export_name)
             continue;
 
@@ -595,7 +594,7 @@ ThrowCompletionOr<ResolvedBinding> SourceTextModule::resolve_export(VM& vm, FlyS
             // 1. Assert: module does not provide the direct binding for this export.
             // FIXME: What does this mean? / How do we check this
 
-            // 2. Return ResolvedBinding Record { [[Module]]: importedModule, [[BindingName]]: namespace }.
+            // 2. Return ResolvedBinding Record { [[Module]]: importedModule, [[BindingName]]: NAMESPACE }.
             return ResolvedBinding {
                 ResolvedBinding::Type::Namespace,
                 imported_module.ptr(),
@@ -607,34 +606,35 @@ ThrowCompletionOr<ResolvedBinding> SourceTextModule::resolve_export(VM& vm, FlyS
             // 1. Assert: module imports a specific binding for this export.
             // FIXME: What does this mean? / How do we check this
 
-            // 2. Return ? importedModule.ResolveExport(e.[[ImportName]], resolveSet).
+            // 2. Return importedModule.ResolveExport(e.[[ImportName]], resolveSet).
             return imported_module->resolve_export(vm, entry.local_or_import_name.value(), resolve_set);
         }
     }
 
-    // 7. If SameValue(exportName, "default") is true, then
+    // 7. If exportName is "default", then
     if (export_name == "default"sv) {
         // a. Assert: A default export was not explicitly defined by this module.
         // FIXME: What does this mean? / How do we check this
 
         // b. Return null.
         return ResolvedBinding::null();
+
         // c. NOTE: A default export cannot be provided by an export * from "mod" declaration.
     }
 
     // 8. Let starResolution be null.
-    ResolvedBinding star_resolution = ResolvedBinding::null();
+    auto star_resolution = ResolvedBinding::null();
 
     // 9. For each ExportEntry Record e of module.[[StarExportEntries]], do
-    for (auto& entry : m_star_export_entries) {
+    for (auto const& entry : m_star_export_entries) {
         // a. Assert: e.[[ModuleRequest]] is not null.
         // b. Let importedModule be GetImportedModule(module, e.[[ModuleRequest]]).
         auto imported_module = get_imported_module(entry.module_request());
 
-        // c. Let resolution be ? importedModule.ResolveExport(exportName, resolveSet).
+        // c. Let resolution be importedModule.ResolveExport(exportName, resolveSet).
-        auto resolution = TRY(imported_module->resolve_export(vm, export_name, resolve_set));
+        auto resolution = imported_module->resolve_export(vm, export_name, resolve_set);
 
-        // d. If resolution is ambiguous, return ambiguous.
+        // d. If resolution is AMBIGUOUS, return AMBIGUOUS.
         if (resolution.is_ambiguous())
             return ResolvedBinding::ambiguous();
 
@@ -654,19 +654,20 @@ ThrowCompletionOr<ResolvedBinding> SourceTextModule::resolve_export(VM& vm, FlyS
             // 1. Assert: There is more than one * import that includes the requested name.
             // FIXME: Assert this
 
-            // 2. If resolution.[[Module]] and starResolution.[[Module]] are not the same Module Record, return ambiguous.
+            // 2. If resolution.[[Module]] and starResolution.[[Module]] are not the same Module Record, return AMBIGUOUS.
             if (resolution.module != star_resolution.module)
                 return ResolvedBinding::ambiguous();
 
-            // 3. If resolution.[[BindingName]] is namespace and starResolution.[[BindingName]] is not namespace, or if resolution.[[BindingName]] is not namespace and starResolution.[[BindingName]] is namespace, return ambiguous.
+            // 3. If resolution.[[BindingName]] is not starResolution.[[BindingName]] and either resolution.[[BindingName]]
+            //    or starResolution.[[BindingName]] is NAMESPACE, return AMBIGUOUS.
             if (resolution.is_namespace() != star_resolution.is_namespace())
                 return ResolvedBinding::ambiguous();
 
-            // 4. If resolution.[[BindingName]] is a String, starResolution.[[BindingName]] is a String, and SameValue(resolution.[[BindingName]], starResolution.[[BindingName]]) is false, return ambiguous.
+            // 4. If resolution.[[BindingName]] is a String, starResolution.[[BindingName]] is a String, and
-            if (!resolution.is_namespace() && resolution.export_name != star_resolution.export_name) {
+            //    resolution.[[BindingName]] is not starResolution.[[BindingName]], return ambiguous.
-                // Note: Because we know from the previous if that either both are namespaces or both are string we can check just one
+            // NOTE: We know from the previous step that either both are namespaces or both are string, so we can check just one.
+            if (!resolution.is_namespace() && resolution.export_name != star_resolution.export_name)
                 return ResolvedBinding::ambiguous();
-            }
         }
     }
 
@@ -704,7 +705,7 @@ ThrowCompletionOr<void> SourceTextModule::execute_module(VM& vm, GC::Ptr<Promise
     ExecutionContext* module_context = nullptr;
     ALLOCATE_EXECUTION_CONTEXT_ON_NATIVE_STACK(module_context, registers_and_constants_and_locals_count, 0);
 
-    // Note: This is not in the spec but we require it.
+    // NOTE: This is not in the spec but we require it.
     module_context->is_strict_mode = true;
 
     // 2. Set the Function of moduleContext to null.

Libraries/LibJS/SourceTextModule.h

@@ -25,8 +25,8 @@ public:
 
     Program const& parse_node() const { return *m_ecmascript_code; }
 
-    virtual ThrowCompletionOr<Vector<FlyString>> get_exported_names(VM& vm, Vector<Module*> export_star_set) override;
+    virtual Vector<FlyString> get_exported_names(VM& vm, HashTable<Module const*>& export_star_set) override;
-    virtual ThrowCompletionOr<ResolvedBinding> resolve_export(VM& vm, FlyString const& export_name, Vector<ResolvedBinding> resolve_set = {}) override;
+    virtual ResolvedBinding resolve_export(VM& vm, FlyString const& export_name, Vector<ResolvedBinding> resolve_set = {}) override;
 
     Object* import_meta() { return m_import_meta; }
     void set_import_meta(Badge<VM>, Object* import_meta) { m_import_meta = import_meta; }

Libraries/LibJS/SyntheticModule.cpp

@@ -27,14 +27,14 @@ SyntheticModule::SyntheticModule(Vector<FlyString> export_names, SyntheticModule
 }
 
 // 1.2.3.1 GetExportedNames( exportStarSet ), https://tc39.es/proposal-json-modules/#sec-smr-getexportednames
-ThrowCompletionOr<Vector<FlyString>> SyntheticModule::get_exported_names(VM&, Vector<Module*>)
+Vector<FlyString> SyntheticModule::get_exported_names(VM&, HashTable<Module const*>&)
 {
     // 1. Return module.[[ExportNames]].
     return m_export_names;
 }
 
 // 1.2.3.2 ResolveExport( exportName, resolveSet ), https://tc39.es/proposal-json-modules/#sec-smr-resolveexport
-ThrowCompletionOr<ResolvedBinding> SyntheticModule::resolve_export(VM&, FlyString const& export_name, Vector<ResolvedBinding>)
+ResolvedBinding SyntheticModule::resolve_export(VM&, FlyString const& export_name, Vector<ResolvedBinding>)
 {
     // 1. If module.[[ExportNames]] does not contain exportName, return null.
     if (!m_export_names.contains_slow(export_name))
@@ -74,7 +74,7 @@ ThrowCompletionOr<void> SyntheticModule::link(VM& vm)
 }
 
 // 1.2.3.4 Evaluate ( ), https://tc39.es/proposal-json-modules/#sec-smr-Evaluate
-ThrowCompletionOr<Promise*> SyntheticModule::evaluate(VM& vm)
+ThrowCompletionOr<GC::Ref<Promise>> SyntheticModule::evaluate(VM& vm)
 {
     // Note: Has some changes from PR: https://github.com/tc39/proposal-json-modules/pull/13.
     // 1. Suspend the currently running execution context.
@@ -119,7 +119,8 @@ ThrowCompletionOr<Promise*> SyntheticModule::evaluate(VM& vm)
         // Note: This value probably isn't visible to JS code? But undefined is fine anyway.
         promise->fulfill(js_undefined());
     }
-    return promise.ptr();
+
+    return promise;
 }
 
 // 1.2.2 SetSyntheticModuleExport ( module, exportName, exportValue ), https://tc39.es/proposal-json-modules/#sec-setsyntheticmoduleexport

Libraries/LibJS/SyntheticModule.h

@@ -23,9 +23,9 @@ public:
     ThrowCompletionOr<void> set_synthetic_module_export(FlyString const& export_name, Value export_value);
 
     virtual ThrowCompletionOr<void> link(VM& vm) override;
-    virtual ThrowCompletionOr<Promise*> evaluate(VM& vm) override;
+    virtual ThrowCompletionOr<GC::Ref<Promise>> evaluate(VM& vm) override;
-    virtual ThrowCompletionOr<Vector<FlyString>> get_exported_names(VM& vm, Vector<Module*> export_star_set) override;
+    virtual Vector<FlyString> get_exported_names(VM& vm, HashTable<Module const*>& export_star_set) override;
-    virtual ThrowCompletionOr<ResolvedBinding> resolve_export(VM& vm, FlyString const& export_name, Vector<ResolvedBinding> resolve_set) override;
+    virtual ResolvedBinding resolve_export(VM& vm, FlyString const& export_name, Vector<ResolvedBinding> resolve_set) override;
     virtual PromiseCapability& load_requested_modules(GC::Ptr<GraphLoadingState::HostDefined>) override;
 
 private: