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

Move ordinary function [[ConstructorKind]] to CodeBlock #3439

Merged
merged 1 commit into from
Oct 30, 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
20 changes: 5 additions & 15 deletions boa_engine/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,6 @@ unsafe impl Trace for ClassFieldDefinition {
pub(crate) enum FunctionKind {
/// A bytecode function.
Ordinary {
/// The `[[ConstructorKind]]` internal slot.
constructor_kind: ConstructorKind,

/// The `[[Fields]]` internal slot.
fields: ThinVec<ClassFieldDefinition>,

Expand Down Expand Up @@ -248,15 +245,8 @@ impl OrdinaryFunction {
}

/// Returns true if the function object is a derived constructor.
pub(crate) const fn is_derived_constructor(&self) -> bool {
if let FunctionKind::Ordinary {
constructor_kind, ..
} = self.kind
{
constructor_kind.is_derived()
} else {
false
}
pub(crate) fn is_derived_constructor(&self) -> bool {
self.code.is_derived_constructor()
}

/// Does this function have the `[[ClassFieldInitializerName]]` internal slot set to non-empty value.
Expand Down Expand Up @@ -330,9 +320,9 @@ impl OrdinaryFunction {
&self.kind
}

/// Gets a mutable reference to the [`FunctionKind`] of the `Function`.
pub(crate) fn kind_mut(&mut self) -> &mut FunctionKind {
&mut self.kind
/// Check if function is [`FunctionKind::Ordinary`].
pub(crate) const fn is_ordinary(&self) -> bool {
matches!(self.kind(), FunctionKind::Ordinary { .. })
}
}

Expand Down
6 changes: 6 additions & 0 deletions boa_engine/src/bytecompiler/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ impl ByteCompiler<'_, '_> {
}
compiler.emit_opcode(Opcode::SetReturnValue);

// 17. If ClassHeritageopt is present, set F.[[ConstructorKind]] to derived.
compiler.code_block_flags.set(
CodeBlockFlags::IS_DERIVED_CONSTRUCTOR,
class.super_ref().is_some(),
);

let code = Gc::new(compiler.finish());
let index = self.push_function_to_constants(code);
self.emit(
Expand Down
35 changes: 17 additions & 18 deletions boa_engine/src/object/internal_methods/function.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
builtins::function::{arguments::Arguments, FunctionKind, ThisMode},
builtins::function::{arguments::Arguments, ThisMode},
context::intrinsics::StandardConstructors,
environments::{FunctionSlots, ThisBindingStatus},
native_function::NativeFunctionObject,
Expand Down Expand Up @@ -48,13 +48,15 @@ pub(crate) fn function_call(
let function = object.as_function().expect("not a function");
let realm = function.realm().clone();

if let FunctionKind::Ordinary { .. } = function.kind() {
if function.code.is_class_constructor() {
return Err(JsNativeError::typ()
.with_message("class constructor cannot be invoked without 'new'")
.with_realm(realm)
.into());
}
if function.code.is_class_constructor() {
debug_assert!(
function.is_ordinary(),
"only ordinary functions can be classes"
);
return Err(JsNativeError::typ()
.with_message("class constructor cannot be invoked without 'new'")
.with_realm(realm)
.into());
}

let code = function.code.clone();
Expand Down Expand Up @@ -172,17 +174,14 @@ fn function_construct(
let function = object.as_function().expect("not a function");
let realm = function.realm().clone();

let FunctionKind::Ordinary {
constructor_kind, ..
} = function.kind()
else {
unreachable!("not a constructor")
};
debug_assert!(
function.is_ordinary(),
"only ordinary functions can be constructed"
);

let code = function.code.clone();
let environments = function.environments.clone();
let script_or_module = function.script_or_module.clone();
let constructor_kind = *constructor_kind;
drop(object);

let env_fp = environments.len() as u32;
Expand All @@ -191,7 +190,9 @@ fn function_construct(

let at = context.vm.stack.len() - argument_count;

let this = if constructor_kind.is_base() {
let this = if code.is_derived_constructor() {
None
} else {
// If the prototype of the constructor is not an object, then use the default object
// prototype as prototype for the new object
// see <https://tc39.es/ecma262/#sec-ordinarycreatefromconstructor>
Expand All @@ -207,8 +208,6 @@ fn function_construct(
this.initialize_instance_elements(this_function_object, context)?;

Some(this)
} else {
None
};

let frame = CallFrame::new(code.clone(), script_or_module, environments, realm)
Expand Down
14 changes: 11 additions & 3 deletions boa_engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! This module is for the `CodeBlock` which implements a function representation in the VM

use crate::{
builtins::function::{ConstructorKind, FunctionKind, OrdinaryFunction, ThisMode},
builtins::function::{FunctionKind, OrdinaryFunction, ThisMode},
environments::{BindingLocator, CompileTimeEnvironment},
object::{JsObject, ObjectData, PROTOTYPE},
property::PropertyDescriptor,
Expand Down Expand Up @@ -69,6 +69,9 @@ bitflags! {
/// The `[[ClassFieldInitializerName]]` internal slot.
const IN_CLASS_FIELD_INITIALIZER = 0b0010_0000;

/// `[[ConstructorKind]]`
const IS_DERIVED_CONSTRUCTOR = 0b0100_0000;

/// Trace instruction execution to `stdout`.
#[cfg(feature = "trace")]
const TRACEABLE = 0b1000_0000;
Expand Down Expand Up @@ -241,6 +244,13 @@ impl CodeBlock {
.contains(CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER)
}

/// Returns true if this function is a derived constructor.
pub(crate) fn is_derived_constructor(&self) -> bool {
self.flags
.get()
.contains(CodeBlockFlags::IS_DERIVED_CONSTRUCTOR)
}

/// 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)> {
Expand Down Expand Up @@ -838,7 +848,6 @@ pub(crate) fn create_function_object(
home_object: None,
script_or_module,
kind: FunctionKind::Ordinary {
constructor_kind: ConstructorKind::Base,
fields: ThinVec::new(),
private_methods: ThinVec::new(),
},
Expand Down Expand Up @@ -903,7 +912,6 @@ pub(crate) fn create_function_object_fast(
FunctionKind::Async
} else {
FunctionKind::Ordinary {
constructor_kind: ConstructorKind::Base,
fields: ThinVec::new(),
private_methods: ThinVec::new(),
}
Expand Down
16 changes: 0 additions & 16 deletions boa_engine/src/vm/opcode/push/class/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::{
builtins::function::{ConstructorKind, FunctionKind},
error::JsNativeError,
object::PROTOTYPE,
vm::{opcode::Operation, CompletionType},
Expand Down Expand Up @@ -69,21 +68,6 @@ impl Operation for PushClassPrototype {
class_object.set_prototype(Some(constructor_parent));
}

let mut class_object_mut = class_object.borrow_mut();
let class_function = class_object_mut
.as_function_mut()
.expect("class must be function object");

// 17. If ClassHeritageopt is present, set F.[[ConstructorKind]] to derived.
if let FunctionKind::Ordinary {
constructor_kind, ..
} = class_function.kind_mut()
{
*constructor_kind = ConstructorKind::Derived;
}

drop(class_object_mut);

context.vm.push(class);
context.vm.push(proto_parent);

Expand Down
Loading