Move RefCell of CompileTimeEnvironments to field bindings

All field except the `bindings` field are mutated after `CompileTimeEnvironment` creation. It moves the `RefCell` to `bindings` field, this cleans the methods callsites.
boa-dev · Jul 4, 2023 · 4a5c665 · 4a5c665
1 parent 9b1f2f4
commit 4a5c665
Show file tree

Hide file tree

Showing 14 changed files with 91 additions and 138 deletions.
diff --git a/boa_engine/src/bytecompiler/declarations.rs b/boa_engine/src/bytecompiler/declarations.rs
@@ -174,7 +174,7 @@ impl ByteCompiler<'_, '_> {
                 //    would not produce any Early Errors for script, then
                 if !lex_names.contains(&f) {
                     // a. If env.HasLexicalDeclaration(F) is false, then
-                    if !self.current_environment.borrow().has_lex_binding(f) {
+                    if !self.current_environment.has_lex_binding(f) {
                         // i. Let fnDefinable be ? env.CanDeclareGlobalVar(F).
                         let fn_definable = self.context.can_declare_global_function(f)?;
 
@@ -308,7 +308,7 @@ impl ByteCompiler<'_, '_> {
     where
         &'a N: Into<NodeRef<'a>>,
     {
-        let mut env = self.current_environment.borrow_mut();
+        let env = &self.current_environment;
 
         // 1. Let declarations be the LexicallyScopedDeclarations of code.
         let declarations = lexically_scoped_declarations(block);
@@ -532,7 +532,7 @@ impl ByteCompiler<'_, '_> {
                     let fn_definable = if !binding_exists && var_environment_is_global {
                         // a. If varEnv.HasLexicalDeclaration(F) is false, then
                         // b. Else,
-                        if self.current_environment.borrow().has_lex_binding(f) {
+                        if self.current_environment.has_lex_binding(f) {
                             // i. Let fnDefinable be false.
                             false
                         } else {

diff --git a/boa_engine/src/bytecompiler/env.rs b/boa_engine/src/bytecompiler/env.rs
@@ -1,4 +1,4 @@
-use std::{cell::RefCell, rc::Rc};
+use std::rc::Rc;
 
 use super::ByteCompiler;
 use crate::environments::{BindingLocator, BindingLocatorError, CompileTimeEnvironment};
@@ -7,10 +7,10 @@ use boa_ast::expression::Identifier;
 impl ByteCompiler<'_, '_> {
     /// Push either a new declarative or function environment on the compile time environment stack.
     pub(crate) fn push_compile_environment(&mut self, function_scope: bool) {
-        self.current_environment = Rc::new(RefCell::new(CompileTimeEnvironment::new(
+        self.current_environment = Rc::new(CompileTimeEnvironment::new(
             self.current_environment.clone(),
             function_scope,
-        )));
+        ));
     }
 
     /// Pops the top compile time environment and returns its index in the compile time environments array.
@@ -20,45 +20,37 @@ impl ByteCompiler<'_, '_> {
         self.compile_environments
             .push(self.current_environment.clone());
 
-        let outer = {
-            let env = self.current_environment.borrow();
-            env.outer().expect("cannot pop the global environment")
-        };
+        let outer = self
+            .current_environment
+            .outer()
+            .expect("cannot pop the global environment");
         self.current_environment = outer;
 
         index
     }
 
     /// Get the binding locator of the binding at bytecode compile time.
     pub(crate) fn get_binding_value(&self, name: Identifier) -> BindingLocator {
-        self.current_environment
-            .borrow()
-            .get_binding_recursive(name)
+        self.current_environment.get_binding_recursive(name)
     }
 
     /// Return if a declarative binding exists at bytecode compile time.
     /// This does not include bindings on the global object.
     pub(crate) fn has_binding(&self, name: Identifier) -> bool {
-        self.current_environment
-            .borrow()
-            .has_binding_recursive(name)
+        self.current_environment.has_binding_recursive(name)
     }
 
     /// Check if a binding name exists in a environment.
     /// If strict is `false` check until a function scope is reached.
     pub(crate) fn has_binding_eval(&self, name: Identifier, strict: bool) -> bool {
-        self.current_environment
-            .borrow()
-            .has_binding_eval(name, strict)
+        self.current_environment.has_binding_eval(name, strict)
     }
 
     #[cfg(feature = "annex-b")]
     /// Check if a binding name exists in a environment.
     /// Stop when a function scope is reached.
     pub(crate) fn has_binding_until_var(&self, name: Identifier) -> bool {
-        self.current_environment
-            .borrow()
-            .has_binding_until_var(name)
+        self.current_environment.has_binding_until_var(name)
     }
 
     /// Create a mutable binding at bytecode compile time.
@@ -70,7 +62,6 @@ impl ByteCompiler<'_, '_> {
     pub(crate) fn create_mutable_binding(&mut self, name: Identifier, function_scope: bool) {
         assert!(self
             .current_environment
-            .borrow_mut()
             .create_mutable_binding(name, function_scope));
     }
 
@@ -81,7 +72,6 @@ impl ByteCompiler<'_, '_> {
         function_scope: bool,
     ) -> BindingLocator {
         self.current_environment
-            .borrow()
             .initialize_mutable_binding(name, function_scope)
     }
 
@@ -93,7 +83,6 @@ impl ByteCompiler<'_, '_> {
     /// Panics if the global environment does not exist.
     pub(crate) fn create_immutable_binding(&mut self, name: Identifier, strict: bool) {
         self.current_environment
-            .borrow_mut()
             .create_immutable_binding(name, strict);
     }
 
@@ -103,19 +92,15 @@ impl ByteCompiler<'_, '_> {
     ///
     /// Panics if the global environment does not exist or a the binding was not created on the current environment.
     pub(crate) fn initialize_immutable_binding(&self, name: Identifier) -> BindingLocator {
-        self.current_environment
-            .borrow()
-            .initialize_immutable_binding(name)
+        self.current_environment.initialize_immutable_binding(name)
     }
 
     /// Return the binding locator for a set operation on an existing binding.
     pub(crate) fn set_mutable_binding(
         &self,
         name: Identifier,
     ) -> Result<BindingLocator, BindingLocatorError> {
-        self.current_environment
-            .borrow()
-            .set_mutable_binding_recursive(name)
+        self.current_environment.set_mutable_binding_recursive(name)
     }
 
     #[cfg(feature = "annex-b")]
@@ -125,7 +110,6 @@ impl ByteCompiler<'_, '_> {
         name: Identifier,
     ) -> Result<BindingLocator, BindingLocatorError> {
         self.current_environment
-            .borrow()
             .set_mutable_binding_var_recursive(name)
     }
 }
diff --git a/boa_engine/src/bytecompiler/expression/assign.rs b/boa_engine/src/bytecompiler/expression/assign.rs
@@ -57,7 +57,7 @@ impl ByteCompiler<'_, '_> {
                 Access::Variable { name } => {
                     let binding = self.get_binding_value(name);
                     let index = self.get_or_insert_binding(binding);
-                    let lex = self.current_environment.borrow().is_lex_binding(name);
+                    let lex = self.current_environment.is_lex_binding(name);
 
                     if lex {
                         self.emit(Opcode::GetName, &[index]);

diff --git a/boa_engine/src/bytecompiler/expression/update.rs b/boa_engine/src/bytecompiler/expression/update.rs
@@ -25,7 +25,7 @@ impl ByteCompiler<'_, '_> {
             Access::Variable { name } => {
                 let binding = self.get_binding_value(name);
                 let index = self.get_or_insert_binding(binding);
-                let lex = self.current_environment.borrow().is_lex_binding(name);
+                let lex = self.current_environment.is_lex_binding(name);
 
                 if lex {
                     self.emit(Opcode::GetName, &[index]);

diff --git a/boa_engine/src/bytecompiler/function.rs b/boa_engine/src/bytecompiler/function.rs
@@ -1,4 +1,4 @@
-use std::{cell::RefCell, rc::Rc};
+use std::rc::Rc;
 
 use crate::{
     builtins::function::ThisMode,
@@ -90,7 +90,7 @@ impl FunctionCompiler {
         mut self,
         parameters: &FormalParameterList,
         body: &FunctionBody,
-        outer_env: Rc<RefCell<CompileTimeEnvironment>>,
+        outer_env: Rc<CompileTimeEnvironment>,
         context: &mut Context<'_>,
     ) -> Gc<CodeBlock> {
         self.strict = self.strict || body.strict();

diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs
@@ -11,10 +11,7 @@ mod module;
 mod statement;
 mod utils;
 
-use std::{
-    cell::{Cell, RefCell},
-    rc::Rc,
-};
+use std::{cell::Cell, rc::Rc};
 
 use crate::{
     builtins::function::ThisMode,
@@ -244,10 +241,10 @@ pub struct ByteCompiler<'ctx, 'host> {
     pub(crate) functions: Vec<Gc<CodeBlock>>,
 
     /// Compile time environments in this function.
-    pub(crate) compile_environments: Vec<Rc<RefCell<CompileTimeEnvironment>>>,
+    pub(crate) compile_environments: Vec<Rc<CompileTimeEnvironment>>,
 
     /// The environment that is currently active.
-    pub(crate) current_environment: Rc<RefCell<CompileTimeEnvironment>>,
+    pub(crate) current_environment: Rc<CompileTimeEnvironment>,
 
     pub(crate) code_block_flags: CodeBlockFlags,
 
@@ -276,7 +273,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
         name: Sym,
         strict: bool,
         json_parse: bool,
-        current_environment: Rc<RefCell<CompileTimeEnvironment>>,
+        current_environment: Rc<CompileTimeEnvironment>,
         // TODO: remove when we separate scripts from the context
         context: &'ctx mut Context<'host>,
     ) -> ByteCompiler<'ctx, 'host> {
@@ -633,7 +630,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
             Access::Variable { name } => {
                 let binding = self.get_binding_value(name);
                 let index = self.get_or_insert_binding(binding);
-                let lex = self.current_environment.borrow().is_lex_binding(name);
+                let lex = self.current_environment.is_lex_binding(name);
 
                 if !lex {
                     self.emit(Opcode::GetLocator, &[index]);