From d6c0ae5197424e190f5fd3e24ca6f5b3263b35e9 Mon Sep 17 00:00:00 2001 From: HalidOdat Date: Wed, 2 Mar 2022 18:00:25 +0100 Subject: [PATCH] Fix Iterator - Remove `JsValue::get_field()` --- boa_engine/src/builtins/iterable/mod.rs | 124 ++++++++++++++++++++---- boa_engine/src/builtins/map/mod.rs | 17 ++-- boa_engine/src/builtins/set/mod.rs | 40 ++++---- boa_engine/src/value/mod.rs | 17 ---- boa_engine/src/vm/mod.rs | 66 ++++++------- 5 files changed, 164 insertions(+), 100 deletions(-) diff --git a/boa_engine/src/builtins/iterable/mod.rs b/boa_engine/src/builtins/iterable/mod.rs index cf2317daeef..f5371d77097 100644 --- a/boa_engine/src/builtins/iterable/mod.rs +++ b/boa_engine/src/builtins/iterable/mod.rs @@ -23,6 +23,8 @@ pub struct IteratorPrototypes { impl IteratorPrototypes { pub(crate) fn init(context: &mut Context) -> Self { + let _timer = Profiler::global().start_event("IteratorPrototypes::init", "init"); + let iterator_prototype = create_iterator_prototype(context); Self { array_iterator: ArrayIterator::create_prototype(iterator_prototype.clone(), context), @@ -77,7 +79,10 @@ impl IteratorPrototypes { /// `CreateIterResultObject( value, done )` /// /// Generates an object supporting the `IteratorResult` interface. +#[inline] pub fn create_iter_result_object(value: JsValue, done: bool, context: &mut Context) -> JsValue { + let _timer = Profiler::global().start_event("create_iter_result_object", "init"); + // 1. Assert: Type(done) is Boolean. // 2. Let obj be ! OrdinaryObjectCreate(%Object.prototype%). let obj = context.construct_object(); @@ -106,6 +111,7 @@ impl JsValue { /// - [ECMA reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-getiterator + #[inline] pub fn get_iterator( &self, context: &mut Context, @@ -168,6 +174,7 @@ impl JsValue { /// - [ECMA reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-%iteratorprototype%-object +#[inline] fn create_iterator_prototype(context: &mut Context) -> JsObject { let _timer = Profiler::global().start_event("Iterator Prototype", "init"); @@ -182,13 +189,58 @@ fn create_iterator_prototype(context: &mut Context) -> JsObject { iterator_prototype } +#[derive(Debug)] +pub struct IteratorResult { + object: JsObject, +} + +impl IteratorResult { + /// Get `done` property of iterator result object. + /// + /// More information: + /// - [ECMA reference][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-iteratorclose + #[inline] + pub fn complete(&self, context: &mut Context) -> JsResult { + // 1. Return ToBoolean(? Get(iterResult, "done")). + Ok(self.object.get("done", context)?.to_boolean()) + } + + /// Get `value` property of iterator result object. + /// + /// More information: + /// - [ECMA reference][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-iteratorvalue + #[inline] + pub fn value(&self, context: &mut Context) -> JsResult { + // 1. Return ? Get(iterResult, "value"). + self.object.get("value", context) + } +} +/// An Iterator Record is a Record value used to encapsulate an +/// `Iterator` or `AsyncIterator` along with the next method. +/// +/// More information: +/// - [ECMA reference][spec] +/// +/// [spec]:https://tc39.es/ecma262/#table-iterator-record-fields #[derive(Debug)] pub struct IteratorRecord { + /// `[[Iterator]]` + /// + /// An object that conforms to the Iterator or AsyncIterator interface. iterator_object: JsValue, + + /// `[[NextMethod]]` + /// + /// The next method of the `[[Iterator]]` object. next_function: JsValue, } impl IteratorRecord { + #[inline] pub fn new(iterator_object: JsValue, next_function: JsValue) -> Self { Self { iterator_object, @@ -196,10 +248,12 @@ impl IteratorRecord { } } + #[inline] pub(crate) fn iterator_object(&self) -> &JsValue { &self.iterator_object } + #[inline] pub(crate) fn next_function(&self) -> &JsValue { &self.next_function } @@ -210,12 +264,50 @@ impl IteratorRecord { /// - [ECMA reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-iteratornext - pub(crate) fn next(&self, context: &mut Context) -> JsResult { - let next = context.call(&self.next_function, &self.iterator_object, &[])?; - let done = next.get_field("done", context)?.to_boolean(); + #[inline] + pub(crate) fn next( + &self, + value: Option, + context: &mut Context, + ) -> JsResult { + let _timer = Profiler::global().start_event("IteratorRecord::next", "iterator"); + + // 1. If value is not present, then + // a. Let result be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]]). + // 2. Else, + // a. Let result be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]], « value »). + let result = if let Some(value) = value { + context.call(&self.next_function, &self.iterator_object, &[value])? + } else { + context.call(&self.next_function, &self.iterator_object, &[])? + }; + + // 3. If Type(result) is not Object, throw a TypeError exception. + // 4. Return result. + if let Some(o) = result.as_object() { + Ok(IteratorResult { object: o.clone() }) + } else { + context.throw_type_error("next value should be an object") + } + } + + #[inline] + pub(crate) fn step(&self, context: &mut Context) -> JsResult> { + let _timer = Profiler::global().start_event("IteratorRecord::step", "iterator"); + + // 1. Let result be ? IteratorNext(iteratorRecord). + let result = self.next(None, context)?; - let value = next.get_field("value", context)?; - Ok(IteratorResult { value, done }) + // 2. Let done be ? IteratorComplete(result). + let done = result.complete(context)?; + + // 3. If done is true, return false. + if done { + return Ok(None); + } + + // 4. Return result. + Ok(Some(result)) } /// Cleanup the iterator @@ -224,16 +316,18 @@ impl IteratorRecord { /// - [ECMA reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-iteratorclose + #[inline] pub(crate) fn close( &self, completion: JsResult, context: &mut Context, ) -> JsResult { + let _timer = Profiler::global().start_event("IteratorRecord::close", "iterator"); + // 1. Assert: Type(iteratorRecord.[[Iterator]]) is Object. // 2. Let iterator be iteratorRecord.[[Iterator]]. // 3. Let innerResult be GetMethod(iterator, "return"). let inner_result = self.iterator_object.get_method("return", context); - //let mut inner_result = self.iterator_object.get_field("return", context); // 4. If innerResult.[[Type]] is normal, then if let Ok(inner_value) = inner_result { @@ -281,6 +375,8 @@ pub(crate) fn iterable_to_list( items: &JsValue, method: Option, ) -> JsResult> { + let _timer = Profiler::global().start_event("iterable_to_list", "iterator"); + // 1. If method is present, then let iterator_record = if let Some(method) = method { // a. Let iteratorRecord be ? GetIterator(items, sync, method). @@ -300,21 +396,11 @@ pub(crate) fn iterable_to_list( // b. If next is not false, then // i. Let nextValue be ? IteratorValue(next). // ii. Append nextValue to the end of the List values. - loop { - let next = iterator_record.next(context)?; - if next.done { - break; - } - - values.push(next.value); + while let Some(next) = iterator_record.step(context)? { + let next_value = next.value(context)?; + values.push(next_value); } // 6. Return values. Ok(values) } - -#[derive(Debug)] -pub struct IteratorResult { - pub value: JsValue, - pub done: bool, -} diff --git a/boa_engine/src/builtins/map/mod.rs b/boa_engine/src/builtins/map/mod.rs index 47dc6c5c821..06579579660 100644 --- a/boa_engine/src/builtins/map/mod.rs +++ b/boa_engine/src/builtins/map/mod.rs @@ -15,7 +15,7 @@ use self::{map_iterator::MapIterator, ordered_map::OrderedMap}; use super::JsArgs; use crate::{ - builtins::{iterable::IteratorResult, BuiltIn}, + builtins::BuiltIn, context::StandardObjects, object::{ internal_methods::get_prototype_from_constructor, ConstructorBuilder, FunctionBuilder, @@ -547,19 +547,20 @@ pub(crate) fn add_entries_from_iterable( // 3. Repeat, loop { // a. Let next be ? IteratorStep(iteratorRecord). - // c. Let nextItem be ? IteratorValue(next). - let IteratorResult { value, done } = iterator_record.next(context)?; + let next = iterator_record.step(context)?; // b. If next is false, return target. - if done { + // c. Let nextItem be ? IteratorValue(next). + let next_item = if let Some(next) = next { + next.value(context)? + } else { return Ok(target.clone().into()); - } + }; - let next_item = if let Some(obj) = value.as_object() { + let next_item = if let Some(obj) = next_item.as_object() { obj - } // d. If Type(nextItem) is not Object, then - else { + } else { // i. Let error be ThrowCompletion(a newly created TypeError object). let err = context .throw_type_error("cannot get key and value from primitive item of `iterable`"); diff --git a/boa_engine/src/builtins/set/mod.rs b/boa_engine/src/builtins/set/mod.rs index 1d33efec2fe..fff3de1f9d7 100644 --- a/boa_engine/src/builtins/set/mod.rs +++ b/boa_engine/src/builtins/set/mod.rs @@ -118,53 +118,53 @@ impl Set { args: &[JsValue], context: &mut Context, ) -> JsResult { - // 1 + // 1. If NewTarget is undefined, throw a TypeError exception. if new_target.is_undefined() { return context .throw_type_error("calling a builtin Set constructor without new is forbidden"); } - // 2 + // 2. Let set be ? OrdinaryCreateFromConstructor(NewTarget, "%Set.prototype%", « [[SetData]] »). + // 3. Set set.[[SetData]] to a new empty List. let prototype = get_prototype_from_constructor(new_target, StandardObjects::set_object, context)?; + let set = JsObject::from_proto_and_data(prototype, ObjectData::set(OrderedSet::default())); - let obj = JsObject::from_proto_and_data(prototype, ObjectData::set(OrderedSet::default())); - + // 4. If iterable is either undefined or null, return set. let iterable = args.get_or_undefined(0); - // 4 if iterable.is_null_or_undefined() { - return Ok(obj.into()); + return Ok(set.into()); } - // 5 - let adder = obj.get("add", context)?; + // 5. Let adder be ? Get(set, "add"). + let adder = set.get("add", context)?; - // 6 + // 6. If IsCallable(adder) is false, throw a TypeError exception. let adder = adder.as_callable().ok_or_else(|| { context.construct_type_error("'add' of 'newTarget' is not a function") })?; - // 7 + // 7. Let iteratorRecord be ? GetIterator(iterable). let iterator_record = iterable.clone().get_iterator(context, None, None)?; - // 8.a - let mut next = iterator_record.next(context)?; - - // 8 - while !next.done { + // 8. Repeat, + // a. Let next be ? IteratorStep(iteratorRecord). + // b. If next is false, return set. + // c. Let nextValue be ? IteratorValue(next). + // d. Let status be Completion(Call(adder, set, « nextValue »)). + // e. IfAbruptCloseIterator(status, iteratorRecord). + while let Some(next) = iterator_record.step(context)? { // c - let next_value = next.value; + let next_value = next.value(context)?; // d, e - if let Err(status) = adder.call(&obj.clone().into(), &[next_value], context) { + if let Err(status) = adder.call(&set.clone().into(), &[next_value], context) { return iterator_record.close(Err(status), context); } - - next = iterator_record.next(context)?; } // 8.b - Ok(obj.into()) + Ok(set.into()) } /// `get Set [ @@species ]` diff --git a/boa_engine/src/value/mod.rs b/boa_engine/src/value/mod.rs index e3178f2fa78..df3818a3b52 100644 --- a/boa_engine/src/value/mod.rs +++ b/boa_engine/src/value/mod.rs @@ -316,23 +316,6 @@ impl JsValue { } } - /** - Resolve the property in the object and get its value, or undefined if this is not an object or the field doesn't exist - `get_field` receives a Property from get_prop(). It should then return the `[[Get]]` result value if that's set, otherwise fall back to `[[Value]]` - */ - pub(crate) fn get_field(&self, key: K, context: &mut Context) -> JsResult - where - K: Into, - { - let _timer = Profiler::global().start_event("Value::get_field", "value"); - if let Self::Object(ref obj) = *self { - obj.clone() - .__get__(&key.into(), obj.clone().into(), context) - } else { - Ok(Self::undefined()) - } - } - /// Set the kind of an object. #[inline] pub fn set_data(&self, data: ObjectData) { diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index c7b5bc9ae10..90639f1ccfc 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -220,13 +220,8 @@ impl Context { let array = self.vm.pop(); let iterator = IteratorRecord::new(iterator, next_function); - loop { - let next = iterator.next(self)?; - - if next.done { - break; - } - Array::push(&array, &[next.value], self)?; + while let Some(next) = iterator.step(self)? { + Array::push(&array, &[next.value(self)?], self)?; } self.vm.push(array); @@ -980,12 +975,8 @@ impl Context { let iterator_record = rest_argument.get_iterator(self, None, None)?; let mut rest_arguments = Vec::new(); - loop { - let next = iterator_record.next(self)?; - if next.done { - break; - } - rest_arguments.push(next.value); + while let Some(next) = iterator_record.step(self)? { + rest_arguments.push(next.value(self)?); } arguments.append(&mut rest_arguments); @@ -1036,12 +1027,8 @@ impl Context { let iterator_record = rest_argument.get_iterator(self, None, None)?; let mut rest_arguments = Vec::new(); - loop { - let next = iterator_record.next(self)?; - if next.done { - break; - } - rest_arguments.push(next.value); + while let Some(next) = iterator_record.step(self)? { + rest_arguments.push(next.value(self)?); } arguments.append(&mut rest_arguments); @@ -1171,23 +1158,34 @@ impl Context { let iterator = self.vm.pop(); let iterator_record = IteratorRecord::new(iterator.clone(), next_function.clone()); - let iterator_result = iterator_record.next(self)?; + let next = iterator_record.step(self)?; self.vm.push(iterator); self.vm.push(next_function); - self.vm.push(iterator_result.value); + if let Some(next) = next { + let value = next.value(self)?; + self.vm.push(value); + } else { + self.vm.push(JsValue::undefined()); + } } Opcode::IteratorNextFull => { let next_function = self.vm.pop(); let iterator = self.vm.pop(); let iterator_record = IteratorRecord::new(iterator.clone(), next_function.clone()); - let iterator_result = iterator_record.next(self)?; + let next = iterator_record.step(self)?; self.vm.push(iterator); self.vm.push(next_function); - self.vm.push(iterator_result.done); - self.vm.push(iterator_result.value); + if let Some(next) = next { + let value = next.value(self)?; + self.vm.push(false); + self.vm.push(value); + } else { + self.vm.push(true); + self.vm.push(JsValue::undefined()); + } } Opcode::IteratorClose => { let done = self.vm.pop(); @@ -1205,12 +1203,8 @@ impl Context { let iterator_record = IteratorRecord::new(iterator.clone(), next_function.clone()); let mut values = Vec::new(); - loop { - let next = iterator_record.next(self)?; - if next.done { - break; - } - values.push(next.value); + while let Some(result) = iterator_record.step(self)? { + values.push(result.value(self)?); } let array = Array::create_array_from_list(values, self); @@ -1226,18 +1220,18 @@ impl Context { let iterator = self.vm.pop(); let iterator_record = IteratorRecord::new(iterator.clone(), next_function.clone()); - let iterator_result = iterator_record.next(self)?; - if iterator_result.done { + if let Some(next) = iterator_record.step(self)? { + self.vm.push(iterator); + self.vm.push(next_function); + let value = next.value(self)?; + self.vm.push(value); + } else { self.vm.frame_mut().pc = address as usize; self.vm.frame_mut().loop_env_stack_dec(); self.vm.frame_mut().try_env_stack_dec(); self.realm.environments.pop(); self.vm.push(iterator); self.vm.push(next_function); - } else { - self.vm.push(iterator); - self.vm.push(next_function); - self.vm.push(iterator_result.value); } } Opcode::ConcatToString => {