diff --git a/src/CHANGELOG.md b/src/CHANGELOG.md index a61937a..39fff9f 100644 --- a/src/CHANGELOG.md +++ b/src/CHANGELOG.md @@ -14,6 +14,11 @@ * 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`, unifying heterogenous and atomic vectors. @@ -21,6 +26,10 @@ * Iterating over references of a `Rep` 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 diff --git a/src/callable/core.rs b/src/callable/core.rs index 82df160..1e398bb 100644 --- a/src/callable/core.rs +++ b/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()), diff --git a/src/callable/primitive/c.rs b/src/callable/primitive/c.rs index 74399ed..6d70491 100644 --- a/src/callable/primitive/c.rs +++ b/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::*}; diff --git a/src/callable/primitive/list.rs b/src/callable/primitive/list.rs index 24cb79f..d2c75d1 100644 --- a/src/callable/primitive/list.rs +++ b/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::*; diff --git a/src/callable/primitive/quote.rs b/src/callable/primitive/quote.rs index 4fa47c1..2ce3fcc 100644 --- a/src/callable/primitive/quote.rs +++ b/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") + } +} diff --git a/src/callable/primitive/substitute.rs b/src/callable/primitive/substitute.rs index 89c96a9..86f0f26 100644 --- a/src/callable/primitive/substitute.rs +++ b/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 + "}} + } } diff --git a/src/callable/primitive/type_reflection.rs b/src/callable/primitive/type_reflection.rs index ccbf9e6..bbd5f6e 100644 --- a/src/callable/primitive/type_reflection.rs +++ b/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()))) } } diff --git a/src/context/core.rs b/src/context/core.rs index 954d0dd..63d67db 100644 --- a/src/context/core.rs +++ b/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::, _>>()? - .into_iter() - .flatten() - .collect::>(), - ))) - } - - 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::, _>>()? .into_iter() diff --git a/src/lang.rs b/src/lang.rs index c37c92e..2c415c7 100644 --- a/src/lang.rs +++ b/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::, 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::, _>>()? + .into_iter() + .flatten() + .collect::>(), + ))) + } + 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!( diff --git a/src/object/ast.rs b/src/object/ast.rs index 7d41ff0..e173119 100644 --- a/src/object/ast.rs +++ b/src/object/ast.rs @@ -100,6 +100,13 @@ pub struct ExprList { pub values: Vec, } +impl ExprList { + pub fn push_named(&mut self, key: Option, 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 = self