Repository: extendr/extendr
Pull request #167 (2021-02-01 22:53:31)

Numeric wrappers
This PR adds the wrappers Int, Real and Logical as well as the types IntIn, RealIn and LogicalIn for parameters.

For example, when returning an integer vector, we should be able to do:

```
fn add(a: IntIn, b:IntIn) -> Robj {
   Int(a.zip(b).map(|(a, b)| a + b)).into()
}
```
(Need to confirm this).

We may need to implement IntoIterator for them all.

Also, this may be too complex. I'm happy if we just keep the iterators.


Ilia-Kosenkov: @andy-thomason,
I am not sure I follow why we need such extra types. I think in discord and various issue topics here there were discussions of why there is a dedicated `Bool` type but not `Integer`/`Real`, etc. It is good to have iterators, but why not declare `wrappers::Integer(i32)` to completely separate opaque R integer type and transparent Rust type.

<details>
<summary>

[Rust playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&code=%23%5Bderive(Debug%2C%20Clone%2C%20PartialEq%2C%20Eq)%5D%0Astruct%20Integer(i32)%3B%0A%0Aimpl%20Integer%20%7B%0A%20%20%20%20pub%20const%20NA%20%3A%20Integer%20%3D%20Integer(std%3A%3Ai32%3A%3AMIN)%3B%0A%0A%20%20%20%20pub%20fn%20is_na(%26self)%20-%3E%20bool%20%7B%0A%20%20%20%20%20%20%20%20self.0%20%3D%3D%20(Integer%3A%3ANA).0%0A%20%20%20%20%7D%0A%20%20%20%20%0A%20%20%20%20%2F%2F%20For%20the%20sake%20of%20example%0A%20%20%20%20pub%20fn%20into_slice%3C%27a%3E(input%20%3A%20%26%27a%20%5BInteger%5D)%20-%3E%20Option%3C%26%27a%20%5Bi32%5D%3E%20%7B%0A%20%20%20%20%20%20%20%20if%20input.iter().any(Integer%3A%3Ais_na)%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20None%0A%20%20%20%20%20%20%20%20%7D%20else%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20unsafe%20%7B%20Some(std%3A%3Amem%3A%3Atransmute(input))%20%7D%0A%20%20%20%20%20%20%20%20%7D%0A%20%20%20%20%7D%0A%7D%0A%0Aimpl%20From%3C%26Integer%3E%20for%20Option%3Ci32%3E%20%7B%0A%20%20%20%20fn%20from(input%20%3A%20%26Integer)%20-%3E%20Self%20%7B%0A%20%20%20%20%20%20%20%20if%20input.is_na()%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20None%0A%20%20%20%20%20%20%20%20%7D%20else%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20Some(input.0)%0A%20%20%20%20%20%20%20%20%7D%0A%20%20%20%20%7D%0A%7D%0A%0Aimpl%20From%3COption%3Ci32%3E%3E%20for%20Integer%20%7B%0A%20%20%20%20fn%20from(input%20%3A%20Option%3A%3A%3Ci32%3E)%20-%3E%20Self%20%7B%0A%20%20%20%20%20%20%20%20if%20let%20Some(int_val)%20%3D%20input%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20Integer(int_val)%0A%20%20%20%20%20%20%20%20%7D%20else%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20Integer%3A%3ANA%0A%20%20%20%20%20%20%20%20%7D%0A%20%20%20%20%7D%0A%7D%0A%0Afn%20test_fn%3C%27a%3E(a%20%3A%20%26%27a%20%5BInteger%5D%2C%20b%20%3A%20%26%27a%20%5BInteger%5D)%20-%3E%20Vec%3CInteger%3E%20%7B%0A%20%20%20%20let%20a%20%3D%20Integer%3A%3Ainto_slice(a).unwrap()%3B%0A%20%20%20%20let%20b%20%3D%20Integer%3A%3Ainto_slice(b).unwrap()%3B%0A%20%20%20%20a%0A%20%20%20%20%20%20%20%20.iter()%0A%20%20%20%20%20%20%20%20.zip(b)%0A%20%20%20%20%20%20%20%20.map(%7C(x%2C%20y)%7C%20Some(x%20%2B%20y).into())%0A%20%20%20%20%20%20%20%20.collect%3A%3A%3CVec%3CInteger%3E%3E()%0A%7D%0A%0Afn%20main()%20%7B%0A%0A%20%20%20%20let%20x%20%3D%20vec!%5BInteger(1)%2C%20Integer(2)%2C%20Integer(3)%5D%3B%0A%20%20%20%20let%20y%20%3D%20vec!%5BInteger(100)%2C%20Integer(200)%2C%20Integer(300)%5D%3B%0A%20%20%20%20%0A%20%20%20%20let%20z%20%3D%20test_fn(%26x%2C%20%26y)%3B%0A%20%20%20%20%0A%20%20%20%20println!(%22size_of%3A%3A%3CInteger%3E()%20%3D%20%7B%7D%22%2C%20std%3A%3Amem%3A%3Asize_of%3A%3A%3CInteger%3E())%3B%0A%20%20%20%20%0A%20%20%20%20for%20item%20in%20z.iter()%20%7B%0A%20%20%20%20%20%20%20%20println!(%22%7B%3A%3F%7D%22%2C%20item)%3B%0A%20%20%20%20%7D%0A%20%20%20%20%0A%7D)
</summary>

