From 2878e361c90c30203542aa9ca34b2494733da864 Mon Sep 17 00:00:00 2001
From: HalidOdat <halidodat@gmail.com>
Date: Mon, 7 Mar 2022 18:33:57 +0100
Subject: [PATCH] Resolve review comments

- Make `IfAbruptCloseIterator` a pub(crate) macro since it's used in more places in the spec.
---
 boa_engine/src/builtins/array/mod.rs    | 28 +++++++------------------
 boa_engine/src/builtins/iterable/mod.rs | 20 ++++++++++++++++++
 2 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/boa_engine/src/builtins/array/mod.rs b/boa_engine/src/builtins/array/mod.rs
index f70298aa8c6..03f5124dcbe 100644
--- a/boa_engine/src/builtins/array/mod.rs
+++ b/boa_engine/src/builtins/array/mod.rs
@@ -19,7 +19,7 @@ use tap::{Conv, Pipe};
 use super::JsArgs;
 use crate::{
     builtins::array::array_iterator::ArrayIterator,
-    builtins::iterable::IteratorHint,
+    builtins::iterable::{if_abrupt_close_iterator, IteratorHint},
     builtins::BuiltIn,
     builtins::Number,
     context::intrinsics::StandardConstructors,
@@ -398,15 +398,10 @@ impl Array {
             //     a. If IsCallable(mapfn) is false, throw a TypeError exception.
             //     b. Let mapping be true.
             Some(mapfn.as_callable().ok_or_else(|| {
-                context.construct_type_error(format!("{} is not a function", mapfn.display()))
+                context.construct_type_error(format!("{} is not a function", mapfn.type_of()))
             })?)
         };
 
-        if items.is_null_or_undefined() {
-            return context
-                .throw_type_error("Array.from requires an array-like object or iterator");
-        }
-
         // 4. Let usingIterator be ? GetMethod(items, @@iterator).
         let using_iterator = items
             .get_method(WellKnownSymbols::iterator(), context)?
@@ -434,24 +429,14 @@ impl Array {
             let iterator_record =
                 items.get_iterator(context, Some(IteratorHint::Sync), Some(using_iterator))?;
 
-            // IfAbruptCloseIterator is a shorthand for a sequence of algorithm steps that use an Iterator Record
-            macro_rules! if_abrupt_close_iterator {
-                ($val:expr, $iterator_record:expr) => { match $val {
-                    // 2. Else if value is a Completion Record, set value to value.
-                    Ok(val) => val,
-                    // 1. If value is an abrupt completion, return ? IteratorClose(iteratorRecord, value).
-                    Err(err) => return $iterator_record.close(Err(err), context)
-                } }
-            }
-
             // d. Let k be 0.
             // e. Repeat,
             //     i. If k ≥ 2^53 - 1 (MAX_SAFE_INTEGER), then
             //     ...
             //     x. Set k to k + 1.
             for k in 0..9_007_199_254_740_991_u64 {
-                let next =
-                    if_abrupt_close_iterator!(iterator_record.step(context), iterator_record);
+                // iii. Let next be ? IteratorStep(iteratorRecord).
+                let next = iterator_record.step(context)?;
 
                 // iv. If next is false, then
                 if next.is_none() {
@@ -474,16 +459,17 @@ impl Array {
                     let mapped_value = mapfn.call(this_arg, &[next_value, k.into()], context);
 
                     // 2. IfAbruptCloseIterator(mappedValue, iteratorRecord).
-                    if_abrupt_close_iterator!(mapped_value, iterator_record)
+                    if_abrupt_close_iterator!(mapped_value, iterator_record, context)
                 } else {
                     // vii. Else, let mappedValue be nextValue.
                     next_value
                 };
+
                 // viii. Let defineStatus be CreateDataPropertyOrThrow(A, Pk, mappedValue).
                 let define_status = a.create_data_property_or_throw(k, mapped_value, context);
 
                 // ix. IfAbruptCloseIterator(defineStatus, iteratorRecord).
-                if_abrupt_close_iterator!(define_status, iterator_record);
+                if_abrupt_close_iterator!(define_status, iterator_record, context);
             }
 
             // NOTE: The loop above has to return before it reaches iteration limit,
diff --git a/boa_engine/src/builtins/iterable/mod.rs b/boa_engine/src/builtins/iterable/mod.rs
index a5c773c6c70..08eebd96ea0 100644
--- a/boa_engine/src/builtins/iterable/mod.rs
+++ b/boa_engine/src/builtins/iterable/mod.rs
@@ -411,3 +411,23 @@ pub(crate) fn iterable_to_list(
     // 6. Return values.
     Ok(values)
 }
+
+/// A shorthand for a sequence of algorithm steps that use an Iterator Record
+///
+/// More information:
+///  - [ECMA reference][spec]
+///
+///  [spec]: https://tc39.es/ecma262/#sec-ifabruptcloseiterator
+macro_rules! if_abrupt_close_iterator {
+    ($value:expr, $iterator_record:expr, $context:expr) => {
+        match $value {
+            // 1. If value is an abrupt completion, return ? IteratorClose(iteratorRecord, value).
+            Err(err) => return $iterator_record.close(Err(err), $context),
+            // 2. Else if value is a Completion Record, set value to value.
+            Ok(value) => value,
+        }
+    };
+}
+
+// Export macro to crate level
+pub(crate) use if_abrupt_close_iterator;