From fadd0464a8438e4d70989d291ffc473082391eed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Wed, 28 Oct 2020 23:49:13 +0100 Subject: [PATCH 01/15] Use `to_length` when accessing array length in `Array` module --- boa/src/builtins/array/mod.rs | 123 ++++++++++++++------------- boa/src/builtins/function/mod.rs | 2 +- boa/src/builtins/map/map_iterator.rs | 1 + boa/src/syntax/ast/node/array/mod.rs | 2 +- 4 files changed, 66 insertions(+), 62 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 1041a747f49..712cc1e2cfc 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -224,11 +224,15 @@ impl Array { /// /// `array_obj` can be any array with prototype already set (it will be wiped and /// recreated from `array_contents`) - pub(crate) fn construct_array(array_obj: &Value, array_contents: &[Value]) -> Result { + pub(crate) fn construct_array( + array_obj: &Value, + array_contents: &[Value], + context: &mut Context, + ) -> Result { let array_obj_ptr = array_obj.clone(); // Wipe existing contents of the array object - let orig_length = array_obj.get_field("length").as_number().unwrap() as i32; + let orig_length = array_obj.get_field("length").to_length(context)?; for n in 0..orig_length { array_obj_ptr.remove_property(n); } @@ -248,17 +252,21 @@ impl Array { /// Utility function which takes an existing array object and puts additional /// values on the end, correctly rewriting the length - pub(crate) fn add_to_array_object(array_ptr: &Value, add_values: &[Value]) -> Result { - let orig_length = array_ptr.get_field("length").as_number().unwrap() as i32; + pub(crate) fn add_to_array_object( + array_ptr: &Value, + add_values: &[Value], + context: &mut Context, + ) -> Result { + let orig_length = array_ptr.get_field("length").to_length(context)?; for (n, value) in add_values.iter().enumerate() { - let new_index = orig_length.wrapping_add(n as i32); + let new_index = orig_length.wrapping_add(n); array_ptr.set_field(new_index, value); } array_ptr.set_field( "length", - Value::from(orig_length.wrapping_add(add_values.len() as i32)), + Value::from(orig_length.wrapping_add(add_values.len())), ); Ok(array_ptr.clone()) @@ -294,7 +302,7 @@ impl Array { /// /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.concat /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/concat - pub(crate) fn concat(this: &Value, args: &[Value], _: &mut Context) -> Result { + pub(crate) fn concat(this: &Value, args: &[Value], context: &mut Context) -> Result { if args.is_empty() { // If concat is called with no arguments, it returns the original array return Ok(this.clone()); @@ -304,19 +312,19 @@ impl Array { // one) let mut new_values: Vec = Vec::new(); - let this_length = this.get_field("length").as_number().unwrap() as i32; + let this_length = this.get_field("length").to_length(context)?; for n in 0..this_length { new_values.push(this.get_field(n)); } for concat_array in args { - let concat_length = concat_array.get_field("length").as_number().unwrap() as i32; + let concat_length = concat_array.get_field("length").to_length(context)?; for n in 0..concat_length { new_values.push(concat_array.get_field(n)); } } - Self::construct_array(this, &new_values) + Self::construct_array(this, &new_values, context) } /// `Array.prototype.push( ...items )` @@ -331,8 +339,8 @@ impl Array { /// /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.push /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/push - pub(crate) fn push(this: &Value, args: &[Value], _: &mut Context) -> Result { - let new_array = Self::add_to_array_object(this, args)?; + pub(crate) fn push(this: &Value, args: &[Value], context: &mut Context) -> Result { + let new_array = Self::add_to_array_object(this, args, context)?; Ok(new_array.get_field("length")) } @@ -346,8 +354,8 @@ impl Array { /// /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.pop /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/pop - pub(crate) fn pop(this: &Value, _: &[Value], _: &mut Context) -> Result { - let curr_length = this.get_field("length").as_number().unwrap() as i32; + pub(crate) fn pop(this: &Value, _: &[Value], context: &mut Context) -> Result { + let curr_length = this.get_field("length").to_length(context)?; if curr_length < 1 { return Ok(Value::undefined()); @@ -377,7 +385,7 @@ impl Array { let callback_arg = args.get(0).expect("Could not get `callbackFn` argument."); let this_arg = args.get(1).cloned().unwrap_or_else(Value::undefined); - let length = this.get_field("length").as_number().unwrap() as i32; + let length = this.get_field("length").to_length(context)?; for i in 0..length { let element = this.get_field(i); @@ -412,7 +420,7 @@ impl Array { }; let mut elem_strs = Vec::new(); - let length = this.get_field("length").as_number().unwrap() as i32; + let length = this.get_field("length").to_length(context)?; for n in 0..length { let elem_str = this.get_field(n).to_string(context)?.to_string(); elem_strs.push(elem_str); @@ -473,10 +481,10 @@ impl Array { /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.reverse /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reverse #[allow(clippy::else_if_without_else)] - pub(crate) fn reverse(this: &Value, _: &[Value], _: &mut Context) -> Result { - let len = this.get_field("length").as_number().unwrap() as i32; + pub(crate) fn reverse(this: &Value, _: &[Value], context: &mut Context) -> Result { + let len = this.get_field("length").to_length(context)?; - let middle: i32 = len.wrapping_div(2); + let middle = len.wrapping_div(2); for lower in 0..middle { let upper = len.wrapping_sub(lower).wrapping_sub(1); @@ -512,8 +520,8 @@ impl Array { /// /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.shift /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/shift - pub(crate) fn shift(this: &Value, _: &[Value], _: &mut Context) -> Result { - let len = this.get_field("length").as_number().unwrap() as i32; + pub(crate) fn shift(this: &Value, _: &[Value], context: &mut Context) -> Result { + let len = this.get_field("length").to_length(context)?; if len == 0 { this.set_field("length", 0); @@ -553,10 +561,10 @@ impl Array { /// /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.unshift /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/unshift - pub(crate) fn unshift(this: &Value, args: &[Value], _: &mut Context) -> Result { - let len = this.get_field("length").as_number().unwrap() as i32; + pub(crate) fn unshift(this: &Value, args: &[Value], context: &mut Context) -> Result { + let len = this.get_field("length").to_length(context)?; - let arg_c: i32 = args.len() as i32; + let arg_c = args.len(); if arg_c > 0 { for k in (1..=len).rev() { @@ -571,12 +579,7 @@ impl Array { } } for j in 0..arg_c { - this.set_field( - j, - args.get(j as usize) - .expect("Could not get argument") - .clone(), - ); + this.set_field(j, args.get(j).expect("Could not get argument").clone()); } } @@ -611,7 +614,7 @@ impl Array { Value::undefined() }; let mut i = 0; - let max_len = this.get_field("length").as_number().unwrap() as i32; + let max_len = this.get_field("length").to_length(context)?; let mut len = max_len; while i < len { let element = this.get_field(i); @@ -620,10 +623,7 @@ impl Array { if !result.to_boolean() { return Ok(Value::from(false)); } - len = min( - max_len, - this.get_field("length").as_number().unwrap() as i32, - ); + len = min(max_len, this.get_field("length").to_length(context)?); i += 1; } Ok(Value::from(true)) @@ -669,7 +669,7 @@ impl Array { }) .collect(); - Self::construct_array(&new, &values) + Self::construct_array(&new, &values, context) } /// `Array.prototype.indexOf( searchElement[, fromIndex ] )` @@ -691,23 +691,24 @@ impl Array { /// /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.indexof /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/indexOf - pub(crate) fn index_of(this: &Value, args: &[Value], _: &mut Context) -> Result { + pub(crate) fn index_of(this: &Value, args: &[Value], context: &mut Context) -> Result { // If no arguments, return -1. Not described in spec, but is what chrome does. if args.is_empty() { return Ok(Value::from(-1)); } let search_element = args[0].clone(); - let len = this.get_field("length").as_number().unwrap() as i32; + let len = this.get_field("length").to_length(context)?; let mut idx = match args.get(1) { Some(from_idx_ptr) => { - let from_idx = from_idx_ptr.as_number().unwrap() as i32; + let from_idx = from_idx_ptr.as_number().unwrap() as isize; if from_idx < 0 { - len + from_idx + let k = len as isize + from_idx; + max(0, k) as usize } else { - from_idx + from_idx as usize } } None => 0, @@ -802,7 +803,7 @@ impl Array { } let callback = &args[0]; let this_arg = args.get(1).cloned().unwrap_or_else(Value::undefined); - let len = this.get_field("length").as_number().unwrap() as i32; + let len = this.get_field("length").to_length(context)?; for i in 0..len { let element = this.get_field(i); let arguments = [element.clone(), Value::from(i), this.clone()]; @@ -837,7 +838,7 @@ impl Array { let this_arg = args.get(1).cloned().unwrap_or_else(Value::undefined); - let length = this.get_field("length").as_number().unwrap() as i32; + let length = this.get_field("length").to_length(context)?; for i in 0..length { let element = this.get_field(i); @@ -846,11 +847,12 @@ impl Array { let result = context.call(predicate_arg, &this_arg, &arguments)?; if result.to_boolean() { - return Ok(Value::rational(f64::from(i))); + // TODO: make the case safe + return Ok(Value::integer(i as i32)); } } - Ok(Value::rational(-1_f64)) + Ok(Value::integer(-1)) } /// `Array.prototype.fill( value[, start[, end]] )` @@ -865,16 +867,16 @@ impl Array { /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.fill /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/fill pub(crate) fn fill(this: &Value, args: &[Value], context: &mut Context) -> Result { - let len: i32 = this.get_field("length").as_number().unwrap() as i32; + let len = this.get_field("length").to_length(context)? as isize; let default_value = Value::undefined(); let value = args.get(0).unwrap_or(&default_value); - let relative_start = args.get(1).unwrap_or(&default_value).to_number(context)? as i32; + let relative_start = args.get(1).unwrap_or(&default_value).to_number(context)? as isize; let relative_end_val = args.get(2).unwrap_or(&default_value); let relative_end = if relative_end_val.is_undefined() { len } else { - relative_end_val.to_number(context)? as i32 + relative_end_val.to_number(context)? as isize }; let start = if relative_start < 0 { max(len + relative_start, 0) @@ -904,10 +906,14 @@ impl Array { /// /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.includes /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes - pub(crate) fn includes_value(this: &Value, args: &[Value], _: &mut Context) -> Result { + pub(crate) fn includes_value( + this: &Value, + args: &[Value], + context: &mut Context, + ) -> Result { let search_element = args.get(0).cloned().unwrap_or_else(Value::undefined); - let length = this.get_field("length").as_number().unwrap() as i32; + let length = this.get_field("length").to_length(context)?; for idx in 0..length { let check_element = this.get_field(idx).clone(); @@ -936,14 +942,14 @@ impl Array { /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/slice pub(crate) fn slice(this: &Value, args: &[Value], context: &mut Context) -> Result { let new_array = Self::new_array(context)?; - let len = this.get_field("length").as_number().unwrap() as i32; + let len = this.get_field("length").to_length(context)? as isize; let start = match args.get(0) { - Some(v) => v.as_number().unwrap() as i32, + Some(v) => v.as_number().unwrap() as isize, None => 0, }; let end = match args.get(1) { - Some(v) => v.as_number().unwrap() as i32, + Some(v) => v.as_number().unwrap() as isize, None => len, }; @@ -989,7 +995,7 @@ impl Array { let callback = args.get(0).cloned().unwrap_or_else(Value::undefined); let this_val = args.get(1).cloned().unwrap_or_else(Value::undefined); - let length = this.get_field("length").as_number().unwrap() as i32; + let length = this.get_field("length").to_length(context)?; let new = Self::new_array(context)?; @@ -1011,7 +1017,7 @@ impl Array { }) .collect::>(); - Self::construct_array(&new, &values) + Self::construct_array(&new, &values, context) } /// Array.prototype.some ( callbackfn [ , thisArg ] ) @@ -1042,7 +1048,7 @@ impl Array { Value::undefined() }; let mut i = 0; - let max_len = this.get_field("length").as_number().unwrap() as i32; + let max_len = this.get_field("length").to_length(context)?; let mut len = max_len; while i < len { let element = this.get_field(i); @@ -1052,10 +1058,7 @@ impl Array { return Ok(Value::from(true)); } // the length of the array must be updated because the callback can mutate it. - len = min( - max_len, - this.get_field("length").as_number().unwrap() as i32, - ); + len = min(max_len, this.get_field("length").to_length(context)?); i += 1; } Ok(Value::from(false)) diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index 3eea345a585..1588531d3e0 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -119,7 +119,7 @@ impl Function { ) { // Create array of values let array = Array::new_array(context).unwrap(); - Array::add_to_array_object(&array, &args_list[index..]).unwrap(); + Array::add_to_array_object(&array, &args_list[index..], context).unwrap(); // Create binding local_env diff --git a/boa/src/builtins/map/map_iterator.rs b/boa/src/builtins/map/map_iterator.rs index 63e80f6061c..adf1eba5f2d 100644 --- a/boa/src/builtins/map/map_iterator.rs +++ b/boa/src/builtins/map/map_iterator.rs @@ -106,6 +106,7 @@ impl MapIterator { let result = Array::construct_array( &Array::new_array(context)?, &[key.clone(), value.clone()], + context, )?; return Ok(create_iter_result_object( context, result, false, diff --git a/boa/src/syntax/ast/node/array/mod.rs b/boa/src/syntax/ast/node/array/mod.rs index 4749777c650..a2e31f3ba0b 100644 --- a/boa/src/syntax/ast/node/array/mod.rs +++ b/boa/src/syntax/ast/node/array/mod.rs @@ -61,7 +61,7 @@ impl Executable for ArrayDecl { } } - Array::add_to_array_object(&array, &elements)?; + Array::add_to_array_object(&array, &elements, context)?; Ok(array) } } From 8dcf6c38ed4c7b96bc78aa01263247251a05b6bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Thu, 29 Oct 2020 00:16:22 +0100 Subject: [PATCH 02/15] Change stray `length` cast to `to_length` --- boa/src/builtins/array/mod.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 712cc1e2cfc..394e7311f6f 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -745,17 +745,14 @@ impl Array { /// /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.lastindexof /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/lastIndexOf - pub(crate) fn last_index_of(this: &Value, args: &[Value], _: &mut Context) -> Result { + pub(crate) fn last_index_of(this: &Value, args: &[Value], context: &mut Context) -> Result { // If no arguments, return -1. Not described in spec, but is what chrome does. if args.is_empty() { return Ok(Value::from(-1)); } let search_element = args[0].clone(); - let len = this - .get_field("length") - .as_number() - .expect("length was not a number") as i32; + let len = this.get_field("length").to_length(context)? as isize; let mut idx = match args.get(1) { Some(from_idx_ptr) => { From 03af4565e9d66618b4b4724eebf3e9e78344da89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Thu, 29 Oct 2020 00:21:02 +0100 Subject: [PATCH 03/15] Handle +/-Inf and NaNs in array functions It still needs some work to handle the cases correctly, but doesnt't panic now --- boa/src/builtins/array/mod.rs | 51 +++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 394e7311f6f..a08dd3569f1 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -702,13 +702,17 @@ impl Array { let mut idx = match args.get(1) { Some(from_idx_ptr) => { - let from_idx = from_idx_ptr.as_number().unwrap() as isize; + let from_idx = from_idx_ptr.to_number(context)?; - if from_idx < 0 { - let k = len as isize + from_idx; - max(0, k) as usize + if !from_idx.is_finite() { + return Ok(Value::from(-1)) } else { - from_idx as usize + if from_idx < 0.0 { + let k = len as isize + from_idx as isize; + max(0, k) as usize + } else { + from_idx as usize + } } } None => 0, @@ -756,12 +760,14 @@ impl Array { let mut idx = match args.get(1) { Some(from_idx_ptr) => { - let from_idx = from_idx_ptr.as_number().unwrap() as i32; + let from_idx = from_idx_ptr.to_integer(context)?; - if from_idx >= 0 { - min(from_idx, len - 1) + if !from_idx.is_finite() { + return Ok(Value::from(-1)) + } else if from_idx >= 0.0 { + min(from_idx as isize, len - 1) } else { - len + from_idx + len + from_idx as isize } } None => len - 1, @@ -771,7 +777,8 @@ impl Array { let check_element = this.get_field(idx).clone(); if check_element.strict_equals(&search_element) { - return Ok(Value::from(idx)); + // TODO: make the cast from isize to i32 safe + return Ok(Value::from(idx as i32)); } idx -= 1; @@ -942,23 +949,27 @@ impl Array { let len = this.get_field("length").to_length(context)? as isize; let start = match args.get(0) { - Some(v) => v.as_number().unwrap() as isize, - None => 0, + Some(v) => v.to_integer(interpreter)?, + None => 0.0, }; let end = match args.get(1) { - Some(v) => v.as_number().unwrap() as isize, - None => len, + Some(v) => v.to_integer(interpreter)?, + None => len as f64, }; - let from = if start < 0 { - max(len.wrapping_add(start), 0) + let from = if !start.is_finite() { + 0 + } else if start < 0.0 { + max(len.wrapping_add(start as isize), 0) } else { - min(start, len) + min(start as isize, len) }; - let to = if end < 0 { - max(len.wrapping_add(end), 0) + let to = if !end.is_finite() { + 0 + } else if end < 0.0 { + max(len.wrapping_add(end as isize), 0) } else { - min(end, len) + min(end as isize, len) }; let span = max(to.wrapping_sub(from), 0); From 7b1fc2c3a05b994f0803233cd2a040fb9e3ec090 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Thu, 29 Oct 2020 15:00:55 +0100 Subject: [PATCH 04/15] Collapse the collapsible else-if statement --- boa/src/builtins/array/mod.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index a08dd3569f1..efe2b9560d3 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -705,14 +705,12 @@ impl Array { let from_idx = from_idx_ptr.to_number(context)?; if !from_idx.is_finite() { - return Ok(Value::from(-1)) + return Ok(Value::from(-1)); + } else if from_idx < 0.0 { + let k = len as isize + from_idx as isize; + max(0, k) as usize } else { - if from_idx < 0.0 { - let k = len as isize + from_idx as isize; - max(0, k) as usize - } else { - from_idx as usize - } + from_idx as usize } } None => 0, From 5b4a973432ebc746dd051ab0682c46db3fc89855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Thu, 29 Oct 2020 15:01:08 +0100 Subject: [PATCH 05/15] Reformat array module --- boa/src/builtins/array/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index efe2b9560d3..ab2edf4972a 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -747,7 +747,11 @@ impl Array { /// /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.lastindexof /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/lastIndexOf - pub(crate) fn last_index_of(this: &Value, args: &[Value], context: &mut Context) -> Result { + pub(crate) fn last_index_of( + this: &Value, + args: &[Value], + context: &mut Context, + ) -> Result { // If no arguments, return -1. Not described in spec, but is what chrome does. if args.is_empty() { return Ok(Value::from(-1)); @@ -761,7 +765,7 @@ impl Array { let from_idx = from_idx_ptr.to_integer(context)?; if !from_idx.is_finite() { - return Ok(Value::from(-1)) + return Ok(Value::from(-1)); } else if from_idx >= 0.0 { min(from_idx as isize, len - 1) } else { From c0e02d6c7a1557d14af12feb846e413475e15d79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Thu, 29 Oct 2020 15:11:39 +0100 Subject: [PATCH 06/15] Fix the variable names post-rebase --- boa/src/builtins/array/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index ab2edf4972a..bf44174ccbe 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -951,11 +951,11 @@ impl Array { let len = this.get_field("length").to_length(context)? as isize; let start = match args.get(0) { - Some(v) => v.to_integer(interpreter)?, + Some(v) => v.to_integer(context)?, None => 0.0, }; let end = match args.get(1) { - Some(v) => v.to_integer(interpreter)?, + Some(v) => v.to_integer(context)?, None => len as f64, }; From 2c75c0a029de3367eba25cf1fa9e5b665653578a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Mon, 2 Nov 2020 21:35:25 +0100 Subject: [PATCH 07/15] Use safe casts instead of `as` --- boa/src/builtins/array/mod.rs | 55 ++++++++++++++++++++++------------- boa/src/value/conversions.rs | 6 ++++ 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index bf44174ccbe..3eb0c4232cd 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -22,7 +22,11 @@ use crate::{ value::{same_value_zero, Value}, BoaProfiler, Context, Result, }; -use std::cmp::{max, min}; +use num_traits::*; +use std::{ + cmp::{max, min}, + convert::{TryFrom, TryInto}, +}; /// JavaScript `Array` built-in implementation. #[derive(Debug, Clone, Copy)] @@ -160,7 +164,7 @@ impl Array { context: &mut Context, ) -> Result { let prototype = context.standard_objects().array_object().prototype(); - let array = Array::array_create(this, items.len() as u32, Some(prototype), context)?; + let array = Array::array_create(this, items.len().try_into()?, Some(prototype), context)?; for (k, item) in items.iter().enumerate() { array.set_field(k, item.clone()); @@ -707,10 +711,10 @@ impl Array { if !from_idx.is_finite() { return Ok(Value::from(-1)); } else if from_idx < 0.0 { - let k = len as isize + from_idx as isize; - max(0, k) as usize + let k = isize::try_from(len)? + f64_to_isize(from_idx)?; + usize::try_from(max(0, k))? } else { - from_idx as usize + f64_to_usize(from_idx)? } } None => 0, @@ -758,7 +762,7 @@ impl Array { } let search_element = args[0].clone(); - let len = this.get_field("length").to_length(context)? as isize; + let len: isize = this.get_field("length").to_length(context)?.try_into()?; let mut idx = match args.get(1) { Some(from_idx_ptr) => { @@ -766,10 +770,10 @@ impl Array { if !from_idx.is_finite() { return Ok(Value::from(-1)); - } else if from_idx >= 0.0 { - min(from_idx as isize, len - 1) + } else if from_idx < 0.0 { + len + f64_to_isize(from_idx)? } else { - len + from_idx as isize + min(f64_to_isize(from_idx)?, len - 1) } } None => len - 1, @@ -779,8 +783,7 @@ impl Array { let check_element = this.get_field(idx).clone(); if check_element.strict_equals(&search_element) { - // TODO: make the cast from isize to i32 safe - return Ok(Value::from(idx as i32)); + return Ok(Value::from(i32::try_from(idx)?)); } idx -= 1; @@ -853,8 +856,7 @@ impl Array { let result = context.call(predicate_arg, &this_arg, &arguments)?; if result.to_boolean() { - // TODO: make the case safe - return Ok(Value::integer(i as i32)); + return Ok(Value::integer(i32::try_from(i)?)); } } @@ -873,16 +875,17 @@ impl Array { /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.fill /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/fill pub(crate) fn fill(this: &Value, args: &[Value], context: &mut Context) -> Result { - let len = this.get_field("length").to_length(context)? as isize; + let len: isize = this.get_field("length").to_length(context)?.try_into()?; let default_value = Value::undefined(); let value = args.get(0).unwrap_or(&default_value); - let relative_start = args.get(1).unwrap_or(&default_value).to_number(context)? as isize; + let relative_start = + f64_to_isize(args.get(1).unwrap_or(&default_value).to_number(context)?)?; let relative_end_val = args.get(2).unwrap_or(&default_value); let relative_end = if relative_end_val.is_undefined() { len } else { - relative_end_val.to_number(context)? as isize + f64_to_isize(relative_end_val.to_number(context)?)? }; let start = if relative_start < 0 { max(len + relative_start, 0) @@ -948,7 +951,7 @@ impl Array { /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/slice pub(crate) fn slice(this: &Value, args: &[Value], context: &mut Context) -> Result { let new_array = Self::new_array(context)?; - let len = this.get_field("length").to_length(context)? as isize; + let len: isize = this.get_field("length").to_length(context)?.try_into()?; let start = match args.get(0) { Some(v) => v.to_integer(context)?, @@ -962,16 +965,16 @@ impl Array { let from = if !start.is_finite() { 0 } else if start < 0.0 { - max(len.wrapping_add(start as isize), 0) + max(len.wrapping_add(f64_to_isize(start)?), 0) } else { - min(start as isize, len) + min(f64_to_isize(start)?, len) }; let to = if !end.is_finite() { 0 } else if end < 0.0 { - max(len.wrapping_add(end as isize), 0) + max(len.wrapping_add(f64_to_isize(end)?), 0) } else { - min(end as isize, len) + min(f64_to_isize(end)?, len) }; let span = max(to.wrapping_sub(from), 0); @@ -1260,3 +1263,13 @@ impl Array { ArrayIterator::create_array_iterator(context, this.clone(), ArrayIterationKind::KeyAndValue) } } + +fn f64_to_isize(v: f64) -> Result { + v.to_isize() + .ok_or_else(|| Value::string("cannot convert f64 to isize - out of range")) +} + +fn f64_to_usize(v: f64) -> Result { + v.to_usize() + .ok_or_else(|| Value::string("cannot convert f64 to usize - out of range")) +} diff --git a/boa/src/value/conversions.rs b/boa/src/value/conversions.rs index 1a1ae39d442..93946945765 100644 --- a/boa/src/value/conversions.rs +++ b/boa/src/value/conversions.rs @@ -186,3 +186,9 @@ where } } } + +impl From for Value { + fn from(err: std::num::TryFromIntError) -> Self { + Value::string(format!("{}", err)) + } +} From 6365cf36090ea65ffbf689ff8c0e86db2378d799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Mon, 2 Nov 2020 22:14:11 +0100 Subject: [PATCH 08/15] Fix the `fill` function --- boa/src/builtins/array/mod.rs | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 3eb0c4232cd..aaa8c132684 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -879,23 +879,31 @@ impl Array { let default_value = Value::undefined(); let value = args.get(0).unwrap_or(&default_value); - let relative_start = - f64_to_isize(args.get(1).unwrap_or(&default_value).to_number(context)?)?; + let relative_start_val = args.get(1).unwrap_or(&default_value); + let relative_start = if relative_start_val.is_undefined() { + 0.0 + } else { + relative_start_val.to_number(context)? + }; let relative_end_val = args.get(2).unwrap_or(&default_value); let relative_end = if relative_end_val.is_undefined() { - len + len as f64 } else { - f64_to_isize(relative_end_val.to_number(context)?)? + relative_end_val.to_number(context)? }; - let start = if relative_start < 0 { - max(len + relative_start, 0) + let start = if !relative_start.is_finite() { + 0 + } else if relative_start < 0.0 { + max(len + f64_to_isize(relative_start)?, 0) } else { - min(relative_start, len) + min(f64_to_isize(relative_start)?, len) }; - let fin = if relative_end < 0 { - max(len + relative_end, 0) + let fin = if !relative_end.is_finite() { + 0 + } else if relative_end < 0.0 { + max(len + f64_to_isize(relative_end)?, 0) } else { - min(relative_end, len) + min(f64_to_isize(relative_end)?, len) }; for i in start..fin { From a86ea85626d82e86a0935783f7f6ebb8a262ddcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Tue, 3 Nov 2020 23:57:06 +0100 Subject: [PATCH 09/15] Implement `to_integer_or_infinity` from the ECMA spec and use it in `fill` and `slice` --- boa/src/builtins/array/mod.rs | 81 +++++++------------- boa/src/value/mod.rs | 2 + boa/src/value/tests.rs | 75 +++++++++++++++++++ boa/src/value/to_integer_or_infinity.rs | 99 +++++++++++++++++++++++++ 4 files changed, 203 insertions(+), 54 deletions(-) create mode 100644 boa/src/value/to_integer_or_infinity.rs diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index aaa8c132684..79cb03fed4d 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -875,36 +875,20 @@ impl Array { /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.fill /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/fill pub(crate) fn fill(this: &Value, args: &[Value], context: &mut Context) -> Result { - let len: isize = this.get_field("length").to_length(context)?.try_into()?; + let len = this.get_field("length").to_length(context)?; let default_value = Value::undefined(); let value = args.get(0).unwrap_or(&default_value); - let relative_start_val = args.get(1).unwrap_or(&default_value); - let relative_start = if relative_start_val.is_undefined() { - 0.0 - } else { - relative_start_val.to_number(context)? - }; - let relative_end_val = args.get(2).unwrap_or(&default_value); - let relative_end = if relative_end_val.is_undefined() { - len as f64 - } else { - relative_end_val.to_number(context)? - }; - let start = if !relative_start.is_finite() { - 0 - } else if relative_start < 0.0 { - max(len + f64_to_isize(relative_start)?, 0) - } else { - min(f64_to_isize(relative_start)?, len) - }; - let fin = if !relative_end.is_finite() { - 0 - } else if relative_end < 0.0 { - max(len + f64_to_isize(relative_end)?, 0) - } else { - min(f64_to_isize(relative_end)?, len) - }; + let start = args + .get(1) + .unwrap_or(&default_value) + .to_integer_or_infinity(context)? + .as_relative_start(len); + let fin = args + .get(2) + .unwrap_or(&default_value) + .to_integer_or_infinity(context)? + .as_relative_end(len); for i in start..fin { this.set_field(i, value.clone()); @@ -959,37 +943,26 @@ impl Array { /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/slice pub(crate) fn slice(this: &Value, args: &[Value], context: &mut Context) -> Result { let new_array = Self::new_array(context)?; - let len: isize = this.get_field("length").to_length(context)?.try_into()?; - let start = match args.get(0) { - Some(v) => v.to_integer(context)?, - None => 0.0, - }; - let end = match args.get(1) { - Some(v) => v.to_integer(context)?, - None => len as f64, - }; - - let from = if !start.is_finite() { - 0 - } else if start < 0.0 { - max(len.wrapping_add(f64_to_isize(start)?), 0) - } else { - min(f64_to_isize(start)?, len) - }; - let to = if !end.is_finite() { - 0 - } else if end < 0.0 { - max(len.wrapping_add(f64_to_isize(end)?), 0) - } else { - min(f64_to_isize(end)?, len) - }; + let len = this.get_field("length").to_length(context)?; - let span = max(to.wrapping_sub(from), 0); + let default_value = Value::undefined(); + let from = args + .get(0) + .unwrap_or(&default_value) + .to_integer_or_infinity(context)? + .as_relative_start(len); + let to = args + .get(1) + .unwrap_or(&default_value) + .to_integer_or_infinity(context)? + .as_relative_end(len); + + let span = max(to.saturating_sub(from), 0); let mut new_array_len: i32 = 0; - for i in from..from.wrapping_add(span) { + for i in from..from.saturating_add(span) { new_array.set_field(new_array_len, this.get_field(i)); - new_array_len = new_array_len.wrapping_add(1); + new_array_len = new_array_len.saturating_add(1); } new_array.set_field("length", Value::from(new_array_len)); Ok(new_array) diff --git a/boa/src/value/mod.rs b/boa/src/value/mod.rs index 1793e55acb4..bba628b8fd3 100644 --- a/boa/src/value/mod.rs +++ b/boa/src/value/mod.rs @@ -32,6 +32,7 @@ mod operations; mod rcbigint; mod rcstring; mod rcsymbol; +mod to_integer_or_infinity; mod r#type; pub use conversions::*; @@ -43,6 +44,7 @@ pub use r#type::Type; pub use rcbigint::RcBigInt; pub use rcstring::RcString; pub use rcsymbol::RcSymbol; +pub use to_integer_or_infinity::*; /// A Javascript value #[derive(Trace, Finalize, Debug, Clone)] diff --git a/boa/src/value/tests.rs b/boa/src/value/tests.rs index 0d8420cd6d2..9ee959aead9 100644 --- a/boa/src/value/tests.rs +++ b/boa/src/value/tests.rs @@ -519,6 +519,81 @@ toString: { ); } +#[test] +fn to_integer_or_infinity() { + let mut context = Context::new(); + + assert_eq!( + Value::undefined().to_integer_or_infinity(&mut context), + Ok(IntegerOrInfinity::Undefined) + ); + assert_eq!( + Value::from(NAN).to_integer_or_infinity(&mut context), + Ok(IntegerOrInfinity::Integer(0)) + ); + assert_eq!( + Value::from(0.0).to_integer_or_infinity(&mut context), + Ok(IntegerOrInfinity::Integer(0)) + ); + assert_eq!( + Value::from(-0.0).to_integer_or_infinity(&mut context), + Ok(IntegerOrInfinity::Integer(0)) + ); + + assert_eq!( + Value::from(f64::INFINITY).to_integer_or_infinity(&mut context), + Ok(IntegerOrInfinity::PositiveInfinity) + ); + assert_eq!( + Value::from(f64::NEG_INFINITY).to_integer_or_infinity(&mut context), + Ok(IntegerOrInfinity::NegativeInfinity) + ); + + assert_eq!( + Value::from(10).to_integer_or_infinity(&mut context), + Ok(IntegerOrInfinity::Integer(10)) + ); + assert_eq!( + Value::from(11.0).to_integer_or_infinity(&mut context), + Ok(IntegerOrInfinity::Integer(11)) + ); + assert_eq!( + Value::from("12").to_integer_or_infinity(&mut context), + Ok(IntegerOrInfinity::Integer(12)) + ); + assert_eq!( + Value::from(true).to_integer_or_infinity(&mut context), + Ok(IntegerOrInfinity::Integer(1)) + ); + + assert_eq!(IntegerOrInfinity::Undefined.as_relative_start(10), 0); + assert_eq!(IntegerOrInfinity::NegativeInfinity.as_relative_start(10), 0); + assert_eq!( + IntegerOrInfinity::PositiveInfinity.as_relative_start(10), + 10 + ); + assert_eq!(IntegerOrInfinity::Integer(-1).as_relative_start(10), 9); + assert_eq!(IntegerOrInfinity::Integer(1).as_relative_start(10), 1); + assert_eq!(IntegerOrInfinity::Integer(-11).as_relative_start(10), 0); + assert_eq!(IntegerOrInfinity::Integer(11).as_relative_start(10), 10); + assert_eq!( + IntegerOrInfinity::Integer(isize::MIN).as_relative_start(10), + 0 + ); + + assert_eq!(IntegerOrInfinity::Undefined.as_relative_end(10), 10); + assert_eq!(IntegerOrInfinity::NegativeInfinity.as_relative_end(10), 0); + assert_eq!(IntegerOrInfinity::PositiveInfinity.as_relative_end(10), 10); + assert_eq!(IntegerOrInfinity::Integer(-1).as_relative_end(10), 9); + assert_eq!(IntegerOrInfinity::Integer(1).as_relative_end(10), 1); + assert_eq!(IntegerOrInfinity::Integer(-11).as_relative_end(10), 0); + assert_eq!(IntegerOrInfinity::Integer(11).as_relative_end(10), 10); + assert_eq!( + IntegerOrInfinity::Integer(isize::MIN).as_relative_end(10), + 0 + ); +} + /// Test cyclic conversions that previously caused stack overflows /// Relevant mitigations for these are in `GcObject::ordinary_to_primitive` and /// `GcObject::to_json` diff --git a/boa/src/value/to_integer_or_infinity.rs b/boa/src/value/to_integer_or_infinity.rs new file mode 100644 index 00000000000..e45adbac157 --- /dev/null +++ b/boa/src/value/to_integer_or_infinity.rs @@ -0,0 +1,99 @@ +use super::*; + +use std::convert::TryFrom; + +/// Represents the result of ToIntegerOrInfinity operation +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum IntegerOrInfinity { + Integer(isize), + Undefined, + PositiveInfinity, + NegativeInfinity, +} + +impl IntegerOrInfinity { + /// Represents the algorithm to calculate `relativeStart` (or `k`) in array functions. + pub fn as_relative_start(self, len: usize) -> usize { + match self { + // 1. If relativeStart is -∞, let k be 0. + IntegerOrInfinity::NegativeInfinity => 0, + // 2. Else if relativeStart < 0, let k be max(len + relativeStart, 0). + IntegerOrInfinity::Integer(i) if i < 0 => Self::offset(len, i), + // 3. Else, let k be min(relativeStart, len). + IntegerOrInfinity::Integer(i) => (i as usize).min(len), + + // Special case - postive infinity. `len` is always smaller than +inf, thus from (3) + IntegerOrInfinity::PositiveInfinity => len, + // Special case - `undefined` is treated like 0 + IntegerOrInfinity::Undefined => 0, + } + } + + /// Represents the algorithm to calculate `relativeEnd` (or `final`) in array functions. + pub fn as_relative_end(self, len: usize) -> usize { + match self { + // 1. If end is undefined, let relativeEnd be len + IntegerOrInfinity::Undefined => len, + + // 1. cont, else let relativeEnd be ? ToIntegerOrInfinity(end). + + // 2. If relativeEnd is -∞, let final be 0. + IntegerOrInfinity::NegativeInfinity => 0, + // 3. Else if relativeEnd < 0, let final be max(len + relativeEnd, 0). + IntegerOrInfinity::Integer(i) if i < 0 => Self::offset(len, i), + // 4. Else, let final be min(relativeEnd, len). + IntegerOrInfinity::Integer(i) => (i as usize).min(len), + + // Special case - postive infinity. `len` is always smaller than +inf, thus from (4) + IntegerOrInfinity::PositiveInfinity => len, + } + } + + fn offset(len: usize, i: isize) -> usize { + debug_assert!(i < 0); + if i == isize::MIN { + len.saturating_sub(isize::MAX as usize).saturating_sub(1) + } else { + len.saturating_sub(i.saturating_neg() as usize) + } + } +} + +impl Value { + /// Converts argument to an integer, +∞, or -∞. + /// + /// See: + pub fn to_integer_or_infinity(&self, context: &mut Context) -> Result { + // Special case - `undefined` + if self.is_undefined() { + return Ok(IntegerOrInfinity::Undefined); + } + + // 1. Let number be ? ToNumber(argument). + let number = self.to_number(context)?; + + // 2. If number is NaN, +0𝔽, or -0𝔽, return 0. + if number.is_nan() || number == 0.0 || number == -0.0 { + Ok(IntegerOrInfinity::Integer(0)) + } else if number.is_infinite() && number.is_sign_positive() { + // 3. If number is +∞𝔽, return +∞. + Ok(IntegerOrInfinity::PositiveInfinity) + } else if number.is_infinite() && number.is_sign_negative() { + // 4. If number is -∞𝔽, return -∞. + Ok(IntegerOrInfinity::NegativeInfinity) + } else { + // 5. Let integer be floor(abs(ℝ(number))). + let integer = number.abs().floor(); + let integer = integer.min(Number::MAX_SAFE_INTEGER) as i64; + let integer = isize::try_from(integer)?; + + // 6. If number < +0𝔽, set integer to -integer. + // 7. Return integer. + if number < 0.0 { + Ok(IntegerOrInfinity::Integer(-integer)) + } else { + Ok(IntegerOrInfinity::Integer(integer)) + } + } + } +} From 8fb83b0f023e78ed4cb57a83eedeb81411105a6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Mon, 7 Dec 2020 21:51:28 +0100 Subject: [PATCH 10/15] Use `i64` as `ToIntegerOrInfinity` inner type & move `as_relative_start` and `as_relative_end` to array module --- boa/src/builtins/array/mod.rs | 81 ++++++++++++++------ boa/src/value/mod.rs | 41 +++++++++- boa/src/value/to_integer_or_infinity.rs | 99 ------------------------- 3 files changed, 97 insertions(+), 124 deletions(-) delete mode 100644 boa/src/value/to_integer_or_infinity.rs diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 79cb03fed4d..5e2886d4e05 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -19,7 +19,7 @@ use crate::{ gc::GcObject, object::{ConstructorBuilder, FunctionBuilder, ObjectData, PROTOTYPE}, property::{Attribute, DataDescriptor}, - value::{same_value_zero, Value}, + value::{same_value_zero, IntegerOrInfinity, Value}, BoaProfiler, Context, Result, }; use num_traits::*; @@ -879,16 +879,8 @@ impl Array { let default_value = Value::undefined(); let value = args.get(0).unwrap_or(&default_value); - let start = args - .get(1) - .unwrap_or(&default_value) - .to_integer_or_infinity(context)? - .as_relative_start(len); - let fin = args - .get(2) - .unwrap_or(&default_value) - .to_integer_or_infinity(context)? - .as_relative_end(len); + let start = Self::as_relative_start(context, args.get(1), len)?; + let fin = Self::as_relative_end(context, args.get(2), len)?; for i in start..fin { this.set_field(i, value.clone()); @@ -945,18 +937,8 @@ impl Array { let new_array = Self::new_array(context)?; let len = this.get_field("length").to_length(context)?; - - let default_value = Value::undefined(); - let from = args - .get(0) - .unwrap_or(&default_value) - .to_integer_or_infinity(context)? - .as_relative_start(len); - let to = args - .get(1) - .unwrap_or(&default_value) - .to_integer_or_infinity(context)? - .as_relative_end(len); + let from = Self::as_relative_start(context, args.get(0), len)?; + let to = Self::as_relative_end(context, args.get(1), len)?; let span = max(to.saturating_sub(from), 0); let mut new_array_len: i32 = 0; @@ -1243,6 +1225,59 @@ impl Array { pub(crate) fn entries(this: &Value, _: &[Value], context: &mut Context) -> Result { ArrayIterator::create_array_iterator(context, this.clone(), ArrayIterationKind::KeyAndValue) } + + /// Represents the algorithm to calculate `relativeStart` (or `k`) in array functions. + fn as_relative_start(context: &mut Context, arg: Option<&Value>, len: usize) -> Result { + let default_value = Value::undefined(); + // 1. Let relativeStart be ? ToIntegerOrInfinity(start). + let relative_start = arg + .unwrap_or(&default_value) + .to_integer_or_infinity(context)?; + match relative_start { + // 2. If relativeStart is -∞, let k be 0. + IntegerOrInfinity::NegativeInfinity => Ok(0), + // 3. Else if relativeStart < 0, let k be max(len + relativeStart, 0). + IntegerOrInfinity::Integer(i) if i < 0 => Ok(Self::offset(len as u64, i).try_into()?), + // 4. Else, let k be min(relativeStart, len). + IntegerOrInfinity::Integer(i) => Ok(i.try_into().map(|x: usize| x.min(len))?), + + // Special case - postive infinity. `len` is always smaller than +inf, thus from (4) + IntegerOrInfinity::PositiveInfinity => Ok(len), + } + } + + /// Represents the algorithm to calculate `relativeEnd` (or `final`) in array functions. + fn as_relative_end(context: &mut Context, arg: Option<&Value>, len: usize) -> Result { + let default_value = Value::undefined(); + let value = arg.unwrap_or(&default_value); + // 1. If end is undefined, let relativeEnd be len [and return it] + if value.is_undefined() { + Ok(len) + } else { + // 1. cont, else let relativeEnd be ? ToIntegerOrInfinity(end). + let relative_end = value.to_integer_or_infinity(context)?; + match relative_end { + // 2. If relativeEnd is -∞, let final be 0. + IntegerOrInfinity::NegativeInfinity => Ok(0), + // 3. Else if relativeEnd < 0, let final be max(len + relativeEnd, 0). + IntegerOrInfinity::Integer(i) if i < 0 => { + Ok(Self::offset(len as u64, i).try_into()?) + } + // 4. Else, let final be min(relativeEnd, len). + IntegerOrInfinity::Integer(i) => Ok(i.try_into().map(|x: usize| x.min(len))?), + + // Special case - postive infinity. `len` is always smaller than +inf, thus from (4) + IntegerOrInfinity::PositiveInfinity => Ok(len), + } + } + } + + fn offset(len: u64, i: i64) -> u64 { + // `Number::MIN_SAFE_INTEGER > i64::MIN` so this should always hold + debug_assert!(i < 0 && i != i64::MIN); + // `i.staurating_neg()` will always be less than `u64::MAX` + len.saturating_sub(i.saturating_neg() as u64) + } } fn f64_to_isize(v: f64) -> Result { diff --git a/boa/src/value/mod.rs b/boa/src/value/mod.rs index bba628b8fd3..5220373defc 100644 --- a/boa/src/value/mod.rs +++ b/boa/src/value/mod.rs @@ -32,7 +32,6 @@ mod operations; mod rcbigint; mod rcstring; mod rcsymbol; -mod to_integer_or_infinity; mod r#type; pub use conversions::*; @@ -44,7 +43,6 @@ pub use r#type::Type; pub use rcbigint::RcBigInt; pub use rcstring::RcString; pub use rcsymbol::RcSymbol; -pub use to_integer_or_infinity::*; /// A Javascript value #[derive(Trace, Finalize, Debug, Clone)] @@ -69,6 +67,14 @@ pub enum Value { Symbol(RcSymbol), } +/// Represents the result of ToIntegerOrInfinity operation +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum IntegerOrInfinity { + Integer(i64), + PositiveInfinity, + NegativeInfinity, +} + impl Value { /// Creates a new `undefined` value. #[inline] @@ -865,6 +871,37 @@ impl Value { Err(context.construct_type_error("Property description must be an object")) } } + + /// Converts argument to an integer, +∞, or -∞. + /// + /// See: + pub fn to_integer_or_infinity(&self, context: &mut Context) -> Result { + // 1. Let number be ? ToNumber(argument). + let number = self.to_number(context)?; + + // 2. If number is NaN, +0𝔽, or -0𝔽, return 0. + if number.is_nan() || number == 0.0 || number == -0.0 { + Ok(IntegerOrInfinity::Integer(0)) + } else if number.is_infinite() && number.is_sign_positive() { + // 3. If number is +∞𝔽, return +∞. + Ok(IntegerOrInfinity::PositiveInfinity) + } else if number.is_infinite() && number.is_sign_negative() { + // 4. If number is -∞𝔽, return -∞. + Ok(IntegerOrInfinity::NegativeInfinity) + } else { + // 5. Let integer be floor(abs(ℝ(number))). + let integer = number.abs().floor(); + let integer = integer.min(Number::MAX_SAFE_INTEGER) as i64; + + // 6. If number < +0𝔽, set integer to -integer. + // 7. Return integer. + if number < 0.0 { + Ok(IntegerOrInfinity::Integer(-integer)) + } else { + Ok(IntegerOrInfinity::Integer(integer)) + } + } + } } impl Default for Value { diff --git a/boa/src/value/to_integer_or_infinity.rs b/boa/src/value/to_integer_or_infinity.rs deleted file mode 100644 index e45adbac157..00000000000 --- a/boa/src/value/to_integer_or_infinity.rs +++ /dev/null @@ -1,99 +0,0 @@ -use super::*; - -use std::convert::TryFrom; - -/// Represents the result of ToIntegerOrInfinity operation -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum IntegerOrInfinity { - Integer(isize), - Undefined, - PositiveInfinity, - NegativeInfinity, -} - -impl IntegerOrInfinity { - /// Represents the algorithm to calculate `relativeStart` (or `k`) in array functions. - pub fn as_relative_start(self, len: usize) -> usize { - match self { - // 1. If relativeStart is -∞, let k be 0. - IntegerOrInfinity::NegativeInfinity => 0, - // 2. Else if relativeStart < 0, let k be max(len + relativeStart, 0). - IntegerOrInfinity::Integer(i) if i < 0 => Self::offset(len, i), - // 3. Else, let k be min(relativeStart, len). - IntegerOrInfinity::Integer(i) => (i as usize).min(len), - - // Special case - postive infinity. `len` is always smaller than +inf, thus from (3) - IntegerOrInfinity::PositiveInfinity => len, - // Special case - `undefined` is treated like 0 - IntegerOrInfinity::Undefined => 0, - } - } - - /// Represents the algorithm to calculate `relativeEnd` (or `final`) in array functions. - pub fn as_relative_end(self, len: usize) -> usize { - match self { - // 1. If end is undefined, let relativeEnd be len - IntegerOrInfinity::Undefined => len, - - // 1. cont, else let relativeEnd be ? ToIntegerOrInfinity(end). - - // 2. If relativeEnd is -∞, let final be 0. - IntegerOrInfinity::NegativeInfinity => 0, - // 3. Else if relativeEnd < 0, let final be max(len + relativeEnd, 0). - IntegerOrInfinity::Integer(i) if i < 0 => Self::offset(len, i), - // 4. Else, let final be min(relativeEnd, len). - IntegerOrInfinity::Integer(i) => (i as usize).min(len), - - // Special case - postive infinity. `len` is always smaller than +inf, thus from (4) - IntegerOrInfinity::PositiveInfinity => len, - } - } - - fn offset(len: usize, i: isize) -> usize { - debug_assert!(i < 0); - if i == isize::MIN { - len.saturating_sub(isize::MAX as usize).saturating_sub(1) - } else { - len.saturating_sub(i.saturating_neg() as usize) - } - } -} - -impl Value { - /// Converts argument to an integer, +∞, or -∞. - /// - /// See: - pub fn to_integer_or_infinity(&self, context: &mut Context) -> Result { - // Special case - `undefined` - if self.is_undefined() { - return Ok(IntegerOrInfinity::Undefined); - } - - // 1. Let number be ? ToNumber(argument). - let number = self.to_number(context)?; - - // 2. If number is NaN, +0𝔽, or -0𝔽, return 0. - if number.is_nan() || number == 0.0 || number == -0.0 { - Ok(IntegerOrInfinity::Integer(0)) - } else if number.is_infinite() && number.is_sign_positive() { - // 3. If number is +∞𝔽, return +∞. - Ok(IntegerOrInfinity::PositiveInfinity) - } else if number.is_infinite() && number.is_sign_negative() { - // 4. If number is -∞𝔽, return -∞. - Ok(IntegerOrInfinity::NegativeInfinity) - } else { - // 5. Let integer be floor(abs(ℝ(number))). - let integer = number.abs().floor(); - let integer = integer.min(Number::MAX_SAFE_INTEGER) as i64; - let integer = isize::try_from(integer)?; - - // 6. If number < +0𝔽, set integer to -integer. - // 7. Return integer. - if number < 0.0 { - Ok(IntegerOrInfinity::Integer(-integer)) - } else { - Ok(IntegerOrInfinity::Integer(integer)) - } - } - } -} From 44586a93d1150b88c64e9dbe2f21d618af730d09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Mon, 7 Dec 2020 22:01:02 +0100 Subject: [PATCH 11/15] Remove the `From` impl from `Value` --- boa/src/builtins/array/mod.rs | 38 ++++++++++++++++++++++++++--------- boa/src/value/conversions.rs | 6 ------ 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 5e2886d4e05..dc75cd1d536 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -164,7 +164,8 @@ impl Array { context: &mut Context, ) -> Result { let prototype = context.standard_objects().array_object().prototype(); - let array = Array::array_create(this, items.len().try_into()?, Some(prototype), context)?; + let items_len = items.len().try_into().map_err(err_to_value)?; + let array = Array::array_create(this, items_len, Some(prototype), context)?; for (k, item) in items.iter().enumerate() { array.set_field(k, item.clone()); @@ -711,8 +712,8 @@ impl Array { if !from_idx.is_finite() { return Ok(Value::from(-1)); } else if from_idx < 0.0 { - let k = isize::try_from(len)? + f64_to_isize(from_idx)?; - usize::try_from(max(0, k))? + let k = isize::try_from(len).map_err(err_to_value)? + f64_to_isize(from_idx)?; + usize::try_from(max(0, k)).map_err(err_to_value)? } else { f64_to_usize(from_idx)? } @@ -762,7 +763,11 @@ impl Array { } let search_element = args[0].clone(); - let len: isize = this.get_field("length").to_length(context)?.try_into()?; + let len: isize = this + .get_field("length") + .to_length(context)? + .try_into() + .map_err(err_to_value)?; let mut idx = match args.get(1) { Some(from_idx_ptr) => { @@ -783,7 +788,7 @@ impl Array { let check_element = this.get_field(idx).clone(); if check_element.strict_equals(&search_element) { - return Ok(Value::from(i32::try_from(idx)?)); + return Ok(Value::from(i32::try_from(idx).map_err(err_to_value)?)); } idx -= 1; @@ -856,7 +861,8 @@ impl Array { let result = context.call(predicate_arg, &this_arg, &arguments)?; if result.to_boolean() { - return Ok(Value::integer(i32::try_from(i)?)); + let result = i32::try_from(i).map_err(err_to_value)?; + return Ok(Value::integer(result)); } } @@ -1237,9 +1243,14 @@ impl Array { // 2. If relativeStart is -∞, let k be 0. IntegerOrInfinity::NegativeInfinity => Ok(0), // 3. Else if relativeStart < 0, let k be max(len + relativeStart, 0). - IntegerOrInfinity::Integer(i) if i < 0 => Ok(Self::offset(len as u64, i).try_into()?), + IntegerOrInfinity::Integer(i) if i < 0 => { + Self::offset(len as u64, i).try_into().map_err(err_to_value) + } // 4. Else, let k be min(relativeStart, len). - IntegerOrInfinity::Integer(i) => Ok(i.try_into().map(|x: usize| x.min(len))?), + IntegerOrInfinity::Integer(i) => i + .try_into() + .map(|x: usize| x.min(len)) + .map_err(err_to_value), // Special case - postive infinity. `len` is always smaller than +inf, thus from (4) IntegerOrInfinity::PositiveInfinity => Ok(len), @@ -1261,10 +1272,13 @@ impl Array { IntegerOrInfinity::NegativeInfinity => Ok(0), // 3. Else if relativeEnd < 0, let final be max(len + relativeEnd, 0). IntegerOrInfinity::Integer(i) if i < 0 => { - Ok(Self::offset(len as u64, i).try_into()?) + Self::offset(len as u64, i).try_into().map_err(err_to_value) } // 4. Else, let final be min(relativeEnd, len). - IntegerOrInfinity::Integer(i) => Ok(i.try_into().map(|x: usize| x.min(len))?), + IntegerOrInfinity::Integer(i) => i + .try_into() + .map(|x: usize| x.min(len)) + .map_err(err_to_value), // Special case - postive infinity. `len` is always smaller than +inf, thus from (4) IntegerOrInfinity::PositiveInfinity => Ok(len), @@ -1289,3 +1303,7 @@ fn f64_to_usize(v: f64) -> Result { v.to_usize() .ok_or_else(|| Value::string("cannot convert f64 to usize - out of range")) } + +fn err_to_value(err: std::num::TryFromIntError) -> Value { + Value::string(format!("{}", err)) +} diff --git a/boa/src/value/conversions.rs b/boa/src/value/conversions.rs index 93946945765..1a1ae39d442 100644 --- a/boa/src/value/conversions.rs +++ b/boa/src/value/conversions.rs @@ -186,9 +186,3 @@ where } } } - -impl From for Value { - fn from(err: std::num::TryFromIntError) -> Self { - Value::string(format!("{}", err)) - } -} From 7632e91fd3b9f176f5e59e99239935d1d0a52abf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Mon, 7 Dec 2020 22:04:51 +0100 Subject: [PATCH 12/15] Rename the `err_to_value` helper function --- boa/src/builtins/array/mod.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index dc75cd1d536..1409a3c7501 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -164,7 +164,7 @@ impl Array { context: &mut Context, ) -> Result { let prototype = context.standard_objects().array_object().prototype(); - let items_len = items.len().try_into().map_err(err_to_value)?; + let items_len = items.len().try_into().map_err(interror_to_value)?; let array = Array::array_create(this, items_len, Some(prototype), context)?; for (k, item) in items.iter().enumerate() { @@ -712,8 +712,8 @@ impl Array { if !from_idx.is_finite() { return Ok(Value::from(-1)); } else if from_idx < 0.0 { - let k = isize::try_from(len).map_err(err_to_value)? + f64_to_isize(from_idx)?; - usize::try_from(max(0, k)).map_err(err_to_value)? + let k = isize::try_from(len).map_err(interror_to_value)? + f64_to_isize(from_idx)?; + usize::try_from(max(0, k)).map_err(interror_to_value)? } else { f64_to_usize(from_idx)? } @@ -767,7 +767,7 @@ impl Array { .get_field("length") .to_length(context)? .try_into() - .map_err(err_to_value)?; + .map_err(interror_to_value)?; let mut idx = match args.get(1) { Some(from_idx_ptr) => { @@ -788,7 +788,7 @@ impl Array { let check_element = this.get_field(idx).clone(); if check_element.strict_equals(&search_element) { - return Ok(Value::from(i32::try_from(idx).map_err(err_to_value)?)); + return Ok(Value::from(i32::try_from(idx).map_err(interror_to_value)?)); } idx -= 1; @@ -861,7 +861,7 @@ impl Array { let result = context.call(predicate_arg, &this_arg, &arguments)?; if result.to_boolean() { - let result = i32::try_from(i).map_err(err_to_value)?; + let result = i32::try_from(i).map_err(interror_to_value)?; return Ok(Value::integer(result)); } } @@ -1244,13 +1244,13 @@ impl Array { IntegerOrInfinity::NegativeInfinity => Ok(0), // 3. Else if relativeStart < 0, let k be max(len + relativeStart, 0). IntegerOrInfinity::Integer(i) if i < 0 => { - Self::offset(len as u64, i).try_into().map_err(err_to_value) + Self::offset(len as u64, i).try_into().map_err(interror_to_value) } // 4. Else, let k be min(relativeStart, len). IntegerOrInfinity::Integer(i) => i .try_into() .map(|x: usize| x.min(len)) - .map_err(err_to_value), + .map_err(interror_to_value), // Special case - postive infinity. `len` is always smaller than +inf, thus from (4) IntegerOrInfinity::PositiveInfinity => Ok(len), @@ -1272,13 +1272,13 @@ impl Array { IntegerOrInfinity::NegativeInfinity => Ok(0), // 3. Else if relativeEnd < 0, let final be max(len + relativeEnd, 0). IntegerOrInfinity::Integer(i) if i < 0 => { - Self::offset(len as u64, i).try_into().map_err(err_to_value) + Self::offset(len as u64, i).try_into().map_err(interror_to_value) } // 4. Else, let final be min(relativeEnd, len). IntegerOrInfinity::Integer(i) => i .try_into() .map(|x: usize| x.min(len)) - .map_err(err_to_value), + .map_err(interror_to_value), // Special case - postive infinity. `len` is always smaller than +inf, thus from (4) IntegerOrInfinity::PositiveInfinity => Ok(len), @@ -1304,6 +1304,6 @@ fn f64_to_usize(v: f64) -> Result { .ok_or_else(|| Value::string("cannot convert f64 to usize - out of range")) } -fn err_to_value(err: std::num::TryFromIntError) -> Value { +fn interror_to_value(err: std::num::TryFromIntError) -> Value { Value::string(format!("{}", err)) } From 8a128bb95e41300b42b974bf01af1c0ebf38a190 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Mon, 7 Dec 2020 22:13:06 +0100 Subject: [PATCH 13/15] Reformat code and fix Clippy errors --- boa/src/builtins/array/mod.rs | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 1409a3c7501..5f88d1b17a8 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -712,7 +712,8 @@ impl Array { if !from_idx.is_finite() { return Ok(Value::from(-1)); } else if from_idx < 0.0 { - let k = isize::try_from(len).map_err(interror_to_value)? + f64_to_isize(from_idx)?; + let k = + isize::try_from(len).map_err(interror_to_value)? + f64_to_isize(from_idx)?; usize::try_from(max(0, k)).map_err(interror_to_value)? } else { f64_to_usize(from_idx)? @@ -885,8 +886,8 @@ impl Array { let default_value = Value::undefined(); let value = args.get(0).unwrap_or(&default_value); - let start = Self::as_relative_start(context, args.get(1), len)?; - let fin = Self::as_relative_end(context, args.get(2), len)?; + let start = Self::get_relative_start(context, args.get(1), len)?; + let fin = Self::get_relative_end(context, args.get(2), len)?; for i in start..fin { this.set_field(i, value.clone()); @@ -943,8 +944,8 @@ impl Array { let new_array = Self::new_array(context)?; let len = this.get_field("length").to_length(context)?; - let from = Self::as_relative_start(context, args.get(0), len)?; - let to = Self::as_relative_end(context, args.get(1), len)?; + let from = Self::get_relative_start(context, args.get(0), len)?; + let to = Self::get_relative_end(context, args.get(1), len)?; let span = max(to.saturating_sub(from), 0); let mut new_array_len: i32 = 0; @@ -1233,7 +1234,7 @@ impl Array { } /// Represents the algorithm to calculate `relativeStart` (or `k`) in array functions. - fn as_relative_start(context: &mut Context, arg: Option<&Value>, len: usize) -> Result { + fn get_relative_start(context: &mut Context, arg: Option<&Value>, len: usize) -> Result { let default_value = Value::undefined(); // 1. Let relativeStart be ? ToIntegerOrInfinity(start). let relative_start = arg @@ -1243,9 +1244,9 @@ impl Array { // 2. If relativeStart is -∞, let k be 0. IntegerOrInfinity::NegativeInfinity => Ok(0), // 3. Else if relativeStart < 0, let k be max(len + relativeStart, 0). - IntegerOrInfinity::Integer(i) if i < 0 => { - Self::offset(len as u64, i).try_into().map_err(interror_to_value) - } + IntegerOrInfinity::Integer(i) if i < 0 => Self::offset(len as u64, i) + .try_into() + .map_err(interror_to_value), // 4. Else, let k be min(relativeStart, len). IntegerOrInfinity::Integer(i) => i .try_into() @@ -1258,7 +1259,7 @@ impl Array { } /// Represents the algorithm to calculate `relativeEnd` (or `final`) in array functions. - fn as_relative_end(context: &mut Context, arg: Option<&Value>, len: usize) -> Result { + fn get_relative_end(context: &mut Context, arg: Option<&Value>, len: usize) -> Result { let default_value = Value::undefined(); let value = arg.unwrap_or(&default_value); // 1. If end is undefined, let relativeEnd be len [and return it] @@ -1271,9 +1272,9 @@ impl Array { // 2. If relativeEnd is -∞, let final be 0. IntegerOrInfinity::NegativeInfinity => Ok(0), // 3. Else if relativeEnd < 0, let final be max(len + relativeEnd, 0). - IntegerOrInfinity::Integer(i) if i < 0 => { - Self::offset(len as u64, i).try_into().map_err(interror_to_value) - } + IntegerOrInfinity::Integer(i) if i < 0 => Self::offset(len as u64, i) + .try_into() + .map_err(interror_to_value), // 4. Else, let final be min(relativeEnd, len). IntegerOrInfinity::Integer(i) => i .try_into() From de447ca2b34aaaf0f618bfaa292e09d9e25811d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Mon, 7 Dec 2020 22:40:20 +0100 Subject: [PATCH 14/15] Fix tests and improve the casts in `get_relative_*` a little --- boa/src/builtins/array/mod.rs | 22 ++++-- boa/src/builtins/array/tests.rs | 116 ++++++++++++++++++++++++++++++++ boa/src/value/tests.rs | 29 +------- 3 files changed, 133 insertions(+), 34 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 5f88d1b17a8..f4bf412b2b1 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -1234,7 +1234,11 @@ impl Array { } /// Represents the algorithm to calculate `relativeStart` (or `k`) in array functions. - fn get_relative_start(context: &mut Context, arg: Option<&Value>, len: usize) -> Result { + pub(super) fn get_relative_start( + context: &mut Context, + arg: Option<&Value>, + len: usize, + ) -> Result { let default_value = Value::undefined(); // 1. Let relativeStart be ? ToIntegerOrInfinity(start). let relative_start = arg @@ -1248,9 +1252,10 @@ impl Array { .try_into() .map_err(interror_to_value), // 4. Else, let k be min(relativeStart, len). - IntegerOrInfinity::Integer(i) => i + // Both `as` casts are safe as both variables are non-negative + IntegerOrInfinity::Integer(i) => (i as u64) + .min(len as u64) .try_into() - .map(|x: usize| x.min(len)) .map_err(interror_to_value), // Special case - postive infinity. `len` is always smaller than +inf, thus from (4) @@ -1259,7 +1264,11 @@ impl Array { } /// Represents the algorithm to calculate `relativeEnd` (or `final`) in array functions. - fn get_relative_end(context: &mut Context, arg: Option<&Value>, len: usize) -> Result { + pub(super) fn get_relative_end( + context: &mut Context, + arg: Option<&Value>, + len: usize, + ) -> Result { let default_value = Value::undefined(); let value = arg.unwrap_or(&default_value); // 1. If end is undefined, let relativeEnd be len [and return it] @@ -1276,9 +1285,10 @@ impl Array { .try_into() .map_err(interror_to_value), // 4. Else, let final be min(relativeEnd, len). - IntegerOrInfinity::Integer(i) => i + // Both `as` casts are safe as both variables are non-negative + IntegerOrInfinity::Integer(i) => (i as u64) + .min(len as u64) .try_into() - .map(|x: usize| x.min(len)) .map_err(interror_to_value), // Special case - postive infinity. `len` is always smaller than +inf, thus from (4) diff --git a/boa/src/builtins/array/tests.rs b/boa/src/builtins/array/tests.rs index 869928ccea3..3b9de27817f 100644 --- a/boa/src/builtins/array/tests.rs +++ b/boa/src/builtins/array/tests.rs @@ -1,3 +1,5 @@ +use super::Array; +use crate::builtins::Number; use crate::{forward, Context, Value}; #[test] @@ -1237,3 +1239,117 @@ fn array_spread_non_iterable() { "#; assert_eq!(forward(&mut context, init), "true"); } + +#[test] +fn get_relative_start() { + let mut context = Context::new(); + + assert_eq!(Array::get_relative_start(&mut context, None, 10), Ok(0)); + assert_eq!( + Array::get_relative_start(&mut context, Some(&Value::undefined()), 10), + Ok(0) + ); + assert_eq!( + Array::get_relative_start(&mut context, Some(&Value::from(f64::NEG_INFINITY)), 10), + Ok(0) + ); + assert_eq!( + Array::get_relative_start(&mut context, Some(&Value::from(f64::INFINITY)), 10), + Ok(10) + ); + assert_eq!( + Array::get_relative_start(&mut context, Some(&Value::from(-1)), 10), + Ok(9) + ); + assert_eq!( + Array::get_relative_start(&mut context, Some(&Value::from(1)), 10), + Ok(1) + ); + assert_eq!( + Array::get_relative_start(&mut context, Some(&Value::from(-11)), 10), + Ok(0) + ); + assert_eq!( + Array::get_relative_start(&mut context, Some(&Value::from(11)), 10), + Ok(10) + ); + assert_eq!( + Array::get_relative_start(&mut context, Some(&Value::from(f64::MIN)), 10), + Ok(0) + ); + assert_eq!( + Array::get_relative_start( + &mut context, + Some(&Value::from(Number::MIN_SAFE_INTEGER)), + 10 + ), + Ok(0) + ); + assert_eq!( + Array::get_relative_start(&mut context, Some(&Value::from(f64::MAX)), 10), + Ok(10) + ); + + // This test is relevant only on 32-bit archs (where usize == u32 thus `len` is u32) + assert_eq!( + Array::get_relative_start(&mut context, Some(&Value::from(Number::MAX_SAFE_INTEGER)), 10), + Ok(10) + ); +} + +#[test] +fn get_relative_end() { + let mut context = Context::new(); + + assert_eq!(Array::get_relative_end(&mut context, None, 10), Ok(10)); + assert_eq!( + Array::get_relative_end(&mut context, Some(&Value::undefined()), 10), + Ok(10) + ); + assert_eq!( + Array::get_relative_end(&mut context, Some(&Value::from(f64::NEG_INFINITY)), 10), + Ok(0) + ); + assert_eq!( + Array::get_relative_end(&mut context, Some(&Value::from(f64::INFINITY)), 10), + Ok(10) + ); + assert_eq!( + Array::get_relative_end(&mut context, Some(&Value::from(-1)), 10), + Ok(9) + ); + assert_eq!( + Array::get_relative_end(&mut context, Some(&Value::from(1)), 10), + Ok(1) + ); + assert_eq!( + Array::get_relative_end(&mut context, Some(&Value::from(-11)), 10), + Ok(0) + ); + assert_eq!( + Array::get_relative_end(&mut context, Some(&Value::from(11)), 10), + Ok(10) + ); + assert_eq!( + Array::get_relative_end(&mut context, Some(&Value::from(f64::MIN)), 10), + Ok(0) + ); + assert_eq!( + Array::get_relative_end( + &mut context, + Some(&Value::from(Number::MIN_SAFE_INTEGER)), + 10 + ), + Ok(0) + ); + assert_eq!( + Array::get_relative_end(&mut context, Some(&Value::from(f64::MAX)), 10), + Ok(10) + ); + + // This test is relevant only on 32-bit archs (where usize == u32 thus `len` is u32) + assert_eq!( + Array::get_relative_end(&mut context, Some(&Value::from(Number::MAX_SAFE_INTEGER)), 10), + Ok(10) + ); +} diff --git a/boa/src/value/tests.rs b/boa/src/value/tests.rs index 9ee959aead9..bf0afd9e46d 100644 --- a/boa/src/value/tests.rs +++ b/boa/src/value/tests.rs @@ -525,7 +525,7 @@ fn to_integer_or_infinity() { assert_eq!( Value::undefined().to_integer_or_infinity(&mut context), - Ok(IntegerOrInfinity::Undefined) + Ok(IntegerOrInfinity::Integer(0)) ); assert_eq!( Value::from(NAN).to_integer_or_infinity(&mut context), @@ -565,33 +565,6 @@ fn to_integer_or_infinity() { Value::from(true).to_integer_or_infinity(&mut context), Ok(IntegerOrInfinity::Integer(1)) ); - - assert_eq!(IntegerOrInfinity::Undefined.as_relative_start(10), 0); - assert_eq!(IntegerOrInfinity::NegativeInfinity.as_relative_start(10), 0); - assert_eq!( - IntegerOrInfinity::PositiveInfinity.as_relative_start(10), - 10 - ); - assert_eq!(IntegerOrInfinity::Integer(-1).as_relative_start(10), 9); - assert_eq!(IntegerOrInfinity::Integer(1).as_relative_start(10), 1); - assert_eq!(IntegerOrInfinity::Integer(-11).as_relative_start(10), 0); - assert_eq!(IntegerOrInfinity::Integer(11).as_relative_start(10), 10); - assert_eq!( - IntegerOrInfinity::Integer(isize::MIN).as_relative_start(10), - 0 - ); - - assert_eq!(IntegerOrInfinity::Undefined.as_relative_end(10), 10); - assert_eq!(IntegerOrInfinity::NegativeInfinity.as_relative_end(10), 0); - assert_eq!(IntegerOrInfinity::PositiveInfinity.as_relative_end(10), 10); - assert_eq!(IntegerOrInfinity::Integer(-1).as_relative_end(10), 9); - assert_eq!(IntegerOrInfinity::Integer(1).as_relative_end(10), 1); - assert_eq!(IntegerOrInfinity::Integer(-11).as_relative_end(10), 0); - assert_eq!(IntegerOrInfinity::Integer(11).as_relative_end(10), 10); - assert_eq!( - IntegerOrInfinity::Integer(isize::MIN).as_relative_end(10), - 0 - ); } /// Test cyclic conversions that previously caused stack overflows From bbecaf3054629fdfe7610dffd82d931a415ac695 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Fija=C5=82kowski?= Date: Mon, 7 Dec 2020 22:48:28 +0100 Subject: [PATCH 15/15] Reformat the code once again --- boa/src/builtins/array/tests.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/boa/src/builtins/array/tests.rs b/boa/src/builtins/array/tests.rs index 3b9de27817f..e4b3a6c5cce 100644 --- a/boa/src/builtins/array/tests.rs +++ b/boa/src/builtins/array/tests.rs @@ -1292,7 +1292,11 @@ fn get_relative_start() { // This test is relevant only on 32-bit archs (where usize == u32 thus `len` is u32) assert_eq!( - Array::get_relative_start(&mut context, Some(&Value::from(Number::MAX_SAFE_INTEGER)), 10), + Array::get_relative_start( + &mut context, + Some(&Value::from(Number::MAX_SAFE_INTEGER)), + 10 + ), Ok(10) ); } @@ -1349,7 +1353,11 @@ fn get_relative_end() { // This test is relevant only on 32-bit archs (where usize == u32 thus `len` is u32) assert_eq!( - Array::get_relative_end(&mut context, Some(&Value::from(Number::MAX_SAFE_INTEGER)), 10), + Array::get_relative_end( + &mut context, + Some(&Value::from(Number::MAX_SAFE_INTEGER)), + 10 + ), Ok(10) ); }