Skip to content

Commit

Permalink
Apply review
Browse files Browse the repository at this point in the history
  • Loading branch information
@raskad
raskad committed Oct 10, 2023
1 parent e8042b3 commit e389020
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 35 deletions.
4 changes: 2 additions & 2 deletions boa_engine/src/bytecompiler/declarations.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ impl ByteCompiler<'_, '_> {

// c. Assert: The following loop will terminate.
// d. Repeat, while thisEnv is not varEnv,
while &this_env != var_env {
while this_env.environment_index() != var_env.environment_index() {
// i. If thisEnv is not an Object Environment Record, then
// 1. NOTE: The environment of with statements cannot contain any lexical
// declaration so it doesn't need to be checked for var/let hoisting conflicts.
Expand Down Expand Up @@ -545,7 +545,7 @@ impl ByteCompiler<'_, '_> {

// 3. Assert: The following loop will terminate.
// 4. Repeat, while thisEnv is not varEnv,
while &this_env != lex_env {
while this_env.environment_index() != lex_env.environment_index() {
// a. If thisEnv is not an Object Environment Record, then
// i. If ! thisEnv.HasBinding(F) is true, then
if this_env.has_binding(f) {
Expand Down
8 changes: 4 additions & 4 deletions boa_engine/src/bytecompiler/expression/assign.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ impl ByteCompiler<'_, '_> {

match access {
Access::Variable { name } => {
let (binding, lex) = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding);
let binding = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding.locator());

if lex {
if binding.is_lexical() {
self.emit_with_varying_operand(Opcode::GetName, index);
} else {
self.emit_with_varying_operand(Opcode::GetNameAndLocator, index);
Expand All @@ -74,7 +74,7 @@ impl ByteCompiler<'_, '_> {
if use_expr {
self.emit_opcode(Opcode::Dup);
}
if lex {
if binding.is_lexical() {
match self.lexical_environment.set_mutable_binding(name) {
Ok(binding) => {
let index = self.get_or_insert_binding(binding);
Expand Down
4 changes: 2 additions & 2 deletions boa_engine/src/bytecompiler/expression/unary.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ impl ByteCompiler<'_, '_> {
UnaryOp::TypeOf => {
match unary.target().flatten() {
Expression::Identifier(identifier) => {
let (binding, _) = self
let binding = self
.lexical_environment
.get_identifier_reference(*identifier);
let index = self.get_or_insert_binding(binding);
let index = self.get_or_insert_binding(binding.locator());
self.emit_with_varying_operand(Opcode::GetNameOrUndefined, index);
}
expr => self.compile_expr(expr, true),
Expand Down
8 changes: 4 additions & 4 deletions boa_engine/src/bytecompiler/expression/update.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ impl ByteCompiler<'_, '_> {

match Access::from_update_target(update.target()) {
Access::Variable { name } => {
let (binding, lex) = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding);
let binding = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding.locator());

if lex {
if binding.is_lexical() {
self.emit_with_varying_operand(Opcode::GetName, index);
} else {
self.emit_with_varying_operand(Opcode::GetNameAndLocator, index);
Expand All @@ -39,7 +39,7 @@ impl ByteCompiler<'_, '_> {
self.emit_opcode(Opcode::Dup);
}

if lex {
if binding.is_lexical() {
match self.lexical_environment.set_mutable_binding(name) {
Ok(binding) => {
let index = self.get_or_insert_binding(binding);
Expand Down
29 changes: 15 additions & 14 deletions boa_engine/src/bytecompiler/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,8 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
fn emit_binding(&mut self, opcode: BindingOpcode, name: Identifier) {
match opcode {
BindingOpcode::Var => {
let (binding, _) = self.variable_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding);
let binding = self.variable_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding.locator());
self.emit_with_varying_operand(Opcode::DefVar, index);
}
BindingOpcode::InitVar => match self.lexical_environment.set_mutable_binding(name) {
Expand All @@ -431,8 +431,8 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
}
},
BindingOpcode::InitLexical => {
let (binding, _) = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding);
let binding = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding.locator());
self.emit_with_varying_operand(Opcode::PutLexicalValue, index);
}
BindingOpcode::SetName => match self.lexical_environment.set_mutable_binding(name) {
Expand Down Expand Up @@ -690,8 +690,8 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
fn access_get(&mut self, access: Access<'_>, use_expr: bool) {
match access {
Access::Variable { name } => {
let (binding, _) = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding);
let binding = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding.locator());
self.emit_with_varying_operand(Opcode::GetName, index);
}
Access::Property { access } => match access {
Expand Down Expand Up @@ -755,10 +755,10 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
{
match access {
Access::Variable { name } => {
let (binding, lex) = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding);
let binding = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding.locator());

if !lex {
if !binding.is_lexical() {
self.emit_with_varying_operand(Opcode::GetLocator, index);
}

Expand All @@ -767,7 +767,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
self.emit(Opcode::Dup, &[]);
}

if lex {
if binding.is_lexical() {
match self.lexical_environment.set_mutable_binding(name) {
Ok(binding) => {
let index = self.get_or_insert_binding(binding);
Expand Down Expand Up @@ -867,8 +867,8 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
}
},
Access::Variable { name } => {
let (binding, _) = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding);
let binding = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding.locator());
self.emit_with_varying_operand(Opcode::DeleteName, index);
}
Access::This => {
Expand Down Expand Up @@ -1171,6 +1171,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
}

/// Compile a [`Declaration`].
#[allow(unused_variables)]
pub fn compile_decl(&mut self, decl: &Declaration, block: bool) {
match decl {
#[cfg(feature = "annex-b")]
Expand All @@ -1179,8 +1180,8 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
.name()
.expect("function declaration must have name");
if self.annex_b_function_names.contains(&name) {
let (binding, _) = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding);
let binding = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding.locator());
self.emit_with_varying_operand(Opcode::GetName, index);

match self.variable_environment.set_mutable_binding_var(name) {
Expand Down
40 changes: 31 additions & 9 deletions boa_engine/src/environments/compile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,6 @@ pub(crate) struct CompileTimeEnvironment {
function_scope: bool,
}

impl PartialEq for CompileTimeEnvironment {
fn eq(&self, other: &Self) -> bool {
self.environment_index == other.environment_index
}
}

// Safety: Nothing in this struct needs tracing, so this is safe.
unsafe impl Trace for CompileTimeEnvironment {
empty_trace!();
Expand Down Expand Up @@ -78,16 +72,16 @@ impl CompileTimeEnvironment {

/// Get the binding locator for a binding with the given name.
/// Fall back to the global environment if the binding is not found.
pub(crate) fn get_identifier_reference(&self, name: Identifier) -> (BindingLocator, bool) {
pub(crate) fn get_identifier_reference(&self, name: Identifier) -> IdentifierReference {
if let Some(binding) = self.bindings.borrow().get(&name) {
(
IdentifierReference::new(
BindingLocator::declarative(name, self.environment_index, binding.index),
binding.lex,
)
} else if let Some(outer) = &self.outer {
outer.get_identifier_reference(name)
} else {
(BindingLocator::global(name), false)
IdentifierReference::new(BindingLocator::global(name), false)
}
}

Expand All @@ -96,6 +90,11 @@ impl CompileTimeEnvironment {
self.bindings.borrow().len() as u32
}

/// Returns the index of this environment.
pub(crate) fn environment_index(&self) -> u32 {
self.environment_index
}

/// Check if the environment is a function environment.
pub(crate) const fn is_function(&self) -> bool {
self.function_scope
Expand Down Expand Up @@ -201,3 +200,26 @@ impl CompileTimeEnvironment {
self.outer.clone()
}
}

/// A reference to an identifier in a compile time environment.
pub(crate) struct IdentifierReference {
locator: BindingLocator,
lexical: bool,
}

impl IdentifierReference {
/// Create a new identifier reference.
pub(crate) fn new(locator: BindingLocator, lexical: bool) -> Self {
Self { locator, lexical }
}

/// Get the binding locator for this identifier reference.
pub(crate) fn locator(&self) -> BindingLocator {
self.locator
}

/// Check if this identifier reference is lexical.
pub(crate) fn is_lexical(&self) -> bool {
self.lexical
}
}

0 comments on commit e389020

Please sign in to comment.