Refactor: Change Realm::global_object field from Value to GcObject (#1067)

* Refactor: Change Realm::global_object field from Value to GcObject

* Style: Add allow atributte to avoid clippy false positive

boa/src/builtins/array/array_iterator.rs

@@ -1,7 +1,7 @@
 use crate::{
     builtins::{function::make_builtin_fn, iterable::create_iter_result_object, Array, Value},
     gc::{Finalize, Trace},
-    object::ObjectData,
+    object::{GcObject, ObjectData},
     property::{Attribute, DataDescriptor},
     BoaProfiler, Context, Result,
 };
@@ -118,20 +118,17 @@ impl ArrayIterator {
     ///  - [ECMA reference][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-%arrayiteratorprototype%-object
-    pub(crate) fn create_prototype(context: &mut Context, iterator_prototype: Value) -> Value {
+    pub(crate) fn create_prototype(context: &mut Context, iterator_prototype: Value) -> GcObject {
         let _timer = BoaProfiler::global().start_event(Self::NAME, "init");
 
         // Create prototype
-        let array_iterator = Value::new_object(context);
+        let mut array_iterator = context.construct_object();
         make_builtin_fn(Self::next, "next", &array_iterator, 0, context);
-        array_iterator
-            .as_object()
-            .expect("array iterator prototype object")
-            .set_prototype_instance(iterator_prototype);
+        array_iterator.set_prototype_instance(iterator_prototype);
 
         let to_string_tag = context.well_known_symbols().to_string_tag_symbol();
         let to_string_tag_property = DataDescriptor::new("Array Iterator", Attribute::CONFIGURABLE);
-        array_iterator.set_property(to_string_tag, to_string_tag_property);
+        array_iterator.insert(to_string_tag, to_string_tag_property);
         array_iterator
     }
 }

boa/src/builtins/function/mod.rs

@@ -222,7 +222,7 @@ pub fn create_unmapped_arguments_object(arguments_list: &[Value]) -> Value {
 pub fn make_builtin_fn<N>(
     function: NativeFunction,
     name: N,
-    parent: &Value,
+    parent: &GcObject,
     length: usize,
     interpreter: &Context,
 ) where
@@ -243,7 +243,7 @@ pub fn make_builtin_fn<N>(
     function.insert_property("length", length, attribute);
     function.insert_property("name", name.as_str(), attribute);
 
-    parent.as_object().unwrap().insert_property(
+    parent.clone().insert_property(
         name,
         function,
         Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE,

boa/src/builtins/global_this/mod.rs

@@ -30,7 +30,7 @@ impl BuiltIn for GlobalThis {
 
         (
             Self::NAME,
-            context.global_object().clone(),
+            context.global_object().clone().into(),
             Self::attribute(),
         )
     }

boa/src/builtins/iterable/mod.rs

@@ -21,21 +21,20 @@ impl IteratorPrototypes {
     pub(crate) fn init(context: &mut Context) -> Self {
         let iterator_prototype = create_iterator_prototype(context);
         Self {
-            iterator_prototype: iterator_prototype
-                .as_object()
-                .expect("Iterator prototype is not an object"),
-            array_iterator: ArrayIterator::create_prototype(context, iterator_prototype.clone())
-                .as_object()
-                .expect("Array Iterator Prototype is not an object"),
-            string_iterator: StringIterator::create_prototype(context, iterator_prototype.clone())
-                .as_object()
-                .expect("String Iterator Prototype is not an object"),
-            map_iterator: MapIterator::create_prototype(context, iterator_prototype.clone())
-                .as_object()
-                .expect("Map Iterator Prototype is not an object"),
-            for_in_iterator: ForInIterator::create_prototype(context, iterator_prototype)
-                .as_object()
-                .expect("For In Iterator Prototype is not an object"),
+            array_iterator: ArrayIterator::create_prototype(
+                context,
+                iterator_prototype.clone().into(),
+            ),
+            string_iterator: StringIterator::create_prototype(
+                context,
+                iterator_prototype.clone().into(),
+            ),
+            map_iterator: MapIterator::create_prototype(context, iterator_prototype.clone().into()),
+            for_in_iterator: ForInIterator::create_prototype(
+                context,
+                iterator_prototype.clone().into(),
+            ),
+            iterator_prototype,
         }
     }
 
@@ -99,7 +98,7 @@ pub fn get_iterator(context: &mut Context, iterable: Value) -> Result<IteratorRe
 ///  - [ECMA reference][spec]
 ///
 /// [spec]: https://tc39.es/ecma262/#sec-%iteratorprototype%-object
-fn create_iterator_prototype(context: &mut Context) -> Value {
+fn create_iterator_prototype(context: &mut Context) -> GcObject {
     let _timer = BoaProfiler::global().start_event("Iterator Prototype", "init");
 
     let symbol_iterator = context.well_known_symbols().iterator_symbol();
@@ -110,8 +109,7 @@ fn create_iterator_prototype(context: &mut Context) -> Value {
             0,
         )
         .build();
-    // TODO: return GcObject
-    iterator_prototype.into()
+    iterator_prototype
 }
 
 #[derive(Debug)]

boa/src/builtins/map/map_iterator.rs

@@ -1,6 +1,6 @@
 use crate::{
     builtins::{function::make_builtin_fn, iterable::create_iter_result_object, Array, Value},
-    object::ObjectData,
+    object::{GcObject, ObjectData},
     property::{Attribute, DataDescriptor},
     BoaProfiler, Context, Result,
 };
@@ -138,20 +138,17 @@ impl MapIterator {
     ///  - [ECMA reference][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-%mapiteratorprototype%-object
-    pub(crate) fn create_prototype(context: &mut Context, iterator_prototype: Value) -> Value {
+    pub(crate) fn create_prototype(context: &mut Context, iterator_prototype: Value) -> GcObject {
         let _timer = BoaProfiler::global().start_event(Self::NAME, "init");
 
         // Create prototype
-        let map_iterator = Value::new_object(context);
+        let mut map_iterator = context.construct_object();
         make_builtin_fn(Self::next, "next", &map_iterator, 0, context);
-        map_iterator
-            .as_object()
-            .expect("map iterator prototype object")
-            .set_prototype_instance(iterator_prototype);
+        map_iterator.set_prototype_instance(iterator_prototype);
 
         let to_string_tag = context.well_known_symbols().to_string_tag_symbol();
         let to_string_tag_property = DataDescriptor::new("Map Iterator", Attribute::CONFIGURABLE);
-        map_iterator.set_property(to_string_tag, to_string_tag_property);
+        map_iterator.insert(to_string_tag, to_string_tag_property);
         map_iterator
     }
 }

boa/src/builtins/map/mod.rs

@@ -80,7 +80,11 @@ impl Map {
         let map_prototype = context
             .global_object()
             .clone()
-            .get_field("Map", context)?
+            .get(
+                &"Map".into(),
+                context.global_object().clone().into(),
+                context,
+            )?
             .get_field(PROTOTYPE, context)?
             .as_object()
             .expect("'Map' global property should be an object");

boa/src/builtins/mod.rs

@@ -90,11 +90,7 @@ pub fn init(context: &mut Context) {
         console::Console::init,
     ];
 
-    let global_object = if let Value::Object(global) = context.global_object() {
-        global.clone()
-    } else {
-        unreachable!("global object should always be an object")
-    };
+    let global_object = context.global_object().clone();
 
     for init in &globals {
         let (name, value, attribute) = init(context);

boa/src/builtins/number/mod.rs

@@ -80,7 +80,7 @@ impl BuiltIn for Number {
         .static_method(Self::number_is_integer, "isInteger", 1)
         .build();
 
-        let global = context.global_object().clone();
+        let global = context.global_object();
         make_builtin_fn(
             Self::parse_int,
             "parseInt",

boa/src/builtins/object/for_in_iterator.rs

@@ -3,7 +3,7 @@ use crate::value::RcString;
 use crate::{
     builtins::{function::make_builtin_fn, iterable::create_iter_result_object},
     gc::{Finalize, Trace},
-    object::ObjectData,
+    object::{GcObject, ObjectData},
     property::{Attribute, DataDescriptor},
     BoaProfiler, Context, Result, Value,
 };
@@ -125,21 +125,18 @@ impl ForInIterator {
     ///  - [ECMA reference][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-%foriniteratorprototype%-object
-    pub(crate) fn create_prototype(context: &mut Context, iterator_prototype: Value) -> Value {
+    pub(crate) fn create_prototype(context: &mut Context, iterator_prototype: Value) -> GcObject {
         let _timer = BoaProfiler::global().start_event(Self::NAME, "init");
 
         // Create prototype
-        let for_in_iterator = Value::new_object(context);
+        let mut for_in_iterator = context.construct_object();
         make_builtin_fn(Self::next, "next", &for_in_iterator, 0, context);
-        for_in_iterator
-            .as_object()
-            .expect("for in iterator prototype object")
-            .set_prototype_instance(iterator_prototype);
+        for_in_iterator.set_prototype_instance(iterator_prototype);
 
         let to_string_tag = context.well_known_symbols().to_string_tag_symbol();
         let to_string_tag_property =
             DataDescriptor::new("For In Iterator", Attribute::CONFIGURABLE);
-        for_in_iterator.set_property(to_string_tag, to_string_tag_property);
+        for_in_iterator.insert(to_string_tag, to_string_tag_property);
         for_in_iterator
     }
 }

boa/src/builtins/string/string_iterator.rs

@@ -3,7 +3,7 @@ use crate::{
         function::make_builtin_fn, iterable::create_iter_result_object, string::code_point_at,
     },
     gc::{Finalize, Trace},
-    object::ObjectData,
+    object::{GcObject, ObjectData},
     property::{Attribute, DataDescriptor},
     BoaProfiler, Context, Result, Value,
 };
@@ -69,21 +69,18 @@ impl StringIterator {
     ///  - [ECMA reference][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-%arrayiteratorprototype%-object
-    pub(crate) fn create_prototype(context: &mut Context, iterator_prototype: Value) -> Value {
+    pub(crate) fn create_prototype(context: &mut Context, iterator_prototype: Value) -> GcObject {
         let _timer = BoaProfiler::global().start_event("String Iterator", "init");
 
         // Create prototype
-        let array_iterator = Value::new_object(context);
+        let mut array_iterator = context.construct_object();
         make_builtin_fn(Self::next, "next", &array_iterator, 0, context);
-        array_iterator
-            .as_object()
-            .expect("array iterator prototype object")
-            .set_prototype_instance(iterator_prototype);
+        array_iterator.set_prototype_instance(iterator_prototype);
 
         let to_string_tag = context.well_known_symbols().to_string_tag_symbol();
         let to_string_tag_property =
             DataDescriptor::new("String Iterator", Attribute::CONFIGURABLE);
-        array_iterator.set_property(to_string_tag, to_string_tag_property);
+        array_iterator.insert(to_string_tag, to_string_tag_property);
         array_iterator
     }
 }

boa/src/context.rs

@@ -337,7 +337,7 @@ impl Context {
 
     /// Return the global object.
     #[inline]
-    pub fn global_object(&self) -> &Value {
+    pub fn global_object(&self) -> &GcObject {
         &self.realm().global_object
     }
 
@@ -552,11 +552,8 @@ impl Context {
         body: NativeFunction,
     ) -> Result<()> {
         let function = self.create_builtin_function(name, length, body)?;
-        let global = self.global_object();
-        global
-            .as_object()
-            .unwrap()
-            .insert_property(name, function, Attribute::all());
+        let mut global = self.global_object().clone();
+        global.insert_property(name, function, Attribute::all());
         Ok(())
     }
 
@@ -663,10 +660,7 @@ impl Context {
 
         let class = class_builder.build();
         let property = DataDescriptor::new(class, T::ATTRIBUTE);
-        self.global_object()
-            .as_object()
-            .unwrap()
-            .insert(T::NAME, property);
+        self.global_object().clone().insert(T::NAME, property);
         Ok(())
     }
 
@@ -693,10 +687,7 @@ impl Context {
         V: Into<Value>,
     {
         let property = DataDescriptor::new(value, attribute);
-        self.global_object()
-            .as_object()
-            .unwrap()
-            .insert(key, property);
+        self.global_object().clone().insert(key, property);
     }
 
     /// Evaluates the given code.

boa/src/object/internal_methods.rs

@@ -6,7 +6,7 @@
 //! [spec]: https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots
 
 use crate::{
-    object::{GcObject, Object},
+    object::{GcObject, Object, ObjectData},
     property::{AccessorDescriptor, Attribute, DataDescriptor, PropertyDescriptor, PropertyKey},
     value::{same_value, Value},
     BoaProfiler, Context, Result,
@@ -555,6 +555,11 @@ impl GcObject {
     pub fn is_constructable(&self) -> bool {
         self.borrow().is_constructable()
     }
+
+    /// Returns true if the GcObject is the global for a Realm
+    pub fn is_global(&self) -> bool {
+        matches!(self.borrow().data, ObjectData::Global)
+    }
 }
 
 impl Object {

boa/src/realm.rs

@@ -4,7 +4,7 @@
 //!
 //! A realm is represented in this implementation as a Realm struct with the fields specified from the spec.
 
-use crate::object::Object;
+use crate::object::{GcObject, Object, ObjectData};
 use crate::{
     environment::{
         declarative_environment_record::DeclarativeEnvironmentRecord,
@@ -22,36 +22,42 @@ use rustc_hash::{FxHashMap, FxHashSet};
 /// In the specification these are called Realm Records.
 #[derive(Debug)]
 pub struct Realm {
-    pub global_object: Value,
+    pub global_object: GcObject,
     pub global_env: Gc<GcCell<GlobalEnvironmentRecord>>,
     pub environment: LexicalEnvironment,
 }
 
 impl Realm {
+    #[allow(clippy::field_reassign_with_default)]
     pub fn create() -> Self {
         let _timer = BoaProfiler::global().start_event("Realm::create", "realm");
         // Create brand new global object
         // Global has no prototype to pass None to new_obj
-        let global = Value::from(Object::default());
+        let mut global = Object::default();
 
         // Allow identification of the global object easily
-        global.set_data(crate::object::ObjectData::Global);
+        global.data = ObjectData::Global;
+
+        let gc_global = GcObject::new(global);
 
         // We need to clone the global here because its referenced from separate places (only pointer is cloned)
-        let global_env = new_global_environment(global.clone(), global.clone());
+        let global_env = new_global_environment(gc_global.clone(), gc_global.clone().into());
 
         Self {
-            global_object: global.clone(),
+            global_object: gc_global.clone(),
             global_env,
-            environment: LexicalEnvironment::new(global),
+            environment: LexicalEnvironment::new(gc_global.into()),
         }
     }
 }
 
 // Similar to new_global_environment in lexical_environment, except we need to return a GlobalEnvirionment
-fn new_global_environment(global: Value, this_value: Value) -> Gc<GcCell<GlobalEnvironmentRecord>> {
+fn new_global_environment(
+    global: GcObject,
+    this_value: Value,
+) -> Gc<GcCell<GlobalEnvironmentRecord>> {
     let obj_rec = ObjectEnvironmentRecord {
-        bindings: global,
+        bindings: Value::Object(global),
         outer_env: None,
         /// Object Environment Records created for with statements (13.11)
         /// can provide their binding object as an implicit this value for use in function calls.

boa/src/syntax/ast/node/call/mod.rs

@@ -79,7 +79,11 @@ impl Executable for Call {
                     obj.get_field(field.to_property_key(context)?, context)?,
                 )
             }
-            _ => (context.global_object().clone(), self.expr().run(context)?), // 'this' binding should come from the function's self-contained environment
+            _ => (
+                // 'this' binding should come from the function's self-contained environment
+                context.global_object().clone().into(),
+                self.expr().run(context)?,
+            ),
         };
         let mut v_args = Vec::with_capacity(self.args().len());
         for arg in self.args() {

boa/src/syntax/ast/node/template/mod.rs

@@ -119,7 +119,10 @@ impl Executable for TaggedTemplate {
                     obj.get_field(field.to_property_key(context)?, context)?,
                 )
             }
-            _ => (context.global_object().clone(), self.tag.run(context)?),
+            _ => (
+                context.global_object().clone().into(),
+                self.tag.run(context)?,
+            ),
         };
 
         let mut args = Vec::new();

boa/src/value/mod.rs

@@ -260,14 +260,6 @@ impl Value {
         true
     }
 
-    /// Returns true if the value the global for a Realm
-    pub fn is_global(&self) -> bool {
-        match self {
-            Value::Object(object) => matches!(object.borrow().data, ObjectData::Global),
-            _ => false,
-        }
-    }
-
     /// Returns true if the value is an object
     #[inline]
     pub fn is_object(&self) -> bool {