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

Jump to bottom

[Merged by Bors] - Fix spread arguments in function calls #2216

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - Fix spread arguments in function calls #2216

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
62 changes: 42 additions & 20 deletions boa_engine/src/bytecompiler/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1234,15 +1234,30 @@ impl<'b> ByteCompiler<'b> {
}
Node::ClassExpr(class) => self.class(class, true)?,
Node::SuperCall(super_call) => {
for arg in super_call.args() {
self.compile_expr(arg, true)?;
let contains_spread = super_call
.args()
.iter()
.any(|arg| matches!(arg, Node::Spread(_)));

if contains_spread {
self.emit_opcode(Opcode::PushNewArray);
for arg in super_call.args() {
self.compile_expr(arg, true)?;
if let Node::Spread(_) = arg {
self.emit_opcode(Opcode::InitIterator);
self.emit_opcode(Opcode::PushIteratorToArray);
} else {
self.emit_opcode(Opcode::PushValueToArray);
}
}
} else {
for arg in super_call.args() {
self.compile_expr(arg, true)?;
}
}

let last_is_rest_parameter =
matches!(super_call.args().last(), Some(Node::Spread(_)));

if last_is_rest_parameter {
self.emit(Opcode::SuperCallWithRest, &[super_call.args().len() as u32]);
if contains_spread {
self.emit_opcode(Opcode::SuperCallSpread);
} else {
self.emit(Opcode::SuperCall, &[super_call.args().len() as u32]);
}
Expand Down Expand Up @@ -2144,24 +2159,31 @@ impl<'b> ByteCompiler<'b> {
}
}

for arg in call.args().iter() {
self.compile_expr(arg, true)?;
}
let contains_spread = call.args().iter().any(|arg| matches!(arg, Node::Spread(_)));

let last_is_rest_parameter = matches!(call.args().last(), Some(Node::Spread(_)));
if contains_spread {
self.emit_opcode(Opcode::PushNewArray);
for arg in call.args() {
self.compile_expr(arg, true)?;
if let Node::Spread(_) = arg {
self.emit_opcode(Opcode::InitIterator);
self.emit_opcode(Opcode::PushIteratorToArray);
} else {
self.emit_opcode(Opcode::PushValueToArray);
}
}
} else {
for arg in call.args() {
self.compile_expr(arg, true)?;
}
}

match kind {
CallKind::CallEval if last_is_rest_parameter => {
self.emit(Opcode::CallEvalWithRest, &[call.args().len() as u32]);
}
CallKind::CallEval if contains_spread => self.emit_opcode(Opcode::CallEvalSpread),
CallKind::CallEval => self.emit(Opcode::CallEval, &[call.args().len() as u32]),
CallKind::Call if last_is_rest_parameter => {
self.emit(Opcode::CallWithRest, &[call.args().len() as u32]);
}
CallKind::Call if contains_spread => self.emit_opcode(Opcode::CallSpread),
CallKind::Call => self.emit(Opcode::Call, &[call.args().len() as u32]),
CallKind::New if last_is_rest_parameter => {
self.emit(Opcode::NewWithRest, &[call.args().len() as u32]);
}
CallKind::New if contains_spread => self.emit_opcode(Opcode::NewSpread),
CallKind::New => self.emit(Opcode::New, &[call.args().len() as u32]),
}

Expand Down
9 changes: 9 additions & 0 deletions boa_engine/src/object/property_map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,15 @@ impl PropertyMap {
self.indexed_properties = IndexedProperties::Dense(properties);
}

/// Returns the vec of dense indexed properties if they exist.
pub(crate) fn dense_indexed_properties(&self) -> Option<&Vec<JsValue>> {
if let IndexedProperties::Dense(properties) = &self.indexed_properties {
Some(properties)
} else {
None
}
}

/// An iterator visiting all key-value pairs in arbitrary order. The iterator element type is `(PropertyKey, &'a Property)`.
///
/// This iterator does not recurse down the prototype chain.
Expand Down
8 changes: 4 additions & 4 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 @@ -199,13 +199,9 @@ impl CodeBlock {
| Opcode::LogicalOr
| Opcode::Coalesce
| Opcode::CallEval
| Opcode::CallEvalWithRest
| Opcode::Call
| Opcode::CallWithRest
| Opcode::New
| Opcode::NewWithRest
| Opcode::SuperCall
| Opcode::SuperCallWithRest
| Opcode::ForInLoopInitIterator
| Opcode::ForInLoopNext
| Opcode::ConcatToString
Expand Down Expand Up @@ -372,6 +368,10 @@ impl CodeBlock {
| Opcode::PushClassField
| Opcode::SuperCallDerived
| Opcode::Await
| Opcode::CallEvalSpread
| Opcode::CallSpread
| Opcode::NewSpread
| Opcode::SuperCallSpread
| Opcode::Nop => String::new(),
}
}
Expand Down
113 changes: 53 additions & 60 deletions boa_engine/src/vm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1561,21 +1561,18 @@ impl Context {
}
self.vm.push(result);
}
Opcode::SuperCallWithRest => {
let argument_count = self.vm.read::<u32>();
let rest_argument = self.vm.pop();
let mut arguments = Vec::with_capacity(argument_count as usize);
for _ in 0..(argument_count - 1) {
arguments.push(self.vm.pop());
}
arguments.reverse();

let iterator_record = rest_argument.get_iterator(self, None, None)?;
let mut rest_arguments = Vec::new();
while let Some(next) = iterator_record.step(self)? {
rest_arguments.push(next.value(self)?);
}
arguments.append(&mut rest_arguments);
Opcode::SuperCallSpread => {
// Get the arguments that are stored as an array object on the stack.
let arguments_array = self.vm.pop();
let arguments_array_object = arguments_array
.as_object()
.expect("arguments array in call spread function must be an object");
let arguments = arguments_array_object
.borrow()
.properties()
.dense_indexed_properties()
.expect("arguments array in call spread function must be dense")
.clone();

let (new_target, active_function) = {
let this_env = self
Expand Down Expand Up @@ -1742,27 +1739,26 @@ impl Context {
self.vm.push(result);
}
}
Opcode::CallEvalWithRest => {
Opcode::CallEvalSpread => {
if self.vm.stack_size_limit <= self.vm.stack.len() {
return self.throw_range_error("Maximum call stack size exceeded");
}
let argument_count = self.vm.read::<u32>();
let rest_argument = self.vm.pop();
let mut arguments = Vec::with_capacity(argument_count as usize);
for _ in 0..(argument_count - 1) {
arguments.push(self.vm.pop());
}
arguments.reverse();

// Get the arguments that are stored as an array object on the stack.
let arguments_array = self.vm.pop();
let arguments_array_object = arguments_array
.as_object()
.expect("arguments array in call spread function must be an object");
let arguments = arguments_array_object
.borrow()
.properties()
.dense_indexed_properties()
.expect("arguments array in call spread function must be dense")
.clone();

let func = self.vm.pop();
let this = self.vm.pop();

let iterator_record = rest_argument.get_iterator(self, None, None)?;
let mut rest_arguments = Vec::new();
while let Some(next) = iterator_record.step(self)? {
rest_arguments.push(next.value(self)?);
}
arguments.append(&mut rest_arguments);

let object = match func {
JsValue::Object(ref object) if object.is_callable() => object.clone(),
_ => return self.throw_type_error("not a callable function"),
Expand Down Expand Up @@ -1809,27 +1805,26 @@ impl Context {

self.vm.push(result);
}
Opcode::CallWithRest => {
Opcode::CallSpread => {
if self.vm.stack_size_limit <= self.vm.stack.len() {
return self.throw_range_error("Maximum call stack size exceeded");
}
let argument_count = self.vm.read::<u32>();
let rest_argument = self.vm.pop();
let mut arguments = Vec::with_capacity(argument_count as usize);
for _ in 0..(argument_count - 1) {
arguments.push(self.vm.pop());
}
arguments.reverse();

// Get the arguments that are stored as an array object on the stack.
let arguments_array = self.vm.pop();
let arguments_array_object = arguments_array
.as_object()
.expect("arguments array in call spread function must be an object");
let arguments = arguments_array_object
.borrow()
.properties()
.dense_indexed_properties()
.expect("arguments array in call spread function must be dense")
.clone();

let func = self.vm.pop();
let this = self.vm.pop();

let iterator_record = rest_argument.get_iterator(self, None, None)?;
let mut rest_arguments = Vec::new();
while let Some(next) = iterator_record.step(self)? {
rest_arguments.push(next.value(self)?);
}
arguments.append(&mut rest_arguments);

let object = match func {
JsValue::Object(ref object) if object.is_callable() => object.clone(),
_ => return self.throw_type_error("not a callable function"),
Expand Down Expand Up @@ -1858,25 +1853,23 @@ impl Context {

self.vm.push(result);
}
Opcode::NewWithRest => {
Opcode::NewSpread => {
if self.vm.stack_size_limit <= self.vm.stack.len() {
return self.throw_range_error("Maximum call stack size exceeded");
}
let argument_count = self.vm.read::<u32>();
let rest_argument = self.vm.pop();
let mut arguments = Vec::with_capacity(argument_count as usize);
for _ in 0..(argument_count - 1) {
arguments.push(self.vm.pop());
}
arguments.reverse();
let func = self.vm.pop();
// Get the arguments that are stored as an array object on the stack.
let arguments_array = self.vm.pop();
let arguments_array_object = arguments_array
.as_object()
.expect("arguments array in call spread function must be an object");
let arguments = arguments_array_object
.borrow()
.properties()
.dense_indexed_properties()
.expect("arguments array in call spread function must be dense")
.clone();

let iterator_record = rest_argument.get_iterator(self, None, None)?;
let mut rest_arguments = Vec::new();
while let Some(next) = iterator_record.step(self)? {
rest_arguments.push(next.value(self)?);
}
arguments.append(&mut rest_arguments);
let func = self.vm.pop();

let result = func
.as_constructor()
Expand Down
48 changes: 24 additions & 24 deletions boa_engine/src/vm/opcode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -877,12 +877,12 @@ pub enum Opcode {
/// Stack: argument_1, ... argument_n **=>**
SuperCall,

/// Execute the `super()` method where the last argument is a rest parameter.
/// Execute the `super()` method where the arguments contain spreads.
///
/// Operands: argument_count: `u32`
/// Operands:
///
/// Stack: argument_1, ... argument_n **=>**
SuperCallWithRest,
/// Stack: arguments_array **=>**
SuperCallSpread,

/// Execute the `super()` method when no constructor of the class is defined.
///
Expand Down Expand Up @@ -941,12 +941,12 @@ pub enum Opcode {
/// Stack: func, this, argument_1, ... argument_n **=>** result
CallEval,

/// Call a function named "eval" where the last argument is a rest parameter.
/// Call a function named "eval" where the arguments contain spreads.
///
/// Operands: argument_count: `u32`
/// Operands:
///
/// Stack: func, this, argument_1, ... argument_n **=>** result
CallEvalWithRest,
/// Stack: arguments_array, func, this **=>** result
CallEvalSpread,

/// Call a function.
///
Expand All @@ -955,12 +955,12 @@ pub enum Opcode {
/// Stack: func, this, argument_1, ... argument_n **=>** result
Call,

/// Call a function where the last argument is a rest parameter.
/// Call a function where the arguments contain spreads.
///
/// Operands: argument_count: `u32`
/// Operands:
///
/// Stack: func, this, argument_1, ... argument_n **=>** result
CallWithRest,
/// Stack: arguments_array, func, this **=>** result
CallSpread,

/// Call construct on a function.
///
Expand All @@ -969,12 +969,12 @@ pub enum Opcode {
/// Stack: func, argument_1, ... argument_n **=>** result
New,

/// Call construct on a function where the last argument is a rest parameter.
/// Call construct on a function where the arguments contain spreads.
///
/// Operands: argument_count: `u32`
/// Operands:
///
/// Stack: func, argument_1, ... argument_n **=>** result
NewWithRest,
/// Stack: arguments_array, func **=>** result
NewSpread,

/// Return from a function.
///
Expand Down Expand Up @@ -1292,7 +1292,7 @@ impl Opcode {
Self::This => "This",
Self::Super => "Super",
Self::SuperCall => "SuperCall",
Self::SuperCallWithRest => "SuperCallWithRest",
Self::SuperCallSpread => "SuperCallWithRest",
Self::SuperCallDerived => "SuperCallDerived",
Self::Case => "Case",
Self::Default => "Default",
Expand All @@ -1301,11 +1301,11 @@ impl Opcode {
Self::GetGenerator => "GetGenerator",
Self::GetGeneratorAsync => "GetGeneratorAsync",
Self::CallEval => "CallEval",
Self::CallEvalWithRest => "CallEvalWithRest",
Self::CallEvalSpread => "CallEvalSpread",
Self::Call => "Call",
Self::CallWithRest => "CallWithRest",
Self::CallSpread => "CallSpread",
Self::New => "New",
Self::NewWithRest => "NewWithRest",
Self::NewSpread => "NewSpread",
Self::Return => "Return",
Self::PushDeclarativeEnvironment => "PushDeclarativeEnvironment",
Self::PushFunctionEnvironment => "PushFunctionEnvironment",
Expand Down Expand Up @@ -1434,7 +1434,7 @@ impl Opcode {
Self::This => "INST - This",
Self::Super => "INST - Super",
Self::SuperCall => "INST - SuperCall",
Self::SuperCallWithRest => "INST - SuperCallWithRest",
Self::SuperCallSpread => "INST - SuperCallWithRest",
Self::SuperCallDerived => "INST - SuperCallDerived",
Self::Case => "INST - Case",
Self::Default => "INST - Default",
Expand All @@ -1443,11 +1443,11 @@ impl Opcode {
Self::GetGenerator => "INST - GetGenerator",
Self::GetGeneratorAsync => "INST - GetGeneratorAsync",
Self::CallEval => "INST - CallEval",
Self::CallEvalWithRest => "INST - CallEvalWithRest",
Self::CallEvalSpread => "INST - CallEvalSpread",
Self::Call => "INST - Call",
Self::CallWithRest => "INST - CallWithRest",
Self::CallSpread => "INST - CallSpread",
Self::New => "INST - New",
Self::NewWithRest => "INST - NewWithRest",
Self::NewSpread => "INST - NewSpread",
Self::Return => "INST - Return",
Self::PushDeclarativeEnvironment => "INST - PushDeclarativeEnvironment",
Self::PushFunctionEnvironment => "INST - PushFunctionEnvironment",
Expand Down