Unify async iterators and iterators compilation (#2976)

* Close active iterators when returning from async generator * Fix remaining async generator tests * Fix remaining async generator tests * Replace some reserved opcodes * Fix for await of loop flag * Add tests for fix * Remove whitespace * Fix test display and improve test * Change `usize` to `u32`
boa-dev · Jun 6, 2023 · a989462 · a989462
1 parent 6879c59
commit a989462
Show file tree

Hide file tree

Showing 40 changed files with 1,460 additions and 1,193 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -14,7 +14,7 @@
       "program": "${workspaceFolder}/target/debug/boa",
       "args": ["${workspaceFolder}/tests/js/test.js", "--debug-object"],
       "sourceLanguages": ["rust"],
-      "preLaunchTask": "Cargo Build"
+      "preLaunchTask": "Cargo Build boa_cli"
     },
     {
       "type": "lldb",
@@ -32,7 +32,7 @@
         "tests/js"
       ],
       "sourceLanguages": ["rust"],
-      "preLaunchTask": "Cargo Build"
+      "preLaunchTask": "Cargo Build boa_cli"
     }
   ]
 }
diff --git a/.vscode/tasks.json b/.vscode/tasks.json
@@ -13,6 +13,16 @@
         "clear": true
       }
     },
+    {
+      "type": "process",
+      "label": "Cargo Build boa_cli",
+      "command": "cargo",
+      "args": ["build", "-p", "boa_cli"],
+      "group": "build",
+      "presentation": {
+        "clear": true
+      }
+    },
     {
       "type": "process",
       "label": "Cargo Run",

diff --git a/boa_cli/src/main.rs b/boa_cli/src/main.rs
@@ -487,7 +487,7 @@ struct Jobs(RefCell<VecDeque<NativeJob>>);
 
 impl JobQueue for Jobs {
     fn enqueue_promise_job(&self, job: NativeJob, _: &mut Context<'_>) {
-        self.0.borrow_mut().push_front(job);
+        self.0.borrow_mut().push_back(job);
     }
 
     fn run_jobs(&self, context: &mut Context<'_>) {
@@ -506,6 +506,6 @@ impl JobQueue for Jobs {
 
     fn enqueue_future_job(&self, future: FutureJob, _: &mut Context<'_>) {
         let job = pollster::block_on(future);
-        self.0.borrow_mut().push_front(job);
+        self.0.borrow_mut().push_back(job);
     }
 }
diff --git a/boa_engine/src/builtins/array/mod.rs b/boa_engine/src/builtins/array/mod.rs
@@ -561,7 +561,7 @@ impl Array {
         };
 
         // c. Let iteratorRecord be ? GetIterator(items, sync, usingIterator).
-        let iterator_record =
+        let mut iterator_record =
             items.get_iterator(context, Some(IteratorHint::Sync), Some(using_iterator))?;
 
         // d. Let k be 0.
@@ -571,19 +571,16 @@ impl Array {
         //     x. Set k to k + 1.
         for k in 0..9_007_199_254_740_991_u64 {
             // iii. Let next be ? IteratorStep(iteratorRecord).
-            let next = iterator_record.step(context)?;
+            if iterator_record.step(context)? {
+                // 1. Perform ? Set(A, "length", 𝔽(k), true).
+                a.set(utf16!("length"), k, true, context)?;
+                // 2. Return A.
+                return Ok(a.into());
+            }
 
             // iv. If next is false, then
-            let Some(next) = next else {
-                    // 1. Perform ? Set(A, "length", 𝔽(k), true).
-                    a.set(utf16!("length"), k, true, context)?;
-
-                    // 2. Return A.
-                    return Ok(a.into());
-                };
-
             // v. Let nextValue be ? IteratorValue(next).
-            let next_value = next.value(context)?;
+            let next_value = iterator_record.value(context)?;
 
             // vi. If mapping is true, then
             let mapped_value = if let Some(mapfn) = mapping {

diff --git a/boa_engine/src/builtins/async_generator/mod.rs b/boa_engine/src/builtins/async_generator/mod.rs
@@ -18,7 +18,7 @@ use crate::{
     realm::Realm,
     symbol::JsSymbol,
     value::JsValue,
-    vm::GeneratorResumeKind,
+    vm::{CompletionRecord, GeneratorResumeKind},
     Context, JsArgs, JsError, JsResult,
 };
 use boa_gc::{Finalize, Trace};
@@ -46,7 +46,7 @@ pub(crate) enum AsyncGeneratorState {
 #[derive(Debug, Clone, Finalize, Trace)]
 pub(crate) struct AsyncGeneratorRequest {
     /// The `[[Completion]]` slot.
-    pub(crate) completion: (JsResult<JsValue>, bool),
+    pub(crate) completion: CompletionRecord,
 
     /// The `[[Capability]]` slot.
     capability: PromiseCapability,
@@ -164,7 +164,7 @@ impl AsyncGenerator {
         }
 
         // 7. Let completion be NormalCompletion(value).
-        let completion = (Ok(args.get_or_undefined(0).clone()), false);
+        let completion = CompletionRecord::Normal(args.get_or_undefined(0).clone());
 
         // 8. Perform AsyncGeneratorEnqueue(generator, completion, promiseCapability).
         generator.enqueue(completion.clone(), promise_capability.clone());
@@ -232,7 +232,8 @@ impl AsyncGenerator {
         if_abrupt_reject_promise!(generator, promise_capability, context);
 
         // 5. Let completion be Completion Record { [[Type]]: return, [[Value]]: value, [[Target]]: empty }.
-        let completion = (Ok(args.get_or_undefined(0).clone()), true);
+        let return_value = args.get_or_undefined(0).clone();
+        let completion = CompletionRecord::Return(return_value.clone());
 
         // 6. Perform AsyncGeneratorEnqueue(generator, completion, promiseCapability).
         generator.enqueue(completion.clone(), promise_capability.clone());
@@ -246,14 +247,8 @@ impl AsyncGenerator {
             generator.state = AsyncGeneratorState::AwaitingReturn;
 
             // b. Perform ! AsyncGeneratorAwaitReturn(generator).
-            let next = generator
-                .queue
-                .front()
-                .cloned()
-                .expect("queue cannot be empty here");
             drop(generator_obj_mut);
-            let (completion, _) = &next.completion;
-            Self::await_return(generator_object.clone(), completion.clone(), context);
+            Self::await_return(generator_object.clone(), return_value, context);
         }
         // 9. Else if state is suspendedYield, then
         else if state == AsyncGeneratorState::SuspendedYield {
@@ -346,10 +341,8 @@ impl AsyncGenerator {
         }
 
         // 8. Let completion be ThrowCompletion(exception).
-        let completion = (
-            Err(JsError::from_opaque(args.get_or_undefined(0).clone())),
-            false,
-        );
+        let completion =
+            CompletionRecord::Throw(JsError::from_opaque(args.get_or_undefined(0).clone()));
 
         // 9. Perform AsyncGeneratorEnqueue(generator, completion, promiseCapability).
         generator.enqueue(completion.clone(), promise_capability.clone());
@@ -384,7 +377,7 @@ impl AsyncGenerator {
     /// [spec]: https://tc39.es/ecma262/#sec-asyncgeneratorenqueue
     pub(crate) fn enqueue(
         &mut self,
-        completion: (JsResult<JsValue>, bool),
+        completion: CompletionRecord,
         promise_capability: PromiseCapability,
     ) {
         // 1. Let request be AsyncGeneratorRequest { [[Completion]]: completion, [[Capability]]: promiseCapability }.
@@ -469,7 +462,7 @@ impl AsyncGenerator {
         generator: &JsObject,
         state: AsyncGeneratorState,
         mut generator_context: GeneratorContext,
-        completion: (JsResult<JsValue>, bool),
+        completion: CompletionRecord,
         context: &mut Context<'_>,
     ) {
         // 1. Assert: generator.[[AsyncGeneratorState]] is either suspendedStart or suspendedYield.
@@ -491,15 +484,9 @@ impl AsyncGenerator {
             .state = AsyncGeneratorState::Executing;
 
         let (value, resume_kind) = match completion {
-            (Ok(value), r#return) => (
-                value,
-                if r#return {
-                    GeneratorResumeKind::Return
-                } else {
-                    GeneratorResumeKind::Normal
-                },
-            ),
-            (Err(value), _) => (value.to_opaque(context), GeneratorResumeKind::Throw),
+            CompletionRecord::Normal(val) => (val, GeneratorResumeKind::Normal),
+            CompletionRecord::Return(val) => (val, GeneratorResumeKind::Return),
+            CompletionRecord::Throw(err) => (err.to_opaque(context), GeneratorResumeKind::Throw),
         };
         // 6. Push genContext onto the execution context stack; genContext is now the running execution context.
 
@@ -527,19 +514,12 @@ impl AsyncGenerator {
     ///  - [ECMAScript reference][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-asyncgeneratorawaitreturn
-    pub(crate) fn await_return(
-        generator: JsObject,
-        completion: JsResult<JsValue>,
-        context: &mut Context<'_>,
-    ) {
+    pub(crate) fn await_return(generator: JsObject, value: JsValue, context: &mut Context<'_>) {
         // 1. Let queue be generator.[[AsyncGeneratorQueue]].
         // 2. Assert: queue is not empty.
         // 3. Let next be the first element of queue.
         // 4. Let completion be Completion(next.[[Completion]]).
 
-        // 5. Assert: completion.[[Type]] is return.
-        let value = completion.expect("completion must be a return completion");
-
         // Note: The spec is currently broken here.
         // See: https://github.com/tc39/ecma262/pull/2683
 
@@ -681,29 +661,24 @@ impl AsyncGenerator {
             let next = queue.front().expect("must have entry");
 
             // b. Let completion be Completion(next.[[Completion]]).
-            match &next.completion {
+            match next.completion.clone() {
                 // c. If completion.[[Type]] is return, then
-                (completion, true) => {
+                CompletionRecord::Return(val) => {
                     // i. Set generator.[[AsyncGeneratorState]] to awaiting-return.
                     gen.state = AsyncGeneratorState::AwaitingReturn;
+                    drop(generator_borrow_mut);
 
                     // ii. Perform ! AsyncGeneratorAwaitReturn(generator).
-                    let completion = completion.clone();
-                    drop(generator_borrow_mut);
-                    Self::await_return(generator.clone(), completion, context);
+                    Self::await_return(generator.clone(), val, context);
 
                     // iii. Set done to true.
                     break;
                 }
                 // d. Else,
-                (completion, false) => {
+                completion => {
                     // i. If completion.[[Type]] is normal, then
-                    let completion = if completion.is_ok() {
-                        // 1. Set completion to NormalCompletion(undefined).
-                        Ok(JsValue::undefined())
-                    } else {
-                        completion.clone()
-                    };
+                    // 1. Set completion to NormalCompletion(undefined).
+                    let completion = completion.consume().map(|_| JsValue::undefined());
 
                     // ii. Perform AsyncGeneratorCompleteStep(generator, completion, true).
                     let next = queue.pop_front().expect("must have entry");

diff --git a/boa_engine/src/builtins/intl/list_format/mod.rs b/boa_engine/src/builtins/intl/list_format/mod.rs
@@ -469,7 +469,7 @@ fn string_list_from_iterable(
     }
 
     // 2. Let iteratorRecord be ? GetIterator(iterable).
-    let iterator = iterable.get_iterator(context, None, None)?;
+    let mut iterator = iterable.get_iterator(context, None, None)?;
 
     // 3. Let list be a new empty List.
     let mut list = Vec::new();
@@ -478,9 +478,9 @@ fn string_list_from_iterable(
     // 5. Repeat, while next is not false,
     //     a. Set next to ? IteratorStep(iteratorRecord).
     //     b. If next is not false, then
-    while let Some(item) = iterator.step(context)? {
+    while !iterator.step(context)? {
+        let item = iterator.value(context)?;
         //    i. Let nextValue be ? IteratorValue(next).
-        let item = item.value(context)?;
         //    ii. If Type(nextValue) is not String, then
         let Some(s) = item.as_string().cloned() else {
             //    1. Let error be ThrowCompletion(a newly created TypeError object).

diff --git a/boa_engine/src/builtins/iterable/async_from_sync_iterator.rs b/boa_engine/src/builtins/iterable/async_from_sync_iterator.rs
@@ -9,7 +9,7 @@ use crate::{
     object::{FunctionObjectBuilder, JsObject, ObjectData},
     realm::Realm,
     string::utf16,
-    Context, JsArgs, JsNativeError, JsResult, JsValue,
+    Context, JsArgs, JsResult, JsValue,
 };
 use boa_gc::{Finalize, Trace};
 use boa_profiler::Profiler;
@@ -84,7 +84,7 @@ impl AsyncFromSyncIterator {
 
         // 4. Let iteratorRecord be the Iterator Record { [[Iterator]]: asyncIterator, [[NextMethod]]: nextMethod, [[Done]]: false }.
         // 5. Return iteratorRecord.
-        IteratorRecord::new(async_iterator, next_method, false)
+        IteratorRecord::new(async_iterator, next_method)
     }
 
     /// `%AsyncFromSyncIteratorPrototype%.next ( [ value ] )`
@@ -117,7 +117,15 @@ impl AsyncFromSyncIterator {
         // a. Let result be Completion(IteratorNext(syncIteratorRecord, value)).
         // 6. Else,
         // a. Let result be Completion(IteratorNext(syncIteratorRecord)).
-        let result = sync_iterator_record.next(args.get(0), context);
+        let iterator = sync_iterator_record.iterator().clone();
+        let next = sync_iterator_record.next_method();
+        let result = next
+            .call(
+                &iterator.into(),
+                args.get(0).map_or(&[], std::slice::from_ref),
+                context,
+            )
+            .and_then(IteratorResult::from_value);
 
         // 7. IfAbruptRejectPromise(result, promiseCapability).
         if_abrupt_reject_promise!(result, promise_capability, context);
@@ -187,36 +195,15 @@ impl AsyncFromSyncIterator {
             }
         };
 
+        // 11. If Type(result) is not Object, then
+        // a. Perform ! Call(promiseCapability.[[Reject]], undefined, « a newly created TypeError object »).
+        let result = result.and_then(IteratorResult::from_value);
+
         // 10. IfAbruptRejectPromise(result, promiseCapability).
         if_abrupt_reject_promise!(result, promise_capability, context);
 
-        let Some(result) = result.as_object() else {
-            // 11. If Type(result) is not Object, then
-            // a. Perform ! Call(promiseCapability.[[Reject]], undefined, « a newly created TypeError object »).
-            promise_capability
-                .reject()
-                .call(
-                    &JsValue::Undefined,
-                    &[JsNativeError::typ()
-                        .with_message("iterator return function returned non-object")
-                        .to_opaque(context)
-                        .into()],
-                    context,
-                )
-                .expect("cannot fail according to spec");
-
-            // b. Return promiseCapability.[[Promise]].
-            return Ok(promise_capability.promise().clone().into());
-        };
-
         // 12. Return AsyncFromSyncIteratorContinuation(result, promiseCapability).
-        Self::continuation(
-            &IteratorResult {
-                object: result.clone(),
-            },
-            &promise_capability,
-            context,
-        )
+        Self::continuation(&result, &promise_capability, context)
     }
 
     /// `%AsyncFromSyncIteratorPrototype%.throw ( [ value ] )`
@@ -280,36 +267,15 @@ impl AsyncFromSyncIterator {
             }
         };
 
+        // 11. If Type(result) is not Object, then
+        // a. Perform ! Call(promiseCapability.[[Reject]], undefined, « a newly created TypeError object »).
+        let result = result.and_then(IteratorResult::from_value);
+
         // 10. IfAbruptRejectPromise(result, promiseCapability).
         if_abrupt_reject_promise!(result, promise_capability, context);
 
-        let Some(result) = result.as_object() else {
-            // 11. If Type(result) is not Object, then
-            // a. Perform ! Call(promiseCapability.[[Reject]], undefined, « a newly created TypeError object »).
-            promise_capability
-                .reject()
-                .call(
-                    &JsValue::Undefined,
-                    &[JsNativeError::typ()
-                        .with_message("iterator throw function returned non-object")
-                        .to_opaque(context)
-                        .into()],
-                    context,
-                )
-                .expect("cannot fail according to spec");
-
-            // b. Return promiseCapability.[[Promise]].
-            return Ok(promise_capability.promise().clone().into());
-        };
-
         // 12. Return AsyncFromSyncIteratorContinuation(result, promiseCapability).
-        Self::continuation(
-            &IteratorResult {
-                object: result.clone(),
-            },
-            &promise_capability,
-            context,
-        )
+        Self::continuation(&result, &promise_capability, context)
     }
 
     /// `AsyncFromSyncIteratorContinuation ( result, promiseCapability )`