```Rust
#[derive(Debug, Clone, PartialEq, Eq)]
struct Integer(i32);

impl Integer {
    pub const NA : Integer = Integer(std::i32::MIN);

    pub fn is_na(&self) -> bool {
        self.0 == (Integer::NA).0
    }
    
    // For the sake of example
    pub fn into_slice<'a>(input : &'a [Integer]) -> Option<&'a [i32]> {
        if input.iter().any(Integer::is_na) {
            None
        } else {
            unsafe { Some(std::mem::transmute(input)) }
        }
    }
}

impl From<&Integer> for Option<i32> {
    fn from(input : &Integer) -> Self {
        if input.is_na() {
            None
        } else {
            Some(input.0)
        }
    }
}

impl From<Option<i32>> for Integer {
    fn from(input : Option::<i32>) -> Self {
        if let Some(int_val) = input {
            Integer(int_val)
        } else {
            Integer::NA
        }
    }
}

fn test_fn<'a>(a : &'a [Integer], b : &'a [Integer]) -> Vec<Integer> {
    let a = Integer::into_slice(a).unwrap();
    let b = Integer::into_slice(b).unwrap();
    a
        .iter()
        .zip(b)
        .map(|(x, y)| Some(x + y).into())
        .collect::<Vec<Integer>>()
}

fn main() {

    let x = vec![Integer(1), Integer(2), Integer(3)];
    let y = vec![Integer(100), Integer(200), Integer(300)];
    
    let z = test_fn(&x, &y);
    
    println!("size_of::<Integer>() = {}", std::mem::size_of::<Integer>());
    
    for item in z.iter() {
        println!("{:?}", item);
    }
    
}
```

</details>

Then all other container types can be built atop of `&[Integer]`. Same for other vectors.

 
andy-thomason: This is primarily a normalisation of the wrapper types as others mostly wrap an iterator.

I've had second thoughts about the wrappers and I'll probably just leave the iterators with Int, Real and Logical aliases.


andy-thomason: We don't need anything like Bool for i32 and f64 as they both support IsNA.
andy-thomason: Although we could add a low performance wrapper for correctness.
andy-thomason: Now it is a little simpler:

The iterator *is* the wrapper and the name is shortend:

fn add(a: Int, b: Int) -> Robj {
  a.zip(b).map(|(a,b)| a+b).collect_robj()
}

fn add_real(a: Real, b: Real) -> Robj {
  a.zip(b).map(|(a,b)| a+b).collect_robj()
}


Patch:
diff --git a/ci-cargo.ps1 b/ci-cargo.ps1
new file mode 100644
index 000000000..0e5adfd3c
--- /dev/null
+++ b/ci-cargo.ps1
@@ -0,0 +1,71 @@
+function ci-cargo {
+
+    param(
+        [Parameter(Position = 0, ValueFromRemainingArguments)]
+        [String[]]
+        $CargoArgs,
+        [String]
+        $ActionName
+    )
+
+
+    try {    
+        echo "::group::$ActionName"
+        echo "Running cargo $CargoArgs"
+        cargo $CargoArgs
+        if($LASTEXITCODE -ne  0) {
+            throw $LASTEXITCODE
+        }
+    }
+    catch {
+        if ($ActionName -ne $null -and $ActionName -ne "") {
+            $ActionName = "'$ActionName': "
+        }
+        $err_msg = "$($ActionName)cargo failed with code $LASTEXITCODE (args: $CargoArgs)"
+        echo "::error::$err_msg"
+        Write-Error -Message "$err_msg" -ErrorAction Stop 
+    }
+    finally {
+        echo "::endgroup::"
+    }
+
+    <#
+        .SYNOPSIS
+        Runs cargo with specified args, adapting error handling and output to CI.
+
+        .DESCRIPTION
+        Runs cargo in a `try` block, catches exceptions and non-zero exit codes.
+        Explicitly logs the beginning and the end of cargo execution, as well as the error message.
+
+        .PARAMETER CargoArgs
+        Arguments passed to cargo, as-is.
+        Note that `--` separator is handled by powershell itself,
+        so it should be wrapped in quotes `'--'` and passed as string.
+
+        .PARAMETER ActionName
+        Optional string that is used to format logs and error messages.
+
+        .INPUTS
+        None. You cannot pipe objects.
+
+        .OUTPUTS
+        No explicit output.
+
+        .EXAMPLE
+        PS> ci-cargo --version
+            ::group::
+            Running cargo --version
+            cargo 1.49.0 (d00d64df9 2020-12-05)
+            ::endgroup::
+
+        .EXAMPLE
+        PS> ci-cargo -ActioName "Build"  build
+
+        .EXAMPLE
+        PS> ci-cargo +stable-x86_64-pc-windows-gnu test --features tests-all --target i686-pc-windows-gnu '--' --nocapture -ActionName "Called from documentation"
+
+        .LINK
+        Used by: https://github.com/extendr/extendr
+    #>
+    
+}
diff --git a/extendr-api/src/functions.rs b/extendr-api/src/functions.rs
index 8363a0570..8ed122986 100644
--- a/extendr-api/src/functions.rs
+++ b/extendr-api/src/functions.rs
@@ -34,8 +34,7 @@ pub fn new_env_with_capacity(capacity: usize) -> Robj {
         // Unhashed envirnment
         call!("new.env", FALSE, global_env(), 0).unwrap()
     } else {
-        // Hashed environment, the pidgeon hole principle
-        // ensures there are empty slots in the hash table.
+        // Hashed environment for larger hashmaps.
         call!("new.env", TRUE, global_env(), capacity as i32 * 2 + 1).unwrap()
     }
 }
