Make iterator spec complaint (#1889)

This PR is also related to #577 

Changes:
- Implements `IteratorValue` (`IteratorResult::value()`)
- Implements `IteratorComplete` (`IteratorResult::complete()`)
- Implements `IteratorStep` (`IteratorRecord::step()`)
- Makes  `IteratorNext` (`IteratorRecord::next()`) spec compliant
- Deprecates/removes `JsValue::get_field()`.

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<bool> {
+        // 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<JsValue> {
+        // 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<IteratorResult> {
-        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<JsValue>,
+        context: &mut Context,
+    ) -> JsResult<IteratorResult> {
+        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<Option<IteratorResult>> {
+        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<JsValue>,
         context: &mut Context,
     ) -> JsResult<JsValue> {
+        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<JsValue>,
 ) -> JsResult<Vec<JsValue>> {
+    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,
-}

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`");

boa_engine/src/builtins/set/mod.rs

@@ -118,53 +118,53 @@ impl Set {
         args: &[JsValue],
         context: &mut Context,
     ) -> JsResult<JsValue> {
-        // 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 ]`

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<K>(&self, key: K, context: &mut Context) -> JsResult<Self>
-    where
-        K: Into<PropertyKey>,
-    {
-        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) {

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 => {