From d157a34d71e3cf05199d4bd8b544cb474a65ff17 Mon Sep 17 00:00:00 2001 From: 0x7D2B <72297086+0x7D2B@users.noreply.github.com> Date: Wed, 2 Jun 2021 14:52:47 +0000 Subject: [PATCH] Fix environment record panics (#1285) * Fix environment record panics * Make environment record `new` methods return `Self` instead of `Environment` * Remove unnecessary clippy lint ignore * Remove GcCell from Environment * Remove global_env GcCell --- boa/src/builtins/function/mod.rs | 4 -- .../declarative_environment_record.rs | 68 ++++++++++--------- .../environment/environment_record_trait.rs | 29 ++++---- .../function_environment_record.rs | 38 +++++------ .../environment/global_environment_record.rs | 55 +++++++-------- boa/src/environment/lexical_environment.rs | 23 +++---- .../environment/object_environment_record.rs | 32 ++++----- boa/src/object/gcobject.rs | 20 +++--- boa/src/realm.rs | 43 ++---------- 9 files changed, 133 insertions(+), 179 deletions(-) diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index d91ea4833c..036f0c324a 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -125,14 +125,12 @@ impl Function { // Create binding local_env - .borrow_mut() // Function parameters can share names in JavaScript... .create_mutable_binding(param.name().to_owned(), false, true, context) .expect("Failed to create binding for rest param"); // Set Binding to value local_env - .borrow_mut() .initialize_binding(param.name(), array, context) .expect("Failed to initialize rest param"); } @@ -147,13 +145,11 @@ impl Function { ) { // Create binding local_env - .borrow_mut() .create_mutable_binding(param.name().to_owned(), false, true, context) .expect("Failed to create binding"); // Set Binding to value local_env - .borrow_mut() .initialize_binding(param.name(), value, context) .expect("Failed to intialize binding"); } diff --git a/boa/src/environment/declarative_environment_record.rs b/boa/src/environment/declarative_environment_record.rs index 6ca3c3918f..9890c1e39e 100644 --- a/boa/src/environment/declarative_environment_record.rs +++ b/boa/src/environment/declarative_environment_record.rs @@ -34,30 +34,27 @@ pub struct DeclarativeEnvironmentRecordBinding { /// declarations contained within its scope. #[derive(Debug, Trace, Finalize, Clone)] pub struct DeclarativeEnvironmentRecord { - pub env_rec: FxHashMap, DeclarativeEnvironmentRecordBinding>, + pub env_rec: GcCell, DeclarativeEnvironmentRecordBinding>>, pub outer_env: Option, } impl DeclarativeEnvironmentRecord { - #[allow(clippy::new_ret_no_self)] - pub fn new(env: Option) -> Environment { + pub fn new(env: Option) -> DeclarativeEnvironmentRecord { let _timer = BoaProfiler::global().start_event("new_declarative_environment", "env"); - let boxed_env = Box::new(DeclarativeEnvironmentRecord { - env_rec: FxHashMap::default(), + DeclarativeEnvironmentRecord { + env_rec: GcCell::new(FxHashMap::default()), outer_env: env, - }); - - Gc::new(GcCell::new(boxed_env)) + } } } impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { fn has_binding(&self, name: &str) -> bool { - self.env_rec.contains_key(name) + self.env_rec.borrow().contains_key(name) } fn create_mutable_binding( - &mut self, + &self, name: String, deletion: bool, allow_name_reuse: bool, @@ -65,13 +62,13 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { ) -> Result<()> { if !allow_name_reuse { assert!( - !self.env_rec.contains_key(name.as_str()), + !self.env_rec.borrow().contains_key(name.as_str()), "Identifier {} has already been declared", name ); } - self.env_rec.insert( + self.env_rec.borrow_mut().insert( name.into_boxed_str(), DeclarativeEnvironmentRecordBinding { value: None, @@ -84,18 +81,18 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { } fn create_immutable_binding( - &mut self, + &self, name: String, strict: bool, _context: &mut Context, ) -> Result<()> { assert!( - !self.env_rec.contains_key(name.as_str()), + !self.env_rec.borrow().contains_key(name.as_str()), "Identifier {} has already been declared", name ); - self.env_rec.insert( + self.env_rec.borrow_mut().insert( name.into_boxed_str(), DeclarativeEnvironmentRecordBinding { value: None, @@ -107,13 +104,8 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { Ok(()) } - fn initialize_binding( - &mut self, - name: &str, - value: Value, - _context: &mut Context, - ) -> Result<()> { - if let Some(ref mut record) = self.env_rec.get_mut(name) { + fn initialize_binding(&self, name: &str, value: Value, _context: &mut Context) -> Result<()> { + if let Some(ref mut record) = self.env_rec.borrow_mut().get_mut(name) { if record.value.is_none() { record.value = Some(value); return Ok(()); @@ -124,13 +116,13 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { #[allow(clippy::else_if_without_else)] fn set_mutable_binding( - &mut self, + &self, name: &str, value: Value, mut strict: bool, context: &mut Context, ) -> Result<()> { - if self.env_rec.get(name).is_none() { + if self.env_rec.borrow().get(name).is_none() { if strict { return Err(context.construct_reference_error(format!("{} not found", name))); } @@ -140,16 +132,22 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { return Ok(()); } - let record: &mut DeclarativeEnvironmentRecordBinding = self.env_rec.get_mut(name).unwrap(); - if record.strict { + let (record_strict, record_has_no_value, record_mutable) = { + let env_rec = self.env_rec.borrow(); + let record = env_rec.get(name).unwrap(); + (record.strict, record.value.is_none(), record.mutable) + }; + if record_strict { strict = true } - if record.value.is_none() { + if record_has_no_value { return Err( context.construct_reference_error(format!("{} has not been initialized", name)) ); } - if record.mutable { + if record_mutable { + let mut env_rec = self.env_rec.borrow_mut(); + let record = env_rec.get_mut(name).unwrap(); record.value = Some(value); } else if strict { return Err(context.construct_reference_error(format!( @@ -162,7 +160,7 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { } fn get_binding_value(&self, name: &str, _strict: bool, context: &mut Context) -> Result { - if let Some(binding) = self.env_rec.get(name) { + if let Some(binding) = self.env_rec.borrow().get(name) { if let Some(ref val) = binding.value { Ok(val.clone()) } else { @@ -173,11 +171,11 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { } } - fn delete_binding(&mut self, name: &str) -> bool { - match self.env_rec.get(name) { + fn delete_binding(&self, name: &str) -> bool { + match self.env_rec.borrow().get(name) { Some(binding) => { if binding.can_delete { - self.env_rec.remove(name); + self.env_rec.borrow_mut().remove(name); true } else { false @@ -215,3 +213,9 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { EnvironmentType::Declarative } } + +impl From for Environment { + fn from(env: DeclarativeEnvironmentRecord) -> Environment { + Gc::new(Box::new(env)) + } +} diff --git a/boa/src/environment/environment_record_trait.rs b/boa/src/environment/environment_record_trait.rs index c730d74b64..2ab9ebefcf 100644 --- a/boa/src/environment/environment_record_trait.rs +++ b/boa/src/environment/environment_record_trait.rs @@ -34,7 +34,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { /// Most variable names cannot be reused, but functions in JavaScript are allowed to have multiple /// paraments with the same name. fn create_mutable_binding( - &mut self, + &self, name: String, deletion: bool, allow_name_reuse: bool, @@ -46,7 +46,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { /// If strict is true then attempts to set it after it has been initialized will always throw an exception, /// regardless of the strict mode setting of operations that reference that binding. fn create_immutable_binding( - &mut self, + &self, name: String, strict: bool, context: &mut Context, @@ -55,15 +55,14 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { /// Set the value of an already existing but uninitialized binding in an Environment Record. /// The String value N is the text of the bound name. /// V is the value for the binding and is a value of any ECMAScript language type. - fn initialize_binding(&mut self, name: &str, value: Value, context: &mut Context) - -> Result<()>; + fn initialize_binding(&self, name: &str, value: Value, context: &mut Context) -> Result<()>; /// Set the value of an already existing mutable binding in an Environment Record. /// The String value `name` is the text of the bound name. /// value is the `value` for the binding and may be a value of any ECMAScript language type. S is a Boolean flag. /// If `strict` is true and the binding cannot be set throw a TypeError exception. fn set_mutable_binding( - &mut self, + &self, name: &str, value: Value, strict: bool, @@ -80,7 +79,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { /// The String value name is the text of the bound name. /// If a binding for name exists, remove the binding and return true. /// If the binding exists but cannot be removed return false. If the binding does not exist return true. - fn delete_binding(&mut self, name: &str) -> bool; + fn delete_binding(&self, name: &str) -> bool; /// Determine if an Environment Record establishes a this binding. /// Return true if it does and false if it does not. @@ -115,7 +114,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { self.get_this_binding(context) } else { match self.get_outer_environment_ref() { - Some(outer) => outer.borrow().recursive_get_this_binding(context), + Some(outer) => outer.recursive_get_this_binding(context), None => Ok(Value::Undefined), } } @@ -123,7 +122,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { /// Create mutable binding while handling outer environments fn recursive_create_mutable_binding( - &mut self, + &self, name: String, deletion: bool, scope: VariableScope, @@ -134,14 +133,13 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { VariableScope::Function => self .get_outer_environment_ref() .expect("No function or global environment") - .borrow_mut() .recursive_create_mutable_binding(name, deletion, scope, context), } } /// Create immutable binding while handling outer environments fn recursive_create_immutable_binding( - &mut self, + &self, name: String, deletion: bool, scope: VariableScope, @@ -152,14 +150,13 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { VariableScope::Function => self .get_outer_environment_ref() .expect("No function or global environment") - .borrow_mut() .recursive_create_immutable_binding(name, deletion, scope, context), } } /// Set mutable binding while handling outer environments fn recursive_set_mutable_binding( - &mut self, + &self, name: &str, value: Value, strict: bool, @@ -170,14 +167,13 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { } else { self.get_outer_environment_ref() .expect("Environment stack underflow") - .borrow_mut() .recursive_set_mutable_binding(name, value, strict, context) } } /// Initialize binding while handling outer environments fn recursive_initialize_binding( - &mut self, + &self, name: &str, value: Value, context: &mut Context, @@ -187,7 +183,6 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { } else { self.get_outer_environment_ref() .expect("Environment stack underflow") - .borrow_mut() .recursive_initialize_binding(name, value, context) } } @@ -196,7 +191,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { fn recursive_has_binding(&self, name: &str) -> bool { self.has_binding(name) || match self.get_outer_environment_ref() { - Some(outer) => outer.borrow().recursive_has_binding(name), + Some(outer) => outer.recursive_has_binding(name), None => false, } } @@ -207,7 +202,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize { self.get_binding_value(name, false, context) } else { match self.get_outer_environment_ref() { - Some(outer) => outer.borrow().recursive_get_binding_value(name, context), + Some(outer) => outer.recursive_get_binding_value(name, context), None => context.throw_reference_error(format!("{} is not defined", name)), } } diff --git a/boa/src/environment/function_environment_record.rs b/boa/src/environment/function_environment_record.rs index 0b28e42fd4..9113552519 100644 --- a/boa/src/environment/function_environment_record.rs +++ b/boa/src/environment/function_environment_record.rs @@ -8,8 +8,7 @@ //! from within the function. //! More info: -use gc::{Gc, GcCell}; -use rustc_hash::FxHashMap; +use gc::Gc; use crate::{ environment::{ @@ -59,19 +58,15 @@ pub struct FunctionEnvironmentRecord { } impl FunctionEnvironmentRecord { - #[allow(clippy::new_ret_no_self)] pub fn new( f: GcObject, this: Option, outer: Option, binding_status: BindingStatus, new_target: Value, - ) -> Environment { + ) -> FunctionEnvironmentRecord { let mut func_env = FunctionEnvironmentRecord { - declarative_record: DeclarativeEnvironmentRecord { - env_rec: FxHashMap::default(), - outer_env: outer, // this will come from Environment set as a private property of F - https://tc39.es/ecma262/#sec-ecmascript-function-objects - }, + declarative_record: DeclarativeEnvironmentRecord::new(outer), // the outer environment will come from Environment set as a private property of F - https://tc39.es/ecma262/#sec-ecmascript-function-objects function: f, this_binding_status: binding_status, home_object: Value::undefined(), @@ -82,7 +77,7 @@ impl FunctionEnvironmentRecord { if let Some(v) = this { func_env.bind_this_value(v).unwrap(); } - Gc::new(GcCell::new(Box::new(func_env))) + func_env } pub fn bind_this_value(&mut self, value: Value) -> Result { @@ -123,7 +118,7 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord { } fn create_mutable_binding( - &mut self, + &self, name: String, deletion: bool, allow_name_reuse: bool, @@ -134,7 +129,7 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord { } fn create_immutable_binding( - &mut self, + &self, name: String, strict: bool, context: &mut Context, @@ -143,18 +138,13 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord { .create_immutable_binding(name, strict, context) } - fn initialize_binding( - &mut self, - name: &str, - value: Value, - context: &mut Context, - ) -> Result<()> { + fn initialize_binding(&self, name: &str, value: Value, context: &mut Context) -> Result<()> { self.declarative_record .initialize_binding(name, value, context) } fn set_mutable_binding( - &mut self, + &self, name: &str, value: Value, strict: bool, @@ -169,7 +159,7 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord { .get_binding_value(name, strict, context) } - fn delete_binding(&mut self, name: &str) -> bool { + fn delete_binding(&self, name: &str) -> bool { self.declarative_record.delete_binding(name) } @@ -214,7 +204,7 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord { } fn recursive_create_mutable_binding( - &mut self, + &self, name: String, deletion: bool, _scope: VariableScope, @@ -224,7 +214,7 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord { } fn recursive_create_immutable_binding( - &mut self, + &self, name: String, deletion: bool, _scope: VariableScope, @@ -233,3 +223,9 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord { self.create_immutable_binding(name, deletion, context) } } + +impl From for Environment { + fn from(env: FunctionEnvironmentRecord) -> Environment { + Gc::new(Box::new(env)) + } +} diff --git a/boa/src/environment/global_environment_record.rs b/boa/src/environment/global_environment_record.rs index 2c6c83baf7..8228622bff 100644 --- a/boa/src/environment/global_environment_record.rs +++ b/boa/src/environment/global_environment_record.rs @@ -20,19 +20,18 @@ use crate::{ Context, Result, Value, }; use gc::{Gc, GcCell}; -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::FxHashSet; #[derive(Debug, Trace, Finalize, Clone)] pub struct GlobalEnvironmentRecord { pub object_record: ObjectEnvironmentRecord, pub global_this_binding: GcObject, pub declarative_record: DeclarativeEnvironmentRecord, - pub var_names: FxHashSet>, + pub var_names: GcCell>>, } impl GlobalEnvironmentRecord { - #[allow(clippy::new_ret_no_self)] - pub fn new(global: GcObject, this_value: GcObject) -> Environment { + pub fn new(global: GcObject, this_value: GcObject) -> GlobalEnvironmentRecord { let obj_rec = ObjectEnvironmentRecord { bindings: global.into(), outer_env: None, @@ -44,21 +43,18 @@ impl GlobalEnvironmentRecord { with_environment: false, }; - let dcl_rec = DeclarativeEnvironmentRecord { - env_rec: FxHashMap::default(), - outer_env: None, - }; + let dcl_rec = DeclarativeEnvironmentRecord::new(None); - Gc::new(GcCell::new(Box::new(GlobalEnvironmentRecord { + GlobalEnvironmentRecord { object_record: obj_rec, global_this_binding: this_value, declarative_record: dcl_rec, - var_names: FxHashSet::default(), - }))) + var_names: GcCell::new(FxHashSet::default()), + } } pub fn has_var_declaration(&self, name: &str) -> bool { - self.var_names.contains(name) + self.var_names.borrow().contains(name) } pub fn has_lexical_declaration(&self, name: &str) -> bool { @@ -118,7 +114,7 @@ impl GlobalEnvironmentRecord { obj_rec.initialize_binding(&name, Value::undefined(), context)?; } - let var_declared_names = &mut self.var_names; + let mut var_declared_names = self.var_names.borrow_mut(); if !var_declared_names.contains(name.as_str()) { var_declared_names.insert(name.into_boxed_str()); } @@ -156,7 +152,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { } fn create_mutable_binding( - &mut self, + &self, name: String, deletion: bool, allow_name_reuse: bool, @@ -173,7 +169,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { } fn create_immutable_binding( - &mut self, + &self, name: String, strict: bool, context: &mut Context, @@ -188,12 +184,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { .create_immutable_binding(name, strict, context) } - fn initialize_binding( - &mut self, - name: &str, - value: Value, - context: &mut Context, - ) -> Result<()> { + fn initialize_binding(&self, name: &str, value: Value, context: &mut Context) -> Result<()> { if self.declarative_record.has_binding(&name) { return self .declarative_record @@ -208,7 +199,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { } fn set_mutable_binding( - &mut self, + &self, name: &str, value: Value, strict: bool, @@ -232,7 +223,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { self.object_record.get_binding_value(name, strict, context) } - fn delete_binding(&mut self, name: &str) -> bool { + fn delete_binding(&self, name: &str) -> bool { if self.declarative_record.has_binding(&name) { return self.declarative_record.delete_binding(name); } @@ -241,7 +232,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { if global.has_field(name) { let status = self.object_record.delete_binding(name); if status { - let var_names = &mut self.var_names; + let mut var_names = self.var_names.borrow_mut(); if var_names.contains(name) { var_names.remove(name); return status; @@ -277,7 +268,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { fn set_outer_environment(&mut self, _env: Environment) { // TODO: Implement - panic!("Not implemented yet") + todo!("Not implemented yet") } fn get_environment_type(&self) -> EnvironmentType { @@ -285,7 +276,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { } fn recursive_create_mutable_binding( - &mut self, + &self, name: String, deletion: bool, _scope: VariableScope, @@ -295,7 +286,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { } fn recursive_create_immutable_binding( - &mut self, + &self, name: String, deletion: bool, _scope: VariableScope, @@ -305,7 +296,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { } fn recursive_set_mutable_binding( - &mut self, + &self, name: &str, value: Value, strict: bool, @@ -315,7 +306,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { } fn recursive_initialize_binding( - &mut self, + &self, name: &str, value: Value, context: &mut Context, @@ -323,3 +314,9 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { self.initialize_binding(name, value, context) } } + +impl From for Environment { + fn from(env: GlobalEnvironmentRecord) -> Environment { + Gc::new(Box::new(env)) + } +} diff --git a/boa/src/environment/lexical_environment.rs b/boa/src/environment/lexical_environment.rs index fdf4b8a97b..a36243336d 100644 --- a/boa/src/environment/lexical_environment.rs +++ b/boa/src/environment/lexical_environment.rs @@ -10,11 +10,11 @@ use crate::{ environment::environment_record_trait::EnvironmentRecordTrait, object::GcObject, BoaProfiler, Context, Result, Value, }; -use gc::{Gc, GcCell}; +use gc::Gc; use std::{collections::VecDeque, error, fmt}; /// Environments are wrapped in a Box and then in a GC wrapper -pub type Environment = Gc>>; +pub type Environment = Gc>; /// Give each environment an easy way to declare its own type /// This helps with comparisons @@ -71,14 +71,17 @@ impl LexicalEnvironment { }; // lexical_env.push(global_env); - lexical_env.environment_stack.push_back(global_env); + lexical_env.environment_stack.push_back(global_env.into()); lexical_env } } impl Context { - pub(crate) fn push_environment(&mut self, env: Environment) { - self.realm.environment.environment_stack.push_back(env); + pub(crate) fn push_environment>(&mut self, env: T) { + self.realm + .environment + .environment_stack + .push_back(env.into()); } pub(crate) fn pop_environment(&mut self) -> Option { @@ -87,7 +90,6 @@ impl Context { pub(crate) fn get_this_binding(&mut self) -> Result { self.get_current_environment() - .borrow() .recursive_get_this_binding(self) } @@ -98,7 +100,6 @@ impl Context { scope: VariableScope, ) -> Result<()> { self.get_current_environment() - .borrow_mut() .recursive_create_mutable_binding(name, deletion, scope, self) } @@ -109,7 +110,6 @@ impl Context { scope: VariableScope, ) -> Result<()> { self.get_current_environment() - .borrow_mut() .recursive_create_immutable_binding(name, deletion, scope, self) } @@ -120,13 +120,11 @@ impl Context { strict: bool, ) -> Result<()> { self.get_current_environment() - .borrow_mut() .recursive_set_mutable_binding(name, value, strict, self) } pub(crate) fn initialize_binding(&mut self, name: &str, value: Value) -> Result<()> { self.get_current_environment() - .borrow_mut() .recursive_initialize_binding(name, value, self) } @@ -142,14 +140,11 @@ impl Context { } pub(crate) fn has_binding(&mut self, name: &str) -> bool { - self.get_current_environment() - .borrow() - .recursive_has_binding(name) + self.get_current_environment().recursive_has_binding(name) } pub(crate) fn get_binding_value(&mut self, name: &str) -> Result { self.get_current_environment() - .borrow() .recursive_get_binding_value(name, self) } } diff --git a/boa/src/environment/object_environment_record.rs b/boa/src/environment/object_environment_record.rs index 75f70f35aa..f2e4d81880 100644 --- a/boa/src/environment/object_environment_record.rs +++ b/boa/src/environment/object_environment_record.rs @@ -6,7 +6,7 @@ //! Property keys that are not strings in the form of an `IdentifierName` are not included in the set of bound identifiers. //! More info: [Object Records](https://tc39.es/ecma262/#sec-object-environment-records) -use gc::{Gc, GcCell}; +use gc::Gc; use crate::{ environment::{ @@ -29,9 +29,8 @@ pub struct ObjectEnvironmentRecord { } impl ObjectEnvironmentRecord { - #[allow(clippy::new_ret_no_self)] - pub fn new(object: Value, environment: Option) -> Environment { - Gc::new(GcCell::new(Box::new(ObjectEnvironmentRecord { + pub fn new(object: Value, environment: Option) -> ObjectEnvironmentRecord { + ObjectEnvironmentRecord { bindings: object, outer_env: environment, /// Object Environment Records created for with statements (13.11) @@ -40,7 +39,7 @@ impl ObjectEnvironmentRecord { /// with each object Environment Record. By default, the value of withEnvironment is false /// for any object Environment Record. with_environment: false, - }))) + } } } @@ -57,7 +56,7 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { } fn create_mutable_binding( - &mut self, + &self, name: String, deletion: bool, _allow_name_reuse: bool, @@ -65,7 +64,7 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { ) -> Result<()> { // TODO: could save time here and not bother generating a new undefined object, // only for it to be replace with the real value later. We could just add the name to a Vector instead - let bindings = &mut self.bindings; + let bindings = &self.bindings; let mut prop = DataDescriptor::new( Value::undefined(), Attribute::WRITABLE | Attribute::ENUMERABLE, @@ -77,7 +76,7 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { } fn create_immutable_binding( - &mut self, + &self, _name: String, _strict: bool, _context: &mut Context, @@ -85,12 +84,7 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { Ok(()) } - fn initialize_binding( - &mut self, - name: &str, - value: Value, - context: &mut Context, - ) -> Result<()> { + fn initialize_binding(&self, name: &str, value: Value, context: &mut Context) -> Result<()> { // We should never need to check if a binding has been created, // As all calls to create_mutable_binding are followed by initialized binding // The below is just a check. @@ -99,7 +93,7 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { } fn set_mutable_binding( - &mut self, + &self, name: &str, value: Value, strict: bool, @@ -128,7 +122,7 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { } } - fn delete_binding(&mut self, name: &str) -> bool { + fn delete_binding(&self, name: &str) -> bool { self.bindings.remove_property(name); true } @@ -167,3 +161,9 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { EnvironmentType::Function } } + +impl From for Environment { + fn from(env: ObjectEnvironmentRecord) -> Environment { + Gc::new(Box::new(env)) + } +} diff --git a/boa/src/object/gcobject.rs b/boa/src/object/gcobject.rs index e7c194caaa..1f71f7efa8 100644 --- a/boa/src/object/gcobject.rs +++ b/boa/src/object/gcobject.rs @@ -7,7 +7,11 @@ use crate::{ builtins::function::{ create_unmapped_arguments_object, BuiltInFunction, Function, NativeFunction, }, - environment::function_environment_record::{BindingStatus, FunctionEnvironmentRecord}, + environment::{ + environment_record_trait::EnvironmentRecordTrait, + function_environment_record::{BindingStatus, FunctionEnvironmentRecord}, + lexical_environment::Environment, + }, property::{AccessorDescriptor, Attribute, DataDescriptor, PropertyDescriptor, PropertyKey}, syntax::ast::node::RcStatementList, value::PreferredType, @@ -216,19 +220,19 @@ impl GcObject { { // Add arguments object let arguments_obj = create_unmapped_arguments_object(args); - local_env.borrow_mut().create_mutable_binding( + local_env.create_mutable_binding( "arguments".to_string(), false, true, context, )?; - local_env.borrow_mut().initialize_binding( - "arguments", - arguments_obj, - context, - )?; + local_env.initialize_binding("arguments", arguments_obj, context)?; } - // push the environment first so that it will be used by default parameters + + // Turn local_env into Environment so it can be cloned + let local_env: Environment = local_env.into(); + + // Push the environment first so that it will be used by default parameters context.push_environment(local_env.clone()); // Add argument bindings to the function environment diff --git a/boa/src/realm.rs b/boa/src/realm.rs index 6f40e41fd4..4911858b61 100644 --- a/boa/src/realm.rs +++ b/boa/src/realm.rs @@ -6,16 +6,12 @@ use crate::{ environment::{ - declarative_environment_record::DeclarativeEnvironmentRecord, - global_environment_record::GlobalEnvironmentRecord, - lexical_environment::LexicalEnvironment, - object_environment_record::ObjectEnvironmentRecord, + global_environment_record::GlobalEnvironmentRecord, lexical_environment::LexicalEnvironment, }, object::{GcObject, Object, ObjectData}, BoaProfiler, }; -use gc::{Gc, GcCell}; -use rustc_hash::{FxHashMap, FxHashSet}; +use gc::Gc; /// Representation of a Realm. /// @@ -23,7 +19,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; #[derive(Debug)] pub struct Realm { pub global_object: GcObject, - pub global_env: Gc>, + pub global_env: Gc, pub environment: LexicalEnvironment, } @@ -41,41 +37,12 @@ impl Realm { 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(gc_global.clone(), gc_global.clone()); + let global_env = GlobalEnvironmentRecord::new(gc_global.clone(), gc_global.clone()); Self { global_object: gc_global.clone(), - global_env, + global_env: Gc::new(global_env), environment: LexicalEnvironment::new(gc_global), } } } - -// Similar to new_global_environment in lexical_environment, except we need to return a GlobalEnvirionment -fn new_global_environment( - global: GcObject, - this_value: GcObject, -) -> Gc> { - let obj_rec = ObjectEnvironmentRecord { - bindings: global.into(), - 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. - /// The capability is controlled by a withEnvironment Boolean value that is associated - /// with each object Environment Record. By default, the value of withEnvironment is false - /// for any object Environment Record. - with_environment: false, - }; - - let dcl_rec = DeclarativeEnvironmentRecord { - env_rec: FxHashMap::default(), - outer_env: None, - }; - - Gc::new(GcCell::new(GlobalEnvironmentRecord { - object_record: obj_rec, - global_this_binding: this_value, - declarative_record: dcl_rec, - var_names: FxHashSet::default(), - })) -}