diff --git a/extendr-api/src/iter.rs b/extendr-api/src/iter.rs
index 3606c6153..e9f16db11 100644
--- a/extendr-api/src/iter.rs
+++ b/extendr-api/src/iter.rs
@@ -1,6 +1,24 @@
 use crate::*;
 
-// Iterator over the objects in a VECSXP, EXPRSXP or WEAKREFSXP.
+/// Iterator over the objects in a VECSXP, EXPRSXP or WEAKREFSXP.
+///
+/// ```
+/// use extendr_api::prelude::*;
+/// test! {
+///     let my_list = list!(a = 1, b = 2);
+///     let mut total = 0;
+///     for robj in my_list.as_list_iter().unwrap() {
+///       if let Some(val) = robj.as_integer() {
+///         total += val;
+///       }
+///     }
+///     assert_eq!(total, 3);
+///    
+///     for name in my_list.names().unwrap() {
+///        assert!(name == "a" || name == "b")
+///     }
+/// }
+/// ```
 #[derive(Clone)]
 pub struct ListIter {
     vector: Robj,
@@ -42,37 +60,95 @@ impl Iterator for ListIter {
     }
 }
 
+/// Generalised iterator of numbers and logical. See Int, Real and Logical.
+pub struct SliceIter<T> {
+    // Control lifetime of vector to make sure the memory is not freed.
+    #[allow(dead_code)]
+    vector: Robj,
+    i: usize,
+    len: usize,
+    ptr: *const T,
+}
+
+impl<T> SliceIter<T> {
+    // A new, empty list iterator.
+    pub fn new() -> Self {
+        SliceIter {
+            vector: ().into(),
+            i: 0,
+            len: 0,
+            ptr: std::ptr::null(),
+        }
+    }
+
+    pub fn from_slice(vector: Robj, slice: &[T]) -> Self {
+        SliceIter {
+            vector,
+            i: 0,
+            len: slice.len(),
+            ptr: slice.as_ptr(),
+        }
+    }
+}
+
+/// Basis of Int, Real and Logical.
+impl<T: Copy> Iterator for SliceIter<T> {
+    type Item = T;
+
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        (self.len, Some(self.len))
+    }
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let i = self.i;
+        self.i += 1;
+        if i >= self.len {
+            self.i = self.len;
+            None
+        } else {
+            unsafe { Some(*self.ptr.offset(i as isize)) }
+        }
+    }
+
+    fn nth(&mut self, n: usize) -> Option<Self::Item> {
+        self.i += n;
+        self.next()
+    }
+}
+
 /// Iterator over name-value pairs in lists.
 pub type NamedListIter = std::iter::Zip<StrIter, ListIter>;
 
 /// Iterator over primitives in integer objects.
-pub type IntegerIter<'a> = std::iter::Cloned<std::slice::Iter<'a, i32>>;
+/// ```
+/// use extendr_api::prelude::*;
+///
+/// fn add(a: Int, b: Int) -> Robj {
+///     a.zip(b).map(|(a, b)| a+b).collect_robj()
+/// }
+/// ```
+pub type Int = SliceIter<i32>;
 
 /// Iterator over primitives in real objects.
