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

@@ -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");
     }

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<Box<str>, DeclarativeEnvironmentRecordBinding>,
+    pub env_rec: GcCell<FxHashMap<Box<str>, DeclarativeEnvironmentRecordBinding>>,
     pub outer_env: Option<Environment>,
 }
 
 impl DeclarativeEnvironmentRecord {
-    #[allow(clippy::new_ret_no_self)]
-    pub fn new(env: Option<Environment>) -> Environment {
+    pub fn new(env: Option<Environment>) -> 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<Value> {
-        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<DeclarativeEnvironmentRecord> for Environment {
+    fn from(env: DeclarativeEnvironmentRecord) -> Environment {
+        Gc::new(Box::new(env))
+    }
+}

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)),
             }
         }

boa/src/environment/function_environment_record.rs

@@ -8,8 +8,7 @@
 //! from within the function.
 //! More info: <https://tc39.es/ecma262/#sec-function-environment-records>
 
-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<Value>,
         outer: Option<Environment>,
         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<Value> {
@@ -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<FunctionEnvironmentRecord> for Environment {
+    fn from(env: FunctionEnvironmentRecord) -> Environment {
+        Gc::new(Box::new(env))
+    }
+}

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<Box<str>>,
+    pub var_names: GcCell<FxHashSet<Box<str>>>,
 }
 
 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<GlobalEnvironmentRecord> for Environment {
+    fn from(env: GlobalEnvironmentRecord) -> Environment {
+        Gc::new(Box::new(env))
+    }
+}

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<GcCell<Box<dyn EnvironmentRecordTrait>>>;
+pub type Environment = Gc<Box<dyn EnvironmentRecordTrait>>;
 
 /// 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<T: Into<Environment>>(&mut self, env: T) {
+        self.realm
+            .environment
+            .environment_stack
+            .push_back(env.into());
     }
 
     pub(crate) fn pop_environment(&mut self) -> Option<Environment> {
@@ -87,7 +90,6 @@ impl Context {
 
     pub(crate) fn get_this_binding(&mut self) -> Result<Value> {
         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<Value> {
         self.get_current_environment()
-            .borrow()
             .recursive_get_binding_value(name, self)
     }
 }

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>) -> Environment {
-        Gc::new(GcCell::new(Box::new(ObjectEnvironmentRecord {
+    pub fn new(object: Value, environment: Option<Environment>) -> 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<ObjectEnvironmentRecord> for Environment {
+    fn from(env: ObjectEnvironmentRecord) -> Environment {
+        Gc::new(Box::new(env))
+    }
+}

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

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<GcCell<GlobalEnvironmentRecord>>,
+    pub global_env: Gc<GlobalEnvironmentRecord>,
     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<GcCell<GlobalEnvironmentRecord>> {
-    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(),
-    }))
-}