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

[Merged by Bors] - Fix order dependent execution in assignment. #2378

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 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
234 changes: 147 additions & 87 deletions boa_engine/src/bytecompiler/mod.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion boa_engine/src/vm/opcode/define/class/getter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ impl Operation for DefineClassGetterByName {

fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
let object = context.vm.pop();
let value = context.vm.pop();
let object = context.vm.pop();
let object = object.to_object(context)?;
value
.as_object()
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/vm/opcode/define/class/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ impl Operation for DefineClassMethodByName {

fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
let object = context.vm.pop();
let value = context.vm.pop();
let object = context.vm.pop();
let object = if let Some(object) = object.as_object() {
object.clone()
} else {
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/vm/opcode/define/class/setter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ impl Operation for DefineClassSetterByName {

fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
let object = context.vm.pop();
let value = context.vm.pop();
let object = context.vm.pop();
let object = object.to_object(context)?;
value
.as_object()
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/vm/opcode/define/own_property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ impl Operation for DefineOwnPropertyByName {

fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
let object = context.vm.pop();
let value = context.vm.pop();
let object = context.vm.pop();
let object = if let Some(object) = object.as_object() {
object.clone()
} else {
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/vm/opcode/delete/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ impl Operation for DeletePropertyByValue {
const INSTRUCTION: &'static str = "INST - DeletePropertyByValue";

fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let object = context.vm.pop();
let key = context.vm.pop();
let object = context.vm.pop();
let result = object
.to_object(context)?
.__delete__(&key.to_property_key(context)?, context)?;
Expand Down
4 changes: 2 additions & 2 deletions boa_engine/src/vm/opcode/get/property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ impl Operation for GetPropertyByValue {
const INSTRUCTION: &'static str = "INST - GetPropertyByValue";

fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let object = context.vm.pop();
let key = context.vm.pop();
let object = context.vm.pop();
let object = if let Some(object) = object.as_object() {
object.clone()
} else {
Expand All @@ -78,8 +78,8 @@ impl Operation for GetPropertyByValuePush {
const INSTRUCTION: &'static str = "INST - GetPropertyByValuePush";

fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let object = context.vm.pop();
let key = context.vm.pop();
let object = context.vm.pop();
let object = if let Some(object) = object.as_object() {
object.clone()
} else {
Expand Down
24 changes: 12 additions & 12 deletions boa_engine/src/vm/opcode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ generate_impl! {
///
/// Operands:
///
/// Stack: key, object **=>** value
/// Stack: object, key **=>** value
GetPropertyByValue,

/// Get a property by value from an object an push the key and value on the stack.
Expand All @@ -711,7 +711,7 @@ generate_impl! {
///
/// Operands:
///
/// Stack: key, object **=>** key, value
/// Stack: object, key **=>** key, value
GetPropertyByValuePush,

/// Sets a property by name of an object.
Expand All @@ -720,21 +720,21 @@ generate_impl! {
///
/// Operands: name_index: `u32`
///
/// Stack: value, object **=>**
/// Stack: object, value **=>** value
SetPropertyByName,

/// Defines a own property of an object by name.
///
/// Operands: name_index: `u32`
///
/// Stack: value, object **=>**
/// Stack: object, value **=>**
DefineOwnPropertyByName,

/// Defines a class method by name.
///
/// Operands: name_index: `u32`
///
/// Stack: value, object **=>**
/// Stack: object, value **=>**
DefineClassMethodByName,

/// Sets a property by value of an object.
Expand All @@ -743,7 +743,7 @@ generate_impl! {
///
/// Operands:
///
/// Stack: value, key, object **=>**
/// Stack: object, key, value **=>** value
SetPropertyByValue,

/// Defines a own property of an object by value.
Expand All @@ -766,7 +766,7 @@ generate_impl! {
///
/// Operands: name_index: `u32`
///
/// Stack: value, object **=>**
/// Stack: object, value **=>**
SetPropertyGetterByName,

/// Defines a getter class method by name.
Expand All @@ -775,7 +775,7 @@ generate_impl! {
///
/// Operands: name_index: `u32`
///
/// Stack: value, object **=>**
/// Stack: object, value **=>**
DefineClassGetterByName,

/// Sets a getter property by value of an object.
Expand All @@ -802,7 +802,7 @@ generate_impl! {
///
/// Operands: name_index: `u32`
///
/// Stack: value, object **=>**
/// Stack: object, value **=>**
SetPropertySetterByName,

/// Defines a setter class method by name.
Expand All @@ -811,7 +811,7 @@ generate_impl! {
///
/// Operands: name_index: `u32`
///
/// Stack: value, object **=>**
/// Stack: object, value **=>**
DefineClassSetterByName,

/// Sets a setter property by value of an object.
Expand All @@ -838,7 +838,7 @@ generate_impl! {
///
/// Operands: name_index: `u32`
///
/// Stack: object, value **=>**
/// Stack: object, value **=>** value
AssignPrivateField,

/// Set a private property of a class constructor by it's name.
Expand Down Expand Up @@ -936,7 +936,7 @@ generate_impl! {
///
/// Operands:
///
/// Stack: key, object **=>**
/// Stack: object, key **=>**
DeletePropertyByValue,

/// Copy all properties of one object to another object.
Expand Down
6 changes: 4 additions & 2 deletions boa_engine/src/vm/opcode/set/private.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ impl Operation for AssignPrivateField {
let mut object_borrow_mut = object.borrow_mut();
match object_borrow_mut.get_private_element(name.sym()) {
Some(PrivateElement::Field(_)) => {
object_borrow_mut.set_private_element(name.sym(), PrivateElement::Field(value));
object_borrow_mut
.set_private_element(name.sym(), PrivateElement::Field(value.clone()));
}
Some(PrivateElement::Method(_)) => {
return Err(JsNativeError::typ()
Expand All @@ -38,7 +39,7 @@ impl Operation for AssignPrivateField {
}) => {
let setter = setter.clone();
drop(object_borrow_mut);
setter.call(&object.clone().into(), &[value], context)?;
setter.call(&object.clone().into(), &[value.clone()], context)?;
}
None => {
return Err(JsNativeError::typ()
Expand All @@ -56,6 +57,7 @@ impl Operation for AssignPrivateField {
.with_message("cannot set private property on non-object")
.into());
}
context.vm.push(value);
Ok(ShouldExit::False)
}
}
Expand Down
16 changes: 9 additions & 7 deletions boa_engine/src/vm/opcode/set/property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ impl Operation for SetPropertyByName {
fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();

let object = context.vm.pop();
let value = context.vm.pop();
let object = context.vm.pop();
let object = if let Some(object) = object.as_object() {
object.clone()
} else {
Expand All @@ -33,7 +33,8 @@ impl Operation for SetPropertyByName {
.into_common::<JsString>(false)
.into();

object.set(name, value, context.vm.frame().code.strict, context)?;
object.set(name, value.clone(), context.vm.frame().code.strict, context)?;
context.vm.stack.push(value);
Ok(ShouldExit::False)
}
}
Expand All @@ -50,17 +51,18 @@ impl Operation for SetPropertyByValue {
const INSTRUCTION: &'static str = "INST - SetPropertyByValue";

fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let object = context.vm.pop();
let key = context.vm.pop();
let value = context.vm.pop();
let key = context.vm.pop();
let object = context.vm.pop();
let object = if let Some(object) = object.as_object() {
object.clone()
} else {
object.to_object(context)?
};

let key = key.to_property_key(context)?;
object.set(key, value, context.vm.frame().code.strict, context)?;
object.set(key, value.clone(), context.vm.frame().code.strict, context)?;
context.vm.stack.push(value);
Ok(ShouldExit::False)
}
}
Expand All @@ -78,8 +80,8 @@ impl Operation for SetPropertyGetterByName {

fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
let object = context.vm.pop();
let value = context.vm.pop();
let object = context.vm.pop();
let object = object.to_object(context)?;
let name = context.vm.frame().code.names[index as usize];
let name = context
Expand Down Expand Up @@ -155,8 +157,8 @@ impl Operation for SetPropertySetterByName {

fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let index = context.vm.read::<u32>();
let object = context.vm.pop();
let value = context.vm.pop();
let object = context.vm.pop();
let object = object.to_object(context)?;
let name = context.vm.frame().code.names[index as usize];
let name = context
Expand Down
6 changes: 3 additions & 3 deletions boa_engine/src/vm/opcode/unary_ops/decrement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ impl Operation for DecPost {
fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let value = context.vm.pop();
let value = value.to_numeric(context)?;
context.vm.push(value.clone());
match value {
Numeric::Number(number) => context.vm.push(number - 1f64),
Numeric::BigInt(bigint) => {
context.vm.push(JsBigInt::sub(&bigint, &JsBigInt::one()));
Numeric::BigInt(ref bigint) => {
context.vm.push(JsBigInt::sub(bigint, &JsBigInt::one()));
}
}
context.vm.push(value);
Ok(ShouldExit::False)
}
}
6 changes: 3 additions & 3 deletions boa_engine/src/vm/opcode/unary_ops/increment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ impl Operation for IncPost {
fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let value = context.vm.pop();
let value = value.to_numeric(context)?;
context.vm.push(value.clone());
match value {
Numeric::Number(number) => context.vm.push(number + 1f64),
Numeric::BigInt(bigint) => {
context.vm.push(JsBigInt::add(&bigint, &JsBigInt::one()));
Numeric::BigInt(ref bigint) => {
context.vm.push(JsBigInt::add(bigint, &JsBigInt::one()));
}
}
context.vm.push(value);
Ok(ShouldExit::False)
}
}