From cbc56d5ee25c74ec3c0afacf3e2b52444d57390b Mon Sep 17 00:00:00 2001 From: Ross MacArthur Date: Fri, 10 Jan 2025 15:58:30 +0200 Subject: [PATCH] Improve filters - Fix long standing bug with owned value being passed to a filter arg that is a reference. - Prepare compiler and renderer to handle computed arguments. This is required for list and map literals. - Simplify filter code by implementing the piped value and the arguments in the same way. These are all now converted from a `&mut ValueCow`. - Clarify lifetimes of args vs stack in filters implementation. --- src/compile/mod.rs | 34 ++++-- src/error.rs | 11 ++ src/filters/args.rs | 283 ++++++++++++++++--------------------------- src/filters/mod.rs | 144 +++++++++------------- src/render/core.rs | 77 ++++++------ src/render/mod.rs | 8 +- src/types/program.rs | 21 ++-- src/value/cow.rs | 10 -- tests/filters.rs | 265 +++++++++++++++++++++++++++++++++------- 9 files changed, 465 insertions(+), 388 deletions(-) diff --git a/src/compile/mod.rs b/src/compile/mod.rs index 7e3befc..e5f0acc 100644 --- a/src/compile/mod.rs +++ b/src/compile/mod.rs @@ -15,7 +15,6 @@ pub use crate::compile::search::Searcher; use crate::types::ast; use crate::types::program::{Instr, Template, FIXME}; -use crate::types::span::Span; use crate::{Engine, Result}; /// Compile a template into a program. @@ -60,9 +59,8 @@ impl Compiler { } ast::Stmt::InlineExpr(ast::InlineExpr { expr, .. }) => { - let span = expr.span(); self.compile_expr(expr); - self.pop_emit_expr(span); + self.pop_emit_expr(); } ast::Stmt::Include(ast::Include { name, globals }) => match globals { @@ -139,10 +137,20 @@ impl Compiler { name, args, receiver, - .. + span, }) => { self.compile_expr(*receiver); - self.push(Instr::Apply(name, args)); + let arity = match args { + None => 0, + Some(args) => { + let arity = args.values.len(); + for arg in args.values { + self.compile_base_expr(arg); + } + arity + } + }; + self.push(Instr::Apply(name, arity, span)); } } } @@ -150,24 +158,24 @@ impl Compiler { fn compile_base_expr(&mut self, base_expr: ast::BaseExpr) { match base_expr { ast::BaseExpr::Var(var) => { - self.push(Instr::ExprStart(var)); + self.push(Instr::ExprStartVar(var)); } - ast::BaseExpr::Literal(ast::Literal { value, .. }) => { - self.push(Instr::ExprStartLit(value)); + ast::BaseExpr::Literal(literal) => { + self.push(Instr::ExprStartLiteral(literal)); } } } - fn pop_emit_expr(&mut self, span: Span) { - let emit = match self.instrs.last() { - Some(Instr::Apply(_, None)) => { + fn pop_emit_expr(&mut self) { + let emit = match self.instrs.last_mut() { + Some(Instr::Apply(_, _, _)) => { let instr = self.instrs.pop().unwrap(); match instr { - Instr::Apply(ident, _) => Instr::EmitWith(ident, span), + Instr::Apply(ident, len, span) => Instr::EmitWith(ident, len, span), _ => unreachable!(), } } - _ => Instr::Emit(span), + _ => Instr::Emit, }; self.push(emit); } diff --git a/src/error.rs b/src/error.rs index e6767f2..71dbdd0 100644 --- a/src/error.rs +++ b/src/error.rs @@ -80,6 +80,17 @@ impl Error { } } + /// Constructs a new render error without pretty information. + #[cfg(feature = "filters")] + pub(crate) fn render_plain(reason: impl Into) -> Self { + Self { + kind: ErrorKind::Render, + name: None, + reason: Some(reason.into()), + pretty: None, + } + } + /// Constructs a max include depth error. pub(crate) fn max_include_depth(max: usize) -> Self { Self { diff --git a/src/filters/args.rs b/src/filters/args.rs index 56e4f08..d05aa0e 100644 --- a/src/filters/args.rs +++ b/src/filters/args.rs @@ -1,4 +1,5 @@ use std::collections::BTreeMap; +use std::mem; use crate::filters::FilterArg; use crate::value::ValueCow; @@ -14,11 +15,6 @@ pub enum Error { /// Got &'static str, ), - /// When the value is owned but the filter expects an owned type. - Reference( - /// Expected - &'static str, - ), /// Failed to convert from i64 to the integer type. TryFromInt( /// Type @@ -29,65 +25,50 @@ pub enum Error { } impl FilterArg for () { - type Output<'a> = (); + type Output<'arg> = (); - fn from_value<'a>(v: Value) -> Result> { - Self::from_value_ref(&v) - } - - fn from_value_ref(v: &Value) -> Result> { - match v { + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { + match &**v { Value::None => Ok(()), v => Err(Error::Type("()", v.human())), } } - - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - Self::from_value_ref(&*v) - } } impl FilterArg for bool { - type Output<'a> = bool; - - fn from_value<'a>(v: Value) -> Result> { - Self::from_value_ref(&v) - } - - fn from_value_ref(v: &Value) -> Result> { - match v { - Value::Bool(b) => Ok(*b), + type Output<'arg> = bool; + + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { + match &**v { + &Value::Bool(b) => Ok(b), v => Err(Error::Type("bool", v.human())), } } - - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - Self::from_value_ref(&*v) - } } macro_rules! impl_for_int { ($($ty:ty)+) => { $( impl FilterArg for $ty { - type Output<'a> =$ty; - - fn from_value<'a>(v: Value) -> Result> { - Self::from_value_ref(&v) - } - - fn from_value_ref(v: &Value) -> Result> { - match v { - Value::Integer(i) => (*i).try_into().map_err(|_| { - Error::TryFromInt(stringify!($ty), *i) - }), + type Output<'arg> =$ty; + + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { + match &**v { + &Value::Integer(i) => { + i.try_into().map_err(|_| Error::TryFromInt(stringify!($ty), i)) + }, v => Err(Error::Type(stringify!($ty), v.human())), } } - - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - Self::from_value_ref(&*v) - } } )+ }; @@ -99,22 +80,17 @@ macro_rules! impl_for_float { ($($ty:ty)+) => { $( impl FilterArg for $ty { - type Output<'a> =$ty; - - fn from_value<'a>(v: Value) -> Result> { - Self::from_value_ref(&v) - } - - fn from_value_ref(v: &Value) -> Result> { - match v { - Value::Float(f) => Ok(*f as $ty), + type Output<'arg> =$ty; + + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { + match &**v { + &Value::Float(f) => Ok(f as $ty), v => Err(Error::Type(stringify!($ty), v.human())), } } - - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - Self::from_value_ref(&*v) - } } )+ } @@ -123,23 +99,21 @@ macro_rules! impl_for_float { impl_for_float! { f32 f64 } impl FilterArg for String { - type Output<'a> = String; - - fn from_value<'a>(v: Value) -> Result> { - Self::from_value_ref(&v) - } + type Output<'arg> = String; - fn from_value_ref(v: &Value) -> Result> { + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { match v { - Value::String(s) => Ok(s.to_owned()), - v => Err(Error::Type("string", v.human())), - } - } - - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - match v.take() { - Value::String(s) => Ok(s), - v => Err(Error::Type("string", v.human())), + ValueCow::Borrowed(v) => match v { + Value::String(s) => Ok(s.to_owned()), + v => Err(Error::Type("string", v.human())), + }, + ValueCow::Owned(v) => match mem::take(v) { + Value::String(s) => Ok(s), + _ => Err(Error::Type("string", v.human())), + }, } } } @@ -147,25 +121,13 @@ impl FilterArg for String { pub struct Str; impl FilterArg for Str { - type Output<'a> = &'a str; - - fn from_value<'a>(v: Value) -> Result> { - match v { - Value::String(_) => Err(Error::Reference("string")), - v => Err(Error::Type("&str", v.human())), - } - } + type Output<'arg> = &'arg str; - fn from_value_ref(v: &Value) -> Result> { - match v { - Value::String(s) => Ok(s), - v => Err(Error::Type("&str", v.human())), - } - } - - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - let v: &'a Value = &*v; - match v { + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { + match &**v { Value::String(s) => Ok(s), v => Err(Error::Type("&str", v.human())), } @@ -173,23 +135,21 @@ impl FilterArg for Str { } impl FilterArg for Vec { - type Output<'a> = Vec; - - fn from_value<'a>(v: Value) -> Result> { - Self::from_value_ref(&v) - } + type Output<'arg> = Vec; - fn from_value_ref(v: &Value) -> Result> { + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { match v { - Value::List(l) => Ok(l.clone()), - v => Err(Error::Type("list", v.human())), - } - } - - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - match v.take() { - Value::List(l) => Ok(l), - v => Err(Error::Type("list", v.human())), + ValueCow::Borrowed(v) => match v { + Value::List(l) => Ok(l.to_owned()), + v => Err(Error::Type("list", v.human())), + }, + ValueCow::Owned(v) => match mem::take(v) { + Value::List(l) => Ok(l), + _ => Err(Error::Type("list", v.human())), + }, } } } @@ -197,25 +157,13 @@ impl FilterArg for Vec { pub struct ListRef; impl FilterArg for ListRef { - type Output<'a> = &'a [Value]; - - fn from_value<'a>(v: Value) -> Result> { - match v { - Value::List(_) => Err(Error::Reference("list")), - v => Err(Error::Type("list", v.human())), - } - } + type Output<'arg> = &'arg [Value]; - fn from_value_ref(v: &Value) -> Result> { - match v { - Value::List(l) => Ok(l), - v => Err(Error::Type("list", v.human())), - } - } - - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - let v: &'a Value = &*v; - match v { + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { + match &**v { Value::List(l) => Ok(l), v => Err(Error::Type("list", v.human())), } @@ -223,23 +171,21 @@ impl FilterArg for ListRef { } impl FilterArg for BTreeMap { - type Output<'a> = BTreeMap; - - fn from_value<'a>(v: Value) -> Result> { - Self::from_value_ref(&v) - } + type Output<'arg> = BTreeMap; - fn from_value_ref(v: &Value) -> Result> { + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { match v { - Value::Map(m) => Ok(m.clone()), - v => Err(Error::Type("map", v.human())), - } - } - - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - match v.take() { - Value::Map(m) => Ok(m), - v => Err(Error::Type("map", v.human())), + ValueCow::Borrowed(v) => match v { + Value::Map(m) => Ok(m.to_owned()), + v => Err(Error::Type("map", v.human())), + }, + ValueCow::Owned(v) => match mem::take(v) { + Value::Map(m) => Ok(m), + _ => Err(Error::Type("map", v.human())), + }, } } } @@ -247,25 +193,13 @@ impl FilterArg for BTreeMap { pub struct MapRef; impl FilterArg for MapRef { - type Output<'a> = &'a BTreeMap; - - fn from_value<'a>(v: Value) -> Result> { - match v { - Value::Map(_) => Err(Error::Reference("map")), - v => Err(Error::Type("map", v.human())), - } - } + type Output<'arg> = &'arg BTreeMap; - fn from_value_ref(v: &Value) -> Result> { - match v { - Value::Map(m) => Ok(m), - v => Err(Error::Type("map", v.human())), - } - } - - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - let v: &'a Value = &*v; - match v { + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { + match &**v { Value::Map(m) => Ok(m), v => Err(Error::Type("map", v.human())), } @@ -273,35 +207,28 @@ impl FilterArg for MapRef { } impl FilterArg for Value { - type Output<'a> = Value; - - fn from_value<'a>(v: Value) -> Result> { - Self::from_value_ref(&v) - } + type Output<'arg> = Value; - fn from_value_ref(v: &Value) -> Result> { - Ok(v.to_owned()) - } - - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - Ok(v.take()) + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { + match v { + ValueCow::Borrowed(v) => Ok(v.clone()), + ValueCow::Owned(v) => Ok(mem::take(v)), + } } } pub struct ValueRef; impl FilterArg for ValueRef { - type Output<'a> = &'a Value; - - fn from_value<'a>(_: Value) -> Result> { - Err(Error::Reference("value")) - } - - fn from_value_ref(v: &Value) -> Result> { - Ok(v) - } + type Output<'arg> = &'arg Value; - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> Result> { - Ok(&*v) + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> Result> + where + 'stack: 'arg, + { + Ok(&**v) } } diff --git a/src/filters/mod.rs b/src/filters/mod.rs index 38ae869..41cad76 100644 --- a/src/filters/mod.rs +++ b/src/filters/mod.rs @@ -89,13 +89,12 @@ mod args; mod impls; -use crate::render::{FilterState, Stack}; -use crate::types::ast::BaseExpr; +use crate::render::FilterState; use crate::types::span::Span; use crate::value::ValueCow; use crate::{Error, Result, Value}; -pub(crate) type FilterFn = dyn Fn(FilterState<'_>) -> Result + Send + Sync + 'static; +pub(crate) type FilterFn = dyn Fn(FilterState<'_, '_>) -> Result + Send + Sync + 'static; pub(crate) fn new(f: F) -> Box where @@ -103,7 +102,7 @@ where R: FilterReturn, A: FilterArgs, { - Box::new(move |state: FilterState<'_>| -> Result { + Box::new(move |state: FilterState<'_, '_>| -> Result { let args = A::from_state(state)?; let result = Filter::filter(&f, args); FilterReturn::to_value(result) @@ -128,9 +127,9 @@ where #[cfg_attr(docsrs, doc(cfg(feature = "filters")))] pub trait FilterArgs { #[doc(hidden)] - type Output<'a>; + type Output<'args>; #[doc(hidden)] - fn from_state(state: FilterState<'_>) -> Result>; + fn from_state<'args>(state: FilterState<'_, 'args>) -> Result>; } /// An argument to a filter. @@ -139,13 +138,11 @@ pub trait FilterArgs { #[cfg_attr(docsrs, doc(cfg(feature = "filters")))] pub trait FilterArg { #[doc(hidden)] - type Output<'a>; + type Output<'arg>; #[doc(hidden)] - fn from_value<'a>(v: Value) -> args::Result>; - #[doc(hidden)] - fn from_value_ref(v: &Value) -> args::Result>; - #[doc(hidden)] - fn from_cow_mut<'a>(v: &'a mut ValueCow<'a>) -> args::Result>; + fn from_value<'stack, 'arg>(v: &'arg mut ValueCow<'stack>) -> args::Result> + where + 'stack: 'arg; } /// A return value from a filter. @@ -271,10 +268,9 @@ where { type Output<'a> = (V::Output<'a>,); - fn from_state(state: FilterState<'_>) -> Result> { - check_args(&state, 0)?; - let err = |e| err_expected_val(e, state.source, state.filter.span); - let v = V::from_cow_mut(state.value).map_err(err)?; + fn from_state<'args>(state: FilterState<'_, 'args>) -> Result> { + let [(v, _)] = get_args(state.args)?; + let v = V::from_value(v).map_err(err_expected_val)?; Ok((v,)) } } @@ -286,11 +282,11 @@ where { type Output<'a> = (V::Output<'a>, A::Output<'a>); - fn from_state(state: FilterState<'_>) -> Result> { - check_args(&state, 1)?; - let err = |e| err_expected_val(e, state.source, state.filter.span); - let v = V::from_cow_mut(state.value).map_err(err)?; - let a = get_arg::(state.source, state.stack, state.args, 0)?; + fn from_state<'args>(state: FilterState<'_, 'args>) -> Result> { + let err = |e, sp| err_expected_arg(e, state.source, sp); + let [(v, _), (a, sa)] = get_args(state.args)?; + let v = V::from_value(v).map_err(err_expected_val)?; + let a = A::from_value(a).map_err(|e| err(e, *sa))?; Ok((v, a)) } } @@ -303,12 +299,12 @@ where { type Output<'a> = (V::Output<'a>, A::Output<'a>, B::Output<'a>); - fn from_state(state: FilterState<'_>) -> Result> { - check_args(&state, 2)?; - let err = |e| err_expected_val(e, state.source, state.filter.span); - let v = V::from_cow_mut(state.value).map_err(err)?; - let a = get_arg::(state.source, state.stack, state.args, 0)?; - let b = get_arg::(state.source, state.stack, state.args, 1)?; + fn from_state<'args>(state: FilterState<'_, 'args>) -> Result> { + let err = |e, sp| err_expected_arg(e, state.source, sp); + let [(v, _), (a, sa), (b, sb)] = get_args(state.args)?; + let v = V::from_value(v).map_err(err_expected_val)?; + let a = A::from_value(a).map_err(|e| err(e, *sa))?; + let b = B::from_value(b).map_err(|e| err(e, *sb))?; Ok((v, a, b)) } } @@ -322,13 +318,13 @@ where { type Output<'a> = (V::Output<'a>, A::Output<'a>, B::Output<'a>, C::Output<'a>); - fn from_state(state: FilterState<'_>) -> Result> { - check_args(&state, 3)?; - let err = |e| err_expected_val(e, state.source, state.filter.span); - let v = V::from_cow_mut(state.value).map_err(err)?; - let a = get_arg::(state.source, state.stack, state.args, 0)?; - let b = get_arg::(state.source, state.stack, state.args, 1)?; - let c = get_arg::(state.source, state.stack, state.args, 2)?; + fn from_state<'args>(state: FilterState<'_, 'args>) -> Result> { + let err = |e, sp| err_expected_arg(e, state.source, sp); + let [(v, _), (a, sa), (b, sb), (c, sc)] = get_args(state.args)?; + let v = V::from_value(v).map_err(err_expected_val)?; + let a = A::from_value(a).map_err(|e| err(e, *sa))?; + let b = B::from_value(b).map_err(|e| err(e, *sb))?; + let c = C::from_value(c).map_err(|e| err(e, *sc))?; Ok((v, a, b, c)) } } @@ -349,79 +345,49 @@ where D::Output<'a>, ); - fn from_state(state: FilterState<'_>) -> Result> { - check_args(&state, 4)?; - let err = |e| err_expected_val(e, state.source, state.filter.span); - let v = V::from_cow_mut(state.value).map_err(err)?; - let a = get_arg::(state.source, state.stack, state.args, 0)?; - let b = get_arg::(state.source, state.stack, state.args, 1)?; - let c = get_arg::(state.source, state.stack, state.args, 2)?; - let d = get_arg::(state.source, state.stack, state.args, 3)?; + fn from_state<'args>(state: FilterState<'_, 'args>) -> Result> { + let err = |e, sp| err_expected_arg(e, state.source, sp); + let [(v, _), (a, sa), (b, sb), (c, sc), (d, sd)] = get_args(state.args)?; + let v = V::from_value(v).map_err(err_expected_val)?; + let a = A::from_value(a).map_err(|e| err(e, *sa))?; + let b = B::from_value(b).map_err(|e| err(e, *sb))?; + let c = C::from_value(c).map_err(|e| err(e, *sc))?; + let d = D::from_value(d).map_err(|e| err(e, *sd))?; Ok((v, a, b, c, d)) } } -fn check_args(state: &FilterState<'_>, exp: usize) -> Result<()> { - if state.args.len() == exp { - Ok(()) - } else { - Err(Error::render( - format!("filter expected {exp} arguments"), - state.source, - state.filter.span, +fn get_args<'stack, 'args, const N: usize>( + args: &'args mut [(ValueCow<'stack>, Span)], +) -> Result<&'args mut [(ValueCow<'stack>, Span); N]> { + let n = args.len() - 1; + args.try_into().map_err(|_| { + Error::render_plain(format!( + "filter expects {} arguments, {} provided", + N - 1, + n )) - } -} - -fn get_arg<'a, T>( - source: &str, - stack: &'a Stack<'a>, - args: &'a [BaseExpr], - i: usize, -) -> Result> -where - T: FilterArg, -{ - match &args[i] { - BaseExpr::Var(var) => match stack.lookup_var(source, var)? { - ValueCow::Borrowed(v) => { - T::from_value_ref(v).map_err(|e| err_expected_arg(e, source, var.span())) - } - ValueCow::Owned(v) => { - T::from_value(v).map_err(|e| err_expected_arg(e, source, var.span())) - } - }, - BaseExpr::Literal(lit) => { - T::from_value_ref(&lit.value).map_err(|e| err_expected_arg(e, source, lit.span)) - } - } + }) } -fn err_expected_arg(err: args::Error, source: &str, span: Span) -> Error { +fn err_expected_val(err: args::Error) -> Error { let msg = match err { args::Error::Type(exp, got) => { - format!("filter expected {exp} argument, found {got}") - } - args::Error::Reference(got) => { - format!("filter expected reference argument but this {got} can only be passed as owned",) + format!("filter expects {exp} value, found {got}") } args::Error::TryFromInt(want, value) => { - format!("filter expected {want} argument, but `{value}` is out of range",) + format!("filter expects {want} value, but `{value}` is out of range",) } }; - Error::render(msg, source, span) + Error::render_plain(msg) } - -fn err_expected_val(err: args::Error, source: &str, span: Span) -> Error { +fn err_expected_arg(err: args::Error, source: &str, span: Span) -> Error { let msg = match err { args::Error::Type(exp, got) => { - format!("filter expected {exp} value, found {got}") - } - args::Error::Reference(_) => { - unreachable!() + format!("filter expects {exp} argument, found {got}") } args::Error::TryFromInt(want, value) => { - format!("filter expected {want} value, but `{value}` is out of range",) + format!("filter expects {want} argument, but `{value}` is out of range",) } }; Error::render(msg, source, span) diff --git a/src/render/core.rs b/src/render/core.rs index db312d8..e0067d4 100644 --- a/src/render/core.rs +++ b/src/render/core.rs @@ -6,6 +6,7 @@ use crate::render::stack::{Stack, State}; use crate::render::RendererInner; use crate::types::ast; use crate::types::program::{Instr, Template}; +use crate::types::span::Span; use crate::value::ValueCow; use crate::{EngineBoxFn, Error, Result}; @@ -17,12 +18,12 @@ pub struct RendererImpl<'render, 'stack> { #[cfg(feature = "filters")] #[cfg_attr(internal_debug, derive(Debug))] -pub struct FilterState<'a> { - pub stack: &'a Stack<'a>, - pub source: &'a str, - pub filter: &'a ast::Ident, - pub value: &'a mut ValueCow<'a>, - pub args: &'a [ast::BaseExpr], +pub struct FilterState<'stack, 'args> +where + 'stack: 'args, +{ + pub source: &'stack str, + pub args: &'args mut [(ValueCow<'stack>, Span)], } #[cfg_attr(internal_debug, derive(Debug))] @@ -100,8 +101,8 @@ where t: &'render Template<'render>, pc: &mut usize, ) -> Result> { - // An expression that we are building - let mut expr: Option> = None; + // The expressions that we are building + let mut exprs: Vec<(ValueCow<'stack>, Span)> = Vec::new(); while let Some(instr) = t.instrs.get(*pc) { match instr { @@ -111,23 +112,23 @@ where } Instr::JumpIfTrue(j) => { - if expr.take().unwrap().as_bool() { + if exprs.pop().unwrap().0.as_bool() { *pc = *j; continue; } } Instr::JumpIfFalse(j) => { - if !expr.take().unwrap().as_bool() { + if !exprs.pop().unwrap().0.as_bool() { *pc = *j; continue; } } - Instr::Emit(span) => { - let value = expr.take().unwrap(); + Instr::Emit => { + let (value, span) = exprs.pop().unwrap(); (self.inner.engine.default_formatter)(f, &value) - .map_err(|err| Error::format(err, &t.source, *span))?; + .map_err(|err| Error::format(err, &t.source, span))?; } Instr::EmitRaw(span) => { @@ -137,7 +138,7 @@ where f.write_str(raw)?; } - Instr::EmitWith(name, _span) => { + Instr::EmitWith(name, _arity, _span) => { let name_raw = &t.source[name.span]; match self.inner.engine.functions.get(name_raw) { // The referenced function is a filter, so we apply @@ -145,22 +146,21 @@ where // formatter. #[cfg(feature = "filters")] Some(EngineBoxFn::Filter(filter)) => { - let mut value = expr.take().unwrap(); + let at = exprs.len() - (_arity + 1); + let args = &mut exprs[at..]; let result = filter(FilterState { - stack: &self.stack, source: &t.source, - filter: name, - value: &mut value, - args: &[], + args, }) .map_err(|err| err.enrich(&t.source, name.span))?; + exprs.truncate(at); (self.inner.engine.default_formatter)(f, &result) .map_err(|err| Error::format(err, &t.source, *_span))?; } // The referenced function is a formatter so we simply // emit the value with it. Some(EngineBoxFn::Formatter(formatter)) => { - let value = expr.take().unwrap(); + let (value, _) = exprs.pop().unwrap(); formatter(f, &value) .map_err(|err| Error::format(err, &t.source, name.span))?; } @@ -176,7 +176,7 @@ where } Instr::LoopStart(vars, span) => { - let iterable = expr.take().unwrap(); + let (iterable, _) = exprs.pop().unwrap(); self.stack.push(State::Loop(LoopState::new( &t.source, vars, iterable, *span, )?)); @@ -191,7 +191,7 @@ where } Instr::WithStart(name) => { - let value = expr.take().unwrap(); + let (value, _) = exprs.pop().unwrap(); self.stack.push(State::Var(name, value)) } @@ -201,49 +201,45 @@ where Instr::Include(template_name) => { *pc += 1; + debug_assert!(exprs.is_empty()); return Ok(RenderState::Include { template_name }); } Instr::IncludeWith(template_name) => { *pc += 1; - let globals = expr.take().unwrap(); + let (globals, _) = exprs.pop().unwrap(); + debug_assert!(exprs.is_empty()); return Ok(RenderState::IncludeWith { template_name, globals, }); } - Instr::ExprStart(var) => { + Instr::ExprStartVar(var) => { let value = self.stack.lookup_var(&t.source, var)?; - let prev = expr.replace(value); - debug_assert!(prev.is_none()); + exprs.push((value, var.span())); } - Instr::ExprStartLit(value) => { - let prev = expr.replace(ValueCow::Owned(value.clone())); - debug_assert!(prev.is_none()); + Instr::ExprStartLiteral(literal) => { + let value = ValueCow::Borrowed(&literal.value); + exprs.push((value, literal.span)); } - Instr::Apply(name, _args) => { + Instr::Apply(name, _arity, _span) => { let name_raw = &t.source[name.span]; match self.inner.engine.functions.get(name_raw) { // The referenced function is a filter, so we apply it. #[cfg(feature = "filters")] Some(EngineBoxFn::Filter(filter)) => { - let mut value = expr.take().unwrap(); - let args = _args - .as_ref() - .map(|args| args.values.as_slice()) - .unwrap_or(&[]); + let at = exprs.len() - (_arity + 1); + let args = &mut exprs[at..]; let result = filter(FilterState { - stack: &self.stack, source: &t.source, - filter: name, - value: &mut value, args, }) - .map_err(|e| e.enrich(&t.source, name.span))?; - expr.replace(ValueCow::Owned(result)); + .map_err(|e| e.enrich(&t.source, *_span))?; + exprs.truncate(at); + exprs.push((ValueCow::Owned(result), *_span)); } // The referenced function is a formatter which is not valid // in the middle of an expression. @@ -265,6 +261,7 @@ where } assert!(*pc == t.instrs.len()); + debug_assert!(exprs.is_empty()); Ok(RenderState::Done) } diff --git a/src/render/mod.rs b/src/render/mod.rs index e9f7118..6b5cd36 100644 --- a/src/render/mod.rs +++ b/src/render/mod.rs @@ -49,10 +49,12 @@ pub struct Renderer<'render> { } enum Globals<'render> { + #[cfg(feature = "serde")] Owned(Result), Borrowed(&'render Value), Fn(Box>), } + pub(crate) struct RendererInner<'render> { engine: &'render Engine<'render>, template: &'render Template<'render>, @@ -156,12 +158,11 @@ impl<'render> Renderer<'render> { pub fn to_string(self) -> Result { let Self { globals, inner } = self; match globals { + #[cfg(feature = "serde")] Globals::Owned(result) => { let value = result?; let stack = Stack::new(&value); - let x = to_string(inner, stack); - drop(value); - x + to_string(inner, stack) } Globals::Borrowed(value) => { let stack = Stack::new(value); @@ -181,6 +182,7 @@ impl<'render> Renderer<'render> { { let Self { globals, inner } = self; match globals { + #[cfg(feature = "serde")] Globals::Owned(result) => { let value = result?; let stack = Stack::new(&value); diff --git a/src/types/program.rs b/src/types/program.rs index 7a3e3e3..78c1974 100644 --- a/src/types/program.rs +++ b/src/types/program.rs @@ -5,7 +5,6 @@ use std::borrow::Cow; use crate::types::ast; use crate::types::span::Span; -use crate::Value; pub const FIXME: usize = !0; @@ -27,13 +26,16 @@ pub enum Instr { JumpIfFalse(usize), /// Emit the current expression - Emit(Span), + Emit, /// Emit raw template EmitRaw(Span), - /// Apply the filter or value formatter to the current expression and emit - EmitWith(ast::Ident, Span), + /// Apply the filter or value formatter to the current expression and emit. + /// + /// The second value is the number of arguments to pop from the stack + /// excluding the value itself. + EmitWith(ast::Ident, usize, Span), /// Start a loop over the current expression LoopStart(ast::LoopVars, Span), @@ -54,13 +56,16 @@ pub enum Instr { IncludeWith(ast::String), /// Lookup a variable and start building an expression - ExprStart(ast::Var), + ExprStartVar(ast::Var), /// Start building an expression using a literal - ExprStartLit(Value), + ExprStartLiteral(ast::Literal), - /// Apply the filter to the value at the top of the stack - Apply(ast::Ident, Option), + /// Apply the filter using the value and args on the top of the stack. + /// + /// The second value is the number of arguments to pop from the stack + /// excluding the value itself. + Apply(ast::Ident, usize, Span), } #[cfg(not(internal_debug))] diff --git a/src/value/cow.rs b/src/value/cow.rs index 14d7db3..fedb9f2 100644 --- a/src/value/cow.rs +++ b/src/value/cow.rs @@ -20,13 +20,3 @@ impl Deref for ValueCow<'_> { } } } - -impl ValueCow<'_> { - #[cfg(feature = "filters")] - pub fn take(&mut self) -> Value { - match self { - Self::Borrowed(v) => v.clone(), - Self::Owned(v) => std::mem::take(v), - } - } -} diff --git a/tests/filters.rs b/tests/filters.rs index 4e20e08..7592bd4 100644 --- a/tests/filters.rs +++ b/tests/filters.rs @@ -93,7 +93,7 @@ fn render_filter_arity_5() { } #[test] -fn render_filter_value_types() { +fn render_filter_arg0_types() { let mut engine = Engine::new(); // unit @@ -134,7 +134,7 @@ fn render_filter_value_types() { } #[test] -fn render_filter_arg_types() { +fn render_filter_arg1_types() { let mut engine = Engine::new(); // unit @@ -172,6 +172,132 @@ fn render_filter_arg_types() { engine.add_filter("_", |_: Value, _: BTreeMap| ()); } +#[test] +fn render_filter_arg2_types() { + let mut engine = Engine::new(); + + // unit + engine.add_filter("_", |_: Value, _: Value, _: ()| ()); + + // bool + engine.add_filter("_", |_: Value, _: Value, _: bool| ()); + + // ints + engine.add_filter("_", |_: Value, _: Value, _: u8| ()); + engine.add_filter("_", |_: Value, _: Value, _: u16| ()); + engine.add_filter("_", |_: Value, _: Value, _: u32| ()); + engine.add_filter("_", |_: Value, _: Value, _: u64| ()); + engine.add_filter("_", |_: Value, _: Value, _: u128| ()); + engine.add_filter("_", |_: Value, _: Value, _: usize| ()); + engine.add_filter("_", |_: Value, _: Value, _: i8| ()); + engine.add_filter("_", |_: Value, _: Value, _: i16| ()); + engine.add_filter("_", |_: Value, _: Value, _: i32| ()); + engine.add_filter("_", |_: Value, _: Value, _: i64| ()); + engine.add_filter("_", |_: Value, _: Value, _: i128| ()); + engine.add_filter("_", |_: Value, _: Value, _: isize| ()); + + // floats + engine.add_filter("_", |_: Value, _: Value, _: f32| ()); + engine.add_filter("_", |_: Value, _: Value, _: f64| ()); + + // strings + engine.add_filter("_", |_: Value, _: Value, _: String| ()); + engine.add_filter("_", |_: Value, _: Value, _: &str| ()); + + // list + engine.add_filter("_", |_: Value, _: Value, _: Vec| ()); + + // map + engine.add_filter("_", |_: Value, _: Value, _: BTreeMap| ()); +} + +#[test] +fn render_filter_arg3_types() { + let mut engine = Engine::new(); + + // unit + engine.add_filter("_", |_: Value, _: Value, _: Value, _: ()| ()); + + // bool + engine.add_filter("_", |_: Value, _: Value, _: Value, _: bool| ()); + + // ints + engine.add_filter("_", |_: Value, _: Value, _: Value, _: u8| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: u16| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: u32| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: u64| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: u128| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: usize| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: i8| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: i16| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: i32| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: i64| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: i128| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: isize| ()); + + // floats + engine.add_filter("_", |_: Value, _: Value, _: Value, _: f32| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: f64| ()); + + // strings + engine.add_filter("_", |_: Value, _: Value, _: Value, _: String| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: &str| ()); + + // list + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Vec| ()); + + // map + engine.add_filter( + "_", + |_: Value, _: Value, _: Value, _: BTreeMap| (), + ); +} + +#[test] +fn render_filter_arg4_types() { + let mut engine = Engine::new(); + + // unit + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: ()| ()); + + // bool + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: bool| ()); + + // ints + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: u8| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: u16| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: u32| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: u64| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: u128| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: usize| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: i8| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: i16| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: i32| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: i64| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: i128| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: isize| ()); + + // floats + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: f32| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: f64| ()); + + // strings + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: String| ()); + engine.add_filter("_", |_: Value, _: Value, _: Value, _: Value, _: &str| ()); + + // list + engine.add_filter( + "_", + |_: Value, _: Value, _: Value, _: Value, _: Vec| (), + ); + + // map + engine.add_filter( + "_", + |_: Value, _: Value, _: Value, _: Value, _: BTreeMap| (), + ); +} + #[test] fn render_filter_err_expected_0_args() { let mut engine = Engine::new(); @@ -184,7 +310,7 @@ fn render_filter_err_expected_0_args() { .unwrap_err(); assert_err( &err, - "filter expected 0 arguments", + "filter expects 0 arguments, 1 provided", " --> :1:11 | @@ -208,7 +334,7 @@ fn render_filter_err_expected_n_args() { .unwrap_err(); assert_err( &err, - "filter expected 3 arguments", + "filter expects 3 arguments, 0 provided", " --> :1:11 | @@ -221,7 +347,7 @@ fn render_filter_err_expected_n_args() { } #[test] -fn render_filter_borrowed_value_str() { +fn render_filter_borrowed_arg_str_from_borrowed() { let mut engine = Engine::new(); engine.add_filter("test", |v: &str| v.to_owned()); let result = engine @@ -234,7 +360,21 @@ fn render_filter_borrowed_value_str() { } #[test] -fn render_filter_borrowed_value_list() { +fn render_filter_borrowed_arg_str_from_owned() { + let mut engine = Engine::new(); + engine.add_filter("into_owned", |v: Value| v); + engine.add_filter("test", |v: &str| v.to_owned()); + let result = engine + .compile("{{ name | into_owned | test }}") + .unwrap() + .render(&engine, value! { name: "John Smith" }) + .to_string() + .unwrap(); + assert_eq!(result, "John Smith"); +} + +#[test] +fn render_filter_borrowed_arg_list_from_borrowed() { let mut engine = Engine::new(); engine.add_filter("test", |v: &[Value]| v[0].clone()); let result = engine @@ -247,7 +387,21 @@ fn render_filter_borrowed_value_list() { } #[test] -fn render_filter_borrowed_value_map() { +fn render_filter_borrowed_arg_list_from_owned() { + let mut engine = Engine::new(); + engine.add_filter("into_owned", |v: Value| v); + engine.add_filter("test", |v: &[Value]| v[0].clone()); + let result = engine + .compile("{{ name | into_owned | test }}") + .unwrap() + .render(&engine, value! { name: ["John", "Smith"] }) + .to_string() + .unwrap(); + assert_eq!(result, "John"); +} + +#[test] +fn render_filter_borrowed_arg_map_from_borrowed() { let mut engine = Engine::new(); engine.add_filter("test", |v: &BTreeMap| v["john"].to_owned()); let result = engine @@ -260,7 +414,21 @@ fn render_filter_borrowed_value_map() { } #[test] -fn render_filter_borrowed_value_value() { +fn render_filter_borrowed_arg_map_from_owned() { + let mut engine = Engine::new(); + engine.add_filter("into_owned", |v: Value| v); + engine.add_filter("test", |v: &BTreeMap| v["john"].to_owned()); + let result = engine + .compile("{{ name | into_owned | test }}") + .unwrap() + .render(&engine, value! { name: { john: "Smith" } }) + .to_string() + .unwrap(); + assert_eq!(result, "Smith"); +} + +#[test] +fn render_filter_borrowed_arg_value_from_borrowed() { let mut engine = Engine::new(); engine.add_filter("test", |v: &Value| v.clone()); let result = engine @@ -272,6 +440,20 @@ fn render_filter_borrowed_value_value() { assert_eq!(result, "John Smith"); } +#[test] +fn render_filter_borrowed_arg_value_from_owned() { + let mut engine = Engine::new(); + engine.add_filter("into_owned", |v: Value| v); + engine.add_filter("test", |v: &Value| v.clone()); + let result = engine + .compile("{{ name | into_owned | test }}") + .unwrap() + .render(&engine, value! { name: "John Smith" }) + .to_string() + .unwrap(); + assert_eq!(result, "John Smith"); +} + #[test] fn render_filter_borrowed_arg_str() { let mut engine = Engine::new(); @@ -288,6 +470,30 @@ fn render_filter_borrowed_arg_str() { assert_eq!(result, "JohnSmith"); } +#[test] +fn render_filter_borrowed_args_from_owned() { + let mut engine = Engine::new(); + engine.add_filter("into_owned", |v: Value| v); + engine.add_filter("prepend", |s1: &str, s2: &str| format!("{s2} {s1}")); + let result = engine + .compile( + "{% for name in names | into_owned %}\n\ + {{ surname | prepend: name }}\n\ + {% endfor %}", + ) + .unwrap() + .render( + &engine, + value! { + names: ["John", "James", "Jimothy"], + surname: "Smith" + }, + ) + .to_string() + .unwrap(); + assert_eq!(result, "\nJohn Smith\n\nJames Smith\n\nJimothy Smith\n"); +} + #[test] fn render_filter_err_expected_value_type() { let mut engine = Engine::new(); @@ -300,7 +506,7 @@ fn render_filter_err_expected_value_type() { .unwrap_err(); assert_err( &err, - "filter expected bool value, found string", + "filter expects bool value, found string", " --> :1:11 | @@ -324,7 +530,7 @@ fn render_filter_err_expected_arg_type() { .unwrap_err(); assert_err( &err, - "filter expected bool argument, found integer", + "filter expects bool argument, found integer", " --> :1:17 | @@ -348,7 +554,7 @@ fn render_filter_err_expected_value_try_from_int() { .unwrap_err(); assert_err( &err, - "filter expected i8 value, but `128` is out of range", + "filter expects i8 value, but `128` is out of range", " --> :1:10 | @@ -360,41 +566,6 @@ fn render_filter_err_expected_value_try_from_int() { ); } -#[test] -fn render_filter_err_expected_arg_reference() { - let mut engine = Engine::new(); - engine.add_filter("into_owned", |v: Value| v); - engine.add_filter("prepend", |s1: &str, s2: &str| format!("{s2} {s1}")); - let err = engine - .compile( - "{% for name in names | into_owned %}\n\ - {{ surname | prepend: name }}\n\ - {% endfor %}", - ) - .unwrap() - .render( - &engine, - value! { - names: ["John", "James", "Jimothy"], - surname: "Smith" - }, - ) - .to_string() - .unwrap_err(); - assert_err( - &err, - "filter expected reference argument but this string can only be passed as owned", - " - --> :2:23 - | - 2 | {{ surname | prepend: name }} - | ^^^^ - | - = reason: REASON -", - ); -} - #[test] fn render_filter_err_expected_arg_try_from_int() { let mut engine = Engine::new(); @@ -407,7 +578,7 @@ fn render_filter_err_expected_arg_try_from_int() { .unwrap_err(); assert_err( &err, - "filter expected i8 argument, but `128` is out of range", + "filter expects i8 argument, but `128` is out of range", " --> :1:19 |