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.

Sign up for GitHub

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

[Merged by Bors] - Avoid creating prototype property on methods #2553

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - Avoid creating prototype property on methods #2553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 18 additions & 5 deletions boa_engine/src/builtins/function/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,9 +616,16 @@ impl BuiltInFunctionObject {
let environments = context.realm.environments.pop_to_global();

let function_object = if generator {
crate::vm::create_generator_function_object(code, r#async, context)
crate::vm::create_generator_function_object(code, r#async, false, context)
} else {
crate::vm::create_function_object(code, r#async, false, Some(prototype), context)
crate::vm::create_function_object(
code,
r#async,
false,
Some(prototype),
false,
context,
)
};

context.realm.environments.extend(environments);
Expand All @@ -636,7 +643,7 @@ impl BuiltInFunctionObject {

let environments = context.realm.environments.pop_to_global();
let function_object =
crate::vm::create_generator_function_object(code, r#async, context);
crate::vm::create_generator_function_object(code, r#async, false, context);
context.realm.environments.extend(environments);

Ok(function_object)
Expand All @@ -648,8 +655,14 @@ impl BuiltInFunctionObject {
)?;

let environments = context.realm.environments.pop_to_global();
let function_object =
crate::vm::create_function_object(code, r#async, false, Some(prototype), context);
let function_object = crate::vm::create_function_object(
code,
r#async,
false,
Some(prototype),
false,
context,
);
context.realm.environments.extend(environments);

Ok(function_object)
Expand Down
5 changes: 5 additions & 0 deletions boa_engine/src/bytecompiler/class.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ impl ByteCompiler<'_, '_> {
let index = self.code_block.functions.len() as u32;
self.code_block.functions.push(code);
self.emit(Opcode::GetFunction, &[index]);
self.emit_u8(0);
Copy link
Member

@nekevss nekevss Jan 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on creating an emit method like emit_with_opcode_and_u8 (maybe not the best name) to bind the pushing of the two operands together?

Edit: maybe even better emit_opcode_with_operand_and_bool, it's a long name, but then you can feed index and true/false as the arguments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could maybe generate emit functions for every opcode via a macro e.g. Opcode::GetFunction::emit. These functions could then automatically take the correct arguments.
Do we want to create a dedicated issue for making bytecode generation more typesafe?

Copy link
Member

@nekevss nekevss Jan 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that would make sense to me. Submitted an issue #2561 😄


self.emit_opcode(Opcode::Dup);
if let Some(node) = class.super_ref() {
Expand Down Expand Up @@ -320,6 +321,7 @@ impl ByteCompiler<'_, '_> {
let index = self.code_block.functions.len() as u32;
self.code_block.functions.push(code);
self.emit(Opcode::GetFunction, &[index]);
self.emit_u8(0);
self.emit_opcode(Opcode::PushClassField);
}
ClassElement::PrivateFieldDefinition(name, field) => {
Expand Down Expand Up @@ -362,6 +364,7 @@ impl ByteCompiler<'_, '_> {
let index = self.code_block.functions.len() as u32;
self.code_block.functions.push(code);
self.emit(Opcode::GetFunction, &[index]);
self.emit_u8(0);
self.emit(Opcode::PushClassFieldPrivate, &[name_index]);
}
ClassElement::StaticFieldDefinition(name, field) => {
Expand Down Expand Up @@ -414,6 +417,7 @@ impl ByteCompiler<'_, '_> {
let index = self.code_block.functions.len() as u32;
self.code_block.functions.push(code);
self.emit(Opcode::GetFunction, &[index]);
self.emit_u8(0);
self.emit_opcode(Opcode::SetHomeObject);
self.emit(Opcode::Call, &[0]);
if let Some(name_index) = name_index {
Expand Down Expand Up @@ -454,6 +458,7 @@ impl ByteCompiler<'_, '_> {
let index = self.code_block.functions.len() as u32;
self.code_block.functions.push(code);
self.emit(Opcode::GetFunction, &[index]);
self.emit_u8(0);
self.emit_opcode(Opcode::SetHomeObject);
self.emit(Opcode::Call, &[0]);
self.emit_opcode(Opcode::Pop);
Expand Down
2 changes: 2 additions & 0 deletions boa_engine/src/bytecompiler/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,7 @@ impl<'b, 'host> ByteCompiler<'b, 'host> {
} else {
self.emit(Opcode::GetFunction, &[index]);
}
self.emit_u8(0);

match node_kind {
NodeKind::Declaration => {
Expand Down Expand Up @@ -1140,6 +1141,7 @@ impl<'b, 'host> ByteCompiler<'b, 'host> {
} else {
self.emit(Opcode::GetFunction, &[index]);
}
self.emit_u8(1);

match node_kind {
NodeKind::Declaration => {
Expand Down
14 changes: 9 additions & 5 deletions boa_engine/src/vm/code_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ impl CodeBlock {
| Opcode::GetGenerator
| Opcode::GetGeneratorAsync => {
let operand = self.read::<u32>(*pc);
*pc += size_of::<u32>();
*pc += size_of::<u32>() + size_of::<u8>();
format!(
"{operand:04}: '{:?}' (length: {})",
interner.resolve_expect(self.functions[operand as usize].name),
Expand Down Expand Up @@ -518,6 +518,7 @@ pub(crate) fn create_function_object(
r#async: bool,
arrow: bool,
prototype: Option<JsObject>,
method: bool,
context: &mut Context<'_>,
) -> JsObject {
let _timer = Profiler::global().start_event("JsVmFunction::new", "vm");
Expand Down Expand Up @@ -612,7 +613,7 @@ pub(crate) fn create_function_object(
constructor
.define_property_or_throw(js_string!("name"), name_property, context)
.expect("failed to define the name property of the function");
if !r#async && !arrow {
if !r#async && !arrow && !method {
constructor
.define_property_or_throw(js_string!("prototype"), prototype_property, context)
.expect("failed to define the prototype property of the function");
Expand All @@ -625,6 +626,7 @@ pub(crate) fn create_function_object(
pub(crate) fn create_generator_function_object(
code: Gc<CodeBlock>,
r#async: bool,
method: bool,
context: &mut Context<'_>,
) -> JsObject {
let function_prototype = if r#async {
Expand Down Expand Up @@ -701,9 +703,11 @@ pub(crate) fn create_generator_function_object(
.configurable(false)
.build();

constructor
.define_property_or_throw(js_string!("prototype"), prototype_property, context)
.expect("failed to define the prototype property of the generator function");
if !method {
constructor
.define_property_or_throw(js_string!("prototype"), prototype_property, context)
.expect("failed to define the prototype property of the generator function");
}
constructor
.define_property_or_throw(js_string!("name"), name_property, context)
.expect("failed to define the name property of the generator function");
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/vm/flowgraph/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ impl CodeBlock {
let fn_name = interner
.resolve_expect(self.functions[operand as usize].name)
.to_string();
pc += size_of::<u32>();
pc += size_of::<u32>() + size_of::<u8>();
let label = format!(
"{opcode_str} '{fn_name}' (length: {})",
self.functions[operand as usize].length
Expand Down
12 changes: 8 additions & 4 deletions boa_engine/src/vm/opcode/get/function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ impl Operation for GetArrowFunction {

fn execute(context: &mut Context<'_>) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
context.vm.read::<u8>();
let code = context.vm.frame().code_block.functions[index as usize].clone();
let function = create_function_object(code, false, true, None, context);
let function = create_function_object(code, false, true, None, false, context);
context.vm.push(function);
Ok(ShouldExit::False)
}
Expand All @@ -36,8 +37,9 @@ impl Operation for GetAsyncArrowFunction {

fn execute(context: &mut Context<'_>) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
context.vm.read::<u8>();
let code = context.vm.frame().code_block.functions[index as usize].clone();
let function = create_function_object(code, true, true, None, context);
let function = create_function_object(code, true, true, None, false, context);
context.vm.push(function);
Ok(ShouldExit::False)
}
Expand All @@ -56,8 +58,9 @@ impl Operation for GetFunction {

fn execute(context: &mut Context<'_>) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
let method = context.vm.read::<u8>() != 0;
let code = context.vm.frame().code_block.functions[index as usize].clone();
let function = create_function_object(code, false, false, None, context);
let function = create_function_object(code, false, false, None, method, context);
context.vm.push(function);
Ok(ShouldExit::False)
}
Expand All @@ -76,8 +79,9 @@ impl Operation for GetFunctionAsync {

fn execute(context: &mut Context<'_>) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
let method = context.vm.read::<u8>() != 0;
let code = context.vm.frame().code_block.functions[index as usize].clone();
let function = create_function_object(code, true, false, None, context);
let function = create_function_object(code, true, false, None, method, context);
context.vm.push(function);
Ok(ShouldExit::False)
}
Expand Down
6 changes: 4 additions & 2 deletions boa_engine/src/vm/opcode/get/generator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ impl Operation for GetGenerator {

fn execute(context: &mut Context<'_>) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
let method = context.vm.read::<u8>() != 0;
let code = context.vm.frame().code_block.functions[index as usize].clone();
let function = create_generator_function_object(code, false, context);
let function = create_generator_function_object(code, false, method, context);
context.vm.push(function);
Ok(ShouldExit::False)
}
Expand All @@ -36,8 +37,9 @@ impl Operation for GetGeneratorAsync {

fn execute(context: &mut Context<'_>) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
let method = context.vm.read::<u8>() != 0;
let code = context.vm.frame().code_block.functions[index as usize].clone();
let function = create_generator_function_object(code, true, context);
let function = create_generator_function_object(code, true, method, context);
context.vm.push(function);
Ok(ShouldExit::False)
}
Expand Down
12 changes: 6 additions & 6 deletions boa_engine/src/vm/opcode/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1207,42 +1207,42 @@ generate_impl! {

/// Get arrow function from the pre-compiled inner functions.
///
/// Operands: address: `u32`
/// Operands: address: `u32`, method: `u8`
///
/// Stack: **=>** func
GetArrowFunction,

/// Get async arrow function from the pre-compiled inner functions.
///
/// Operands: address: `u32`
/// Operands: address: `u32`, method: `u8`
///
/// Stack: **=>** func
GetAsyncArrowFunction,

/// Get function from the pre-compiled inner functions.
///
/// Operands: address: `u32`
/// Operands: address: `u32`, method: `u8`
///
/// Stack: **=>** func
GetFunction,

/// Get async function from the pre-compiled inner functions.
///
/// Operands: address: `u32`
/// Operands: address: `u32`, method: `u8`
///
/// Stack: **=>** func
GetFunctionAsync,

/// Get generator function from the pre-compiled inner functions.
///
/// Operands: address: `u32`
/// Operands: address: `u32`, method: `u8`
///
/// Stack: **=>** func
GetGenerator,

/// Get async generator function from the pre-compiled inner functions.
///
/// Operands: address: `u32`
/// Operands: address: `u32`, method: `u8`
///
/// Stack: **=>** func
GetGeneratorAsync,
Expand Down