Apply reviews

boa_ast/src/module_item_list/mod.rs

@@ -237,7 +237,7 @@ impl ModuleItemList {
     ///
     /// Gets the list of import entries of this module.
     ///
-    /// [spec]: https://tc39.es/ecma262/#sec-static-semantics-modulerequests
+    /// [spec]: https://tc39.es/ecma262/#sec-static-semantics-importentries
     #[inline]
     #[must_use]
     pub fn import_entries(&self) -> Vec<ImportEntry> {
@@ -298,11 +298,11 @@ impl ModuleItemList {
         entries
     }
 
-    /// Operation [`ImportEntries`][spec].
+    /// Operation [`ExportEntries`][spec].
     ///
-    /// Gets the list of import entries of this module.
+    /// Gets the list of export entries of this module.
     ///
-    /// [spec]: https://tc39.es/ecma262/#sec-static-semantics-modulerequests
+    /// [spec]: https://tc39.es/ecma262/#sec-static-semantics-exportentries
     #[inline]
     #[must_use]
     pub fn export_entries(&self) -> Vec<ExportEntry> {

boa_engine/src/context/intrinsics.rs

@@ -999,6 +999,8 @@ pub(crate) struct ObjectTemplates {
 
     function_without_proto: ObjectTemplate,
     function_with_prototype_without_proto: ObjectTemplate,
+
+    namespace: ObjectTemplate,
 }
 
 impl ObjectTemplates {
@@ -1104,6 +1106,9 @@ impl ObjectTemplates {
             Attribute::WRITABLE | Attribute::CONFIGURABLE | Attribute::ENUMERABLE,
         );
 
+        let mut namespace = ObjectTemplate::new(root_shape);
+        namespace.property(JsSymbol::to_string_tag().into(), Attribute::empty());
+
         Self {
             iterator_result,
             ordinary_object,
@@ -1121,6 +1126,7 @@ impl ObjectTemplates {
             async_function,
             function_without_proto,
             function_with_prototype_without_proto,
+            namespace,
         }
     }
 
@@ -1288,4 +1294,13 @@ impl ObjectTemplates {
     pub(crate) const fn function_with_prototype_without_proto(&self) -> &ObjectTemplate {
         &self.function_with_prototype_without_proto
     }
+
+    /// Cached namespace object template.
+    ///
+    /// Transitions:
+    ///
+    /// 1. `@@toStringTag`: (`READONLY`, `NON_ENUMERABLE`, `PERMANENT`)
+    pub(crate) const fn namespace(&self) -> &ObjectTemplate {
+        &self.namespace
+    }
 }

boa_engine/src/environments/runtime/declarative/module.rs

@@ -5,25 +5,35 @@ use boa_gc::{Finalize, GcRefCell, Trace};
 
 use crate::{module::Module, JsValue};
 
+/// Type of accessor used to access an indirect binding.
 #[derive(Debug, Clone, Copy)]
 enum BindingAccessor {
     Identifier(Identifier),
     Index(usize),
 }
 
+/// An indirect reference to a binding inside an environment.
 #[derive(Clone, Debug, Trace, Finalize)]
 struct IndirectBinding {
     module: Module,
     #[unsafe_ignore_trace]
     accessor: Cell<BindingAccessor>,
 }
 
+/// The type of binding a [`ModuleEnvironment`] can contain.
 #[derive(Clone, Debug, Trace, Finalize)]
 enum BindingType {
     Direct(Option<JsValue>),
     Indirect(IndirectBinding),
 }
 
+/// A [**Module Environment Record**][spec].
+///
+/// Module environments allow referencing bindings inside other environments, in addition
+/// to the usual declarative environment functionality.
+///
+///
+/// [spec]: https://tc39.es/ecma262/#sec-module-environment-records
 #[derive(Debug, Trace, Finalize)]
 pub(crate) struct ModuleEnvironment {
     bindings: GcRefCell<Vec<BindingType>>,
@@ -88,7 +98,7 @@ impl ModuleEnvironment {
         }
     }
 
-    /// Sets a reference from this environment to an external environment binding.
+    /// Creates an indirect binding reference to another environment binding.
     ///
     /// # Panics
     ///

boa_engine/src/module/mod.rs

@@ -39,15 +39,14 @@ use boa_parser::{Parser, Source};
 use boa_profiler::Profiler;
 
 use crate::object::FunctionObjectBuilder;
-use crate::property::{PropertyDescriptor, PropertyKey};
 use crate::{
     builtins::promise::{PromiseCapability, PromiseState},
     environments::DeclarativeEnvironment,
     object::{JsObject, JsPromise, ObjectData},
     realm::Realm,
     Context, JsError, JsResult, JsString, JsValue,
 };
-use crate::{js_string, JsNativeError, JsSymbol, NativeFunction};
+use crate::{js_string, JsNativeError, NativeFunction};
 
 /// The referrer from which a load request of a module originates.
 #[derive(Debug)]
@@ -138,11 +137,13 @@ impl SimpleModuleLoader {
     }
 
     /// Inserts a new module onto the module map.
+    #[inline]
     pub fn insert(&self, path: PathBuf, module: Module) {
         self.module_map.borrow_mut().insert(path, module);
     }
 
     /// Gets a module from its original path.
+    #[inline]
     pub fn get(&self, path: &Path) -> Option<Module> {
         self.module_map.borrow().get(path).cloned()
     }
@@ -283,6 +284,7 @@ impl Module {
     /// Parses the provided `src` as an ECMAScript module, returning an error if parsing fails.
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-parsemodule
+    #[inline]
     pub fn parse<R: Read>(
         src: Source<'_, R>,
         realm: Option<Realm>,
@@ -304,6 +306,7 @@ impl Module {
     }
 
     /// Gets the realm of this `Module`.
+    #[inline]
     pub fn realm(&self) -> &Realm {
         &self.inner.realm
     }
@@ -325,6 +328,7 @@ impl Module {
     ///
     /// [spec]: https://tc39.es/ecma262/#table-abstract-methods-of-module-records
     #[allow(clippy::missing_panics_doc)]
+    #[inline]
     pub fn load(&self, context: &mut Context<'_>) -> JsPromise {
         match self.kind() {
             ModuleKind::SourceText(_) => SourceTextModule::load(self, context),
@@ -450,6 +454,7 @@ impl Module {
     ///
     /// [spec]: https://tc39.es/ecma262/#table-abstract-methods-of-module-records
     #[allow(clippy::missing_panics_doc)]
+    #[inline]
     pub fn link(&self, context: &mut Context<'_>) -> JsResult<()> {
         match self.kind() {
             ModuleKind::SourceText(_) => SourceTextModule::link(self, context),
@@ -492,6 +497,7 @@ impl Module {
     ///
     /// [spec]: https://tc39.es/ecma262/#table-abstract-methods-of-module-records
     #[allow(clippy::missing_panics_doc)]
+    #[inline]
     pub fn evaluate(&self, context: &mut Context<'_>) -> JsPromise {
         match self.kind() {
             ModuleKind::SourceText(src) => src.evaluate(context),
@@ -556,6 +562,7 @@ impl Module {
     /// assert_eq!(promise.state().unwrap(), PromiseState::Fulfilled(JsValue::undefined()));
     /// ```
     #[allow(clippy::drop_copy)]
+    #[inline]
     pub fn load_link_evaluate(&self, context: &mut Context<'_>) -> JsResult<JsPromise> {
         let main_timer = Profiler::global().start_event("Module evaluation", "Main");
 
@@ -607,28 +614,39 @@ impl Module {
     /// [spec]: https://tc39.es/ecma262/#sec-getmodulenamespace
     /// [ns]: https://tc39.es/ecma262/#sec-module-namespace-exotic-objects
     pub fn namespace(&self, context: &mut Context<'_>) -> JsObject {
+        // 1. Assert: If module is a Cyclic Module Record, then module.[[Status]] is not new or unlinked.
+        // 2. Let namespace be module.[[Namespace]].
+        // 3. If namespace is empty, then
         self.inner
             .namespace
             .borrow_mut()
             .get_or_insert_with(|| {
+                // a. Let exportedNames be module.GetExportedNames().
                 let exported_names = self.get_exported_names(&mut Vec::default());
 
+                // b. Let unambiguousNames be a new empty List.
                 let unambiguous_names = exported_names
                     .into_iter()
+                    // c. For each element name of exportedNames, do
                     .filter_map(|name| {
+                        // i. Let resolution be module.ResolveExport(name).
+                        // ii. If resolution is a ResolvedBinding Record, append name to unambiguousNames.
                         self.resolve_export(name, &mut HashSet::default())
                             .ok()
                             .map(|_| name)
                     })
                     .collect();
 
+                //     d. Set namespace to ModuleNamespaceCreate(module, unambiguousNames).
+                // 4. Return namespace.
                 ModuleNamespace::create(self.clone(), unambiguous_names, context)
             })
             .clone()
     }
 }
 
 impl PartialEq for Module {
+    #[inline]
     fn eq(&self, other: &Self) -> bool {
         std::ptr::eq(self.inner.as_ref(), other.inner.as_ref())
     }
@@ -637,6 +655,7 @@ impl PartialEq for Module {
 impl Eq for Module {}
 
 impl Hash for Module {
+    #[inline]
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
         std::ptr::hash(self.inner.as_ref(), state);
     }
@@ -657,6 +676,10 @@ impl ModuleNamespace {
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-modulenamespacecreate
     pub(crate) fn create(module: Module, names: Vec<Sym>, context: &mut Context<'_>) -> JsObject {
+        // 1. Assert: module.[[Namespace]] is empty.
+        // ignored since this is ensured by `Module::namespace`.
+
+        // 6. Let sortedExports be a List whose elements are the elements of exports ordered as if an Array of the same values had been sorted using %Array.prototype.sort% using undefined as comparefn.
         let mut exports = names
             .into_iter()
             .map(|sym| {
@@ -671,30 +694,30 @@ impl ModuleNamespace {
             .collect::<IndexMap<_, _, _>>();
         exports.sort_keys();
 
-        let namespace = JsObject::from_proto_and_data_with_shared_shape(
-            context.root_shape(),
-            None,
+        // 2. Let internalSlotsList be the internal slots listed in Table 32.
+        // 3. Let M be MakeBasicObject(internalSlotsList).
+        // 4. Set M's essential internal methods to the definitions specified in 10.4.6.
+        // 5. Set M.[[Module]] to module.
+        // 7. Set M.[[Exports]] to sortedExports.
+        // 8. Create own properties of M corresponding to the definitions in 28.3.
+        let namespace = context.intrinsics().templates().namespace().create(
             ObjectData::module_namespace(ModuleNamespace { module, exports }),
+            vec![js_string!("Module").into()],
         );
 
-        namespace.borrow_mut().properties_mut().insert(
-            &PropertyKey::Symbol(JsSymbol::to_string_tag()),
-            PropertyDescriptor::builder()
-                .value(js_string!("Module"))
-                .writable(false)
-                .enumerable(false)
-                .configurable(false)
-                .build(),
-        );
+        // 9. Set module.[[Namespace]] to M.
+        // Ignored because this is done by `Module::namespace`
+
+        // 10. Return M.
         namespace
     }
 
-    /// Gets the export names of the `ModuleNamespace` object.
+    /// Gets the export names of the Module Namespace object.
     pub(crate) const fn exports(&self) -> &IndexMap<JsString, Sym, BuildHasherDefault<FxHasher>> {
         &self.exports
     }
 
-    /// Gest the module associated with this `ModuleNamespace` object.
+    /// Gest the module associated with this Module Namespace object.
     pub(crate) const fn module(&self) -> &Module {
         &self.module
     }