Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jan 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions boa/src/builtins/array/array_iterator.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -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
}
}
4 changes: 2 additions & 2 deletions boa/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion boa/src/builtins/global_this/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl BuiltIn for GlobalThis {

(
Self::NAME,
context.global_object().clone(),
context.global_object().clone().into(),
Self::attribute(),
)
}
Expand Down
34 changes: 16 additions & 18 deletions boa/src/builtins/iterable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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();
Expand All @@ -110,8 +109,7 @@ fn create_iterator_prototype(context: &mut Context) -> Value {
0,
)
.build();
// TODO: return GcObject
iterator_prototype.into()
iterator_prototype
}

#[derive(Debug)]
Expand Down
13 changes: 5 additions & 8 deletions boa/src/builtins/map/map_iterator.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -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
}
}
6 changes: 5 additions & 1 deletion boa/src/builtins/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
6 changes: 1 addition & 5 deletions boa/src/builtins/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion boa/src/builtins/number/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 5 additions & 8 deletions boa/src/builtins/object/for_in_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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
}
}
13 changes: 5 additions & 8 deletions boa/src/builtins/string/string_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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
}
}
19 changes: 5 additions & 14 deletions boa/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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(())
}

Expand Down Expand Up @@ -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(())
}

Expand All @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion boa/src/object/internal_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 15 additions & 9 deletions boa/src/realm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add attribute #[allow(clippy::field_reassign_with_default)] on the create method to make clippy happy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does clippy offer a better suggestion?

Copy link
Contributor Author

@RageKnify RageKnify Jan 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code was triggering a clippy lint, but it seems to be a false positive since the code proposed by clippy wasn't valid.


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.
Expand Down
6 changes: 5 additions & 1 deletion boa/src/syntax/ast/node/call/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading