Skip to content

Commit

Permalink
Refactor builtin Map intrinsics to follow more closely the spec (#1572
Browse files Browse the repository at this point in the history 
)

* Refactor builtin `Map` object and document methods

* Replace some calls to `is_function` with `is_callable`

* Remove `expect_map` methods and cleanup some `expect`s
  • Loading branch information
@jedel1043
jedel1043 committed Sep 12, 2021
1 parent 2f8c35d commit 8ba500a
Show file tree
Hide file tree
Showing 21 changed files with 440 additions and 401 deletions.
23 changes: 10 additions & 13 deletions boa/src/builtins/array/array_iterator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ impl ArrayIterator {
///
/// [spec]: https://tc39.es/ecma262/#sec-createarrayiterator
pub(crate) fn create_array_iterator(
context: &Context,
array: JsValue,
kind: PropertyNameKind,
context: &Context,
) -> JsValue {
let array_iterator = JsValue::new_object(context);
array_iterator.set_data(ObjectData::array_iterator(Self::new(array, kind)));
Expand All @@ -68,9 +68,9 @@ impl ArrayIterator {
let index = array_iterator.next_index;
if array_iterator.array.is_undefined() {
return Ok(create_iter_result_object(
context,
JsValue::undefined(),
true,
context,
));
}
let len = array_iterator
Expand All @@ -82,33 +82,30 @@ impl ArrayIterator {
if array_iterator.next_index >= len {
array_iterator.array = JsValue::undefined();
return Ok(create_iter_result_object(
context,
JsValue::undefined(),
true,
context,
));
}
array_iterator.next_index = index + 1;
match array_iterator.kind {
return match array_iterator.kind {
PropertyNameKind::Key => {
Ok(create_iter_result_object(context, index.into(), false))
Ok(create_iter_result_object(index.into(), false, context))
}
PropertyNameKind::Value => {
let element_value = array_iterator.array.get_field(index, context)?;
Ok(create_iter_result_object(context, element_value, false))
Ok(create_iter_result_object(element_value, false, context))
}
PropertyNameKind::KeyAndValue => {
let element_value = array_iterator.array.get_field(index, context)?;
let result =
Array::create_array_from_list([index.into(), element_value], context);
Ok(create_iter_result_object(context, result.into(), false))
Ok(create_iter_result_object(result.into(), false, context))
}
}
} else {
context.throw_type_error("`this` is not an ArrayIterator")
};
}
} else {
context.throw_type_error("`this` is not an ArrayIterator")
}
context.throw_type_error("`this` is not an ArrayIterator")
}

/// Create the %ArrayIteratorPrototype% object
Expand All @@ -117,7 +114,7 @@ impl ArrayIterator {
/// - [ECMA reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-%arrayiteratorprototype%-object
pub(crate) fn create_prototype(context: &mut Context, iterator_prototype: JsValue) -> JsObject {
pub(crate) fn create_prototype(iterator_prototype: JsValue, context: &mut Context) -> JsObject {
let _timer = BoaProfiler::global().start_event(Self::NAME, "init");

// Create prototype
Expand Down
6 changes: 3 additions & 3 deletions boa/src/builtins/array/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2528,9 +2528,9 @@ impl Array {
context: &mut Context,
) -> JsResult<JsValue> {
Ok(ArrayIterator::create_array_iterator(
context,
this.clone(),
PropertyNameKind::Value,
context,
))
}

Expand All @@ -2546,9 +2546,9 @@ impl Array {
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/values
pub(crate) fn keys(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult<JsValue> {
Ok(ArrayIterator::create_array_iterator(
context,
this.clone(),
PropertyNameKind::Key,
context,
))
}

Expand All @@ -2568,9 +2568,9 @@ impl Array {
context: &mut Context,
) -> JsResult<JsValue> {
Ok(ArrayIterator::create_array_iterator(
context,
this.clone(),
PropertyNameKind::KeyAndValue,
context,
))
}

Expand Down
40 changes: 13 additions & 27 deletions boa/src/builtins/iterable/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,22 @@ impl IteratorPrototypes {
let iterator_prototype = create_iterator_prototype(context);
Self {
array_iterator: ArrayIterator::create_prototype(
context,
iterator_prototype.clone().into(),
context,
),
set_iterator: SetIterator::create_prototype(context, iterator_prototype.clone().into()),
set_iterator: SetIterator::create_prototype(iterator_prototype.clone().into(), context),
string_iterator: StringIterator::create_prototype(
context,
iterator_prototype.clone().into(),
context,
),
regexp_string_iterator: RegExpStringIterator::create_prototype(
context,
iterator_prototype.clone().into(),
context,
),
map_iterator: MapIterator::create_prototype(context, iterator_prototype.clone().into()),
map_iterator: MapIterator::create_prototype(iterator_prototype.clone().into(), context),
for_in_iterator: ForInIterator::create_prototype(
context,
iterator_prototype.clone().into(),
context,
),
iterator_prototype,
}
Expand Down Expand Up @@ -85,7 +85,7 @@ impl IteratorPrototypes {
/// CreateIterResultObject( value, done )
///
/// Generates an object supporting the IteratorResult interface.
pub fn create_iter_result_object(context: &mut Context, value: JsValue, done: bool) -> JsValue {
pub fn create_iter_result_object(value: JsValue, done: bool, context: &mut Context) -> JsValue {
// 1. Assert: Type(done) is Boolean.
// 2. Let obj be ! OrdinaryObjectCreate(%Object.prototype%).
let obj = context.construct_object();
Expand All @@ -101,12 +101,12 @@ pub fn create_iter_result_object(context: &mut Context, value: JsValue, done: bo
}

/// Get an iterator record
pub fn get_iterator(context: &mut Context, iterable: JsValue) -> JsResult<IteratorRecord> {
pub fn get_iterator(iterable: &JsValue, context: &mut Context) -> JsResult<IteratorRecord> {
let iterator_function = iterable.get_field(WellKnownSymbols::iterator(), context)?;
if iterator_function.is_null_or_undefined() {
return Err(context.construct_type_error("Not an iterable"));
}
let iterator_object = context.call(&iterator_function, &iterable, &[])?;
let iterator_object = context.call(&iterator_function, iterable, &[])?;
let next_function = iterator_object.get_field("next", context)?;
if next_function.is_null_or_undefined() {
return Err(context.construct_type_error("Could not find property `next`"));
Expand Down Expand Up @@ -158,8 +158,8 @@ impl IteratorRecord {
let next = context.call(&self.next_function, &self.iterator_object, &[])?;
let done = next.get_field("done", context)?.to_boolean();

let next_result = next.get_field("value", context)?;
Ok(IteratorResult::new(next_result, done))
let value = next.get_field("value", context)?;
Ok(IteratorResult { value, done })
}

/// Cleanup the iterator
Expand Down Expand Up @@ -203,20 +203,6 @@ impl IteratorRecord {

#[derive(Debug)]
pub struct IteratorResult {
value: JsValue,
done: bool,
}

impl IteratorResult {
fn new(value: JsValue, done: bool) -> Self {
Self { value, done }
}

pub fn is_done(&self) -> bool {
self.done
}

pub fn value(self) -> JsValue {
self.value
}
pub value: JsValue,
pub done: bool,
}
156 changes: 68 additions & 88 deletions boa/src/builtins/map/map_iterator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
};
use gc::{Finalize, Trace};

use super::{ordered_map::MapLock, Map};
use super::ordered_map::MapLock;
/// The Map Iterator object represents an iteration over a map. It implements the iterator protocol.
///
/// More information:
Expand All @@ -16,7 +16,7 @@ use super::{ordered_map::MapLock, Map};
/// [spec]: https://tc39.es/ecma262/#sec-array-iterator-objects
#[derive(Debug, Clone, Finalize, Trace)]
pub struct MapIterator {
iterated_map: JsValue,
iterated_map: Option<JsObject>,
map_next_index: usize,
map_iteration_kind: PropertyNameKind,
lock: MapLock,
Expand All @@ -25,17 +25,6 @@ pub struct MapIterator {
impl MapIterator {
pub(crate) const NAME: &'static str = "MapIterator";

/// Constructs a new `MapIterator`, that will iterate over `map`, starting at index 0
fn new(map: JsValue, kind: PropertyNameKind, context: &mut Context) -> JsResult<Self> {
let lock = Map::lock(&map, context)?;
Ok(MapIterator {
iterated_map: map,
map_next_index: 0,
map_iteration_kind: kind,
lock,
})
}

/// Abstract operation CreateMapIterator( map, kind )
///
/// Creates a new iterator over the given map.
Expand All @@ -45,17 +34,29 @@ impl MapIterator {
///
/// [spec]: https://www.ecma-international.org/ecma-262/11.0/index.html#sec-createmapiterator
pub(crate) fn create_map_iterator(
context: &mut Context,
map: JsValue,
map: &JsValue,
kind: PropertyNameKind,
context: &mut Context,
) -> JsResult<JsValue> {
let map_iterator = JsValue::new_object(context);
map_iterator.set_data(ObjectData::map_iterator(Self::new(map, kind, context)?));
map_iterator
.as_object()
.expect("map iterator object")
.set_prototype_instance(context.iterator_prototypes().map_iterator().into());
Ok(map_iterator)
if let Some(map_obj) = map.as_object() {
if let Some(map) = map_obj.borrow_mut().as_map_mut() {
let lock = map.lock(map_obj.clone());
let iter = MapIterator {
iterated_map: Some(map_obj.clone()),
map_next_index: 0,
map_iteration_kind: kind,
lock,
};
let map_iterator = JsValue::new_object(context);
map_iterator.set_data(ObjectData::map_iterator(iter));
map_iterator
.as_object()
.expect("map iterator object")
.set_prototype_instance(context.iterator_prototypes().map_iterator().into());
return Ok(map_iterator);
}
}
context.throw_type_error("`this` is not a Map")
}

/// %MapIteratorPrototype%.next( )
Expand All @@ -67,77 +68,56 @@ impl MapIterator {
///
/// [spec]: https://tc39.es/ecma262/#sec-%mapiteratorprototype%.next
pub(crate) fn next(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult<JsValue> {
if let JsValue::Object(ref object) = this {
let mut object = object.borrow_mut();
if let Some(map_iterator) = object.as_map_iterator_mut() {
let m = &map_iterator.iterated_map;
let mut index = map_iterator.map_next_index;
let item_kind = &map_iterator.map_iteration_kind;
let iterator_object = match this {
JsValue::Object(obj) if obj.borrow().is_map_iterator() => obj,
_ => return context.throw_type_error("`this` is not a MapIterator"),
};

if map_iterator.iterated_map.is_undefined() {
return Ok(create_iter_result_object(
context,
JsValue::undefined(),
true,
));
}
let mut iterator_object = iterator_object.borrow_mut();

let map_iterator = iterator_object
.as_map_iterator_mut()
.expect("checked that obj was a map iterator");

if let JsValue::Object(ref object) = m {
if let Some(entries) = object.borrow().as_map_ref() {
let num_entries = entries.full_len();
while index < num_entries {
let e = entries.get_index(index);
index += 1;
map_iterator.map_next_index = index;
if let Some((key, value)) = e {
match item_kind {
PropertyNameKind::Key => {
return Ok(create_iter_result_object(
context,
key.clone(),
false,
));
}
PropertyNameKind::Value => {
return Ok(create_iter_result_object(
context,
value.clone(),
false,
));
}
PropertyNameKind::KeyAndValue => {
let result = Array::create_array_from_list(
[key.clone(), value.clone()],
context,
);
return Ok(create_iter_result_object(
context,
result.into(),
false,
));
}
}
}
let mut index = map_iterator.map_next_index;
let item_kind = map_iterator.map_iteration_kind;

if let Some(obj) = map_iterator.iterated_map.take() {
let map = obj.borrow();
let entries = map.as_map_ref().expect("iterator should only iterate maps");
let num_entries = entries.full_len();
while index < num_entries {
let e = entries.get_index(index);
index += 1;
map_iterator.map_next_index = index;
if let Some((key, value)) = e {
let item = match item_kind {
PropertyNameKind::Key => {
Ok(create_iter_result_object(key.clone(), false, context))
}
PropertyNameKind::Value => {
Ok(create_iter_result_object(value.clone(), false, context))
}
} else {
return Err(context.construct_type_error("'this' is not a Map"));
}
} else {
return Err(context.construct_type_error("'this' is not a Map"));
PropertyNameKind::KeyAndValue => {
let result = Array::create_array_from_list(
[key.clone(), value.clone()],
context,
);
Ok(create_iter_result_object(result.into(), false, context))
}
};
drop(map);
map_iterator.iterated_map = Some(obj);
return item;
}

map_iterator.iterated_map = JsValue::undefined();
Ok(create_iter_result_object(
context,
JsValue::undefined(),
true,
))
} else {
context.throw_type_error("`this` is not an MapIterator")
}
} else {
context.throw_type_error("`this` is not an MapIterator")
}

Ok(create_iter_result_object(
JsValue::undefined(),
true,
context,
))
}

/// Create the %MapIteratorPrototype% object
Expand All @@ -146,7 +126,7 @@ impl MapIterator {
/// - [ECMA reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-%mapiteratorprototype%-object
pub(crate) fn create_prototype(context: &mut Context, iterator_prototype: JsValue) -> JsObject {
pub(crate) fn create_prototype(iterator_prototype: JsValue, context: &mut Context) -> JsObject {
let _timer = BoaProfiler::global().start_event(Self::NAME, "init");

// Create prototype
Expand Down

0 comments on commit 8ba500a

Please sign in to comment.