Skip to content

Commit

Permalink
Fix super call execution order
Browse files Browse the repository at this point in the history
  • Loading branch information
raskad committed Mar 23, 2023
1 parent 9b53859 commit 9a79ce8
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 47 deletions.
2 changes: 2 additions & 0 deletions boa_engine/src/bytecompiler/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,8 @@ impl ByteCompiler<'_, '_> {
}
Expression::Class(class) => self.class(class, true),
Expression::SuperCall(super_call) => {
self.emit_opcode(Opcode::SuperCallPrepare);

let contains_spread = super_call
.arguments()
.iter()
Expand Down
1 change: 1 addition & 0 deletions boa_engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ impl CodeBlock {
| Opcode::SuperCallDerived
| Opcode::Await
| Opcode::PushNewTarget
| Opcode::SuperCallPrepare
| Opcode::CallEvalSpread
| Opcode::CallSpread
| Opcode::NewSpread
Expand Down
1 change: 1 addition & 0 deletions boa_engine/src/vm/flowgraph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ impl CodeBlock {
| Opcode::CallSpread
| Opcode::NewSpread
| Opcode::SuperCallSpread
| Opcode::SuperCallPrepare
| Opcode::SetPrototype
| Opcode::IsObject
| Opcode::Nop
Expand Down
106 changes: 62 additions & 44 deletions boa_engine/src/vm/opcode/environment/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,46 @@ impl Operation for Super {
}
}

/// `SuperCallPrepare` implements the Opcode Operation for `Opcode::SuperCallPrepare`
///
/// Operation:
/// - Get the super constructor and the new target of the current environment.
#[derive(Debug, Clone, Copy)]
pub(crate) struct SuperCallPrepare;

impl Operation for SuperCallPrepare {
const NAME: &'static str = "SuperCallPrepare";
const INSTRUCTION: &'static str = "INST - SuperCallPrepare";

fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
let this_env = context
.realm
.environments
.get_this_environment()
.as_function_slots()
.expect("super call must be in function environment");
let this_env_borrow = this_env.borrow();
let new_target = this_env_borrow
.new_target()
.expect("must have new target")
.clone();
let active_function = this_env_borrow.function_object().clone();
drop(this_env_borrow);
let super_constructor = active_function
.__get_prototype_of__(context)
.expect("function object must have prototype");

if let Some(constructor) = super_constructor {
context.vm.push(constructor);
} else {
context.vm.push(JsValue::Null);
}
context.vm.push(new_target);

Ok(CompletionType::Normal)
}
}

/// `SuperCall` implements the Opcode Operation for `Opcode::SuperCall`
///
/// Operation:
Expand All @@ -103,33 +143,20 @@ impl Operation for SuperCall {
}
arguments.reverse();

let (new_target, active_function) = {
let this_env = context
.realm
.environments
.get_this_environment()
.as_function_slots()
.expect("super call must be in function environment");
let this_env_borrow = this_env.borrow();
let new_target = this_env_borrow
.new_target()
.expect("must have new target")
.clone();
let active_function = this_env.borrow().function_object().clone();
(new_target, active_function)
};
let super_constructor = active_function
.__get_prototype_of__(context)
.expect("function object must have prototype")
.expect("function object must have prototype");
let new_target_value = context.vm.pop();
let super_constructor = context.vm.pop();

if !super_constructor.is_constructor() {
let new_target = new_target_value
.as_object()
.expect("new target must be object");

let Some(super_constructor) = super_constructor.as_constructor() else {
return Err(JsNativeError::typ()
.with_message("super constructor object must be constructor")
.into());
}
};

let result = super_constructor.__construct__(&arguments, &new_target, context)?;
let result = super_constructor.__construct__(&arguments, new_target, context)?;

let this_env = context
.realm
Expand All @@ -144,6 +171,8 @@ impl Operation for SuperCall {
.into());
}

let active_function = this_env.borrow().function_object().clone();

result.initialize_instance_elements(&active_function, context)?;

context.vm.push(result);
Expand Down Expand Up @@ -175,33 +204,20 @@ impl Operation for SuperCallSpread {
.expect("arguments array in call spread function must be dense")
.clone();

let (new_target, active_function) = {
let this_env = context
.realm
.environments
.get_this_environment()
.as_function_slots()
.expect("super call must be in function environment");
let this_env_borrow = this_env.borrow();
let new_target = this_env_borrow
.new_target()
.expect("must have new target")
.clone();
let active_function = this_env.borrow().function_object().clone();
(new_target, active_function)
};
let super_constructor = active_function
.__get_prototype_of__(context)
.expect("function object must have prototype")
.expect("function object must have prototype");
let new_target_value = context.vm.pop();
let super_constructor = context.vm.pop();

if !super_constructor.is_constructor() {
let new_target = new_target_value
.as_object()
.expect("new target must be object");

let Some(super_constructor) = super_constructor.as_constructor() else {
return Err(JsNativeError::typ()
.with_message("super constructor object must be constructor")
.into());
}
};

let result = super_constructor.__construct__(&arguments, &new_target, context)?;
let result = super_constructor.__construct__(&arguments, new_target, context)?;

let this_env = context
.realm
Expand All @@ -216,6 +232,8 @@ impl Operation for SuperCallSpread {
.into());
}

let active_function = this_env.borrow().function_object().clone();

result.initialize_instance_elements(&active_function, context)?;

context.vm.push(result);
Expand Down
11 changes: 9 additions & 2 deletions boa_engine/src/vm/opcode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1200,18 +1200,25 @@ generate_impl! {
/// Stack: **=>** super
Super,

/// Get the super constructor and the new target of the current environment.
///
/// Operands:
///
/// Stack: **=>** super_constructor, new_target
SuperCallPrepare,

/// Execute the `super()` method.
///
/// Operands: argument_count: `u32`
///
/// Stack: argument_1, ... argument_n **=>**
/// Stack: super_constructor, new_target, argument_1, ... argument_n **=>**
SuperCall,

/// Execute the `super()` method where the arguments contain spreads.
///
/// Operands:
///
/// Stack: arguments_array **=>**
/// Stack: super_constructor, new_target, arguments_array **=>**
SuperCallSpread,

/// Execute the `super()` method when no constructor of the class is defined.
Expand Down
31 changes: 30 additions & 1 deletion boa_engine/src/vm/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{run_test_actions, JsValue, TestAction};
use crate::{builtins::error::ErrorKind, run_test_actions, JsValue, TestAction};
use indoc::indoc;

#[test]
Expand Down Expand Up @@ -178,6 +178,35 @@ fn get_reference_by_super() {
)]);
}

#[test]
fn super_call_constructor_null() {
run_test_actions([TestAction::assert_native_error(
indoc! {r#"
class A extends Object {
constructor() {
Object.setPrototypeOf(A, null);
super(A);
}
}
new A();
"#},
ErrorKind::Type,
"super constructor object must be constructor",
)]);
}

#[test]
fn super_call_get_constructor_before_arguments_execution() {
run_test_actions([TestAction::assert(indoc! {r#"
class A extends Object {
constructor() {
super(Object.setPrototypeOf(A, null));
}
}
new A() instanceof A;
"#})]);
}

#[test]
fn order_of_execution_in_assigment() {
run_test_actions([
Expand Down

0 comments on commit 9a79ce8

Please sign in to comment.