Repository: extendr/extendr
Pull request #303 (2021-10-12 14:02:14)

Better list macro
This PR makes a procedural version of the list! macro which now returns the List type.

There are also a few structural changes to List.

As `Robj` is now a wrapper for `SEXP` we can express lists as an array of `Robj` types.

I have also added a more efficient method to create named lists.



andy-thomason: Thansk for all the feedback.

I'm going to merge it anyway for fear of it getting stale.

We should absolutely not expect it to panic, but R may have other ideas (maybe reserved names and such like).

We could also have `integers![] doubles![] data_frame![] and logicals![]` macros analagous to vec![].



Patch:
diff --git a/extendr-api/src/error.rs b/extendr-api/src/error.rs
index 1b5477db2..cd267ed96 100644
--- a/extendr-api/src/error.rs
+++ b/extendr-api/src/error.rs
@@ -2,6 +2,8 @@
 
 use crate::{throw_r_error, Robj};
 
+use std::convert::Infallible;
+
 /// Throw an R error if a result is an error.
 #[doc(hidden)]
 pub fn unwrap_or_throw<T>(r: std::result::Result<T, &'static str>) -> T {
@@ -193,3 +195,9 @@ impl From<String> for Error {
 //         Error::None
 //     }
 // }
+
+impl From<Infallible> for Error {
+    fn from(_: Infallible) -> Self {
+        Error::Other("".to_string())
+    }
+}
diff --git a/extendr-api/src/rmacros.rs b/extendr-api/src/rmacros.rs
index bbb2ed61c..1ea4e2e84 100644
--- a/extendr-api/src/rmacros.rs
+++ b/extendr-api/src/rmacros.rs
@@ -91,28 +91,6 @@ macro_rules! sym {
     };
 }
 
-/// Create a list.
-///
-/// Example:
-/// ```
-/// use extendr_api::prelude::*;
-/// test! {
-///     let mylist = list!(x=1, y=2);
-///     assert_eq!(mylist, r!(List::from_pairs(vec![("x", r!(1)), ("y", r!(2))])));
-/// }
-/// ```
-///
-/// Panics on error.
-#[macro_export]
-macro_rules! list {
-    () => {
-        call!("list").unwrap()
-    };
-    ($($rest: tt)*) => {
-        call!("list", $($rest)*).unwrap()
-    };
-}
-
 /// Create a dataframe.
 ///
 /// Example:
diff --git a/extendr-api/src/robj/mod.rs b/extendr-api/src/robj/mod.rs
index b40bd2020..3bd1ee268 100644
--- a/extendr-api/src/robj/mod.rs
+++ b/extendr-api/src/robj/mod.rs
@@ -710,6 +710,8 @@ make_typed_slice!(Rint, INTEGER, INTSXP);
 make_typed_slice!(f64, REAL, REALSXP);
 make_typed_slice!(Rfloat, REAL, REALSXP);
 make_typed_slice!(u8, RAW, RAWSXP);
+make_typed_slice!(Robj, VECTOR_PTR, VECSXP);
+make_typed_slice!(Rstr, STRING_PTR, STRSXP);
 
 /// These are helper functions which give access to common properties of R objects.
 #[allow(non_snake_case)]
@@ -806,10 +808,12 @@ impl Robj {
     {
         let iter = names.into_iter();
         let robj = iter.collect_robj();
-        if robj.len() == self.len() {
-            self.set_attrib(wrapper::symbol::names_symbol(), robj)
-        } else {
+        if !robj.is_vector() && !robj.is_pairlist() {
+            Err(Error::ExpectedVector(robj))
+        } else if robj.len() != self.len() {
             Err(Error::NamesLengthMismatch(robj))
+        } else {
+            self.set_attrib(wrapper::symbol::names_symbol(), robj)
         }
     }
 
diff --git a/extendr-api/src/scalar/macros.rs b/extendr-api/src/scalar/macros.rs
index 4be390eee..2066f62a9 100644
--- a/extendr-api/src/scalar/macros.rs
+++ b/extendr-api/src/scalar/macros.rs
@@ -459,7 +459,9 @@ macro_rules! gen_binopassign {
                 fn [< $opname:snake >](&mut self, other: $type) {
                     match (*self, other.into()) {
                         (Some(lhs), Some(rhs)) => {
-                            *self = $expr(lhs, rhs);
+                            let f = $expr;
+                            let _ = (); // confuse clippy.
+                            *self = f(lhs, rhs);
                         },
                         _ => *self = None,
                     }
diff --git a/extendr-api/src/wrapper/list.rs b/extendr-api/src/wrapper/list.rs
index 46b824475..f57516758 100644
--- a/extendr-api/src/wrapper/list.rs
+++ b/extendr-api/src/wrapper/list.rs
@@ -1,3 +1,5 @@
+use std::iter::FromIterator;
+
 use super::*;
 
 #[derive(Debug, PartialEq, Clone)]
@@ -7,23 +9,23 @@ pub struct List {
 
 impl Default for List {
     fn default() -> Self {
-        List::new()
+        List::new(0)
     }
 }
 
 impl List {
-    /// Create a new, empty list.
+    /// Create a new list.
     /// ```
     /// use extendr_api::prelude::*;
     /// test! {
-    ///     let list = List::new();
+    ///     let list = List::new(10);
     ///     assert_eq!(list.is_list(), true);
-    ///     assert_eq!(list.len(), 0);
+    ///     assert_eq!(list.len(), 10);
     /// }
     /// ```
-    pub fn new() -> List {
-        let values: &[Robj] = &[];
-        List::from_values(values)
+    pub fn new(size: usize) -> Self {
+        let robj = Robj::alloc_vector(VECSXP, size);
+        Self { robj }
     }
 
     /// Wrapper for creating a list (VECSXP) object.
@@ -90,6 +92,22 @@ impl List {
         Ok(res)
     }
 
+    /// Build a list using separate names and values iterators.
+    /// Used internally by the list! macro.
+    pub fn from_names_and_values<N, V>(names: N, values: V) -> Result<Self>
+    where
+        N: IntoIterator,
+        N::IntoIter: ExactSizeIterator,
+        N::Item: ToVectorValue + AsRef<str>,
+        V: IntoIterator,
+        V::IntoIter: ExactSizeIterator,
+        V::Item: Into<Robj>,
+    {
+        let mut list = List::from_values(values);
+        list.set_names(names)?;
+        Ok(list)
+    }
+
     /// Return an iterator over the values of this list.
     /// ```
     /// use extendr_api::prelude::*;
@@ -107,15 +125,49 @@ impl List {
     /// ```
     /// use extendr_api::prelude::*;
     /// test! {
-    ///     let mut robj = list!(a=1, 2);
-    ///     let names_and_values : Vec<_> = robj.as_list().unwrap().iter().collect();
+    ///     let mut list = list!(a=1, 2);
+    ///     let names_and_values : Vec<_> = list.iter().collect();
     ///     assert_eq!(names_and_values, vec![("a", r!(1)), ("", r!(2))]);
     /// }
     /// ```
     pub fn iter(&self) -> NamedListIter {
+        // TODO: Make a proper NamedListIter.
         self.names()
             .map(|n| n.zip(self.values()))
-            .unwrap_or_else(|| StrIter::new().zip(ListIter::new()))
+            .unwrap_or_else(|| StrIter::new().zip(self.values()))
+    }
+
+    /// Get the list a slice of `Robj`s.
+    pub fn as_slice(&self) -> &[Robj] {
+        unsafe {
+            let data = DATAPTR(self.robj.get()) as *const Robj;
+            let len = self.robj.len();
+            std::slice::from_raw_parts(data, len)
+        }
+    }
+
+    /// Get a reference to an element in the list.
+    pub fn elt(&self, i: usize) -> Result<Robj> {
+        if i >= self.robj.len() {
+            Err(Error::OutOfRange(self.robj.clone()))
+        } else {
+            unsafe {
+                let sexp = VECTOR_ELT(self.robj.get(), i as R_xlen_t);
+                Ok(Robj::from_sexp(sexp))
+            }
+        }
+    }
+
+    /// Set an element in the list.
+    pub fn set_elt(&mut self, i: usize, value: Robj) -> Result<()> {
+        unsafe {
+            if i >= self.robj.len() {
+                Err(Error::OutOfRange(self.robj.clone()))
+            } else {
+                SET_VECTOR_ELT(self.robj.get(), i as R_xlen_t, value.get());
+                Ok(())
+            }
+        }
     }
 
     /// Convert a List into a HashMap, consuming the list.
@@ -231,7 +283,7 @@ impl ExactSizeIterator for ListIter {
 /// ```
 /// use extendr_api::prelude::*;
 /// test! {
-///     let list = list!(1, 2);
+///     let list = Robj::from(list!(1, 2));
 ///     let vec : FromList<Vec<i32>> = list.try_into()?;
 ///     assert_eq!(vec.0, vec![1, 2]);
 /// }
@@ -249,7 +301,7 @@ where
     /// ```
     /// use extendr_api::prelude::*;
     /// test! {
-    ///     let list = list!(1, 2);
+    ///     let list = Robj::from(list!(1, 2));
     ///     let vec : FromList<Vec<i32>> = list.try_into()?;
     ///     assert_eq!(vec.0, vec![1, 2]);
     /// }
@@ -270,7 +322,7 @@ impl TryFrom<Robj> for ListIter {
     /// ```
     /// use extendr_api::prelude::*;
     /// test! {
-    ///     let list = list!(1, 2);
+    ///     let list = Robj::from(list!(1, 2));
     ///     let vec : ListIter = list.try_into()?;
     ///     assert_eq!(vec.collect::<Vec<_>>(), vec![r!(1), r!(2)]);
     /// }
@@ -286,8 +338,8 @@ impl From<ListIter> for Robj {
     /// ```
     /// use extendr_api::prelude::*;
     /// test! {
-    ///     let listiter = list!(1, 2).as_list().unwrap().values();
-    ///     assert_eq!(Robj::from(listiter), list!(1, 2));
+    ///     let listiter = list!(1, 2).values();
+    ///     assert_eq!(Robj::from(listiter), Robj::from(list!(1, 2)));
     /// }
     /// ```
     fn from(iter: ListIter) -> Self {
@@ -315,3 +367,24 @@ impl<T: AsRef<str>> KeyValue for (T, Robj) {
         self.1
     }
 }
+
+impl<T: Into<Robj>> FromIterator<T> for List {
+    /// Convert an iterator to a List object.
+    fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> Self {
+        crate::single_threaded(|| unsafe {
+            let values: Vec<SEXP> = iter
+                .into_iter()
+                .map(|s| Rf_protect(s.into().get()))
+                .collect();
+
+            let len = values.len();
+            let robj = Robj::alloc_vector(VECSXP, len);
+            for (i, v) in values.into_iter().enumerate() {
+                SET_VECTOR_ELT(robj.get(), i as isize, v);
+            }
+            Rf_unprotect(len as i32);
+
+            List { robj }
+        })
+    }
+}
diff --git a/extendr-macros/src/lib.rs b/extendr-macros/src/lib.rs
index 2bea7b989..b7f5f5918 100644
--- a/extendr-macros/src/lib.rs
+++ b/extendr-macros/src/lib.rs
@@ -60,6 +60,7 @@ mod call;
 mod extendr_function;
 mod extendr_impl;
 mod extendr_module;
+mod list;
 mod pairlist;
 mod pairs;
 mod wrappers;
@@ -113,6 +114,15 @@ pub fn pairlist(item: TokenStream) -> TokenStream {
     pairlist::pairlist(item)
 }
 
+/// Create a List R object from a list of name-value pairs.
+/// ```ignore
+///     assert_eq!(list!(a=1, 2, 3), List::from_pairs(&[("a", 1), ("", 2), ("", 3)]));
+/// ```
+#[proc_macro]
+pub fn list(item: TokenStream) -> TokenStream {
+    list::list(item)
+}
+
 /// Call a function or primitive defined by a text expression with arbitrary parameters.
 /// This currently works by parsing and evaluating the string in R, but will probably acquire
 /// some shortcuts for simple expessions, for example by caching symbols and constant values.
diff --git a/extendr-macros/src/list.rs b/extendr-macros/src/list.rs
new file mode 100644
index 000000000..84de99f4a
--- /dev/null
+++ b/extendr-macros/src/list.rs
@@ -0,0 +1,32 @@
+use proc_macro::TokenStream;
+use quote::quote;
+use syn::parse_macro_input;
+
+use crate::pairs::Pairs;
+
+pub fn list(item: TokenStream) -> TokenStream {
+    let list = parse_macro_input!(item as Pairs);
+
+    let nv = list.names_and_values();
+
+    if nv.is_empty() {
+        TokenStream::from(quote!(List::default()))
+    } else {
+        let values: Vec<proc_macro2::TokenStream> = nv
+            .iter()
+            .map(|(_n, v)| quote!( extendr_api::Robj::from(#v) ))
+            .collect();
+        if nv.iter().any(|(n, _v)| !n.is_empty()) {
+            let names: Vec<proc_macro2::TokenStream> =
+                nv.iter().map(|(n, _v)| quote!( #n )).collect();
+            // Note that this unwrap should not fail.
+            TokenStream::from(quote!(
+                extendr_api::List::from_names_and_values(&[# ( #names ),*], &[# ( #values ),*]).unwrap()
+            ))
+        } else {
+            TokenStream::from(quote!(
+                extendr_api::List::from_values(&[# ( #values ),*])
+            ))
+        }
+    }
+}
diff --git a/extendr-macros/src/pairs.rs b/extendr-macros/src/pairs.rs
index 9e0fc2ec7..fa5bcb1cb 100644
--- a/extendr-macros/src/pairs.rs
+++ b/extendr-macros/src/pairs.rs
@@ -1,7 +1,9 @@
-use syn::{parse::ParseStream, punctuated::Punctuated, Expr, Token};
+//! Internal module for parsing R-like variadic arguments.
+
+use syn::{parse::ParseStream, punctuated::Punctuated, Expr, ExprAssign, ExprPath, Token};
 
 #[derive(Debug)]
-pub struct Pairs {
+pub(crate) struct Pairs {
     pub pairs: Punctuated<Expr, Token![,]>,
 }
 
@@ -21,3 +23,21 @@ impl syn::parse::Parse for Pairs {
         Ok(res)
     }
 }
+
+impl Pairs {
+    // Having parsed a variadic expression, extract the names and values.
+    pub(crate) fn names_and_values(&self) -> Vec<(String, &Expr)> {
+        self.pairs
+            .iter()
+            .map(|e| -> (String, &Expr) {
+                if let Expr::Assign(ExprAssign { left, right, .. }) = e {
+                    if let Expr::Path(ExprPath { path, .. }) = &**left {
+                        let s = path.get_ident().unwrap().to_string();
+                        return (s, right.as_ref());
+                    }
+                }
+                ("".to_owned(), e)
+            })
+            .collect()
+    }
+}


Test patch:
diff --git a/extendr-api/tests/extendr_macro_tests.rs b/extendr-api/tests/extendr_macro_tests.rs
new file mode 100644
index 000000000..96b819678
--- /dev/null
+++ b/extendr-api/tests/extendr_macro_tests.rs
@@ -0,0 +1,18 @@
+// use extendr_api::prelude::*;
+use extendr_api::{list, test, List, Result};
+
+#[test]
+fn test_list() {
+    test! {
+        let l : List = list!();
+        assert_eq!(l, List::default());
+        let l : List = list!(1);
+        assert_eq!(l, List::from_values([1]));
+        let l : List = list!(a=1);
+        assert_eq!(l, List::from_names_and_values(["a"], [1]).unwrap());
+        let l : List = list!(a=1, b=2);
+        assert_eq!(l, List::from_names_and_values(["a", "b"], [1, 2]).unwrap());
+
+        assert!(List::from_names_and_values(["a", "b"], [1, 2, 3]).is_err());
+    }
+}
diff --git a/extendr-api/tests/serializer_tests.rs b/extendr-api/tests/serializer_tests.rs
index 80b4ea0a8..d8a73482f 100644
--- a/extendr-api/tests/serializer_tests.rs
+++ b/extendr-api/tests/serializer_tests.rs
@@ -38,19 +38,19 @@ mod test {
 
             let u = E::Unit;
             let expected = list!(Unit=NULL);
-            assert_eq!(to_robj(&u).unwrap(), expected);
+            assert_eq!(to_robj(&u).unwrap(), r!(expected));
 
             let n = E::Newtype(1);
             let expected = list!(Newtype=1);
-            assert_eq!(to_robj(&n).unwrap(), expected);
+            assert_eq!(to_robj(&n).unwrap(), r!(expected));
 
             let t = E::Tuple(1, 2);
             let expected = list!(Tuple=list!(1, 2));
-            assert_eq!(to_robj(&t).unwrap(), expected);
+            assert_eq!(to_robj(&t).unwrap(), r!(expected));
 
             let s = E::Struct { a: 1 };
             let expected = list!(Struct=list!(a=1));
-            assert_eq!(to_robj(&s).unwrap(), expected);
+            assert_eq!(to_robj(&s).unwrap(), r!(expected));
         }
     }
 
diff --git a/extendr-api/tests/vector_tests.rs b/extendr-api/tests/vector_tests.rs
index f0b2de364..b43bfcfa0 100644
--- a/extendr-api/tests/vector_tests.rs
+++ b/extendr-api/tests/vector_tests.rs
@@ -34,3 +34,35 @@ fn test_strings() {
         assert_eq!(s.elt(0), "xyz");
     }
 }
+
+#[test]
+fn test_list() {
+    test! {
+        let s = List::new(10);
+        assert_eq!(s.len(), 10);
+        assert_eq!(s.rtype(), RType::List);
+
+        let mut s = List::from_values(["x", "y", "z"]);
+        assert_eq!(s.len(), 3);
+        assert_eq!(s.rtype(), RType::List);
+        assert_eq!(s.elt(0)?, r!("x"));
+        assert_eq!(s.elt(1)?, r!("y"));
+        assert_eq!(s.elt(2)?, r!("z"));
+        assert_eq!(s.elt(3).is_err(), true);
+
+        let v = s.as_slice().iter().map(|c| c).collect::<Vec<_>>();
+        assert_eq!(v, vec![&r!("x"), &r!("y"), &r!("z")]);
+
+        s.set_elt(1, r!("q"))?;
+        assert_eq!(s.elt(1)?, r!("q"));
+
+        let s : List = ["x", "y", "z"].iter().collect();
+        assert_eq!(s, list!("x", "y", "z"));
+
+        let v = list!(a="x", b="y", c="z").iter().collect::<Vec<_>>();
+        assert_eq!(v, vec![("a", r!("x")), ("b", r!("y")), ("c", r!("z"))]);
+
+        let s = List::from_values(["x", <&str>::na(), "z"]);
+        assert_eq!(s.elt(1)?.is_na(), true);
+    }
+}