From e1da91292e71f4c83a56119eef2b1f2d93642e54 Mon Sep 17 00:00:00 2001 From: raskad <32105367+raskad@users.noreply.github.com> Date: Wed, 4 Dec 2024 22:18:13 +0000 Subject: [PATCH] Reduce environment allocations (#4002) * Skip environment creation when all bindings in the scope are local * Skip environment creation when possible for arrow functions * Do not allocate space for local bindings in runtime environments * Change RefCell to Cell --- core/ast/src/expression/literal/object.rs | 7 + core/ast/src/function/arrow_function.rs | 7 + core/ast/src/function/async_arrow_function.rs | 7 + core/ast/src/function/async_function.rs | 14 + core/ast/src/function/async_generator.rs | 14 + core/ast/src/function/class.rs | 7 + core/ast/src/function/generator.rs | 14 + core/ast/src/function/ordinary_function.rs | 14 + core/ast/src/scope.rs | 308 +++++++--- core/ast/src/scope_analyzer.rs | 544 +++++++++++++++++- core/ast/src/source.rs | 20 +- .../engine/src/builtins/function/arguments.rs | 16 +- core/engine/src/builtins/function/mod.rs | 11 +- core/engine/src/bytecompiler/class.rs | 30 +- core/engine/src/bytecompiler/declarations.rs | 169 +++--- core/engine/src/bytecompiler/env.rs | 32 +- core/engine/src/bytecompiler/function.rs | 19 +- core/engine/src/bytecompiler/mod.rs | 24 +- .../src/bytecompiler/statement/block.rs | 19 +- .../engine/src/bytecompiler/statement/loop.rs | 96 ++-- .../src/bytecompiler/statement/switch.rs | 17 +- core/engine/src/bytecompiler/statement/try.rs | 10 +- core/engine/src/environments/runtime/mod.rs | 4 +- core/engine/src/module/synthetic.rs | 2 + core/engine/src/vm/code_block.rs | 10 + core/engine/src/vm/opcode/push/environment.rs | 5 +- 26 files changed, 1093 insertions(+), 327 deletions(-) diff --git a/core/ast/src/expression/literal/object.rs b/core/ast/src/expression/literal/object.rs index 88355918b92..3191f20ae71 100644 --- a/core/ast/src/expression/literal/object.rs +++ b/core/ast/src/expression/literal/object.rs @@ -469,6 +469,13 @@ impl ObjectMethodDefinition { pub const fn scopes(&self) -> &FunctionScopes { &self.scopes } + + /// Returns `true` if the object method definition contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } } impl ToIndentedString for ObjectMethodDefinition { diff --git a/core/ast/src/function/arrow_function.rs b/core/ast/src/function/arrow_function.rs index 26bf82437f9..9272febeb13 100644 --- a/core/ast/src/function/arrow_function.rs +++ b/core/ast/src/function/arrow_function.rs @@ -86,6 +86,13 @@ impl ArrowFunction { pub const fn scopes(&self) -> &FunctionScopes { &self.scopes } + + /// Returns `true` if the arrow function contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } } impl ToIndentedString for ArrowFunction { diff --git a/core/ast/src/function/async_arrow_function.rs b/core/ast/src/function/async_arrow_function.rs index a6a360dc676..a800db5bc4a 100644 --- a/core/ast/src/function/async_arrow_function.rs +++ b/core/ast/src/function/async_arrow_function.rs @@ -86,6 +86,13 @@ impl AsyncArrowFunction { pub const fn scopes(&self) -> &FunctionScopes { &self.scopes } + + /// Returns `true` if the function declaration contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } } impl ToIndentedString for AsyncArrowFunction { diff --git a/core/ast/src/function/async_function.rs b/core/ast/src/function/async_function.rs index f80be21962c..401a1694a02 100644 --- a/core/ast/src/function/async_function.rs +++ b/core/ast/src/function/async_function.rs @@ -78,6 +78,13 @@ impl AsyncFunctionDeclaration { pub const fn scopes(&self) -> &FunctionScopes { &self.scopes } + + /// Returns `true` if the async function declaration contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } } impl ToIndentedString for AsyncFunctionDeclaration { @@ -207,6 +214,13 @@ impl AsyncFunctionExpression { pub const fn scopes(&self) -> &FunctionScopes { &self.scopes } + + /// Returns `true` if the async function expression contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } } impl ToIndentedString for AsyncFunctionExpression { diff --git a/core/ast/src/function/async_generator.rs b/core/ast/src/function/async_generator.rs index b7a046e5fc5..72edcb1ef3c 100644 --- a/core/ast/src/function/async_generator.rs +++ b/core/ast/src/function/async_generator.rs @@ -77,6 +77,13 @@ impl AsyncGeneratorDeclaration { pub const fn scopes(&self) -> &FunctionScopes { &self.scopes } + + /// Returns `true` if the async generator declaration contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } } impl ToIndentedString for AsyncGeneratorDeclaration { @@ -206,6 +213,13 @@ impl AsyncGeneratorExpression { pub const fn scopes(&self) -> &FunctionScopes { &self.scopes } + + /// Returns `true` if the async generator expression contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } } impl ToIndentedString for AsyncGeneratorExpression { diff --git a/core/ast/src/function/class.rs b/core/ast/src/function/class.rs index 162b1b6933e..ad5a45d0664 100644 --- a/core/ast/src/function/class.rs +++ b/core/ast/src/function/class.rs @@ -748,6 +748,13 @@ impl ClassMethodDefinition { pub const fn scopes(&self) -> &FunctionScopes { &self.scopes } + + /// Returns `true` if the class method definition contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } } impl ToIndentedString for ClassMethodDefinition { diff --git a/core/ast/src/function/generator.rs b/core/ast/src/function/generator.rs index 7911b922a6d..5ddb3a876d9 100644 --- a/core/ast/src/function/generator.rs +++ b/core/ast/src/function/generator.rs @@ -76,6 +76,13 @@ impl GeneratorDeclaration { pub const fn scopes(&self) -> &FunctionScopes { &self.scopes } + + /// Returns `true` if the generator declaration contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } } impl ToIndentedString for GeneratorDeclaration { @@ -205,6 +212,13 @@ impl GeneratorExpression { pub const fn scopes(&self) -> &FunctionScopes { &self.scopes } + + /// Returns `true` if the generator expression contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } } impl ToIndentedString for GeneratorExpression { diff --git a/core/ast/src/function/ordinary_function.rs b/core/ast/src/function/ordinary_function.rs index 84f8c4c6e37..bd4c456a153 100644 --- a/core/ast/src/function/ordinary_function.rs +++ b/core/ast/src/function/ordinary_function.rs @@ -77,6 +77,13 @@ impl FunctionDeclaration { pub const fn scopes(&self) -> &FunctionScopes { &self.scopes } + + /// Returns `true` if the function declaration contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } } impl ToIndentedString for FunctionDeclaration { @@ -207,6 +214,13 @@ impl FunctionExpression { &self.scopes } + /// Returns `true` if the function expression contains a direct call to `eval`. + #[inline] + #[must_use] + pub const fn contains_direct_eval(&self) -> bool { + self.contains_direct_eval + } + /// Analyze the scope of the function expression. pub fn analyze_scope(&mut self, strict: bool, scope: &Scope, interner: &Interner) -> bool { if !collect_bindings(self, strict, false, scope, interner) { diff --git a/core/ast/src/scope.rs b/core/ast/src/scope.rs index 621189ee79f..08481649955 100644 --- a/core/ast/src/scope.rs +++ b/core/ast/src/scope.rs @@ -3,12 +3,16 @@ //! Scopes are used to track the bindings of identifiers in the AST. use boa_string::JsString; -use rustc_hash::FxHashMap; -use std::{cell::RefCell, fmt::Debug, rc::Rc}; +use std::{ + cell::{Cell, RefCell}, + fmt::Debug, + rc::Rc, +}; #[derive(Clone, Debug, PartialEq)] #[allow(clippy::struct_excessive_bools)] struct Binding { + name: JsString, index: u32, mutable: bool, lex: bool, @@ -50,9 +54,10 @@ impl<'a> arbitrary::Arbitrary<'a> for Scope { #[derive(Debug, PartialEq)] pub(crate) struct Inner { + unique_id: u32, outer: Option, - index: u32, - bindings: RefCell>, + index: Cell, + bindings: RefCell>, function: bool, } @@ -62,8 +67,9 @@ impl Scope { pub fn new_global() -> Self { Self { inner: Rc::new(Inner { + unique_id: 0, outer: None, - index: 0, + index: Cell::default(), bindings: RefCell::default(), function: true, }), @@ -73,20 +79,32 @@ impl Scope { /// Creates a new scope. #[must_use] pub fn new(parent: Self, function: bool) -> Self { - let index = parent.inner.index + 1; + let index = parent.inner.index.get() + 1; Self { inner: Rc::new(Inner { + unique_id: index, outer: Some(parent), - index, + index: Cell::new(index), bindings: RefCell::default(), function, }), } } + /// Checks if the scope has only local bindings. + #[must_use] + pub fn all_bindings_local(&self) -> bool { + // if self.inner.function && self.inn + self.inner + .bindings + .borrow() + .iter() + .all(|binding| !binding.escapes) + } + /// Marks all bindings in this scope as escaping. pub fn escape_all_bindings(&self) { - for binding in self.inner.bindings.borrow_mut().values_mut() { + for binding in self.inner.bindings.borrow_mut().iter_mut() { binding.escapes = true; } } @@ -97,23 +115,29 @@ impl Scope { self.inner .bindings .borrow() - .get(name) + .iter() + .find(|b| &b.name == name) .map_or(false, |binding| binding.lex) } /// Check if the scope has a binding with the given name. #[must_use] pub fn has_binding(&self, name: &JsString) -> bool { - self.inner.bindings.borrow().contains_key(name) + self.inner.bindings.borrow().iter().any(|b| &b.name == name) } /// Get the binding locator for a binding with the given name. /// Fall back to the global scope if the binding is not found. #[must_use] pub fn get_identifier_reference(&self, name: JsString) -> IdentifierReference { - if let Some(binding) = self.inner.bindings.borrow().get(&name) { + if let Some(binding) = self.inner.bindings.borrow().iter().find(|b| b.name == name) { IdentifierReference::new( - BindingLocator::declarative(name, self.inner.index, binding.index), + BindingLocator::declarative( + name, + self.inner.index.get(), + binding.index, + self.inner.unique_id, + ), binding.lex, binding.escapes, ) @@ -131,10 +155,41 @@ impl Scope { self.inner.bindings.borrow().len() as u32 } + /// Returns the number of bindings in this scope that are not local. + #[must_use] + #[allow(clippy::cast_possible_truncation)] + pub fn num_bindings_non_local(&self) -> u32 { + self.inner + .bindings + .borrow() + .iter() + .filter(|binding| binding.escapes) + .count() as u32 + } + + /// Adjust the binding indices to exclude local bindings. + pub(crate) fn reorder_binding_indices(&self) { + let mut bindings = self.inner.bindings.borrow_mut(); + let mut index = 0; + for binding in bindings.iter_mut() { + if !binding.escapes { + binding.index = 0; + continue; + } + binding.index = index; + index += 1; + } + } + /// Returns the index of this scope. #[must_use] pub fn scope_index(&self) -> u32 { - self.inner.index + self.inner.index.get() + } + + /// Set the index of this scope. + pub(crate) fn set_index(&self, index: u32) { + self.inner.index.set(index); } /// Check if the scope is a function scope. @@ -152,21 +207,41 @@ impl Scope { /// Get the locator for a binding name. #[must_use] pub fn get_binding(&self, name: &JsString) -> Option { - self.inner.bindings.borrow().get(name).map(|binding| { - BindingLocator::declarative(name.clone(), self.inner.index, binding.index) - }) + self.inner + .bindings + .borrow() + .iter() + .find(|b| &b.name == name) + .map(|binding| { + BindingLocator::declarative( + name.clone(), + self.inner.index.get(), + binding.index, + self.inner.unique_id, + ) + }) } /// Get the locator for a binding name. #[must_use] pub fn get_binding_reference(&self, name: &JsString) -> Option { - self.inner.bindings.borrow().get(name).map(|binding| { - IdentifierReference::new( - BindingLocator::declarative(name.clone(), self.inner.index, binding.index), - binding.lex, - binding.escapes, - ) - }) + self.inner + .bindings + .borrow() + .iter() + .find(|b| &b.name == name) + .map(|binding| { + IdentifierReference::new( + BindingLocator::declarative( + name.clone(), + self.inner.index.get(), + binding.index, + self.inner.unique_id, + ), + binding.lex, + binding.escapes, + ) + }) } /// Simulate a binding access. @@ -177,7 +252,13 @@ impl Scope { let mut crossed_function_border = false; let mut current = self; loop { - if let Some(binding) = current.inner.bindings.borrow_mut().get_mut(name) { + if let Some(binding) = current + .inner + .bindings + .borrow_mut() + .iter_mut() + .find(|b| &b.name == name) + { if crossed_function_border || eval_or_with { binding.escapes = true; } @@ -198,34 +279,48 @@ impl Scope { #[must_use] #[allow(clippy::cast_possible_truncation)] pub fn create_mutable_binding(&self, name: JsString, function_scope: bool) -> BindingLocator { - let binding_index = self.inner.bindings.borrow().len() as u32; - self.inner.bindings.borrow_mut().insert( - name.clone(), - Binding { - index: binding_index, - mutable: true, - lex: !function_scope, - strict: false, - escapes: self.is_global(), - }, - ); - BindingLocator::declarative(name, self.inner.index, binding_index) + let mut bindings = self.inner.bindings.borrow_mut(); + let binding_index = bindings.len() as u32; + if let Some(binding) = bindings.iter().find(|b| b.name == name) { + return BindingLocator::declarative( + name, + self.inner.index.get(), + binding.index, + self.inner.unique_id, + ); + } + bindings.push(Binding { + name: name.clone(), + index: binding_index, + mutable: true, + lex: !function_scope, + strict: false, + escapes: self.is_global(), + }); + BindingLocator::declarative( + name, + self.inner.index.get(), + binding_index, + self.inner.unique_id, + ) } /// Crate an immutable binding. #[allow(clippy::cast_possible_truncation)] pub(crate) fn create_immutable_binding(&self, name: JsString, strict: bool) { - let binding_index = self.inner.bindings.borrow().len() as u32; - self.inner.bindings.borrow_mut().insert( + let mut bindings = self.inner.bindings.borrow_mut(); + if bindings.iter().any(|b| b.name == name) { + return; + } + let binding_index = bindings.len() as u32; + bindings.push(Binding { name, - Binding { - index: binding_index, - mutable: false, - lex: true, - strict, - escapes: self.is_global(), - }, - ); + index: binding_index, + mutable: false, + lex: true, + strict, + escapes: self.is_global(), + }); } /// Return the binding locator for a mutable binding. @@ -236,25 +331,34 @@ impl Scope { &self, name: JsString, ) -> Result { - Ok(match self.inner.bindings.borrow().get(&name) { - Some(binding) if binding.mutable => IdentifierReference::new( - BindingLocator::declarative(name, self.inner.index, binding.index), - binding.lex, - binding.escapes, - ), - Some(binding) if binding.strict => return Err(BindingLocatorError::MutateImmutable), - Some(_) => return Err(BindingLocatorError::Silent), - None => self.inner.outer.as_ref().map_or_else( - || { - Ok(IdentifierReference::new( - BindingLocator::global(name.clone()), - false, - true, - )) - }, - |outer| outer.set_mutable_binding(name.clone()), - )?, - }) + Ok( + match self.inner.bindings.borrow().iter().find(|b| b.name == name) { + Some(binding) if binding.mutable => IdentifierReference::new( + BindingLocator::declarative( + name, + self.inner.index.get(), + binding.index, + self.inner.unique_id, + ), + binding.lex, + binding.escapes, + ), + Some(binding) if binding.strict => { + return Err(BindingLocatorError::MutateImmutable) + } + Some(_) => return Err(BindingLocatorError::Silent), + None => self.inner.outer.as_ref().map_or_else( + || { + Ok(IdentifierReference::new( + BindingLocator::global(name.clone()), + false, + true, + )) + }, + |outer| outer.set_mutable_binding(name.clone()), + )?, + }, + ) } #[cfg(feature = "annex-b")] @@ -279,25 +383,34 @@ impl Scope { ); } - Ok(match self.inner.bindings.borrow().get(&name) { - Some(binding) if binding.mutable => IdentifierReference::new( - BindingLocator::declarative(name, self.inner.index, binding.index), - binding.lex, - binding.escapes, - ), - Some(binding) if binding.strict => return Err(BindingLocatorError::MutateImmutable), - Some(_) => return Err(BindingLocatorError::Silent), - None => self.inner.outer.as_ref().map_or_else( - || { - Ok(IdentifierReference::new( - BindingLocator::global(name.clone()), - false, - true, - )) - }, - |outer| outer.set_mutable_binding_var(name.clone()), - )?, - }) + Ok( + match self.inner.bindings.borrow().iter().find(|b| b.name == name) { + Some(binding) if binding.mutable => IdentifierReference::new( + BindingLocator::declarative( + name, + self.inner.index.get(), + binding.index, + self.inner.unique_id, + ), + binding.lex, + binding.escapes, + ), + Some(binding) if binding.strict => { + return Err(BindingLocatorError::MutateImmutable) + } + Some(_) => return Err(BindingLocatorError::Silent), + None => self.inner.outer.as_ref().map_or_else( + || { + Ok(IdentifierReference::new( + BindingLocator::global(name.clone()), + false, + true, + )) + }, + |outer| outer.set_mutable_binding_var(name.clone()), + )?, + }, + ) } /// Gets the outer scope of this scope. @@ -358,15 +471,23 @@ pub struct BindingLocator { /// Index of the binding in the scope. binding_index: u32, + + unique_scope_id: u32, } impl BindingLocator { /// Creates a new declarative binding locator that has knows indices. - pub(crate) const fn declarative(name: JsString, scope_index: u32, binding_index: u32) -> Self { + pub(crate) const fn declarative( + name: JsString, + scope_index: u32, + binding_index: u32, + unique_scope_id: u32, + ) -> Self { Self { name, scope: scope_index + 1, binding_index, + unique_scope_id, } } @@ -376,6 +497,7 @@ impl BindingLocator { name, scope: 0, binding_index: 0, + unique_scope_id: 0, } } @@ -480,7 +602,8 @@ impl FunctionScopes { } /// Returns the effective paramter scope for this function. - pub(crate) fn parameter_scope(&self) -> Scope { + #[must_use] + pub fn parameter_scope(&self) -> Scope { if let Some(parameters_eval_scope) = &self.parameters_eval_scope { return parameters_eval_scope.clone(); } @@ -514,6 +637,19 @@ impl FunctionScopes { lexical_scope.escape_all_bindings(); } } + + pub(crate) fn reorder_binding_indices(&self) { + self.function_scope.reorder_binding_indices(); + if let Some(parameters_eval_scope) = &self.parameters_eval_scope { + parameters_eval_scope.reorder_binding_indices(); + } + if let Some(parameters_scope) = &self.parameters_scope { + parameters_scope.reorder_binding_indices(); + } + if let Some(lexical_scope) = &self.lexical_scope { + lexical_scope.reorder_binding_indices(); + } + } } #[cfg(feature = "arbitrary")] diff --git a/core/ast/src/scope_analyzer.rs b/core/ast/src/scope_analyzer.rs index e40534e67a6..b81b0cbcc2d 100644 --- a/core/ast/src/scope_analyzer.rs +++ b/core/ast/src/scope_analyzer.rs @@ -16,8 +16,9 @@ use crate::{ FunctionExpression, GeneratorDeclaration, GeneratorExpression, }, operations::{ - bound_names, lexically_declared_names, lexically_scoped_declarations, var_declared_names, - var_scoped_declarations, LexicallyScopedDeclaration, VarScopedDeclaration, + bound_names, contains, lexically_declared_names, lexically_scoped_declarations, + var_declared_names, var_scoped_declarations, ContainsSymbol, LexicallyScopedDeclaration, + VarScopedDeclaration, }, property::PropertyName, scope::{FunctionScopes, IdentifierReference, Scope}, @@ -104,6 +105,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { try_break!(self.visit_statement_list_mut(&mut node.statements)); if let Some(scope) = &mut node.scope { std::mem::swap(&mut self.scope, scope); + scope.reorder_binding_indices(); } self.direct_eval = direct_eval_old; ControlFlow::Continue(()) @@ -124,6 +126,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { } if let Some(scope) = &mut node.scope { std::mem::swap(&mut self.scope, scope); + scope.reorder_binding_indices(); } self.direct_eval = direct_eval_old; ControlFlow::Continue(()) @@ -139,6 +142,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { std::mem::swap(&mut self.scope, &mut node.scope); try_break!(self.visit_statement_mut(&mut node.statement)); std::mem::swap(&mut self.scope, &mut node.scope); + node.scope.reorder_binding_indices(); self.with = with; ControlFlow::Continue(()) } @@ -155,6 +159,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { } try_break!(self.visit_block_mut(&mut node.block)); std::mem::swap(&mut self.scope, &mut node.scope); + node.scope.reorder_binding_indices(); self.direct_eval = direct_eval_old; ControlFlow::Continue(()) } @@ -180,6 +185,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { try_break!(self.visit_statement_mut(&mut node.inner.body)); if let Some(ForLoopInitializer::Lexical(decl)) = &mut node.inner.init { std::mem::swap(&mut self.scope, &mut decl.scope); + decl.scope.reorder_binding_indices(); } self.direct_eval = direct_eval_old; ControlFlow::Continue(()) @@ -198,6 +204,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { if let Some(scope) = &mut node.target_scope { self.direct_eval = direct_eval_old; std::mem::swap(&mut self.scope, scope); + scope.reorder_binding_indices(); } if let Some(scope) = &mut node.scope { self.direct_eval = node.contains_direct_eval || self.direct_eval; @@ -210,6 +217,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { try_break!(self.visit_statement_mut(&mut node.body)); if let Some(scope) = &mut node.scope { std::mem::swap(&mut self.scope, scope); + scope.reorder_binding_indices(); } self.direct_eval = direct_eval_old; ControlFlow::Continue(()) @@ -228,6 +236,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { if let Some(scope) = &mut node.iterable_scope { self.direct_eval = direct_eval_old; std::mem::swap(&mut self.scope, scope); + scope.reorder_binding_indices(); } if let Some(scope) = &mut node.scope { self.direct_eval = node.contains_direct_eval || self.direct_eval; @@ -240,6 +249,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { try_break!(self.visit_statement_mut(&mut node.body)); if let Some(scope) = &mut node.scope { std::mem::swap(&mut self.scope, scope); + scope.reorder_binding_indices(); } self.direct_eval = direct_eval_old; ControlFlow::Continue(()) @@ -252,7 +262,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, ) } @@ -264,7 +274,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, ) } @@ -276,7 +286,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, ) } @@ -288,7 +298,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, ) } @@ -300,7 +310,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, ) } @@ -312,7 +322,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, ) } @@ -324,7 +334,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, ) } @@ -336,7 +346,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, ) } @@ -348,7 +358,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, ) } @@ -360,7 +370,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, ) } @@ -381,6 +391,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { try_break!(self.visit_class_element_mut(element)); } std::mem::swap(&mut self.scope, &mut node.name_scope); + node.name_scope.reorder_binding_indices(); ControlFlow::Continue(()) } @@ -406,6 +417,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { } if let Some(name_scope) = &mut node.name_scope { std::mem::swap(&mut self.scope, name_scope); + name_scope.reorder_binding_indices(); } ControlFlow::Continue(()) } @@ -414,15 +426,41 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { &mut self, node: &'ast mut ClassElement, ) -> ControlFlow { - if let ClassElement::MethodDefinition(node) = node { - self.visit_function_like( + match node { + ClassElement::MethodDefinition(node) => self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, - ) - } else { - ControlFlow::Continue(()) + ), + ClassElement::FieldDefinition(field) | ClassElement::StaticFieldDefinition(field) => { + try_break!(self.visit_property_name_mut(&mut field.name)); + if let Some(e) = &mut field.field { + try_break!(self.visit_expression_mut(e)); + } + ControlFlow::Continue(()) + } + ClassElement::PrivateFieldDefinition(field) => { + if let Some(e) = &mut field.field { + try_break!(self.visit_expression_mut(e)); + } + ControlFlow::Continue(()) + } + ClassElement::PrivateStaticFieldDefinition(_, e) => { + if let Some(e) = e { + try_break!(self.visit_expression_mut(e)); + } + ControlFlow::Continue(()) + } + ClassElement::StaticBlock(node) => { + let contains_direct_eval = contains(node.statements(), ContainsSymbol::DirectEval); + self.visit_function_like( + &mut FormalParameterList::default(), + &mut node.body, + &mut node.scopes, + contains_direct_eval, + ) + } } } @@ -434,7 +472,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { self.visit_function_like( &mut node.parameters, &mut node.body, - &node.scopes, + &mut node.scopes, node.contains_direct_eval, ) } @@ -484,6 +522,7 @@ impl<'ast> VisitorMut<'ast> for BindingEscapeAnalyzer<'_> { std::mem::swap(&mut self.scope, &mut scope); try_break!(self.visit_module_item_list_mut(&mut node.items)); std::mem::swap(&mut self.scope, &mut scope); + scope.reorder_binding_indices(); ControlFlow::Continue(()) } } @@ -493,7 +532,7 @@ impl BindingEscapeAnalyzer<'_> { &mut self, parameters: &mut FormalParameterList, body: &mut FunctionBody, - scopes: &FunctionScopes, + scopes: &mut FunctionScopes, contains_direct_eval: bool, ) -> ControlFlow<&'static str> { let direct_eval_old = self.direct_eval; @@ -509,6 +548,7 @@ impl BindingEscapeAnalyzer<'_> { std::mem::swap(&mut self.scope, &mut scope); try_break!(self.visit_function_body_mut(body)); std::mem::swap(&mut self.scope, &mut scope); + scopes.reorder_binding_indices(); self.direct_eval = direct_eval_old; ControlFlow::Continue(()) } @@ -1149,6 +1189,470 @@ impl BindingCollectorVisitor<'_> { } } +/// Optimize scope indicies when scopes only contain local bindings. +pub(crate) fn optimize_scope_indicies<'a, N>(node: &'a mut N, scope: &Scope) +where + &'a mut N: Into>, +{ + let mut visitor = ScopeIndexVisitor { + index: scope.scope_index(), + }; + visitor.visit(node.into()); +} + +struct ScopeIndexVisitor { + index: u32, +} + +impl<'ast> VisitorMut<'ast> for ScopeIndexVisitor { + type BreakTy = (); + + fn visit_function_declaration_mut( + &mut self, + node: &'ast mut FunctionDeclaration, + ) -> ControlFlow { + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut None, + false, + contains_direct_eval, + ) + } + + fn visit_generator_declaration_mut( + &mut self, + node: &'ast mut GeneratorDeclaration, + ) -> ControlFlow { + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut None, + false, + contains_direct_eval, + ) + } + + fn visit_async_function_declaration_mut( + &mut self, + node: &'ast mut AsyncFunctionDeclaration, + ) -> ControlFlow { + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut None, + false, + contains_direct_eval, + ) + } + + fn visit_async_generator_declaration_mut( + &mut self, + node: &'ast mut AsyncGeneratorDeclaration, + ) -> ControlFlow { + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut None, + false, + contains_direct_eval, + ) + } + + fn visit_function_expression_mut( + &mut self, + node: &'ast mut FunctionExpression, + ) -> ControlFlow { + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut node.name_scope, + false, + contains_direct_eval, + ) + } + + fn visit_generator_expression_mut( + &mut self, + node: &'ast mut GeneratorExpression, + ) -> ControlFlow { + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut node.name_scope, + false, + contains_direct_eval, + ) + } + + fn visit_async_function_expression_mut( + &mut self, + node: &'ast mut AsyncFunctionExpression, + ) -> ControlFlow { + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut node.name_scope, + false, + contains_direct_eval, + ) + } + + fn visit_async_generator_expression_mut( + &mut self, + node: &'ast mut AsyncGeneratorExpression, + ) -> ControlFlow { + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut node.name_scope, + false, + contains_direct_eval, + ) + } + + fn visit_arrow_function_mut( + &mut self, + node: &'ast mut ArrowFunction, + ) -> ControlFlow { + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut None, + true, + contains_direct_eval, + ) + } + + fn visit_async_arrow_function_mut( + &mut self, + node: &'ast mut AsyncArrowFunction, + ) -> ControlFlow { + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut None, + true, + contains_direct_eval, + ) + } + + fn visit_class_declaration_mut( + &mut self, + node: &'ast mut ClassDeclaration, + ) -> ControlFlow { + let index = self.index; + if !node.name_scope.all_bindings_local() { + self.index += 1; + } + node.name_scope.set_index(self.index); + if let Some(super_ref) = &mut node.super_ref { + try_break!(self.visit_expression_mut(super_ref)); + } + if let Some(constructor) = &mut node.constructor { + try_break!(self.visit_function_expression_mut(constructor)); + } + for element in &mut *node.elements { + try_break!(self.visit_class_element_mut(element)); + } + self.index = index; + ControlFlow::Continue(()) + } + + fn visit_class_expression_mut( + &mut self, + node: &'ast mut ClassExpression, + ) -> ControlFlow { + let index = self.index; + if let Some(scope) = &node.name_scope { + if !scope.all_bindings_local() { + self.index += 1; + } + scope.set_index(self.index); + } + if let Some(super_ref) = &mut node.super_ref { + try_break!(self.visit_expression_mut(super_ref)); + } + if let Some(constructor) = &mut node.constructor { + try_break!(self.visit_function_expression_mut(constructor)); + } + for element in &mut *node.elements { + try_break!(self.visit_class_element_mut(element)); + } + self.index = index; + ControlFlow::Continue(()) + } + + fn visit_class_element_mut( + &mut self, + node: &'ast mut ClassElement, + ) -> ControlFlow { + match node { + ClassElement::MethodDefinition(node) => { + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut None, + false, + contains_direct_eval, + ) + } + ClassElement::FieldDefinition(field) | ClassElement::StaticFieldDefinition(field) => { + try_break!(self.visit_property_name_mut(&mut field.name)); + let index = self.index; + self.index += 1; + field.scope.set_index(self.index); + if let Some(e) = &mut field.field { + try_break!(self.visit_expression_mut(e)); + } + self.index = index; + ControlFlow::Continue(()) + } + ClassElement::PrivateFieldDefinition(field) => { + let index = self.index; + self.index += 1; + field.scope.set_index(self.index); + if let Some(e) = &mut field.field { + try_break!(self.visit_expression_mut(e)); + } + self.index = index; + ControlFlow::Continue(()) + } + ClassElement::PrivateStaticFieldDefinition(_, e) => { + if let Some(e) = e { + try_break!(self.visit_expression_mut(e)); + } + ControlFlow::Continue(()) + } + ClassElement::StaticBlock(node) => { + let contains_direct_eval = contains(node.statements(), ContainsSymbol::DirectEval); + self.visit_function_like( + &mut node.body, + &mut FormalParameterList::default(), + &mut node.scopes, + &mut None, + false, + contains_direct_eval, + ) + } + } + } + + fn visit_object_method_definition_mut( + &mut self, + node: &'ast mut ObjectMethodDefinition, + ) -> ControlFlow { + match &mut node.name { + PropertyName::Literal(_) => {} + PropertyName::Computed(name) => { + try_break!(self.visit_expression_mut(name)); + } + } + let contains_direct_eval = node.contains_direct_eval(); + self.visit_function_like( + &mut node.body, + &mut node.parameters, + &mut node.scopes, + &mut None, + false, + contains_direct_eval, + ) + } + + fn visit_block_mut(&mut self, node: &'ast mut Block) -> ControlFlow { + let index = self.index; + if let Some(scope) = &node.scope { + if !scope.all_bindings_local() { + self.index += 1; + } + scope.set_index(self.index); + } + try_break!(self.visit_statement_list_mut(&mut node.statements)); + self.index = index; + ControlFlow::Continue(()) + } + + fn visit_switch_mut(&mut self, node: &'ast mut Switch) -> ControlFlow { + let index = self.index; + try_break!(self.visit_expression_mut(&mut node.val)); + if let Some(scope) = &node.scope { + if !scope.all_bindings_local() { + self.index += 1; + } + scope.set_index(self.index); + } + for case in &mut *node.cases { + try_break!(self.visit_case_mut(case)); + } + self.index = index; + ControlFlow::Continue(()) + } + + fn visit_with_mut(&mut self, node: &'ast mut With) -> ControlFlow { + let index = self.index; + try_break!(self.visit_expression_mut(&mut node.expression)); + self.index += 1; + node.scope.set_index(self.index); + try_break!(self.visit_statement_mut(&mut node.statement)); + self.index = index; + ControlFlow::Continue(()) + } + + fn visit_catch_mut(&mut self, node: &'ast mut Catch) -> ControlFlow { + let index = self.index; + if !node.scope.all_bindings_local() { + self.index += 1; + } + node.scope.set_index(self.index); + if let Some(binding) = &mut node.parameter { + try_break!(self.visit_binding_mut(binding)); + } + try_break!(self.visit_block_mut(&mut node.block)); + self.index = index; + ControlFlow::Continue(()) + } + + fn visit_for_loop_mut(&mut self, node: &'ast mut ForLoop) -> ControlFlow { + let index = self.index; + if let Some(ForLoopInitializer::Lexical(decl)) = &mut node.inner.init { + if !decl.scope.all_bindings_local() { + self.index += 1; + } + decl.scope.set_index(self.index); + } + if let Some(fli) = &mut node.inner.init { + try_break!(self.visit_for_loop_initializer_mut(fli)); + } + if let Some(expr) = &mut node.inner.condition { + try_break!(self.visit_expression_mut(expr)); + } + if let Some(expr) = &mut node.inner.final_expr { + try_break!(self.visit_expression_mut(expr)); + } + self.visit_statement_mut(&mut node.inner.body); + self.index = index; + ControlFlow::Continue(()) + } + + fn visit_for_in_loop_mut(&mut self, node: &'ast mut ForInLoop) -> ControlFlow { + { + let index = self.index; + if let Some(scope) = &node.target_scope { + if !scope.all_bindings_local() { + self.index += 1; + } + scope.set_index(self.index); + } + try_break!(self.visit_expression_mut(&mut node.target)); + self.index = index; + } + let index = self.index; + if let Some(scope) = &node.scope { + if !scope.all_bindings_local() { + self.index += 1; + } + scope.set_index(self.index); + } + try_break!(self.visit_iterable_loop_initializer_mut(&mut node.initializer)); + try_break!(self.visit_statement_mut(&mut node.body)); + self.index = index; + ControlFlow::Continue(()) + } + + fn visit_for_of_loop_mut(&mut self, node: &'ast mut ForOfLoop) -> ControlFlow { + { + let index = self.index; + if let Some(scope) = &node.iterable_scope { + if !scope.all_bindings_local() { + self.index += 1; + } + scope.set_index(self.index); + } + try_break!(self.visit_expression_mut(&mut node.iterable)); + self.index = index; + } + let index = self.index; + if let Some(scope) = &node.scope { + if !scope.all_bindings_local() { + self.index += 1; + } + scope.set_index(self.index); + } + try_break!(self.visit_iterable_loop_initializer_mut(&mut node.init)); + try_break!(self.visit_statement_mut(&mut node.body)); + self.index = index; + ControlFlow::Continue(()) + } +} + +impl ScopeIndexVisitor { + fn visit_function_like( + &mut self, + body: &mut FunctionBody, + parameters: &mut FormalParameterList, + scopes: &mut FunctionScopes, + name_scope: &mut Option, + arrow: bool, + contains_direct_eval: bool, + ) -> ControlFlow<()> { + let index = self.index; + if let Some(scope) = name_scope { + if !scope.all_bindings_local() { + self.index += 1; + } + scope.set_index(self.index); + } + if !(arrow && scopes.function_scope.all_bindings_local() && !contains_direct_eval) { + self.index += 1; + } + scopes.function_scope.set_index(self.index); + if let Some(scope) = &scopes.parameters_eval_scope { + if !scope.all_bindings_local() { + self.index += 1; + } + scope.set_index(self.index); + } + try_break!(self.visit_formal_parameter_list_mut(parameters)); + if let Some(scope) = &scopes.parameters_scope { + if !scope.all_bindings_local() { + self.index += 1; + } + scope.set_index(self.index); + } + if let Some(scope) = &scopes.lexical_scope { + if !scope.all_bindings_local() { + self.index += 1; + } + scope.set_index(self.index); + } + try_break!(self.visit_function_body_mut(body)); + self.index = index; + ControlFlow::Continue(()) + } +} + /// `GlobalDeclarationInstantiation ( script, env )` /// /// More information: diff --git a/core/ast/src/source.rs b/core/ast/src/source.rs index f1220a76213..e7a4ad1633b 100644 --- a/core/ast/src/source.rs +++ b/core/ast/src/source.rs @@ -7,7 +7,7 @@ use crate::{ scope::Scope, scope_analyzer::{ analyze_binding_escapes, collect_bindings, eval_declaration_instantiation_scope, - EvalDeclarationBindings, + optimize_scope_indicies, EvalDeclarationBindings, }, visitor::{VisitWith, Visitor, VisitorMut}, ModuleItemList, StatementList, @@ -56,7 +56,11 @@ impl Script { if !collect_bindings(self, self.strict(), false, scope, interner) { return false; } - analyze_binding_escapes(self, false, scope.clone(), interner) + if !analyze_binding_escapes(self, false, scope.clone(), interner) { + return false; + } + optimize_scope_indicies(self, scope); + true } /// Analyze the scope of the script in eval mode. @@ -84,10 +88,14 @@ impl Script { if !collect_bindings(self, strict, true, lexical_scope, interner) { return Err(String::from("Failed to analyze scope")); } - if !analyze_binding_escapes(self, true, lexical_scope.clone(), interner) { return Err(String::from("Failed to analyze scope")); } + variable_scope.escape_all_bindings(); + lexical_scope.escape_all_bindings(); + variable_scope.reorder_binding_indices(); + lexical_scope.reorder_binding_indices(); + optimize_scope_indicies(self, lexical_scope); Ok(bindings) } @@ -158,7 +166,11 @@ impl Module { if !collect_bindings(self, true, false, scope, interner) { return false; } - analyze_binding_escapes(self, false, scope.clone(), interner) + if !analyze_binding_escapes(self, false, scope.clone(), interner) { + return false; + } + optimize_scope_indicies(self, &self.scope.clone()); + true } } diff --git a/core/engine/src/builtins/function/arguments.rs b/core/engine/src/builtins/function/arguments.rs index 83fd821287e..01f28a23ffd 100644 --- a/core/engine/src/builtins/function/arguments.rs +++ b/core/engine/src/builtins/function/arguments.rs @@ -1,4 +1,5 @@ use crate::{ + bytecompiler::ToJsString, environments::DeclarativeEnvironment, object::{ internal_methods::{ @@ -11,8 +12,9 @@ use crate::{ property::{DescriptorKind, PropertyDescriptor, PropertyKey}, Context, JsData, JsResult, JsValue, }; -use boa_ast::{function::FormalParameterList, operations::bound_names}; +use boa_ast::{function::FormalParameterList, operations::bound_names, scope::Scope}; use boa_gc::{Finalize, Gc, Trace}; +use boa_interner::Interner; use rustc_hash::FxHashMap; use thin_vec::{thin_vec, ThinVec}; @@ -141,7 +143,11 @@ impl MappedArguments { } impl MappedArguments { - pub(crate) fn binding_indices(formals: &FormalParameterList) -> ThinVec> { + pub(crate) fn binding_indices( + formals: &FormalParameterList, + scope: &Scope, + interner: &Interner, + ) -> ThinVec> { // Section 17-19 are done first, for easier object creation in 11. // // The section 17-19 differs from the spec, due to the way the runtime environments work. @@ -180,8 +186,10 @@ impl MappedArguments { let mut bindings = FxHashMap::default(); let mut property_index = 0; for name in bound_names(formals) { - // NOTE(HalidOdat): Offset by +1 to account for the first binding ("argument"). - let binding_index = bindings.len() as u32 + 1; + let binding_index = scope + .get_binding(&name.to_js_string(interner)) + .expect("binding must exist") + .binding_index(); let entry = bindings .entry(name) diff --git a/core/engine/src/builtins/function/mod.rs b/core/engine/src/builtins/function/mod.rs index bd662fe4d7c..94751379289 100644 --- a/core/engine/src/builtins/function/mod.rs +++ b/core/engine/src/builtins/function/mod.rs @@ -657,6 +657,7 @@ impl BuiltInFunctionObject { context.realm().scope().clone(), context.realm().scope().clone(), function.scopes(), + function.contains_direct_eval(), context.interner_mut(), ); @@ -1028,10 +1029,12 @@ pub(crate) fn function_call( last_env += 1; } - context.vm.environments.push_function( - code.constant_scope(last_env), - FunctionSlots::new(this, function_object.clone(), None), - ); + if code.has_function_scope() { + context.vm.environments.push_function( + code.constant_scope(last_env), + FunctionSlots::new(this, function_object.clone(), None), + ); + } Ok(CallValue::Ready) } diff --git a/core/engine/src/bytecompiler/class.rs b/core/engine/src/bytecompiler/class.rs index bfc69002291..1e360be4920 100644 --- a/core/engine/src/bytecompiler/class.rs +++ b/core/engine/src/bytecompiler/class.rs @@ -79,14 +79,7 @@ impl ByteCompiler<'_> { .map_or(Sym::EMPTY_STRING, Identifier::sym) .to_js_string(self.interner()); - let outer_scope = if let Some(name_scope) = class.name_scope { - let outer_scope = self.lexical_scope.clone(); - let scope_index = self.push_scope(name_scope); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); - Some(outer_scope) - } else { - None - }; + let outer_scope = self.push_declarative_scope(class.name_scope); let mut compiler = ByteCompiler::new( class_name.clone(), @@ -103,10 +96,12 @@ impl ByteCompiler<'_> { compiler.code_block_flags |= CodeBlockFlags::IS_CLASS_CONSTRUCTOR; if let Some(expr) = &class.constructor { + compiler.code_block_flags |= CodeBlockFlags::HAS_FUNCTION_SCOPE; let _ = compiler.push_scope(expr.scopes().function_scope()); compiler.length = expr.parameters().length(); compiler.params = expr.parameters().clone(); + compiler.parameter_scope = expr.scopes().parameter_scope(); compiler.function_declaration_instantiation( expr.body(), @@ -122,11 +117,13 @@ impl ByteCompiler<'_> { compiler.emit_opcode(Opcode::PushUndefined); } else if class.super_ref.is_some() { // We push an empty, unused function scope since the compiler expects a function scope. + compiler.code_block_flags |= CodeBlockFlags::HAS_FUNCTION_SCOPE; let _ = compiler.push_scope(&Scope::new(compiler.lexical_scope.clone(), true)); compiler.emit_opcode(Opcode::SuperCallDerived); compiler.emit_opcode(Opcode::BindThisValue); } else { // We push an empty, unused function scope since the compiler expects a function scope. + compiler.code_block_flags |= CodeBlockFlags::HAS_FUNCTION_SCOPE; let _ = compiler.push_scope(&Scope::new(compiler.lexical_scope.clone(), true)); compiler.emit_opcode(Opcode::PushUndefined); } @@ -181,9 +178,11 @@ impl ByteCompiler<'_> { let mut static_elements = Vec::new(); let mut static_field_name_count = 0; - if outer_scope.is_some() { + if let Some(scope) = class.name_scope { + let binding = scope.get_identifier_reference(class_name.clone()); + let index = self.get_or_insert_binding(binding); self.emit_opcode(Opcode::Dup); - self.emit_binding(BindingOpcode::InitLexical, class_name.clone()); + self.emit_binding_access(Opcode::PutLexicalValue, &index); } for element in class.elements { @@ -293,6 +292,7 @@ impl ByteCompiler<'_> { ); // Function environment + field_compiler.code_block_flags |= CodeBlockFlags::HAS_FUNCTION_SCOPE; let _ = field_compiler.push_scope(field.scope()); let is_anonymous_function = if let Some(node) = &field.field() { field_compiler.compile_expr(node, true); @@ -327,6 +327,7 @@ impl ByteCompiler<'_> { self.interner, self.in_with, ); + field_compiler.code_block_flags |= CodeBlockFlags::HAS_FUNCTION_SCOPE; let _ = field_compiler.push_scope(field.scope()); if let Some(node) = field.field() { field_compiler.compile_expr(node, true); @@ -368,6 +369,7 @@ impl ByteCompiler<'_> { self.interner, self.in_with, ); + field_compiler.code_block_flags |= CodeBlockFlags::HAS_FUNCTION_SCOPE; let _ = field_compiler.push_scope(field.scope()); let is_anonymous_function = if let Some(node) = &field.field() { field_compiler.compile_expr(node, true); @@ -411,6 +413,7 @@ impl ByteCompiler<'_> { self.interner, self.in_with, ); + compiler.code_block_flags |= CodeBlockFlags::HAS_FUNCTION_SCOPE; let _ = compiler.push_scope(block.scopes().function_scope()); compiler.function_declaration_instantiation( @@ -474,12 +477,7 @@ impl ByteCompiler<'_> { self.emit_opcode(Opcode::Swap); self.emit_opcode(Opcode::Pop); - if let Some(outer_scope) = outer_scope { - self.pop_scope(); - self.lexical_scope = outer_scope; - self.emit_opcode(Opcode::PopEnvironment); - } - + self.pop_declarative_scope(outer_scope); self.emit_opcode(Opcode::PopPrivateEnvironment); if !expression { diff --git a/core/engine/src/bytecompiler/declarations.rs b/core/engine/src/bytecompiler/declarations.rs index e040c99a5d7..06b91d9293c 100644 --- a/core/engine/src/bytecompiler/declarations.rs +++ b/core/engine/src/bytecompiler/declarations.rs @@ -479,41 +479,46 @@ impl ByteCompiler<'_> { // 16. For each Parse Node f of functionsToInitialize, do for function in functions_to_initialize { // a. Let fn be the sole element of the BoundNames of f. - let (name, generator, r#async, parameters, body, scopes) = match &function { - VarScopedDeclaration::FunctionDeclaration(f) => ( - f.name(), - false, - false, - f.parameters(), - f.body(), - f.scopes().clone(), - ), - VarScopedDeclaration::GeneratorDeclaration(f) => ( - f.name(), - true, - false, - f.parameters(), - f.body(), - f.scopes().clone(), - ), - VarScopedDeclaration::AsyncFunctionDeclaration(f) => ( - f.name(), - false, - true, - f.parameters(), - f.body(), - f.scopes().clone(), - ), - VarScopedDeclaration::AsyncGeneratorDeclaration(f) => ( - f.name(), - true, - true, - f.parameters(), - f.body(), - f.scopes().clone(), - ), - VarScopedDeclaration::VariableDeclaration(_) => continue, - }; + let (name, generator, r#async, parameters, body, scopes, contains_direct_eval) = + match &function { + VarScopedDeclaration::FunctionDeclaration(f) => ( + f.name(), + false, + false, + f.parameters(), + f.body(), + f.scopes().clone(), + f.contains_direct_eval(), + ), + VarScopedDeclaration::GeneratorDeclaration(f) => ( + f.name(), + true, + false, + f.parameters(), + f.body(), + f.scopes().clone(), + f.contains_direct_eval(), + ), + VarScopedDeclaration::AsyncFunctionDeclaration(f) => ( + f.name(), + false, + true, + f.parameters(), + f.body(), + f.scopes().clone(), + f.contains_direct_eval(), + ), + VarScopedDeclaration::AsyncGeneratorDeclaration(f) => ( + f.name(), + true, + true, + f.parameters(), + f.body(), + f.scopes().clone(), + f.contains_direct_eval(), + ), + VarScopedDeclaration::VariableDeclaration(_) => continue, + }; let code = FunctionCompiler::new() .name(name.sym().to_js_string(self.interner())) @@ -527,6 +532,7 @@ impl ByteCompiler<'_> { self.variable_scope.clone(), self.lexical_scope.clone(), &scopes, + contains_direct_eval, self.interner, ); @@ -737,43 +743,48 @@ impl ByteCompiler<'_> { // 17. For each Parse Node f of functionsToInitialize, do for function in functions_to_initialize { // a. Let fn be the sole element of the BoundNames of f. - let (name, generator, r#async, parameters, body, scopes) = match &function { - VarScopedDeclaration::FunctionDeclaration(f) => ( - f.name(), - false, - false, - f.parameters(), - f.body(), - f.scopes().clone(), - ), - VarScopedDeclaration::GeneratorDeclaration(f) => ( - f.name(), - true, - false, - f.parameters(), - f.body(), - f.scopes().clone(), - ), - VarScopedDeclaration::AsyncFunctionDeclaration(f) => ( - f.name(), - false, - true, - f.parameters(), - f.body(), - f.scopes().clone(), - ), - VarScopedDeclaration::AsyncGeneratorDeclaration(f) => ( - f.name(), - true, - true, - f.parameters(), - f.body(), - f.scopes().clone(), - ), - VarScopedDeclaration::VariableDeclaration(_) => { - continue; - } - }; + let (name, generator, r#async, parameters, body, scopes, contains_direct_eval) = + match &function { + VarScopedDeclaration::FunctionDeclaration(f) => ( + f.name(), + false, + false, + f.parameters(), + f.body(), + f.scopes().clone(), + f.contains_direct_eval(), + ), + VarScopedDeclaration::GeneratorDeclaration(f) => ( + f.name(), + true, + false, + f.parameters(), + f.body(), + f.scopes().clone(), + f.contains_direct_eval(), + ), + VarScopedDeclaration::AsyncFunctionDeclaration(f) => ( + f.name(), + false, + true, + f.parameters(), + f.body(), + f.scopes().clone(), + f.contains_direct_eval(), + ), + VarScopedDeclaration::AsyncGeneratorDeclaration(f) => ( + f.name(), + true, + true, + f.parameters(), + f.body(), + f.scopes().clone(), + f.contains_direct_eval(), + ), + VarScopedDeclaration::VariableDeclaration(_) => { + continue; + } + }; let code = FunctionCompiler::new() .name(name.sym().to_js_string(self.interner())) @@ -788,6 +799,7 @@ impl ByteCompiler<'_> { self.variable_scope.clone(), self.lexical_scope.clone(), &scopes, + contains_direct_eval, self.interner, ); @@ -963,10 +975,7 @@ impl ByteCompiler<'_> { } // 19-20 - if let Some(scope) = scopes.parameters_eval_scope() { - let scope_index = self.push_scope(scope); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); - } + drop(self.push_declarative_scope(scopes.parameters_eval_scope())); let scope = self.lexical_scope.clone(); @@ -1057,8 +1066,7 @@ impl ByteCompiler<'_> { // visibility of declarations in the function body. // b. Let varEnv be NewDeclarativeEnvironment(env). // c. Set the VariableEnvironment of calleeContext to varEnv. - let scope_index = self.push_scope(scope); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); + drop(self.push_declarative_scope(Some(scope))); let mut variable_scope = self.lexical_scope.clone(); @@ -1177,10 +1185,7 @@ impl ByteCompiler<'_> { } // 30-31 - if let Some(scope) = scopes.lexical_scope() { - let scope_index = self.push_scope(scope); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); - } + drop(self.push_declarative_scope(scopes.lexical_scope())); // 35. Let privateEnv be the PrivateEnvironment of calleeContext. // 36. For each Parse Node f of functionsToInitialize, do diff --git a/core/engine/src/bytecompiler/env.rs b/core/engine/src/bytecompiler/env.rs index 59b65184e35..1fddff71ac2 100644 --- a/core/engine/src/bytecompiler/env.rs +++ b/core/engine/src/bytecompiler/env.rs @@ -1,5 +1,7 @@ use boa_ast::scope::Scope; +use crate::vm::{Constant, Opcode}; + use super::ByteCompiler; impl ByteCompiler<'_> { @@ -9,8 +11,7 @@ impl ByteCompiler<'_> { self.current_open_environments_count += 1; let index = self.constants.len() as u32; - self.constants - .push(crate::vm::Constant::Scope(scope.clone())); + self.constants.push(Constant::Scope(scope.clone())); if scope.is_function() { self.variable_scope = scope.clone(); @@ -21,6 +22,33 @@ impl ByteCompiler<'_> { index } + /// Push a declarative scope. + /// + /// Returns the outer scope. + #[must_use] + pub(crate) fn push_declarative_scope(&mut self, scope: Option<&Scope>) -> Option { + let mut scope = scope?.clone(); + if !scope.all_bindings_local() { + self.current_open_environments_count += 1; + let index = self.constants.len() as u32; + self.constants.push(Constant::Scope(scope.clone())); + self.emit_with_varying_operand(Opcode::PushScope, index); + } + std::mem::swap(&mut self.lexical_scope, &mut scope); + Some(scope) + } + + /// Pop a declarative scope. + pub(crate) fn pop_declarative_scope(&mut self, scope: Option) { + if let Some(mut scope) = scope { + std::mem::swap(&mut self.lexical_scope, &mut scope); + if !scope.all_bindings_local() { + self.current_open_environments_count -= 1; + self.emit_opcode(Opcode::PopEnvironment); + } + } + } + /// Pops the top scope. pub(crate) fn pop_scope(&mut self) { self.current_open_environments_count -= 1; diff --git a/core/engine/src/bytecompiler/function.rs b/core/engine/src/bytecompiler/function.rs index 3fef0a612d6..5909783b025 100644 --- a/core/engine/src/bytecompiler/function.rs +++ b/core/engine/src/bytecompiler/function.rs @@ -94,6 +94,7 @@ impl FunctionCompiler { } /// Compile a function statement list and it's parameters into bytecode. + #[allow(clippy::too_many_arguments)] pub(crate) fn compile( mut self, parameters: &FormalParameterList, @@ -101,6 +102,7 @@ impl FunctionCompiler { variable_environment: Scope, lexical_environment: Scope, scopes: &FunctionScopes, + contains_direct_eval: bool, interner: &mut Interner, ) -> Gc { self.strict = self.strict || body.strict(); @@ -129,11 +131,19 @@ impl FunctionCompiler { } if let Some(scope) = self.name_scope { - compiler.code_block_flags |= CodeBlockFlags::HAS_BINDING_IDENTIFIER; - let _ = compiler.push_scope(&scope); + if !scope.all_bindings_local() { + compiler.code_block_flags |= CodeBlockFlags::HAS_BINDING_IDENTIFIER; + let _ = compiler.push_scope(&scope); + } + } + + if self.arrow && scopes.function_scope().all_bindings_local() && !contains_direct_eval { + compiler.variable_scope = scopes.function_scope().clone(); + compiler.lexical_scope = scopes.function_scope().clone(); + } else { + compiler.code_block_flags |= CodeBlockFlags::HAS_FUNCTION_SCOPE; + let _ = compiler.push_scope(scopes.function_scope()); } - // Function environment - let _ = compiler.push_scope(scopes.function_scope()); // Taken from: // - 15.9.3 Runtime Semantics: EvaluateAsyncConciseBody: @@ -185,6 +195,7 @@ impl FunctionCompiler { compiler.compile_statement_list(body.statement_list(), false, false); compiler.params = parameters.clone(); + compiler.parameter_scope = scopes.parameter_scope(); let code = compiler.finish(); diff --git a/core/engine/src/bytecompiler/mod.rs b/core/engine/src/bytecompiler/mod.rs index da428e4e2d2..657261f3e80 100644 --- a/core/engine/src/bytecompiler/mod.rs +++ b/core/engine/src/bytecompiler/mod.rs @@ -122,6 +122,7 @@ pub(crate) struct FunctionSpec<'a> { body: &'a FunctionBody, pub(crate) scopes: &'a FunctionScopes, pub(crate) name_scope: Option<&'a Scope>, + pub(crate) contains_direct_eval: bool, } impl<'a> From<&'a FunctionDeclaration> for FunctionSpec<'a> { @@ -133,6 +134,7 @@ impl<'a> From<&'a FunctionDeclaration> for FunctionSpec<'a> { body: function.body(), scopes: function.scopes(), name_scope: None, + contains_direct_eval: function.contains_direct_eval(), } } } @@ -146,6 +148,7 @@ impl<'a> From<&'a GeneratorDeclaration> for FunctionSpec<'a> { body: function.body(), scopes: function.scopes(), name_scope: None, + contains_direct_eval: function.contains_direct_eval(), } } } @@ -159,6 +162,7 @@ impl<'a> From<&'a AsyncFunctionDeclaration> for FunctionSpec<'a> { body: function.body(), scopes: function.scopes(), name_scope: None, + contains_direct_eval: function.contains_direct_eval(), } } } @@ -172,6 +176,7 @@ impl<'a> From<&'a AsyncGeneratorDeclaration> for FunctionSpec<'a> { body: function.body(), scopes: function.scopes(), name_scope: None, + contains_direct_eval: function.contains_direct_eval(), } } } @@ -185,6 +190,7 @@ impl<'a> From<&'a FunctionExpression> for FunctionSpec<'a> { body: function.body(), scopes: function.scopes(), name_scope: function.name_scope(), + contains_direct_eval: function.contains_direct_eval(), } } } @@ -198,6 +204,7 @@ impl<'a> From<&'a ArrowFunction> for FunctionSpec<'a> { body: function.body(), scopes: function.scopes(), name_scope: None, + contains_direct_eval: function.contains_direct_eval(), } } } @@ -211,6 +218,7 @@ impl<'a> From<&'a AsyncArrowFunction> for FunctionSpec<'a> { body: function.body(), scopes: function.scopes(), name_scope: None, + contains_direct_eval: function.contains_direct_eval(), } } } @@ -224,6 +232,7 @@ impl<'a> From<&'a AsyncFunctionExpression> for FunctionSpec<'a> { body: function.body(), scopes: function.scopes(), name_scope: function.name_scope(), + contains_direct_eval: function.contains_direct_eval(), } } } @@ -237,6 +246,7 @@ impl<'a> From<&'a GeneratorExpression> for FunctionSpec<'a> { body: function.body(), scopes: function.scopes(), name_scope: function.name_scope(), + contains_direct_eval: function.contains_direct_eval(), } } } @@ -250,6 +260,7 @@ impl<'a> From<&'a AsyncGeneratorExpression> for FunctionSpec<'a> { body: function.body(), scopes: function.scopes(), name_scope: function.name_scope(), + contains_direct_eval: function.contains_direct_eval(), } } } @@ -270,6 +281,7 @@ impl<'a> From<&'a ClassMethodDefinition> for FunctionSpec<'a> { body: method.body(), scopes: method.scopes(), name_scope: None, + contains_direct_eval: method.contains_direct_eval(), } } } @@ -290,6 +302,7 @@ impl<'a> From<&'a ObjectMethodDefinition> for FunctionSpec<'a> { body: method.body(), scopes: method.scopes(), name_scope: None, + contains_direct_eval: method.contains_direct_eval(), } } } @@ -388,6 +401,9 @@ pub struct ByteCompiler<'ctx> { /// Parameters passed to this function. pub(crate) params: FormalParameterList, + /// Scope of the function parameters. + pub(crate) parameter_scope: Scope, + /// Bytecode pub(crate) bytecode: Vec, @@ -500,6 +516,7 @@ impl<'ctx> ByteCompiler<'ctx> { local_binding_registers: FxHashMap::default(), this_mode: ThisMode::Global, params: FormalParameterList::default(), + parameter_scope: Scope::default(), current_open_environments_count: 0, register_allocator, @@ -1518,6 +1535,7 @@ impl<'ctx> ByteCompiler<'ctx> { self.variable_scope.clone(), self.lexical_scope.clone(), scopes, + function.contains_direct_eval, self.interner, ); @@ -1595,6 +1613,7 @@ impl<'ctx> ByteCompiler<'ctx> { self.variable_scope.clone(), self.lexical_scope.clone(), scopes, + function.contains_direct_eval, self.interner, ); @@ -1638,6 +1657,7 @@ impl<'ctx> ByteCompiler<'ctx> { self.variable_scope.clone(), self.lexical_scope.clone(), scopes, + function.contains_direct_eval, self.interner, ); @@ -1756,7 +1776,9 @@ impl<'ctx> ByteCompiler<'ctx> { let mapped_arguments_binding_indices = self .emitted_mapped_arguments_object_opcode - .then(|| MappedArguments::binding_indices(&self.params)) + .then(|| { + MappedArguments::binding_indices(&self.params, &self.parameter_scope, self.interner) + }) .unwrap_or_default(); let max_local_binding_register_index = diff --git a/core/engine/src/bytecompiler/statement/block.rs b/core/engine/src/bytecompiler/statement/block.rs index f4fa8bdb638..99da2023627 100644 --- a/core/engine/src/bytecompiler/statement/block.rs +++ b/core/engine/src/bytecompiler/statement/block.rs @@ -1,25 +1,12 @@ -use crate::{bytecompiler::ByteCompiler, vm::Opcode}; +use crate::bytecompiler::ByteCompiler; use boa_ast::statement::Block; impl ByteCompiler<'_> { /// Compile a [`Block`] `boa_ast` node pub(crate) fn compile_block(&mut self, block: &Block, use_expr: bool) { - let outer_scope = if let Some(scope) = block.scope() { - let outer_scope = self.lexical_scope.clone(); - let scope_index = self.push_scope(scope); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); - Some(outer_scope) - } else { - None - }; - + let scope = self.push_declarative_scope(block.scope()); self.block_declaration_instantiation(block); self.compile_statement_list(block.statement_list(), use_expr, true); - - if let Some(outer_scope) = outer_scope { - self.pop_scope(); - self.lexical_scope = outer_scope; - self.emit_opcode(Opcode::PopEnvironment); - } + self.pop_declarative_scope(scope); } } diff --git a/core/engine/src/bytecompiler/statement/loop.rs b/core/engine/src/bytecompiler/statement/loop.rs index e0589efd7d9..b02c93bcc1f 100644 --- a/core/engine/src/bytecompiler/statement/loop.rs +++ b/core/engine/src/bytecompiler/statement/loop.rs @@ -22,6 +22,7 @@ impl ByteCompiler<'_> { use_expr: bool, ) { let mut let_binding_indices = None; + let mut outer_scope_local = None; let mut outer_scope = None; if let Some(init) = for_loop.init() { @@ -31,9 +32,16 @@ impl ByteCompiler<'_> { self.compile_var_decl(decl); } ForLoopInitializer::Lexical(decl) => { - outer_scope = Some(self.lexical_scope.clone()); - let scope_index = self.push_scope(decl.scope()); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); + let scope_index = if decl.scope().all_bindings_local() { + outer_scope_local = Some(self.lexical_scope.clone()); + self.lexical_scope = decl.scope().clone(); + None + } else { + outer_scope = Some(self.lexical_scope.clone()); + let scope_index = self.push_scope(decl.scope()); + self.emit_with_varying_operand(Opcode::PushScope, scope_index); + Some(scope_index) + }; let names = bound_names(decl.declaration()); if decl.declaration().is_const() { @@ -41,8 +49,8 @@ impl ByteCompiler<'_> { let mut indices = Vec::new(); for name in &names { let name = name.to_js_string(self.interner()); - let binding = self - .lexical_scope + let binding = decl + .scope() .get_binding_reference(&name) .expect("binding must exist"); let index = self.get_or_insert_binding(binding); @@ -62,8 +70,10 @@ impl ByteCompiler<'_> { self.emit_binding_access(Opcode::GetName, index); } - self.emit_opcode(Opcode::PopEnvironment); - self.emit_with_varying_operand(Opcode::PushScope, *scope_index); + if let Some(index) = scope_index { + self.emit_opcode(Opcode::PopEnvironment); + self.emit_with_varying_operand(Opcode::PushScope, *index); + } for index in let_binding_indices.iter().rev() { self.emit_binding_access(Opcode::PutLexicalValue, index); @@ -85,8 +95,10 @@ impl ByteCompiler<'_> { self.emit_binding_access(Opcode::GetName, index); } - self.emit_opcode(Opcode::PopEnvironment); - self.emit_with_varying_operand(Opcode::PushScope, *scope_index); + if let Some(index) = scope_index { + self.emit_opcode(Opcode::PopEnvironment); + self.emit_with_varying_operand(Opcode::PushScope, *index); + } for index in let_binding_indices.iter().rev() { self.emit_binding_access(Opcode::PutLexicalValue, index); @@ -115,11 +127,10 @@ impl ByteCompiler<'_> { self.patch_jump(exit); self.pop_loop_control_info(); - if let Some(outer_scope) = outer_scope { - self.pop_scope(); - self.lexical_scope = outer_scope; - self.emit_opcode(Opcode::PopEnvironment); + if let Some(outer_scope_local) = outer_scope_local { + self.lexical_scope = outer_scope_local; } + self.pop_declarative_scope(outer_scope); } pub(crate) fn compile_for_in_loop( @@ -138,17 +149,9 @@ impl ByteCompiler<'_> { } } } - if let Some(scope) = for_in_loop.target_scope() { - let outer_scope = self.lexical_scope.clone(); - let scope_index = self.push_scope(scope); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); - self.compile_expr(for_in_loop.target(), true); - self.pop_scope(); - self.lexical_scope = outer_scope; - self.emit_opcode(Opcode::PopEnvironment); - } else { - self.compile_expr(for_in_loop.target(), true); - } + let outer_scope = self.push_declarative_scope(for_in_loop.target_scope()); + self.compile_expr(for_in_loop.target(), true); + self.pop_declarative_scope(outer_scope); let early_exit = self.jump_if_null_or_undefined(); self.emit_opcode(Opcode::CreateForInIterator); @@ -163,13 +166,7 @@ impl ByteCompiler<'_> { self.emit_opcode(Opcode::IteratorValue); - let mut outer_scope = None; - - if let Some(scope) = for_in_loop.scope() { - outer_scope = Some(self.lexical_scope.clone()); - let scope_index = self.push_scope(scope); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); - } + let outer_scope = self.push_declarative_scope(for_in_loop.scope()); match for_in_loop.initializer() { IterableLoopInitializer::Identifier(ident) => { @@ -208,12 +205,7 @@ impl ByteCompiler<'_> { } self.compile_stmt(for_in_loop.body(), use_expr, true); - - if let Some(outer_scope) = outer_scope { - self.pop_scope(); - self.lexical_scope = outer_scope; - self.emit_opcode(Opcode::PopEnvironment); - } + self.pop_declarative_scope(outer_scope); self.emit(Opcode::Jump, &[Operand::U32(start_address)]); @@ -234,17 +226,9 @@ impl ByteCompiler<'_> { label: Option, use_expr: bool, ) { - if let Some(scope) = for_of_loop.iterable_scope() { - let outer_scope = self.lexical_scope.clone(); - let scope_index = self.push_scope(scope); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); - self.compile_expr(for_of_loop.iterable(), true); - self.pop_scope(); - self.lexical_scope = outer_scope; - self.emit_opcode(Opcode::PopEnvironment); - } else { - self.compile_expr(for_of_loop.iterable(), true); - } + let outer_scope = self.push_declarative_scope(for_of_loop.iterable_scope()); + self.compile_expr(for_of_loop.iterable(), true); + self.pop_declarative_scope(outer_scope); if for_of_loop.r#await() { self.emit_opcode(Opcode::GetAsyncIterator); @@ -271,14 +255,7 @@ impl ByteCompiler<'_> { let exit = self.jump_if_true(); self.emit_opcode(Opcode::IteratorValue); - let mut outer_scope = None; - - if let Some(scope) = for_of_loop.scope() { - outer_scope = Some(self.lexical_scope.clone()); - let scope_index = self.push_scope(scope); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); - } - + let outer_scope = self.push_declarative_scope(for_of_loop.scope()); let handler_index = self.push_handler(); match for_of_loop.initializer() { @@ -354,12 +331,7 @@ impl ByteCompiler<'_> { self.patch_jump(exit); } - if let Some(outer_scope) = outer_scope { - self.pop_scope(); - self.lexical_scope = outer_scope; - self.emit_opcode(Opcode::PopEnvironment); - } - + self.pop_declarative_scope(outer_scope); self.emit(Opcode::Jump, &[Operand::U32(start_address)]); self.patch_jump(exit); diff --git a/core/engine/src/bytecompiler/statement/switch.rs b/core/engine/src/bytecompiler/statement/switch.rs index d1bb4b8d38c..3a53d9cc39a 100644 --- a/core/engine/src/bytecompiler/statement/switch.rs +++ b/core/engine/src/bytecompiler/statement/switch.rs @@ -5,15 +5,7 @@ impl ByteCompiler<'_> { /// Compile a [`Switch`] `boa_ast` node pub(crate) fn compile_switch(&mut self, switch: &Switch, use_expr: bool) { self.compile_expr(switch.val(), true); - - let outer_scope = if let Some(scope) = switch.scope() { - let outer_scope = self.lexical_scope.clone(); - let scope_index = self.push_scope(scope); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); - Some(outer_scope) - } else { - None - }; + let outer_scope = self.push_declarative_scope(switch.scope()); self.block_declaration_instantiation(switch); @@ -55,11 +47,6 @@ impl ByteCompiler<'_> { } self.pop_switch_control_info(); - - if let Some(outer_scope) = outer_scope { - self.pop_scope(); - self.lexical_scope = outer_scope; - self.emit_opcode(Opcode::PopEnvironment); - } + self.pop_declarative_scope(outer_scope); } } diff --git a/core/engine/src/bytecompiler/statement/try.rs b/core/engine/src/bytecompiler/statement/try.rs index ad0b626c5ad..98073bdf320 100644 --- a/core/engine/src/bytecompiler/statement/try.rs +++ b/core/engine/src/bytecompiler/statement/try.rs @@ -108,11 +108,7 @@ impl ByteCompiler<'_> { } pub(crate) fn compile_catch_stmt(&mut self, catch: &Catch, _has_finally: bool, use_expr: bool) { - // stack: exception - - let outer_scope = self.lexical_scope.clone(); - let scope_index = self.push_scope(catch.scope()); - self.emit_with_varying_operand(Opcode::PushScope, scope_index); + let outer_scope = self.push_declarative_scope(Some(catch.scope())); if let Some(binding) = catch.parameter() { match binding { @@ -130,9 +126,7 @@ impl ByteCompiler<'_> { self.compile_catch_finally_block(catch.block(), use_expr); - self.pop_scope(); - self.lexical_scope = outer_scope; - self.emit_opcode(Opcode::PopEnvironment); + self.pop_declarative_scope(outer_scope); } pub(crate) fn compile_finally_stmt(&mut self, finally: &Finally, has_catch: bool) { diff --git a/core/engine/src/environments/runtime/mod.rs b/core/engine/src/environments/runtime/mod.rs index ba2a03dda5f..4dce94a9f89 100644 --- a/core/engine/src/environments/runtime/mod.rs +++ b/core/engine/src/environments/runtime/mod.rs @@ -186,7 +186,7 @@ impl EnvironmentStack { /// Push a function environment on the environments stack. pub(crate) fn push_function(&mut self, scope: Scope, function_slots: FunctionSlots) { - let num_bindings = scope.num_bindings(); + let num_bindings = scope.num_bindings_non_local(); let (poisoned, with) = { // Check if the outer environment is a declarative environment. @@ -214,7 +214,7 @@ impl EnvironmentStack { /// Push a module environment on the environments stack. pub(crate) fn push_module(&mut self, scope: Scope) { - let num_bindings = scope.num_bindings(); + let num_bindings = scope.num_bindings_non_local(); self.stack.push(Environment::Declarative(Gc::new( DeclarativeEnvironment::new(DeclarativeEnvironmentKind::Module( ModuleEnvironment::new(num_bindings, scope), diff --git a/core/engine/src/module/synthetic.rs b/core/engine/src/module/synthetic.rs index 5e8309e0b6c..99e8f278264 100644 --- a/core/engine/src/module/synthetic.rs +++ b/core/engine/src/module/synthetic.rs @@ -307,6 +307,8 @@ impl SyntheticModule { }) .collect::>(); + module_scope.escape_all_bindings(); + let cb = Gc::new(compiler.finish()); let mut envs = EnvironmentStack::new(global_env); diff --git a/core/engine/src/vm/code_block.rs b/core/engine/src/vm/code_block.rs index 6c461b1b21a..dd44daf1e8e 100644 --- a/core/engine/src/vm/code_block.rs +++ b/core/engine/src/vm/code_block.rs @@ -66,6 +66,9 @@ bitflags! { /// Arrow and method functions don't have `"prototype"` property. const HAS_PROTOTYPE_PROPERTY = 0b1000_0000; + /// If the function requires a function scope. + const HAS_FUNCTION_SCOPE = 0b1_0000_0000; + /// Trace instruction execution to `stdout`. #[cfg(feature = "trace")] const TRACEABLE = 0b1000_0000_0000_0000; @@ -271,6 +274,13 @@ impl CodeBlock { .contains(CodeBlockFlags::HAS_PROTOTYPE_PROPERTY) } + /// Returns true if this function requires a function scope. + pub(crate) fn has_function_scope(&self) -> bool { + self.flags + .get() + .contains(CodeBlockFlags::HAS_FUNCTION_SCOPE) + } + /// Find exception [`Handler`] in the code block given the current program counter (`pc`). #[inline] pub(crate) fn find_handler(&self, pc: u32) -> Option<(usize, &Handler)> { diff --git a/core/engine/src/vm/opcode/push/environment.rs b/core/engine/src/vm/opcode/push/environment.rs index cfd66eca846..81fb983569b 100644 --- a/core/engine/src/vm/opcode/push/environment.rs +++ b/core/engine/src/vm/opcode/push/environment.rs @@ -17,7 +17,10 @@ impl PushScope { #[allow(clippy::unnecessary_wraps)] fn operation(context: &mut Context, index: usize) -> JsResult { let scope = context.vm.frame().code_block().constant_scope(index); - context.vm.environments.push_lexical(scope.num_bindings()); + context + .vm + .environments + .push_lexical(scope.num_bindings_non_local()); Ok(CompletionType::Normal) } }