Skip to content

Commit

Permalink
Remove unneded num_bindings in Opcodes and CodeBlock (#2967)
Browse files Browse the repository at this point in the history
* Remove redundant `num_bindings` field from `CodeBlock`

* Remove redundant num_bindings parameter from push_function_inherits

* Remove redundant num_bindings operand from environment opcodes

* Make pop_compile_environment() return an index

* Move boolean `CodeBlock` flags to bitflags

* Fix ci

* Add doc
  • Loading branch information
HalidOdat authored May 27, 2023
1 parent 4ea80f4 commit 67c5652
Show file tree
Hide file tree
Showing 22 changed files with 210 additions and 224 deletions.
2 changes: 1 addition & 1 deletion boa_cli/src/debug/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ fn set_trace_flag_in_function_object(object: &JsObject, value: bool) -> JsResult
let code = function.codeblock().ok_or_else(|| {
JsNativeError::typ().with_message("native functions do not have bytecode")
})?;
code.set_trace(value);
code.set_traceable(value);
Ok(())
}

Expand Down
9 changes: 5 additions & 4 deletions boa_engine/src/builtins/eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,14 +232,15 @@ impl Eval {
);

compiler.push_compile_environment(strict);
let push_env = compiler.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);

let push_env = compiler.emit_opcode_with_operand(Opcode::PushDeclarativeEnvironment);

compiler.eval_declaration_instantiation(&body, strict)?;
compiler.compile_statement_list(body.statements(), true, false);

let env_info = compiler.pop_compile_environment();
compiler.patch_jump_with_target(push_env.0, env_info.num_bindings);
compiler.patch_jump_with_target(push_env.1, env_info.index);
let env_index = compiler.pop_compile_environment();
compiler.patch_jump_with_target(push_env, env_index);

compiler.emit_opcode(Opcode::PopEnvironment);