-pub type RealIter<'a> = std::iter::Cloned<std::slice::Iter<'a, f64>>;
+/// ```
+/// use extendr_api::prelude::*;
+///
+/// fn add1(a: Real) -> Robj {
+///     a.map(|a| a + 1.0).collect_robj()
+/// }
+/// ```
+pub type Real = SliceIter<f64>;
 
 /// Iterator over primitives in logical objects.
-pub type LogicalIter<'a> = std::iter::Cloned<std::slice::Iter<'a, Bool>>;
-
-/// Iterator over the objects in a vector or string.
-///
 /// ```
 /// use extendr_api::prelude::*;
-/// test! {
-///     let my_list = list!(a = 1, b = 2);
-///     let mut total = 0;
-///     for robj in my_list.as_list_iter().unwrap() {
-///       if let Some(val) = robj.as_integer() {
-///         total += val;
-///       }
-///     }
-///     assert_eq!(total, 3);
-///    
-///     for name in my_list.names().unwrap() {
-///        assert!(name == "a" || name == "b")
-///     }
+///
+/// fn all_true(mut a: Logical) -> bool {
+///     a.all(|a| a.is_true())
 /// }
 /// ```
+pub type Logical = SliceIter<Bool>;
+
 #[derive(Clone)]
 pub struct PairlistIter {
     root_obj: Robj,
diff --git a/extendr-api/src/lib.rs b/extendr-api/src/lib.rs
index 8f42ef53c..0d07fe4cc 100644
--- a/extendr-api/src/lib.rs
+++ b/extendr-api/src/lib.rs
@@ -41,6 +41,7 @@
 //! test! {
 //!     // An R object with a single string "hello"
 //!     let character = r!("hello");
+//!     let character = r!(["hello", "goodbye"]);
 //!    
 //!     // An R integer object with a single number 1L.
 //!     // Note that in Rust, 1 is an integer and 1.0 is a real.
@@ -52,6 +53,8 @@
 //!    
 //!     // An R real vector.
 //!     let real_vector = r!([1.0, 2.0]);
+//!     let real_vector = &[1.0, 2.0].iter().collect_robj();
+//!     let real_vector = r!(vec![1.0, 2.0]);
 //!    
 //!     // An R function object.
 //!     let function = R!(function(x, y) { x + y })?;
@@ -59,8 +62,16 @@
 //!     // A named list using the list! macro.
 //!     let list = list!(a = 1, b = 2);
 //!    
+//!     // An unnamed list (of R objects) using the List wrapper.
+//!     let list = r!(List(vec![1, 2, 3]));
+//!     let list = r!(List(vec!["a", "b", "c"]));
+//!     let list = r!(List(&[r!("a"), r!(1), r!(2.0)]));
+//!
 //!     // A symbol
 //!     let sym = sym!(wombat);
+//!
+//!     // A R vector using collect_robj()
+//!     let vector = (0..3).map(|x| x * 10).collect_robj();
 //! }
 //! ```
 //!
@@ -484,17 +495,17 @@ mod tests {
     }
 
     #[extendr]
-    pub fn f64_iter(x: RealIter) -> RealIter {
+    pub fn f64_iter(x: Real) -> Real {
         x
     }
 
     #[extendr]
-    pub fn i32_iter(x: IntegerIter) -> IntegerIter {
+    pub fn i32_iter(x: Int) -> Int {
         x
     }
 
     #[extendr]
-    pub fn bool_iter(x: LogicalIter) -> LogicalIter {
+    pub fn bool_iter(x: Logical) -> Logical {
         x
     }
 
@@ -641,19 +652,19 @@ mod tests {
                 assert_eq!(new_owned(wrap__bool_slice(robj.get())), robj);
 
                 // #[extendr]
-                // pub fn f64_iter(x: RealIter) -> RealIter { x }
+                // pub fn f64_iter(x: Real) -> Real { x }
 
                 let robj = r!([1., 2., 3.]);
                 assert_eq!(new_owned(wrap__f64_iter(robj.get())), robj);
 
                 // #[extendr]
-                // pub fn i32_iter(x: IntegerIter) -> IntegerIter { x }
+                // pub fn i32_iter(x: Int) -> Int { x }
 
                 let robj = r!([1, 2, 3]);
                 assert_eq!(new_owned(wrap__i32_iter(robj.get())), robj);
 
                 // #[extendr]
-                // pub fn bool_iter(x: LogicalIter) -> LogicalIter { x }
+                // pub fn bool_iter(x: Logical) -> Logical { x }
 
                 let robj = r!([TRUE, FALSE, TRUE]);
                 assert_eq!(new_owned(wrap__bool_iter(robj.get())), robj);
diff --git a/extendr-api/src/logical.rs b/extendr-api/src/logical.rs
index c8660331d..a87ae26f7 100644
--- a/extendr-api/src/logical.rs
+++ b/extendr-api/src/logical.rs
@@ -16,7 +16,7 @@ impl Bool {
 
     /// Test if TRUE
     pub fn is_true(&self) -> bool {
-        self.0 != 0
+        self.0 == 1
     }
 
     /// Test if FALSE
diff --git a/extendr-api/src/prelude.rs b/extendr-api/src/prelude.rs
index a2b2c4291..e4bc2e65c 100644
--- a/extendr-api/src/prelude.rs
+++ b/extendr-api/src/prelude.rs
@@ -51,5 +51,5 @@ pub use ndarray::*;
 pub use extendr_macros::{extendr, extendr_module};
 
 pub use super::iter::{
-    EnvIter, IntegerIter, ListIter, LogicalIter, PairlistIter, PairlistTagIter, RealIter, StrIter,
+    EnvIter, Int, ListIter, Logical, PairlistIter, PairlistTagIter, Real, StrIter,
 };
diff --git a/extendr-api/src/robj/from_robj.rs b/extendr-api/src/robj/from_robj.rs
index 9983eb1b1..f00898144 100644
--- a/extendr-api/src/robj/from_robj.rs
+++ b/extendr-api/src/robj/from_robj.rs
@@ -153,9 +153,9 @@ macro_rules! impl_iter_from_robj {
 
 impl_iter_from_robj!(StrIter, as_str_iter, "Not a character vector.");
 impl_iter_from_robj!(ListIter, as_list_iter, "Not a list.");
-impl_iter_from_robj!(IntegerIter<'a>, as_integer_iter, "Not an integer vector.");
-impl_iter_from_robj!(RealIter<'a>, as_real_iter, "Not a real vector.");
-impl_iter_from_robj!(LogicalIter<'a>, as_logical_iter, "Not a logical vector.");
+impl_iter_from_robj!(Int, as_integer_iter, "Not an integer vector.");
+impl_iter_from_robj!(Real, as_real_iter, "Not a real vector.");
+impl_iter_from_robj!(Logical, as_logical_iter, "Not a logical vector.");
 
 /// Pass-through Robj conversion.
 impl<'a> FromRobj<'a> for Robj {
diff --git a/extendr-api/src/robj/into_robj.rs b/extendr-api/src/robj/into_robj.rs
index 4791a2d52..9b9d06a93 100644
--- a/extendr-api/src/robj/into_robj.rs
+++ b/extendr-api/src/robj/into_robj.rs
@@ -497,23 +497,23 @@ impl_from_into_iter! {&'a [T]}
 impl_from_as_iterator! {Range<T>}
 impl_from_as_iterator! {RangeInclusive<T>}
 
-impl<'a> From<RealIter<'a>> for Robj {
+impl From<Real> for Robj {
     /// Convert a real iterator into a vector.
-    fn from(val: RealIter) -> Self {
+    fn from(val: Real) -> Self {
         val.collect_robj()
     }
 }
 
-impl<'a> From<IntegerIter<'a>> for Robj {
+impl From<Int> for Robj {
     /// Convert an integer iterator into a vector.
-    fn from(val: IntegerIter) -> Self {
+    fn from(val: Int) -> Self {
         val.collect_robj()
     }
 }
 
-impl<'a> From<LogicalIter<'a>> for Robj {
+impl From<Logical> for Robj {
     /// Convert a logical iterator into a vector.
-    fn from(val: LogicalIter) -> Self {
+    fn from(val: Logical) -> Self {
         val.collect_robj()
     }
 }
diff --git a/extendr-api/src/robj/mod.rs b/extendr-api/src/robj/mod.rs
index 7b4602ff9..5fcdc066b 100644
--- a/extendr-api/src/robj/mod.rs
+++ b/extendr-api/src/robj/mod.rs
@@ -299,12 +299,9 @@ impl Robj {
     /// assert_eq!(tot, 6);
     /// }
     /// ```
-    pub fn as_integer_iter<'a>(&self) -> Option<IntegerIter<'a>>
-    where
-        Self: 'a,
-    {
+    pub fn as_integer_iter(&self) -> Option<Int> {
         if let Some(slice) = self.as_integer_slice() {
-            Some(slice.iter().cloned())
+            Some(Int::from_slice(self.to_owned(), slice))
         } else {
             None
         }
@@ -375,9 +372,9 @@ impl Robj {
     ///     assert_eq!((nt, nf, nna), (1, 1, 1));
     /// }
     /// ```
-    pub fn as_logical_iter(&self) -> Option<LogicalIter> {
+    pub fn as_logical_iter(&self) -> Option<Logical> {
         if let Some(slice) = self.as_logical_slice() {
-            Some(slice.iter().cloned())
+            Some(Logical::from_slice(self.to_owned(), slice))
         } else {
             None
         }
@@ -417,9 +414,9 @@ impl Robj {
     ///     assert_eq!(tot, 6.);
     /// }
     /// ```
-    pub fn as_real_iter(&self) -> Option<RealIter> {
+    pub fn as_real_iter(&self) -> Option<Real> {
         if let Some(slice) = self.as_real_slice() {
-            Some(slice.iter().cloned())
+            Some(Real::from_slice(self.to_owned(), slice))
         } else {
             None
         }
@@ -904,11 +901,7 @@ impl Robj {
     ///    assert_eq!(dim, vec![2, 2]);
     /// }
     /// ```
-    pub fn dim<'a>(&self) -> Option<IntegerIter<'a>>
-    where
-        Self: 'a,
-        Robj: 'a,
-    {
+    pub fn dim(&self) -> Option<Int> {
         if let Some(dim) = self.get_attrib(dim_symbol()) {
             dim.as_integer_iter()
         } else {
diff --git a/extendr-api/src/robj/rinternals.rs b/extendr-api/src/robj/rinternals.rs
index 0c0a7bf08..ba7476608 100644
--- a/extendr-api/src/robj/rinternals.rs
+++ b/extendr-api/src/robj/rinternals.rs
@@ -111,7 +111,9 @@ impl Robj {
     /// test! {
     ///    let my_fun = base_env().find_function(sym!(ls)).unwrap();
     ///    assert_eq!(my_fun.is_function(), true);
-    ///    assert!(base_env().find_function(sym!(qwertyuiop)).is_none());
+    ///
+    ///    // Note: this may crash on some versions of windows which don't support unwinding.
+    ///    // assert!(base_env().find_function(sym!(qwertyuiop)).is_none());
     /// }
     /// ```
     pub fn find_function<K: Into<Robj>>(&self, key: K) -> Option<Robj> {
@@ -157,7 +159,9 @@ impl Robj {
     ///        .find_var(sym!(iris)).unwrap().eval_promise().unwrap();
     ///    assert_eq!(iris_dataframe.is_frame(), true);
     ///    assert_eq!(iris_dataframe.len(), 5);
-    ///    assert_eq!(global_env().find_var(sym!(imnotasymbol)), None);
+    ///
+    ///    // Note: this may crash on some versions of windows which don't support unwinding.
+    ///    //assert_eq!(global_env().find_var(sym!(imnotasymbol)), None);
     /// }
     /// ```
     pub fn find_var<K: Into<Robj>>(&self, key: K) -> Option<Robj> {
diff --git a/extendr-api/src/thread_safety.rs b/extendr-api/src/thread_safety.rs
index 65a0080b3..ded9a44df 100644
--- a/extendr-api/src/thread_safety.rs
+++ b/extendr-api/src/thread_safety.rs
@@ -96,7 +96,7 @@ pub fn throw_r_error<S: AsRef<str>>(s: S) {
 }
 
 /// Wrap an R function such as Rf_findFunction and convert errors and panics into results.
-/// ```
+/// ```ignore
 /// use extendr_api::prelude::*;
 /// test! {
 ///    let res = catch_r_error(|| unsafe {
@@ -138,6 +138,8 @@ where
         let cleandata = std::mem::transmute(&x);
         let cont = R_MakeUnwindCont();
         Rf_protect(cont);
+
+        // Note that catch_unwind does not work for 32 bit windows targets.
         let res = match std::panic::catch_unwind(|| {
             R_UnwindProtect(fun, data, cleanfun, cleandata, cont)
         }) {
diff --git a/extendr-api/src/wrapper.rs b/extendr-api/src/wrapper.rs
index 8c7ff3cbe..3dce4cfdc 100644
--- a/extendr-api/src/wrapper.rs
+++ b/extendr-api/src/wrapper.rs
@@ -1,10 +1,10 @@
 //! Wrappers are lightweight proxies for references to R datatypes.
 //! They do not contain an Robj (see array.rs for an example of this).
 
+#[doc(hidden)]
 use crate::*;
 #[doc(hidden)]
 use libR_sys::*;
-#[doc(hidden)]
 
 /// Wrapper for creating symbols.
 ///
@@ -195,7 +195,7 @@ where
     T::IntoIter: ExactSizeIterator,
     T::Item: Into<Robj>,
 {
-    /// Make a list object from an array of Robjs.
+    /// Make a list object from an iterator of Robjs.
     /// ```
     /// use extendr_api::prelude::*;
     /// test! {
@@ -214,7 +214,7 @@ where
     T::IntoIter: ExactSizeIterator,
     T::Item: Into<Robj>,
 {
-    /// Make an expression object from an array of Robjs.
+    /// Make an expression object from an iterator of Robjs.
     /// ```
     /// use extendr_api::prelude::*;
     /// test! {


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 82d0000ee..2c2be3cc1 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -10,6 +10,11 @@ on:
       - main
       - master
 
+# cargo error handling is wrapped by ci-cargo, for simplicity and better logging.
+# To use ci-cargo in a step, source ci-cargo.ps1 (using `. ./ci-cargo.ps`, note the extra dot in front).
+# ci-cargo does not handle correctly -- separator, so wrap it in quotes ('--')
+# A special named parameter called `ActionName` can be used to set the name of current ci-cargo action,
+# used as `ci-cargo .... -ActionName "Called for documentation purposes"`.
 jobs:
   # All tests under this job are run with pre-computed libR-sys bindings.
   tests_no_bindgen:
@@ -113,7 +118,6 @@ jobs:
         run: |
           Del alias:R
           echo "LD_LIBRARY_PATH=$(R -s -e 'cat(normalizePath(R.home()))')/lib" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append
-
       # Check code formatting. As this doesn't depend on the platform, do this only on one platform.
       - name: Check code formatting
         if: matrix.config.check_fmt
@@ -123,34 +127,19 @@ jobs:
       # Required by Windows builds, does not affect other platforms
       - name: Build
         run: |
+          . ./ci-cargo.ps1
           foreach($target in ($env:BUILD_TARGETS).Split(',')) {
-            echo "::group::Build for $target target"
-
-            cargo build $(if($target -ne 'default') {"--target=$target"} ) 
-            if (!$?) { 
-              echo "::error::Build for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
+            ci-cargo build $(if($target -ne 'default') {"--target=$target"} ) -ActionName "Building for $target target"
           }
-
       # For each target in the BUILD_TARGETS comma-separated list, run cargo test with appropriate target
       # Required by Windows builds, does not affect other platforms
+      # ! ci-cargo requires '--' to be wrapped in quotes (passed as an explicit string)
       - name: Run tests
         run: |
+          . ./ci-cargo.ps1
           foreach($target in ($env:BUILD_TARGETS).Split(',')) {
-            echo "::group::Test for $target target"
-            
-            cargo test $(if($target -ne 'default') {"--target=$target"} ) -- --nocapture
-            if (!$?) { 
-              echo "::error::Test for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
+            ci-cargo test $(if($target -ne 'default') {"--target=$target"} ) '--' --nocapture -ActionName "Testing for $target target"
           }
-
       - name: Query dependencies for integration testing
         run: |
           install.packages('remotes')
@@ -199,7 +188,7 @@ jobs:
   bindgen_no_cross_compile:
     runs-on: ${{ matrix.config.os }}
 
-    name: ${{ matrix.config.os }} w/ bindgen no cross-compile (R-${{ matrix.config.r }} rust-${{ matrix.config.rust-version }})
+    name: ${{ matrix.config.os }} w/ bindgen (R-${{ matrix.config.r }} rust-${{ matrix.config.rust-version }})
             
     strategy:
       fail-fast: false
@@ -223,8 +212,6 @@ jobs:
     steps:
       - uses: actions/checkout@v2
       
-      # Default action allows to install only one toolchain,
-      # so Rust is set up manually.
       # 1. Update rustup
       # 2. For each target add respective toolchain &
       #    update target for that toolchain
@@ -269,7 +256,6 @@ jobs:
           r-version: ${{ matrix.config.r }}
           windows-path-include-mingw: false
       
-
       # 1. Inspect targets. If empty, assume 'x86_64' arch
       # 1.5 Make symlink to please i686 toolchain (https://github.com/rust-lang/cargo/issues/8990)
       # 2. Install msys2 packages (--needed skips already installed)
@@ -318,6 +304,7 @@ jobs:
       # 3.x Run build commands, checking for errors, providing explicit toolchain & target
       - name: Build
         run: |
+          . ./ci-cargo.ps1
           $targets = $env:BUILD_TARGETS.Split(',')
           $libclang_paths = $env:LIBCLANG_PATHS.Split([System.IO.Path]::PathSeparator)
           $itt = 0..($targets.Length - 1)
@@ -337,33 +324,21 @@ jobs:
               $env:LIBCLANG_PATH = $null
             }
 
-            echo "::group::Build extendr-api for $toolchain"
-
-            cargo +$toolchain build --manifest-path extendr-api/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} )
-            if (!$?) { 
-              echo "::error::Build extendr-api for $toolchain" 
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
-          
-            echo "::group::Build extendr-engine for $toolchain"
-
-            cargo +$toolchain build --manifest-path extendr-engine/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) 
-            if (!$?) { 
-              echo "::error::Build extendr-engine for $toolchain"
-              throw "Last exit code $LASTEXITCODE"
-            }
 
-            echo "::endgroup::"
+            ci-cargo +$toolchain build --manifest-path extendr-api/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) -ActionName "Build extendr-api for $toolchain"
+         
+            ci-cargo +$toolchain build --manifest-path extendr-engine/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) -ActionName "Build extendr-engine for $toolchain"
+            
           }
         env: 
           BUILD_TARGETS: ${{ join(matrix.config.rust-targets, ',') }}
           RUST_TOOLCHAIN: ${{ matrix.config.rust-version }}
 
       # Identical to the build process, but running more test commands
+      # ! ci-cargo requires '--' to be wrapped in quotes (passed as an explicit string)
       - name: Run tests
         run: |
+          . ./ci-cargo.ps1
           $targets = $env:BUILD_TARGETS.Split(',')
           $libclang_paths = $env:LIBCLANG_PATHS.Split([System.IO.Path]::PathSeparator)
           $itt = 0..($targets.Length - 1)
@@ -383,45 +358,14 @@ jobs:
               $env:LIBCLANG_PATH = $null
             }
 
-            echo "::group::Test extendr-engine \w tests-all for $target target"
-
-            cargo +$toolchain test --manifest-path extendr-engine/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) -- --nocapture 
-            if (!$?) { 
-              echo "::error::Test extendr-engine \w tests-all for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
-          
-            echo "::group::Test extendr-api \w tests-all for $target target"
-
-            cargo +$toolchain test --manifest-path extendr-api/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) -- --nocapture 
-            if (!$?) { 
-              echo "::error::Test extendr-api \w tests-all for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
-                    
-            echo "::group::Test extendr-api \w tests-minimal for $target target"
-
-            cargo +$toolchain test --manifest-path extendr-api/Cargo.toml --features tests-minimal $(if($target -ne 'default') {"--target=$target"} ) -- --nocapture
-            if (!$?) { 
-              echo "::error::Test extendr-api \w tests-minimal for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
-          
-            echo "::group::Test extendr-macros for $target target"
+            ci-cargo +$toolchain test --manifest-path extendr-engine/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) '--' --nocapture -ActionName "Test extendr-engine \w tests-all for $target target"
+            
+            ci-cargo +$toolchain test --manifest-path extendr-api/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) '--' --nocapture -ActionName "Test extendr-api \w tests-all for $target target"
 
-            cargo +$toolchain test --manifest-path extendr-macros/Cargo.toml  $(if($target -ne 'default') {"--target=$target"} ) -- --nocapture
-            if (!$?) { 
-              echo "::error::Test extendr-macros for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
+            ci-cargo +$toolchain test --manifest-path extendr-api/Cargo.toml --features tests-minimal $(if($target -ne 'default') {"--target=$target"} ) '--' --nocapture -ActionName "Test extendr-api \w tests-minimal for $target target"
+                      
+            ci-cargo +$toolchain test --manifest-path extendr-macros/Cargo.toml  $(if($target -ne 'default') {"--target=$target"} ) '--' --nocapture -ActionName "Test extendr-macros for $target target"
             
-            echo "::endgroup::"
           }
         env: 
           BUILD_TARGETS: ${{ join(matrix.config.rust-targets, ',') }}
diff --git a/extendr-api/src/robj/tests.rs b/extendr-api/src/robj/tests.rs
index b2872283d..de75734ab 100644
--- a/extendr-api/src/robj/tests.rs
+++ b/extendr-api/src/robj/tests.rs
@@ -227,7 +227,7 @@ fn output_iterator_test() {
 // Test that we can use Iterators as the input to functions.
 // eg.
 // #[extendr]
-// fn fred(a: RealIter, b: RealIter) -> RealIter {
+// fn fred(a: Real, b: Real) -> Robj {
 // }
 #[test]
 fn input_iterator_test() {
@@ -244,18 +244,18 @@ fn input_iterator_test() {
 
         let src: &[i32] = &[1, 2, 3];
         let robj = Robj::from(src);
-        let iter = <IntegerIter>::from_robj(&robj).unwrap();
+        let iter = <Int>::from_robj(&robj).unwrap();
         assert_eq!(iter.collect::<Vec<_>>(), src);
 
         let src: &[f64] = &[1., 2., 3.];
         let robj = Robj::from(src);
-        let iter = <RealIter>::from_robj(&robj).unwrap();
+        let iter = <Real>::from_robj(&robj).unwrap();
         assert_eq!(iter.collect::<Vec<_>>(), src);
 
         /*
         let src: &[Bool] = &[TRUE, FALSE, TRUE];
         let robj = Robj::from(src);
-        let iter = <LogicalIter>::from_robj(&robj).unwrap();
+        let iter = <Logical>::from_robj(&robj).unwrap();
         assert_eq!(iter.collect::<Vec<_>>(), src);
         */
     }