Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unneded num_bindings in Opcodes and CodeBlock #2967

Merged
merged 7 commits into from
May 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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