let code_block = Gc::new(compiler.finish());
Expand Down
42 changes: 17 additions & 25 deletions boa_engine/src/bytecompiler/class.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{ByteCompiler, Literal};
use crate::vm::{BindingOpcode, Opcode};
use crate::vm::{BindingOpcode, CodeBlockFlags, Opcode};
use boa_ast::{
expression::Identifier,
function::{Class, ClassElement, FormalParameterList},
Expand Down Expand Up @@ -27,7 +27,7 @@ impl ByteCompiler<'_, '_> {

if let Some(class_name) = class.name() {
if class.has_binding_identifier() {
compiler.has_binding_identifier = true;
compiler.code_block_flags |= CodeBlockFlags::HAS_BINDING_IDENTIFIER;
compiler.push_compile_environment(false);
compiler.create_immutable_binding(class_name, true);
}
Expand All @@ -39,7 +39,7 @@ impl ByteCompiler<'_, '_> {
compiler.length = expr.parameters().length();
compiler.params = expr.parameters().clone();

let (env_labels, _) = compiler.function_declaration_instantiation(
let (env_label, _) = compiler.function_declaration_instantiation(
expr.body(),
expr.parameters(),
false,
Expand All @@ -49,23 +49,20 @@ impl ByteCompiler<'_, '_> {

compiler.compile_statement_list(expr.body().statements(), false, false);

let env_info = compiler.pop_compile_environment();
let env_index = compiler.pop_compile_environment();

if let Some(env_labels) = env_labels {
compiler.patch_jump_with_target(env_labels.0, env_info.num_bindings);
compiler.patch_jump_with_target(env_labels.1, env_info.index);
if let Some(env_label) = env_label {
compiler.patch_jump_with_target(env_label, env_index);
compiler.pop_compile_environment();
} else {
compiler.num_bindings = env_info.num_bindings;
compiler.is_class_constructor = true;
compiler.code_block_flags |= CodeBlockFlags::IS_CLASS_CONSTRUCTOR;
}
} else {
if class.super_ref().is_some() {
compiler.emit_opcode(Opcode::SuperCallDerived);
}
let env_info = compiler.pop_compile_environment();
compiler.num_bindings = env_info.num_bindings;
compiler.is_class_constructor = true;
compiler.pop_compile_environment();
compiler.code_block_flags |= CodeBlockFlags::IS_CLASS_CONSTRUCTOR;
}

if class.name().is_some() && class.has_binding_identifier() {
Expand All @@ -81,11 +78,11 @@ impl ByteCompiler<'_, '_> {
self.emit(Opcode::GetFunction, &[index]);
self.emit_u8(0);

let class_env: Option<(super::Label, super::Label)> = match class.name() {
let class_env: Option<super::Label> = match class.name() {
Some(name) if class.has_binding_identifier() => {
self.push_compile_environment(false);
self.create_immutable_binding(name, true);
Some(self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment))
Some(self.emit_opcode_with_operand(Opcode::PushDeclarativeEnvironment))
}
_ => None,
};
Expand Down Expand Up @@ -266,9 +263,8 @@ impl ByteCompiler<'_, '_> {
} else {
field_compiler.emit_opcode(Opcode::PushUndefined);
}
let env_info = field_compiler.pop_compile_environment();
field_compiler.pop_compile_environment();
field_compiler.num_bindings = env_info.num_bindings;
field_compiler.pop_compile_environment();
field_compiler.emit_opcode(Opcode::Return);

let mut code = field_compiler.finish();
Expand Down Expand Up @@ -298,9 +294,8 @@ impl ByteCompiler<'_, '_> {
} else {
field_compiler.emit_opcode(Opcode::PushUndefined);
}
let env_info = field_compiler.pop_compile_environment();
field_compiler.pop_compile_environment();
field_compiler.num_bindings = env_info.num_bindings;
field_compiler.pop_compile_environment();
field_compiler.emit_opcode(Opcode::Return);

let mut code = field_compiler.finish();
Expand Down Expand Up @@ -340,9 +335,8 @@ impl ByteCompiler<'_, '_> {
} else {
field_compiler.emit_opcode(Opcode::PushUndefined);
}
let env_info = field_compiler.pop_compile_environment();
field_compiler.pop_compile_environment();
field_compiler.num_bindings = env_info.num_bindings;
field_compiler.pop_compile_environment();
field_compiler.emit_opcode(Opcode::Return);

let mut code = field_compiler.finish();
Expand Down Expand Up @@ -392,9 +386,8 @@ impl ByteCompiler<'_, '_> {
);

compiler.compile_statement_list(body.statements(), false, false);
let env_info = compiler.pop_compile_environment();
compiler.pop_compile_environment();
compiler.num_bindings = env_info.num_bindings;
compiler.pop_compile_environment();

let code = Gc::new(compiler.finish());
let index = self.functions.len() as u32;
Expand Down Expand Up @@ -547,9 +540,8 @@ impl ByteCompiler<'_, '_> {
self.emit_opcode(Opcode::Pop);

if let Some(class_env) = class_env {
let env_info = self.pop_compile_environment();
self.patch_jump_with_target(class_env.0, env_info.num_bindings);
self.patch_jump_with_target(class_env.1, env_info.index);
let env_index = self.pop_compile_environment();
self.patch_jump_with_target(class_env, env_index);
self.emit_opcode(Opcode::PopEnvironment);
}

Expand Down
14 changes: 6 additions & 8 deletions boa_engine/src/bytecompiler/declarations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ impl ByteCompiler<'_, '_> {
.name(name.sym())
.generator(generator)
.r#async(r#async)
.strict(self.strict)
.strict(self.strict())
.binding_identifier(Some(name.sym()))
.compile(
parameters,
Expand Down Expand Up @@ -672,7 +672,7 @@ impl ByteCompiler<'_, '_> {
.name(name.sym())
.generator(generator)
.r#async(r#async)
.strict(self.strict)
.strict(self.strict())
.binding_identifier(Some(name.sym()))
.compile(
parameters,
Expand Down Expand Up @@ -777,8 +777,8 @@ impl ByteCompiler<'_, '_> {
arrow: bool,
strict: bool,
generator: bool,
) -> (Option<(Label, Label)>, bool) {
let mut env_labels = None;
) -> (Option<Label>, bool) {
let mut env_label = None;
let mut additional_env = false;

// 1. Let calleeContext be the running execution context.
Expand Down Expand Up @@ -987,8 +987,7 @@ impl ByteCompiler<'_, '_> {
// b. Let varEnv be NewDeclarativeEnvironment(env).
// c. Set the VariableEnvironment of calleeContext to varEnv.
self.push_compile_environment(true);
self.function_environment_push_location = self.next_opcode_location();
env_labels = Some(self.emit_opcode_with_two_operands(Opcode::PushFunctionEnvironment));
env_label = Some(self.emit_opcode_with_operand(Opcode::PushFunctionEnvironment));

// d. Let instantiatedVarNames be a new empty List.
let mut instantiated_var_names = Vec::new();
Expand Down Expand Up @@ -1052,7 +1051,6 @@ impl ByteCompiler<'_, '_> {
}

// d. Let varEnv be env.

instantiated_var_names
};

Expand Down Expand Up @@ -1151,6 +1149,6 @@ impl ByteCompiler<'_, '_> {
}

// 37. Return unused.
(env_labels, additional_env)
(env_label, additional_env)
}
}
25 changes: 5 additions & 20 deletions boa_engine/src/bytecompiler/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,6 @@ use crate::environments::{BindingLocator, CompileTimeEnvironment};
use boa_ast::expression::Identifier;
use boa_gc::{Gc, GcRefCell};

/// Info returned by the [`ByteCompiler::pop_compile_environment`] method.
#[derive(Debug, Clone, Copy)]
pub(crate) struct PopEnvironmentInfo {
/// Number of bindings declared.
pub(crate) num_bindings: u32,
/// Index in the compile time envs array.
pub(crate) index: u32,
}

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) {
Expand All @@ -21,26 +12,20 @@ impl ByteCompiler<'_, '_> {
)));
}

/// Pops the top compile time environment and returns its index and number of bindings.
/// Pops the top compile time environment and returns its index in the compile time environments array.
#[track_caller]
pub(crate) fn pop_compile_environment(&mut self) -> PopEnvironmentInfo {
pub(crate) fn pop_compile_environment(&mut self) -> u32 {
let index = self.compile_environments.len() as u32;
self.compile_environments
.push(self.current_environment.clone());

let (num_bindings, outer) = {
let outer = {
let env = self.current_environment.borrow();
(
env.num_bindings(),
env.outer().expect("cannot pop the global environment"),
)
env.outer().expect("cannot pop the global environment")
};
self.current_environment = outer;

PopEnvironmentInfo {
num_bindings,
index,
}
index
}

/// Get the binding locator of the binding at bytecode compile time.
Expand Down
19 changes: 9 additions & 10 deletions boa_engine/src/bytecompiler/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::{
builtins::function::ThisMode,
bytecompiler::ByteCompiler,
environments::CompileTimeEnvironment,
vm::{CodeBlock, Opcode},
vm::{CodeBlock, CodeBlockFlags, Opcode},
Context,
};
use boa_ast::function::{FormalParameterList, FunctionBody};
Expand Down Expand Up @@ -109,15 +109,15 @@ impl FunctionCompiler {
}

if let Some(binding_identifier) = self.binding_identifier {
compiler.has_binding_identifier = true;
compiler.code_block_flags |= CodeBlockFlags::HAS_BINDING_IDENTIFIER;
compiler.push_compile_environment(false);
compiler.create_immutable_binding(binding_identifier.into(), self.strict);
}

// Function environment
compiler.push_compile_environment(true);

let (env_labels, additional_env) = compiler.function_declaration_instantiation(
let (env_label, additional_env) = compiler.function_declaration_instantiation(
body,
parameters,
self.arrow,
Expand All @@ -127,18 +127,17 @@ impl FunctionCompiler {

compiler.compile_statement_list(body.statements(), false, false);

if let Some(env_labels) = env_labels {
let env_info = compiler.pop_compile_environment();
compiler.patch_jump_with_target(env_labels.0, env_info.num_bindings);
compiler.patch_jump_with_target(env_labels.1, env_info.index);
if let Some(env_labels) = env_label {
let env_index = compiler.pop_compile_environment();
compiler.patch_jump_with_target(env_labels, env_index);
}

if additional_env {
compiler.parameters_env_bindings =
Some(compiler.pop_compile_environment().num_bindings);
compiler.pop_compile_environment();
compiler.code_block_flags |= CodeBlockFlags::PARAMETERS_ENV_BINDINGS;
}

compiler.num_bindings = compiler.pop_compile_environment().num_bindings;
compiler.pop_compile_environment();

if self.binding_identifier.is_some() {
compiler.pop_compile_environment();
Expand Down
Loading

0 comments on commit 67c5652

Please sign in to comment.