fix(lang): function calling and promises

Internal changes:
* `eval_list_eager()` was removed from the `Context` trait and added as a member method for `CallStack` (because we need a callstack to force arguments).
* `eval_list_lazy()` now boxes all expressions in promises (including literals).
  This is necessary to box `..a`-style ellipsis arguments in a list-call promise, which requires
  access to the underlying expression (needed to solve #216).

Bugs: 
* `substitute()` now works on datatypes such as literals or calls (#199).
* accessing variable collected via 'rest-args' does now force evaluation of calls (#216).

Author: sebffischer

src/CHANGELOG.md

@@ -14,13 +14,22 @@
 * The `typeof()` primitive was added
 * Type stability for numeric operations (@69)
 
+## Noteable Bugs Addressed:
+
+* `substitute()` now works on datatypes such as literals or calls (#199).
+* accessing variable collected via 'rest-args' does now force evaluation of calls (#216).
+
 ## Internals
 
 * The `List` is now represented as a `Rep<Obj>`, unifying heterogenous and atomic vectors.
   This included a considerable refactor.
 * Iterating over references of a `Rep<T>` was made much simpler and new methods were added
   and unused ones removed.
 * The `RepType` struct that was introduced in 0.4.0 was removed again (#189).
+* `eval_list_eager()` was removed from the `Context` trait and added as a member method for `CallStack`.
+* `eval_list_lazy()` now boxes all expressions in promises (including literals).
+  This is necessary to box `..a`-style ellipsis arguments in a list-call promise, which requires
+  access to the underlying expression (needed to solve #216).
 
 ## Notable Bugs Addressed
 

src/callable/core.rs

@@ -83,6 +83,30 @@ pub trait Callable: CallableFormals {
             }
         }
 
+        let mut ellipsis_expr = ExprList::new();
+
+        for (k, v) in ellipsis.iter_pairs() {
+            if let Obj::Promise(_, e, _) = v {
+                ellipsis_expr.push_named(k.as_option(), e)
+            } else {
+                // all arguments must be boxed in promises to allow for NSE
+                unreachable!()
+            }
+        }
+
+        let list = crate::callable::builtins::BUILTIN
+            .get("list")
+            .cloned()
+            .unwrap();
+
+        // convert the expr_ellipsis to an Obj::Promise where the expression is a call into List
+
+        let ellipsis_promise = Obj::Promise(
+            None,
+            Expr::Call(Box::new(Expr::Primitive(list)), ellipsis_expr),
+            stack.last_frame().env().clone(),
+        );
+
         // add back in parameter defaults that weren't filled with args
         for (param, default) in formals.into_iter() {
             matched_args.push_named(
@@ -92,7 +116,7 @@ pub trait Callable: CallableFormals {
         }
 
         if let Some(Expr::Ellipsis(Some(name))) = remainder.get(0) {
-            matched_args.push_named(Character::Some(name), Obj::List(ellipsis.clone()));
+            matched_args.push_named(Character::Some(name), ellipsis_promise);
         } else if !remainder.is_empty() {
             matched_args.push_named(
                 Character::Some("...".to_string()),

src/callable/primitive/c.rs

@@ -1,7 +1,6 @@
 use r_derive::*;
 
 use crate::callable::core::*;
-use crate::context::Context;
 use crate::object::types::*;
 use crate::object::*;
 use crate::{formals, lang::*};

src/callable/primitive/list.rs

@@ -1,7 +1,6 @@
 use r_derive::*;
 
 use crate::callable::core::*;
-use crate::context::Context;
 use crate::formals;
 use crate::lang::*;
 use crate::object::*;

src/callable/primitive/quote.rs

@@ -25,8 +25,12 @@ use crate::object::*;
 ///
 /// ```custom,{class=r-repl}
 /// quote(x + y)
+/// quote(1)
 /// ```
-///
+/// ## Differentes to the R implementation
+/// While R treats literals as expressions this implementation of `quote` differentiates between
+/// the literal `1` and the length-1 vector "`c(1)`".
+/// Thereby the return type of `quote()` can be expected to be an object of type `Expression`.
 #[doc(alias = "quote")]
 #[builtin(sym = "quote")]
 #[derive(Debug, Clone, PartialEq)]
@@ -39,3 +43,14 @@ impl Callable for PrimitiveQuote {
         Ok(Obj::Expr(args.get(0).unwrap_or(Expr::Null)))
     }
 }
+
+#[cfg(test)]
+
+mod tests {
+    use crate::{r, r_expect};
+
+    #[test]
+    fn literals_dont_evaluate() {
+        r_expect!(typeof(quote(1)) == "expression")
+    }
+}

src/callable/primitive/substitute.rs

@@ -126,7 +126,7 @@ impl Callable for PrimitiveSubstitute {
 
 #[cfg(test)]
 mod test {
-    use crate::r;
+    use crate::{r, r_expect};
 
     #[test]
     fn function_param_promises() {
@@ -191,4 +191,19 @@ mod test {
             r! { quote(3 + 4) }
         );
     }
+
+    #[test]
+    fn literals_evaluate_to_themselves() {
+        r_expect!(substitute(1) == 1);
+        r_expect!(substitute("a") == "a");
+        r_expect!(substitute(true) == true);
+        r_expect!(substitute(1L) == 1L);
+    }
+
+    #[test]
+    fn calls_work() {
+        r_expect! {{"
+            eval(substitute(fn(x) x))(1) == 1
+        "}}
+    }
 }

src/callable/primitive/type_reflection.rs

@@ -16,22 +16,7 @@ impl Callable for PrimitiveTypeOf {
     fn call_matched(&self, args: List, _ellipsis: List, stack: &mut CallStack) -> EvalResult {
         let mut args = Obj::List(args);
         let x = args.try_get_named("x")?.force(stack)?;
-
-        let t = match x {
-            Obj::Null => "null",
-            Obj::Vector(v) => match v {
-                Vector::Character(_) => "character",
-                Vector::Integer(_) => "integer",
-                Vector::Double(_) => "double",
-                Vector::Logical(_) => "logical",
-            },
-            Obj::List(_) => "list",
-            Obj::Expr(_) => "expression",
-            Obj::Promise(..) => "promise",
-            Obj::Function(..) => "function",
-            Obj::Environment(..) => "environment",
-        };
-        EvalResult::Ok(Obj::Vector(Vector::Character(vec![t.to_string()].into())))
+        EvalResult::Ok(Obj::Vector(Vector::Character(vec![x.type_of()].into())))
     }
 }
 

src/context/core.rs

@@ -1,7 +1,6 @@
 use std::rc::Rc;
 
 use crate::lang::{EvalResult, Signal};
-use crate::object::types::Character;
 use crate::object::*;
 use crate::{error::*, internal_err};
 
@@ -88,7 +87,7 @@ pub trait Context: std::fmt::Debug + std::fmt::Display {
                             internal_err!()
                         }
                     }
-                    // Avoid creating a new closure just to point to another, just reuse it
+                    // Avoid creating a new promise just to point to another, just reuse it
                     (k, Expr::Symbol(s)) => match self.env().get(s.clone()) {
                         Ok(c @ Obj::Promise(..)) => {
                             let k = k.map_or(OptionNA::NA, OptionNA::Some);
@@ -109,46 +108,8 @@ pub trait Context: std::fmt::Debug + std::fmt::Display {
                         Ok(List::from(elem).iter_pairs())
                     }
                     (k, v) => {
-                        let k = k.map_or(OptionNA::NA, OptionNA::Some);
-                        if let Ok(elem) = self.eval(v) {
-                            Ok(List::from(vec![(k, elem)]).iter_pairs())
-                        } else {
-                            internal_err!()
-                        }
-                    }
-                })
-                .collect::<Result<Vec<_>, _>>()?
-                .into_iter()
-                .flatten()
-                .collect::<Vec<_>>(),
-        )))
-    }
-
-    fn eval_list_eager(&mut self, l: ExprList) -> EvalResult {
-        Ok(Obj::List(List::from(
-            l.into_iter()
-                .map(|pair| match pair {
-                    (_, Expr::Ellipsis(None)) => {
-                        if let Ok(Obj::List(ellipsis)) = self.get_ellipsis() {
-                            Ok(ellipsis.iter_pairs())
-                        } else {
-                            Ok(List::from(Vec::<(Character, Obj)>::new()).iter_pairs())
-                        }
+                        Ok(List::from(vec![(k, Obj::Promise(None, v, self.env()))]).iter_pairs())
                     }
-                    (_, Expr::Ellipsis(Some(name))) => {
-                        if let Ok(Obj::List(more)) = self.get(name) {
-                            Ok(more.iter_pairs())
-                        } else {
-                            Ok(List::from(Vec::<(Character, Obj)>::new()).iter_pairs())
-                        }
-                    }
-                    (k, v) => match self.eval_and_finalize(v) {
-                        Ok(elem) => {
-                            let k = k.map_or(OptionNA::NA, OptionNA::Some);
-                            Ok(List::from(vec![(k, elem)]).iter_pairs())
-                        }
-                        Err(e) => Err(e),
-                    },
                 })
                 .collect::<Result<Vec<_>, _>>()?
                 .into_iter()

src/lang.rs

@@ -79,6 +79,23 @@ impl ViewMut for Obj {
 }
 
 impl Obj {
+    pub fn type_of(&self) -> String {
+        match self {
+            Obj::Null => "null",
+            Obj::Vector(v) => match v {
+                Vector::Character(_) => "character",
+                Vector::Integer(_) => "integer",
+                Vector::Double(_) => "double",
+                Vector::Logical(_) => "logical",
+            },
+            Obj::List(_) => "list",
+            Obj::Expr(_) => "expression",
+            Obj::Promise(..) => "promise",
+            Obj::Function(..) => "function",
+            Obj::Environment(..) => "environment",
+        }
+        .to_string()
+    }
     pub fn with_visibility(self, visibility: bool) -> EvalResult {
         Signal::Return(self, visibility).into()
     }
@@ -633,6 +650,50 @@ pub struct CallStack {
 }
 
 impl CallStack {
+    pub fn eval_list_eager(&mut self, l: ExprList) -> EvalResult {
+        Ok(Obj::List(List::from(
+            l.into_iter()
+                .map(|pair| match pair {
+                    (_, Expr::Ellipsis(None)) => {
+                        if let Ok(Obj::List(ellipsis)) = self.get_ellipsis() {
+                            // Each iterator yields a ()
+                            let x = ellipsis
+                                .iter_pairs()
+                                .map(|(k, v)| match v {
+                                    Obj::Promise(_, expr, _) => {
+                                        Ok((k, self.eval_and_finalize(expr)?))
+                                    }
+                                    _ => Ok((k, v)),
+                                })
+                                .collect::<Result<Vec<(Character, Obj)>, Signal>>()?;
+                            let l = List::from(x);
+                            Ok(l.iter_pairs())
+                        } else {
+                            Ok(List::from(Vec::<(Character, Obj)>::new()).iter_pairs())
+                        }
+                    }
+                    (_, Expr::Ellipsis(Some(name))) => {
+                        if let Ok(Obj::List(more)) = self.get(name) {
+                            Ok(more.iter_pairs())
+                        } else {
+                            Ok(List::from(Vec::<(Character, Obj)>::new()).iter_pairs())
+                        }
+                    }
+                    (k, v) => match self.eval_and_finalize(v) {
+                        Ok(elem) => {
+                            let k = k.map_or(OptionNA::NA, OptionNA::Some);
+                            Ok(List::from(vec![(k, elem)]).iter_pairs())
+                        }
+                        Err(e) => Err(e),
+                    },
+                })
+                .collect::<Result<Vec<_>, _>>()?
+                .into_iter()
+                .flatten()
+                .collect::<Vec<_>>(),
+        )))
+    }
+
     pub fn parse(&self, input: &str) -> ParseResult {
         let config: SessionParserConfig = self.session.clone().into();
         config.parse_input(input)
@@ -1220,6 +1281,21 @@ mod test {
         )
     }
 
+    #[test]
+    fn accessing_ellipsis_forces_evaluation() {
+        assert_eq!(
+            CallStack::default()
+                .map_session(|s| s.with_experiments(vec![Experiment::RestArgs]))
+                .parse_and_eval(
+                    "
+                    f = fn(...) { . }
+                    f(sum(1))
+                    ",
+                ),
+            r! { list(1) }
+        )
+    }
+
     #[test]
     fn fn_duplicated_parameters() {
         assert_eq!(

src/object/ast.rs

@@ -100,6 +100,13 @@ pub struct ExprList {
     pub values: Vec<Expr>,
 }
 
+impl ExprList {
+    pub fn push_named(&mut self, key: Option<String>, value: Expr) {
+        self.keys.push(key);
+        self.values.push(value);
+    }
+}
+
 impl fmt::Display for ExprList {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let pairs: Vec<String> = self