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

Move RefCell of CompileTimeEnvironments to field bindings #3108

Merged
merged 2 commits into from
Jul 9, 2023
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
8 changes: 3 additions & 5 deletions boa_engine/src/bytecompiler/declarations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -345,8 +345,6 @@ impl ByteCompiler<'_, '_> {
}
}

drop(env);

// Note: Not sure if the spec is wrong here or if our implementation just differs too much,
// but we need 3.a to be finished for all declarations before 3.b can be done.

Expand Down Expand Up @@ -532,7 +530,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 {
Expand Down
42 changes: 13 additions & 29 deletions boa_engine/src/bytecompiler/env.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{cell::RefCell, rc::Rc};
use std::rc::Rc;

use super::ByteCompiler;
use crate::environments::{BindingLocator, BindingLocatorError, CompileTimeEnvironment};
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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));
}

Expand All @@ -81,7 +72,6 @@ impl ByteCompiler<'_, '_> {
function_scope: bool,
) -> BindingLocator {
self.current_environment
.borrow()
.initialize_mutable_binding(name, function_scope)
}

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

Expand All @@ -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")]
Expand All @@ -125,7 +110,6 @@ impl ByteCompiler<'_, '_> {
name: Identifier,
) -> Result<BindingLocator, BindingLocatorError> {
self.current_environment
.borrow()
.set_mutable_binding_var_recursive(name)
}
}
2 changes: 1 addition & 1 deletion boa_engine/src/bytecompiler/expression/assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/bytecompiler/expression/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
4 changes: 2 additions & 2 deletions boa_engine/src/bytecompiler/function.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{cell::RefCell, rc::Rc};
use std::rc::Rc;

use crate::{
builtins::function::ThisMode,
Expand Down Expand Up @@ -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();
Expand Down
13 changes: 5 additions & 8 deletions boa_engine/src/bytecompiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,

Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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]);
Expand Down
Loading