From d93c7967c0fd987d8d32d9131681d8120be5dcc7 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sat, 10 Sep 2022 10:51:55 +0200 Subject: [PATCH 01/10] Experimentally implement borrowing for arguments --- minijinja/src/defaults.rs | 2 + minijinja/src/environment.rs | 15 ++++++-- minijinja/src/filters.rs | 9 +++-- minijinja/src/functions.rs | 9 ++--- minijinja/src/tests.rs | 8 ++-- minijinja/src/value/argtypes.rs | 68 +++++++++++++++++++++++++++------ 6 files changed, 84 insertions(+), 27 deletions(-) diff --git a/minijinja/src/defaults.rs b/minijinja/src/defaults.rs index cf0e5069..23644c33 100644 --- a/minijinja/src/defaults.rs +++ b/minijinja/src/defaults.rs @@ -118,6 +118,7 @@ pub(crate) fn get_builtin_filters() -> BTreeMap<&'static str, filters::BoxedFilt rv.insert("urlencode", BoxedFilter::new(filters::urlencode)); } } + rv } @@ -151,5 +152,6 @@ pub(crate) fn get_globals() -> BTreeMap<&'static str, Value> { rv.insert("dict", BoxedFunction::new(functions::dict).to_value()); rv.insert("debug", BoxedFunction::new(functions::debug).to_value()); } + rv } diff --git a/minijinja/src/environment.rs b/minijinja/src/environment.rs index e37fd5d3..185d2430 100644 --- a/minijinja/src/environment.rs +++ b/minijinja/src/environment.rs @@ -363,7 +363,12 @@ impl<'source> Environment<'source> { /// [`Filter`](crate::filters::Filter). pub fn add_filter(&mut self, name: &'source str, f: F) where - F: filters::Filter, + F: filters::Filter + + for<'a> filters::Filter< + >::Output, + Rv, + >::Output, + > + 'static, V: for<'a> ArgType<'a>, Rv: FunctionResult, Args: for<'a> FunctionArgs<'a>, @@ -383,9 +388,11 @@ impl<'source> Environment<'source> { /// have a look at [`Test`](crate::tests::Test). pub fn add_test(&mut self, name: &'source str, f: F) where + F: tests::Test + + for<'a> tests::Test<>::Output, Rv, >::Output> + + 'static, V: for<'a> ArgType<'a>, Rv: tests::TestResult, - F: tests::Test, Args: for<'a> FunctionArgs<'a>, { self.tests.insert(name, tests::BoxedTest::new(f)); @@ -402,8 +409,10 @@ impl<'source> Environment<'source> { /// functions and other global variables share the same namespace. pub fn add_function(&mut self, name: &'source str, f: F) where + F: functions::Function + + for<'a> functions::Function>::Output> + + 'static, Rv: FunctionResult, - F: functions::Function, Args: for<'a> FunctionArgs<'a>, { self.add_global(name, functions::BoxedFunction::new(f).to_value()); diff --git a/minijinja/src/filters.rs b/minijinja/src/filters.rs index 94ef273d..c69f14eb 100644 --- a/minijinja/src/filters.rs +++ b/minijinja/src/filters.rs @@ -160,7 +160,6 @@ macro_rules! tuple_impls { Func: Fn(&State, V, $($name),*) -> Rv + Send + Sync + 'static, V: for<'a> ArgType<'a>, Rv: FunctionResult, - $($name: for<'a> ArgType<'a>),* { fn apply_to(&self, state: &State, value: V, args: ($($name,)*), _: SealedMarker) -> Rv { #[allow(non_snake_case)] @@ -181,7 +180,9 @@ impl BoxedFilter { /// Creates a new boxed filter. pub fn new(f: F) -> BoxedFilter where - F: Filter, + F: Filter + + for<'a> Filter<>::Output, Rv, >::Output> + + 'static, V: for<'a> ArgType<'a>, Rv: FunctionResult, Args: for<'a> FunctionArgs<'a>, @@ -190,8 +191,8 @@ impl BoxedFilter { move |state, value, args| -> Result { f.apply_to( state, - ArgType::from_value(Some(value))?, - FunctionArgs::from_values(args)?, + V::from_value(Some(value))?, + Args::from_values(args)?, SealedMarker, ) .into_result() diff --git a/minijinja/src/functions.rs b/minijinja/src/functions.rs index 897a0e7c..69470803 100644 --- a/minijinja/src/functions.rs +++ b/minijinja/src/functions.rs @@ -53,7 +53,7 @@ use std::sync::Arc; use crate::error::Error; use crate::utils::SealedMarker; -use crate::value::{ArgType, FunctionArgs, FunctionResult, Object, Value}; +use crate::value::{FunctionArgs, FunctionResult, Object, Value}; use crate::vm::State; type FuncFunc = dyn Fn(&State, &[Value]) -> Result + Sync + Send + 'static; @@ -135,7 +135,6 @@ macro_rules! tuple_impls { where Func: Fn(&State, $($name),*) -> Rv + Send + Sync + 'static, Rv: FunctionResult, - $($name: for<'a> ArgType<'a>),* { fn invoke(&self, state: &State, args: ($($name,)*), _: SealedMarker) -> Rv { #[allow(non_snake_case)] @@ -156,13 +155,13 @@ impl BoxedFunction { /// Creates a new boxed filter. pub fn new(f: F) -> BoxedFunction where - F: Function, + F: Function + for<'a> Function>::Output> + 'static, Rv: FunctionResult, Args: for<'a> FunctionArgs<'a>, { BoxedFunction( - Arc::new(move |env, args| -> Result { - f.invoke(env, FunctionArgs::from_values(args)?, SealedMarker) + Arc::new(move |state, args| -> Result { + f.invoke(state, Args::from_values(args)?, SealedMarker) .into_result() }), std::any::type_name::(), diff --git a/minijinja/src/tests.rs b/minijinja/src/tests.rs index b018876d..63aeca02 100644 --- a/minijinja/src/tests.rs +++ b/minijinja/src/tests.rs @@ -183,7 +183,9 @@ impl BoxedTest { /// Creates a new boxed filter. pub fn new(f: F) -> BoxedTest where - F: Test, + F: Test + + for<'a> Test<>::Output, Rv, >::Output> + + 'static, V: for<'a> ArgType<'a>, Rv: TestResult, Args: for<'a> FunctionArgs<'a>, @@ -192,8 +194,8 @@ impl BoxedTest { let value = Some(value); f.perform( state, - ArgType::from_value(value)?, - FunctionArgs::from_values(args)?, + V::from_value(value)?, + Args::from_values(args)?, SealedMarker, ) .into_result() diff --git a/minijinja/src/value/argtypes.rs b/minijinja/src/value/argtypes.rs index 84244dd2..155bfe8d 100644 --- a/minijinja/src/value/argtypes.rs +++ b/minijinja/src/value/argtypes.rs @@ -41,9 +41,11 @@ impl> FunctionResult for I { /// /// For each argument the conversion is performed via the [`ArgType`] /// trait which is implemented for many common types. -pub trait FunctionArgs<'a>: Sized { +pub trait FunctionArgs<'a> { + type Output; + /// Converts to function arguments from a slice of values. - fn from_values(values: &'a [Value]) -> Result; + fn from_values(values: &'a [Value]) -> Result; } /// A trait implemented by all filter/test argument types. @@ -64,13 +66,15 @@ pub trait FunctionArgs<'a>: Sized { /// to encode optional parameters to filters, functions or tests. Additionally /// it's implemented for [`Rest`] which is used to encode the remaining arguments /// of a function call. -pub trait ArgType<'a>: Sized { +pub trait ArgType<'a> { + type Output; + #[doc(hidden)] - fn from_value(value: Option<&'a Value>) -> Result; + fn from_value(value: Option<&'a Value>) -> Result; #[doc(hidden)] #[inline(always)] - fn from_rest_values(_values: &'a [Value]) -> Result, Error> { + fn from_rest_values(_values: &'a [Value]) -> Result, Error> { Ok(None) } } @@ -80,7 +84,9 @@ macro_rules! tuple_impls { impl<'a, $($name),*> FunctionArgs<'a> for ($($name,)*) where $($name: ArgType<'a>,)* { - fn from_values(values: &'a [Value]) -> Result { + type Output = ($($name::Output,)*); + + fn from_values(values: &'a [Value]) -> Result { #![allow(non_snake_case, unused)] let arg_count = 0 $( + { let $name = (); 1 } @@ -91,7 +97,7 @@ macro_rules! tuple_impls { if let Some(rest) = $rest_name::from_rest_values(rest_values)? { let mut idx = 0; $( - let $alt_name = ArgType::from_value(values.get(idx))?; + let $alt_name = $alt_name::from_value(values.get(idx))?; idx += 1; )* return Ok(( $($alt_name,)* rest ,)); @@ -107,7 +113,7 @@ macro_rules! tuple_impls { { let mut idx = 0; $( - let $name = ArgType::from_value(values.get(idx))?; + let $name = $name::from_value(values.get(idx))?; idx += 1; )* Ok(( $($name,)* )) @@ -256,6 +262,7 @@ macro_rules! primitive_try_from { } impl<'a> ArgType<'a> for $ty { + type Output = Self; fn from_value(value: Option<&Value>) -> Result { match value { Some(value) => TryFrom::try_from(value.clone()), @@ -265,6 +272,7 @@ macro_rules! primitive_try_from { } impl<'a> ArgType<'a> for Option<$ty> { + type Output = Self; fn from_value(value: Option<&Value>) -> Result { match value { Some(value) => { @@ -315,6 +323,30 @@ primitive_try_from!(f64, { ValueRepr::F64(val) => val, }); +impl<'a> ArgType<'a> for &str { + type Output = &'a str; + + fn from_value(value: Option<&'a Value>) -> Result<&'a str, Error> { + match value { + Some(value) => value + .as_str() + .ok_or_else(|| Error::new(ErrorKind::ImpossibleOperation, "value is not a string")), + None => Err(Error::new(ErrorKind::UndefinedError, "missing argument")), + } + } +} + +impl<'a> ArgType<'a> for &Value { + type Output = &'a Value; + + fn from_value(value: Option<&'a Value>) -> Result<&'a Value, Error> { + match value { + Some(value) => Ok(value), + None => Err(Error::new(ErrorKind::UndefinedError, "missing argument")), + } + } +} + /// Utility type to capture remaining arguments. /// /// In some cases you might want to have a variadic function. In that case @@ -351,7 +383,9 @@ impl DerefMut for Rest { } } -impl<'a, T: ArgType<'a>> ArgType<'a> for Rest { +impl<'a, T: ArgType<'a, Output = T>> ArgType<'a> for Rest { + type Output = Self; + fn from_value(_value: Option<&'a Value>) -> Result { Err(Error::new( ErrorKind::ImpossibleOperation, @@ -364,13 +398,15 @@ impl<'a, T: ArgType<'a>> ArgType<'a> for Rest { Ok(Some(Rest( values .iter() - .map(|v| ArgType::from_value(Some(v))) + .map(|v| T::from_value(Some(v))) .collect::>()?, ))) } } impl<'a> ArgType<'a> for Value { + type Output = Self; + fn from_value(value: Option<&'a Value>) -> Result { match value { Some(value) => Ok(value.clone()), @@ -380,6 +416,8 @@ impl<'a> ArgType<'a> for Value { } impl<'a> ArgType<'a> for Option { + type Output = Self; + fn from_value(value: Option<&'a Value>) -> Result { match value { Some(value) => { @@ -395,6 +433,8 @@ impl<'a> ArgType<'a> for Option { } impl<'a> ArgType<'a> for String { + type Output = Self; + fn from_value(value: Option<&'a Value>) -> Result { match value { Some(value) => Ok(value.to_string()), @@ -404,6 +444,8 @@ impl<'a> ArgType<'a> for String { } impl<'a> ArgType<'a> for Option { + type Output = Self; + fn from_value(value: Option<&'a Value>) -> Result { match value { Some(value) => { @@ -430,7 +472,9 @@ impl From for Value { } } -impl<'a, T: ArgType<'a>> ArgType<'a> for Vec { +impl<'a, T: ArgType<'a, Output = T>> ArgType<'a> for Vec { + type Output = Self; + fn from_value(value: Option<&'a Value>) -> Result { match value { None => Ok(Vec::new()), @@ -438,7 +482,7 @@ impl<'a, T: ArgType<'a>> ArgType<'a> for Vec { let values = values.as_slice()?; let mut rv = Vec::new(); for value in values { - rv.push(ArgType::from_value(Some(value))?); + rv.push(T::from_value(Some(value))?); } Ok(rv) } From 000cd580682d45662920b28bf61c9e789e89ba06 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sat, 10 Sep 2022 12:37:17 +0200 Subject: [PATCH 02/10] Use nicer abstractions for filters that borrow --- minijinja/src/filters.rs | 38 ++++++------ minijinja/src/functions.rs | 4 +- minijinja/src/tests.rs | 9 +-- minijinja/src/value/argtypes.rs | 103 +++++++++++++++----------------- 4 files changed, 77 insertions(+), 77 deletions(-) diff --git a/minijinja/src/filters.rs b/minijinja/src/filters.rs index c69f14eb..2c956ec7 100644 --- a/minijinja/src/filters.rs +++ b/minijinja/src/filters.rs @@ -259,8 +259,8 @@ mod builtins { ///

{{ chapter.title|upper }}

/// ``` #[cfg_attr(docsrs, doc(cfg(feature = "builtins")))] - pub fn upper(_state: &State, v: Value) -> String { - v.to_cowstr().to_uppercase() + pub fn upper(_state: &State, v: Cow<'_, str>) -> String { + v.to_uppercase() } /// Converts a value to lowercase. @@ -269,8 +269,8 @@ mod builtins { ///

{{ chapter.title|lower }}

/// ``` #[cfg_attr(docsrs, doc(cfg(feature = "builtins")))] - pub fn lower(_state: &State, v: Value) -> String { - v.to_cowstr().to_lowercase() + pub fn lower(_state: &State, v: Cow<'_, str>) -> String { + v.to_lowercase() } /// Converts a value to title case. @@ -279,10 +279,10 @@ mod builtins { ///

{{ chapter.title|title }}

/// ``` #[cfg_attr(docsrs, doc(cfg(feature = "builtins")))] - pub fn title(_state: &State, v: Value) -> String { + pub fn title(_state: &State, v: Cow<'_, str>) -> String { let mut rv = String::new(); let mut capitalize = true; - for c in v.to_cowstr().chars() { + for c in v.chars() { if c.is_ascii_punctuation() || c.is_whitespace() { rv.push(c); capitalize = true; @@ -305,9 +305,13 @@ mod builtins { /// -> Goodbye World /// ``` #[cfg_attr(docsrs, doc(cfg(feature = "builtins")))] - pub fn replace(_state: &State, v: Value, from: Value, to: Value) -> String { - v.to_cowstr() - .replace(&from.to_cowstr() as &str, &to.to_cowstr() as &str) + pub fn replace( + _state: &State, + v: Cow<'_, str>, + from: Cow<'_, str>, + to: Cow<'_, str>, + ) -> String { + v.replace(&from as &str, &to as &str) } /// Returns the "length" of the value @@ -409,30 +413,30 @@ mod builtins { /// Trims a value #[cfg_attr(docsrs, doc(cfg(feature = "builtins")))] - pub fn trim(_state: &State, s: Value, chars: Option) -> String { + pub fn trim(_state: &State, s: Cow<'_, str>, chars: Option>) -> String { match chars { Some(chars) => { - let chars = chars.to_cowstr().chars().collect::>(); - s.to_cowstr().trim_matches(&chars[..]).to_string() + let chars = chars.chars().collect::>(); + s.trim_matches(&chars[..]).to_string() } - None => s.to_cowstr().trim().to_string(), + None => s.trim().to_string(), } } /// Joins a sequence by a character #[cfg_attr(docsrs, doc(cfg(feature = "builtins")))] - pub fn join(_state: &State, val: Value, joiner: Option) -> Result { + pub fn join(_state: &State, val: Value, joiner: Option>) -> Result { if val.is_undefined() || val.is_none() { return Ok(String::new()); } - let joiner = joiner.as_ref().map_or(Cow::Borrowed(""), |x| x.to_cowstr()); + let joiner = joiner.as_ref().unwrap_or(&Cow::Borrowed("")); if let Some(s) = val.as_str() { let mut rv = String::new(); for c in s.chars() { if !rv.is_empty() { - rv.push_str(&joiner); + rv.push_str(joiner); } rv.push(c); } @@ -441,7 +445,7 @@ mod builtins { let mut rv = String::new(); for item in val.as_slice()? { if !rv.is_empty() { - rv.push_str(&joiner); + rv.push_str(joiner); } if let Some(s) = item.as_str() { rv.push_str(s); diff --git a/minijinja/src/functions.rs b/minijinja/src/functions.rs index 69470803..56c70ef5 100644 --- a/minijinja/src/functions.rs +++ b/minijinja/src/functions.rs @@ -78,8 +78,8 @@ pub(crate) struct BoxedFunction(Arc, &'static str); /// /// The parameters can be marked optional by using `Option`. The last /// argument can also use [`Rest`](crate::value::Rest) to capture the -/// remaining arguments. All types are supported for which [`ArgType`] is -/// implemented. +/// remaining arguments. All types are supported for which +/// [`ArgType`](crate::value::ArgType) is implemented. /// /// For a list of built-in functions see [`functions`](crate::functions). /// diff --git a/minijinja/src/tests.rs b/minijinja/src/tests.rs index 63aeca02..bf34dc33 100644 --- a/minijinja/src/tests.rs +++ b/minijinja/src/tests.rs @@ -212,6 +212,7 @@ impl BoxedTest { mod builtins { use super::*; + use std::borrow::Cow; use std::convert::TryFrom; use crate::value::ValueKind; @@ -266,14 +267,14 @@ mod builtins { /// Checks if the value is starting with a string. #[cfg_attr(docsrs, doc(cfg(feature = "builtins")))] - pub fn is_startingwith(_state: &State, v: String, other: String) -> bool { - v.starts_with(&other) + pub fn is_startingwith(_state: &State, v: Cow<'_, str>, other: Cow<'_, str>) -> bool { + v.starts_with(&other as &str) } /// Checks if the value is ending with a string. #[cfg_attr(docsrs, doc(cfg(feature = "builtins")))] - pub fn is_endingwith(_state: &State, v: String, other: String) -> bool { - v.ends_with(&other) + pub fn is_endingwith(_state: &State, v: Cow<'_, str>, other: Cow<'_, str>) -> bool { + v.ends_with(&other as &str) } #[test] diff --git a/minijinja/src/value/argtypes.rs b/minijinja/src/value/argtypes.rs index 155bfe8d..ad96b34b 100644 --- a/minijinja/src/value/argtypes.rs +++ b/minijinja/src/value/argtypes.rs @@ -58,9 +58,15 @@ pub trait FunctionArgs<'a> { /// * signed integers: [`i8`], [`i16`], [`i32`], [`i64`], [`i128`] /// * floats: [`f64`] /// * bool: [`bool`] -/// * string: [`String`] -/// * values: [`Value`] -/// * vectors: [`Vec`] +/// * string: [`String`], [`&str`], `Cow<'_, str>` +/// * values: [`Value`], `&Value` +/// * vectors: [`Vec`], `&[Value]` +/// +/// Note on that there is an important difference between `String` and `&str`: +/// the former will be valid for all values and an implicit conversion to string +/// via [`ToString`] will take place, for the latter only values which are already +/// strings will be passed. A compromise between the two is `Cow<'_, str>` which +/// will behave like `String` but borrows when possible. /// /// The type is also implemented for optional values (`Option`) which is used /// to encode optional parameters to filters, functions or tests. Additionally @@ -270,22 +276,6 @@ macro_rules! primitive_try_from { } } } - - impl<'a> ArgType<'a> for Option<$ty> { - type Output = Self; - fn from_value(value: Option<&Value>) -> Result { - match value { - Some(value) => { - if value.is_undefined() || value.is_none() { - Ok(None) - } else { - TryFrom::try_from(value.clone()).map(Some) - } - } - None => Ok(None), - } - } - } } } @@ -326,7 +316,7 @@ primitive_try_from!(f64, { impl<'a> ArgType<'a> for &str { type Output = &'a str; - fn from_value(value: Option<&'a Value>) -> Result<&'a str, Error> { + fn from_value(value: Option<&'a Value>) -> Result { match value { Some(value) => value .as_str() @@ -336,6 +326,34 @@ impl<'a> ArgType<'a> for &str { } } +impl<'a, T: ArgType<'a>> ArgType<'a> for Option { + type Output = Option; + + fn from_value(value: Option<&'a Value>) -> Result { + match value { + Some(value) => { + if value.is_undefined() || value.is_none() { + Ok(None) + } else { + T::from_value(Some(value)).map(Some) + } + } + None => Ok(None), + } + } +} + +impl<'a> ArgType<'a> for Cow<'_, str> { + type Output = Cow<'a, str>; + + fn from_value(value: Option<&'a Value>) -> Result, Error> { + match value { + Some(value) => Ok(value.to_cowstr()), + None => Err(Error::new(ErrorKind::UndefinedError, "missing argument")), + } + } +} + impl<'a> ArgType<'a> for &Value { type Output = &'a Value; @@ -347,6 +365,17 @@ impl<'a> ArgType<'a> for &Value { } } +impl<'a> ArgType<'a> for &[Value] { + type Output = &'a [Value]; + + fn from_value(value: Option<&'a Value>) -> Result<&'a [Value], Error> { + match value { + Some(value) => Ok(value.as_slice()?), + None => Err(Error::new(ErrorKind::UndefinedError, "missing argument")), + } + } +} + /// Utility type to capture remaining arguments. /// /// In some cases you might want to have a variadic function. In that case @@ -415,23 +444,6 @@ impl<'a> ArgType<'a> for Value { } } -impl<'a> ArgType<'a> for Option { - type Output = Self; - - fn from_value(value: Option<&'a Value>) -> Result { - match value { - Some(value) => { - if value.is_undefined() || value.is_none() { - Ok(None) - } else { - Ok(Some(value.clone())) - } - } - None => Ok(None), - } - } -} - impl<'a> ArgType<'a> for String { type Output = Self; @@ -443,23 +455,6 @@ impl<'a> ArgType<'a> for String { } } -impl<'a> ArgType<'a> for Option { - type Output = Self; - - fn from_value(value: Option<&'a Value>) -> Result { - match value { - Some(value) => { - if value.is_undefined() || value.is_none() { - Ok(None) - } else { - Ok(Some(value.to_string())) - } - } - None => Ok(None), - } - } -} - impl From for String { fn from(val: Value) -> Self { val.to_string() From 742c8b935fea730d676dd11175c8b2328ee832a5 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sat, 10 Sep 2022 12:39:40 +0200 Subject: [PATCH 03/10] Remove 'static bound which is already a bound to the trait --- minijinja/src/environment.rs | 8 +++----- minijinja/src/filters.rs | 3 +-- minijinja/src/functions.rs | 2 +- minijinja/src/tests.rs | 3 +-- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/minijinja/src/environment.rs b/minijinja/src/environment.rs index 185d2430..787477cc 100644 --- a/minijinja/src/environment.rs +++ b/minijinja/src/environment.rs @@ -368,7 +368,7 @@ impl<'source> Environment<'source> { >::Output, Rv, >::Output, - > + 'static, + >, V: for<'a> ArgType<'a>, Rv: FunctionResult, Args: for<'a> FunctionArgs<'a>, @@ -389,8 +389,7 @@ impl<'source> Environment<'source> { pub fn add_test(&mut self, name: &'source str, f: F) where F: tests::Test - + for<'a> tests::Test<>::Output, Rv, >::Output> - + 'static, + + for<'a> tests::Test<>::Output, Rv, >::Output>, V: for<'a> ArgType<'a>, Rv: tests::TestResult, Args: for<'a> FunctionArgs<'a>, @@ -410,8 +409,7 @@ impl<'source> Environment<'source> { pub fn add_function(&mut self, name: &'source str, f: F) where F: functions::Function - + for<'a> functions::Function>::Output> - + 'static, + + for<'a> functions::Function>::Output>, Rv: FunctionResult, Args: for<'a> FunctionArgs<'a>, { diff --git a/minijinja/src/filters.rs b/minijinja/src/filters.rs index 2c956ec7..f66e4951 100644 --- a/minijinja/src/filters.rs +++ b/minijinja/src/filters.rs @@ -181,8 +181,7 @@ impl BoxedFilter { pub fn new(f: F) -> BoxedFilter where F: Filter - + for<'a> Filter<>::Output, Rv, >::Output> - + 'static, + + for<'a> Filter<>::Output, Rv, >::Output>, V: for<'a> ArgType<'a>, Rv: FunctionResult, Args: for<'a> FunctionArgs<'a>, diff --git a/minijinja/src/functions.rs b/minijinja/src/functions.rs index 56c70ef5..3dd84444 100644 --- a/minijinja/src/functions.rs +++ b/minijinja/src/functions.rs @@ -155,7 +155,7 @@ impl BoxedFunction { /// Creates a new boxed filter. pub fn new(f: F) -> BoxedFunction where - F: Function + for<'a> Function>::Output> + 'static, + F: Function + for<'a> Function>::Output>, Rv: FunctionResult, Args: for<'a> FunctionArgs<'a>, { diff --git a/minijinja/src/tests.rs b/minijinja/src/tests.rs index bf34dc33..c8c7a603 100644 --- a/minijinja/src/tests.rs +++ b/minijinja/src/tests.rs @@ -184,8 +184,7 @@ impl BoxedTest { pub fn new(f: F) -> BoxedTest where F: Test - + for<'a> Test<>::Output, Rv, >::Output> - + 'static, + + for<'a> Test<>::Output, Rv, >::Output>, V: for<'a> ArgType<'a>, Rv: TestResult, Args: for<'a> FunctionArgs<'a>, From 96a685522ced89b68450309ca5a1a112e7cc09ab Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sat, 10 Sep 2022 13:31:54 +0200 Subject: [PATCH 04/10] Change the order of generics for functions --- minijinja/src/environment.rs | 19 ++++++++++++------- minijinja/src/filters.rs | 12 ++++++------ minijinja/src/tests.rs | 14 +++++++------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/minijinja/src/environment.rs b/minijinja/src/environment.rs index 787477cc..c7b18c93 100644 --- a/minijinja/src/environment.rs +++ b/minijinja/src/environment.rs @@ -361,12 +361,13 @@ impl<'source> Environment<'source> { /// Filter functions are functions that can be applied to values in /// templates. For details about filters have a look at /// [`Filter`](crate::filters::Filter). - pub fn add_filter(&mut self, name: &'source str, f: F) + pub fn add_filter(&mut self, name: &'source str, f: F) where - F: filters::Filter + // the crazy bounds here exist to enable borrowing in closures + F: filters::Filter + for<'a> filters::Filter< - >::Output, Rv, + >::Output, >::Output, >, V: for<'a> ArgType<'a>, @@ -386,12 +387,13 @@ impl<'source> Environment<'source> { /// Test functions are similar to filters but perform a check on a value /// where the return value is always true or false. For details about tests /// have a look at [`Test`](crate::tests::Test). - pub fn add_test(&mut self, name: &'source str, f: F) + pub fn add_test(&mut self, name: &'source str, f: F) where - F: tests::Test - + for<'a> tests::Test<>::Output, Rv, >::Output>, - V: for<'a> ArgType<'a>, + // the crazy bounds here exist to enable borrowing in closures + F: tests::Test + + for<'a> tests::Test>::Output, >::Output>, Rv: tests::TestResult, + V: for<'a> ArgType<'a>, Args: for<'a> FunctionArgs<'a>, { self.tests.insert(name, tests::BoxedTest::new(f)); @@ -406,8 +408,11 @@ impl<'source> Environment<'source> { /// /// For details about functions have a look at [`functions`]. Note that /// functions and other global variables share the same namespace. + /// For more details about functions have a look at + /// [`Function`](crate::functions::Function). pub fn add_function(&mut self, name: &'source str, f: F) where + // the crazy bounds here exist to enable borrowing in closures F: functions::Function + for<'a> functions::Function>::Output>, Rv: FunctionResult, diff --git a/minijinja/src/filters.rs b/minijinja/src/filters.rs index f66e4951..c21e78d2 100644 --- a/minijinja/src/filters.rs +++ b/minijinja/src/filters.rs @@ -147,7 +147,7 @@ pub(crate) struct BoxedFilter(Arc); /// ```jinja /// {{ "|".join(1, 2, 3) }} -> 1|2|3 /// ``` -pub trait Filter: Send + Sync + 'static { +pub trait Filter: Send + Sync + 'static { /// Applies a filter to value with the given arguments. #[doc(hidden)] fn apply_to(&self, state: &State, value: V, args: Args, _: SealedMarker) -> Rv; @@ -155,7 +155,7 @@ pub trait Filter: Send + Sync + 'static { macro_rules! tuple_impls { ( $( $name:ident )* ) => { - impl Filter for Func + impl Filter for Func where Func: Fn(&State, V, $($name),*) -> Rv + Send + Sync + 'static, V: for<'a> ArgType<'a>, @@ -178,12 +178,12 @@ tuple_impls! { A B C D } impl BoxedFilter { /// Creates a new boxed filter. - pub fn new(f: F) -> BoxedFilter + pub fn new(f: F) -> BoxedFilter where - F: Filter - + for<'a> Filter<>::Output, Rv, >::Output>, - V: for<'a> ArgType<'a>, + F: Filter + + for<'a> Filter>::Output, >::Output>, Rv: FunctionResult, + V: for<'a> ArgType<'a>, Args: for<'a> FunctionArgs<'a>, { BoxedFilter(Arc::new( diff --git a/minijinja/src/tests.rs b/minijinja/src/tests.rs index c8c7a603..2d234ab1 100644 --- a/minijinja/src/tests.rs +++ b/minijinja/src/tests.rs @@ -149,7 +149,7 @@ impl TestResult for bool { /// ```jinja /// {{ "foo" is containing("o") }} -> true /// ``` -pub trait Test: Send + Sync + 'static { +pub trait Test: Send + Sync + 'static { /// Performs a test to value with the given arguments. #[doc(hidden)] fn perform(&self, state: &State, value: V, args: Args, _: SealedMarker) -> Rv; @@ -157,11 +157,11 @@ pub trait Test: Send + Sync + 'static { macro_rules! tuple_impls { ( $( $name:ident )* ) => { - impl Test for Func + impl Test for Func where Func: Fn(&State, V, $($name),*) -> Rv + Send + Sync + 'static, - V: for<'a> ArgType<'a>, Rv: TestResult, + V: for<'a> ArgType<'a>, $($name: for<'a> ArgType<'a>),* { fn perform(&self, state: &State, value: V, args: ($($name,)*), _: SealedMarker) -> Rv { @@ -181,12 +181,12 @@ tuple_impls! { A B C D } impl BoxedTest { /// Creates a new boxed filter. - pub fn new(f: F) -> BoxedTest + pub fn new(f: F) -> BoxedTest where - F: Test - + for<'a> Test<>::Output, Rv, >::Output>, - V: for<'a> ArgType<'a>, + F: Test + + for<'a> Test>::Output, >::Output>, Rv: TestResult, + V: for<'a> ArgType<'a>, Args: for<'a> FunctionArgs<'a>, { BoxedTest(Arc::new(move |state, value, args| -> Result { From 5276930e5f74180950927d96f4632eed233f081d Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sat, 10 Sep 2022 13:47:24 +0200 Subject: [PATCH 05/10] Added example for parse_args --- examples/dynamic/src/main.rs | 8 +++++--- minijinja/src/value/argtypes.rs | 25 +++++++++++++++++++++++++ minijinja/src/value/mod.rs | 2 +- minijinja/src/value/object.rs | 6 ++++++ 4 files changed, 37 insertions(+), 4 deletions(-) diff --git a/examples/dynamic/src/main.rs b/examples/dynamic/src/main.rs index e86e6184..36c34075 100644 --- a/examples/dynamic/src/main.rs +++ b/examples/dynamic/src/main.rs @@ -2,7 +2,7 @@ use std::fmt; use std::sync::atomic::{AtomicUsize, Ordering}; -use minijinja::value::{FunctionArgs, Object, Value}; +use minijinja::value::{parse_args, Object, Value}; use minijinja::{Environment, Error, State}; #[derive(Debug)] @@ -19,7 +19,8 @@ impl fmt::Display for Cycler { impl Object for Cycler { fn call(&self, _state: &State, args: &[Value]) -> Result { - let _: () = FunctionArgs::from_values(args)?; + // we don't want any args + parse_args::<()>(args)?; let idx = self.idx.fetch_add(1, Ordering::Relaxed); Ok(self.values[idx % self.values.len()].clone()) } @@ -44,7 +45,8 @@ impl fmt::Display for Magic { impl Object for Magic { fn call_method(&self, _state: &State, name: &str, args: &[Value]) -> Result { if name == "make_class" { - let (tag,): (String,) = FunctionArgs::from_values(args)?; + // single string argument + let (tag,): (&str,) = parse_args(args)?; Ok(Value::from(format!("magic-{}", tag))) } else { Err(Error::new( diff --git a/minijinja/src/value/argtypes.rs b/minijinja/src/value/argtypes.rs index ad96b34b..e9dcc3be 100644 --- a/minijinja/src/value/argtypes.rs +++ b/minijinja/src/value/argtypes.rs @@ -42,12 +42,36 @@ impl> FunctionResult for I { /// For each argument the conversion is performed via the [`ArgType`] /// trait which is implemented for many common types. pub trait FunctionArgs<'a> { + #[doc(hidden)] type Output; /// Converts to function arguments from a slice of values. + #[doc(hidden)] fn from_values(values: &'a [Value]) -> Result; } +/// Utility function to convert a slice of values into arguments. +/// +/// This performs the same conversion that [`Function`](crate::functions::Function) +/// performs. It exists so that you one can leverage the same functionality when +/// implementing [`Object::call_method`](crate::value::Object::call_method). +/// +/// ``` +/// use minijinja::value::parse_args; +/// # use minijinja::value::Value; +/// # fn foo() -> Result<(), minijinja::Error> { +/// # let args = vec![Value::from("foo"), Value::from(42i64)]; let args = &args[..]; +/// let (string, num): (&str, i64) = parse_args(args)?; +/// # Ok(()) } fn main() { foo().unwrap(); } +/// ``` +#[inline(always)] +pub fn parse_args<'a, Args>(values: &'a [Value]) -> Result +where + Args: FunctionArgs<'a, Output = Args>, +{ + Args::from_values(values) +} + /// A trait implemented by all filter/test argument types. /// /// This trait is used by [`FunctionArgs`]. It's implemented for many common @@ -73,6 +97,7 @@ pub trait FunctionArgs<'a> { /// it's implemented for [`Rest`] which is used to encode the remaining arguments /// of a function call. pub trait ArgType<'a> { + #[doc(hidden)] type Output; #[doc(hidden)] diff --git a/minijinja/src/value/mod.rs b/minijinja/src/value/mod.rs index cb24cb3c..4059019c 100644 --- a/minijinja/src/value/mod.rs +++ b/minijinja/src/value/mod.rs @@ -83,7 +83,7 @@ use crate::utils::OnDrop; use crate::value::serialize::ValueSerializer; use crate::vm::State; -pub use crate::value::argtypes::{ArgType, FunctionArgs, FunctionResult, Rest}; +pub use crate::value::argtypes::{parse_args, ArgType, FunctionArgs, FunctionResult, Rest}; pub use crate::value::object::Object; mod argtypes; diff --git a/minijinja/src/value/object.rs b/minijinja/src/value/object.rs index 04e3c033..21e383a3 100644 --- a/minijinja/src/value/object.rs +++ b/minijinja/src/value/object.rs @@ -48,6 +48,9 @@ pub trait Object: fmt::Display + fmt::Debug + Any + Sync + Send { /// /// It's the responsibility of the implementer to ensure that an /// error is generated if an invalid method is invoked. + /// + /// To convert the arguments into arguments use the + /// [`parse_args`](crate::value::parse_args) function. fn call_method(&self, state: &State, name: &str, args: &[Value]) -> Result { let _state = state; let _args = args; @@ -61,6 +64,9 @@ pub trait Object: fmt::Display + fmt::Debug + Any + Sync + Send { /// /// The default implementation just generates an error that the object /// cannot be invoked. + /// + /// To convert the arguments into arguments use the + /// [`parse_args`](crate::value::parse_args) function. fn call(&self, state: &State, args: &[Value]) -> Result { let _state = state; let _args = args; From 707f497e74e740cf539c5e15dace13743b82d12b Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sat, 10 Sep 2022 16:23:09 +0200 Subject: [PATCH 06/10] Small docs improvements --- examples/dynamic/src/main.rs | 2 +- minijinja/src/value/argtypes.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/dynamic/src/main.rs b/examples/dynamic/src/main.rs index 36c34075..5862fc05 100644 --- a/examples/dynamic/src/main.rs +++ b/examples/dynamic/src/main.rs @@ -20,7 +20,7 @@ impl fmt::Display for Cycler { impl Object for Cycler { fn call(&self, _state: &State, args: &[Value]) -> Result { // we don't want any args - parse_args::<()>(args)?; + parse_args(args)?; let idx = self.idx.fetch_add(1, Ordering::Relaxed); Ok(self.values[idx % self.values.len()].clone()) } diff --git a/minijinja/src/value/argtypes.rs b/minijinja/src/value/argtypes.rs index e9dcc3be..5db76207 100644 --- a/minijinja/src/value/argtypes.rs +++ b/minijinja/src/value/argtypes.rs @@ -42,7 +42,6 @@ impl> FunctionResult for I { /// For each argument the conversion is performed via the [`ArgType`] /// trait which is implemented for many common types. pub trait FunctionArgs<'a> { - #[doc(hidden)] type Output; /// Converts to function arguments from a slice of values. @@ -61,6 +60,8 @@ pub trait FunctionArgs<'a> { /// # use minijinja::value::Value; /// # fn foo() -> Result<(), minijinja::Error> { /// # let args = vec![Value::from("foo"), Value::from(42i64)]; let args = &args[..]; +/// +/// // args is &[Value] /// let (string, num): (&str, i64) = parse_args(args)?; /// # Ok(()) } fn main() { foo().unwrap(); } /// ``` @@ -97,7 +98,6 @@ where /// it's implemented for [`Rest`] which is used to encode the remaining arguments /// of a function call. pub trait ArgType<'a> { - #[doc(hidden)] type Output; #[doc(hidden)] From 8238fb07676bbfa2dedfef0c1db2faed1ad146dd Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sat, 10 Sep 2022 16:35:10 +0200 Subject: [PATCH 07/10] Rename parse_args utility --- examples/dynamic/src/main.rs | 6 +++--- minijinja/src/value/argtypes.rs | 20 +++++++++++++------- minijinja/src/value/mod.rs | 2 +- minijinja/src/value/object.rs | 4 ++-- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/examples/dynamic/src/main.rs b/examples/dynamic/src/main.rs index 5862fc05..07fe228f 100644 --- a/examples/dynamic/src/main.rs +++ b/examples/dynamic/src/main.rs @@ -2,7 +2,7 @@ use std::fmt; use std::sync::atomic::{AtomicUsize, Ordering}; -use minijinja::value::{parse_args, Object, Value}; +use minijinja::value::{from_args, Object, Value}; use minijinja::{Environment, Error, State}; #[derive(Debug)] @@ -20,7 +20,7 @@ impl fmt::Display for Cycler { impl Object for Cycler { fn call(&self, _state: &State, args: &[Value]) -> Result { // we don't want any args - parse_args(args)?; + from_args(args)?; let idx = self.idx.fetch_add(1, Ordering::Relaxed); Ok(self.values[idx % self.values.len()].clone()) } @@ -46,7 +46,7 @@ impl Object for Magic { fn call_method(&self, _state: &State, name: &str, args: &[Value]) -> Result { if name == "make_class" { // single string argument - let (tag,): (&str,) = parse_args(args)?; + let (tag,): (&str,) = from_args(args)?; Ok(Value::from(format!("magic-{}", tag))) } else { Err(Error::new( diff --git a/minijinja/src/value/argtypes.rs b/minijinja/src/value/argtypes.rs index 5db76207..6287c128 100644 --- a/minijinja/src/value/argtypes.rs +++ b/minijinja/src/value/argtypes.rs @@ -40,7 +40,8 @@ impl> FunctionResult for I { /// arity of 4 parameters. /// /// For each argument the conversion is performed via the [`ArgType`] -/// trait which is implemented for many common types. +/// trait which is implemented for many common types. For manual +/// conversions the [`from_args`] utility should be used. pub trait FunctionArgs<'a> { type Output; @@ -56,17 +57,17 @@ pub trait FunctionArgs<'a> { /// implementing [`Object::call_method`](crate::value::Object::call_method). /// /// ``` -/// use minijinja::value::parse_args; +/// use minijinja::value::from_args; /// # use minijinja::value::Value; /// # fn foo() -> Result<(), minijinja::Error> { /// # let args = vec![Value::from("foo"), Value::from(42i64)]; let args = &args[..]; /// /// // args is &[Value] -/// let (string, num): (&str, i64) = parse_args(args)?; +/// let (string, num): (&str, i64) = from_args(args)?; /// # Ok(()) } fn main() { foo().unwrap(); } /// ``` #[inline(always)] -pub fn parse_args<'a, Args>(values: &'a [Value]) -> Result +pub fn from_args<'a, Args>(values: &'a [Value]) -> Result where Args: FunctionArgs<'a, Output = Args>, { @@ -81,9 +82,9 @@ where /// /// * unsigned integers: [`u8`], [`u16`], [`u32`], [`u64`], [`u128`], [`usize`] /// * signed integers: [`i8`], [`i16`], [`i32`], [`i64`], [`i128`] -/// * floats: [`f64`] +/// * floats: [`f32`], [`f64`] /// * bool: [`bool`] -/// * string: [`String`], [`&str`], `Cow<'_, str>` +/// * string: [`String`], [`&str`], `Cow<'_, str>` ([`char`]) /// * values: [`Value`], `&Value` /// * vectors: [`Vec`], `&[Value]` /// @@ -333,7 +334,12 @@ primitive_int_try_from!(usize); primitive_try_from!(bool, { ValueRepr::Bool(val) => val, }); - +primitive_try_from!(char, { + ValueRepr::Char(val) => val, +}); +primitive_try_from!(f32, { + ValueRepr::F64(val) => val as f32, +}); primitive_try_from!(f64, { ValueRepr::F64(val) => val, }); diff --git a/minijinja/src/value/mod.rs b/minijinja/src/value/mod.rs index 4059019c..e6020f21 100644 --- a/minijinja/src/value/mod.rs +++ b/minijinja/src/value/mod.rs @@ -83,7 +83,7 @@ use crate::utils::OnDrop; use crate::value::serialize::ValueSerializer; use crate::vm::State; -pub use crate::value::argtypes::{parse_args, ArgType, FunctionArgs, FunctionResult, Rest}; +pub use crate::value::argtypes::{from_args, ArgType, FunctionArgs, FunctionResult, Rest}; pub use crate::value::object::Object; mod argtypes; diff --git a/minijinja/src/value/object.rs b/minijinja/src/value/object.rs index 21e383a3..d4ffa810 100644 --- a/minijinja/src/value/object.rs +++ b/minijinja/src/value/object.rs @@ -50,7 +50,7 @@ pub trait Object: fmt::Display + fmt::Debug + Any + Sync + Send { /// error is generated if an invalid method is invoked. /// /// To convert the arguments into arguments use the - /// [`parse_args`](crate::value::parse_args) function. + /// [`from_args`](crate::value::from_args) function. fn call_method(&self, state: &State, name: &str, args: &[Value]) -> Result { let _state = state; let _args = args; @@ -66,7 +66,7 @@ pub trait Object: fmt::Display + fmt::Debug + Any + Sync + Send { /// cannot be invoked. /// /// To convert the arguments into arguments use the - /// [`parse_args`](crate::value::parse_args) function. + /// [`from_args`](crate::value::from_args) function. fn call(&self, state: &State, args: &[Value]) -> Result { let _state = state; let _args = args; From a52f6fbfd258d08a5f89cd8ad28479de696feb55 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 14 Sep 2022 10:50:18 +0200 Subject: [PATCH 08/10] Remove special casing of first value to filters --- minijinja/src/compiler/codegen.rs | 2 +- minijinja/src/environment.rs | 11 ++--- minijinja/src/filters.rs | 76 ++++++++++++------------------- minijinja/src/value/argtypes.rs | 10 ++-- minijinja/src/vm/mod.rs | 3 +- minijinja/src/vm/state.rs | 9 +--- 6 files changed, 42 insertions(+), 69 deletions(-) diff --git a/minijinja/src/compiler/codegen.rs b/minijinja/src/compiler/codegen.rs index cee572aa..0c8b3c6c 100644 --- a/minijinja/src/compiler/codegen.rs +++ b/minijinja/src/compiler/codegen.rs @@ -412,7 +412,7 @@ impl<'source> CodeGenerator<'source> { for arg in &f.args { self.compile_expr(arg)?; } - self.add(Instruction::BuildList(f.args.len())); + self.add(Instruction::BuildList(f.args.len() + 1)); self.add(Instruction::ApplyFilter(f.name)); } ast::Expr::Test(f) => { diff --git a/minijinja/src/environment.rs b/minijinja/src/environment.rs index c7b18c93..f899b9a1 100644 --- a/minijinja/src/environment.rs +++ b/minijinja/src/environment.rs @@ -361,16 +361,11 @@ impl<'source> Environment<'source> { /// Filter functions are functions that can be applied to values in /// templates. For details about filters have a look at /// [`Filter`](crate::filters::Filter). - pub fn add_filter(&mut self, name: &'source str, f: F) + pub fn add_filter(&mut self, name: &'source str, f: F) where // the crazy bounds here exist to enable borrowing in closures - F: filters::Filter - + for<'a> filters::Filter< - Rv, - >::Output, - >::Output, - >, - V: for<'a> ArgType<'a>, + F: filters::Filter + + for<'a> filters::Filter>::Output>, Rv: FunctionResult, Args: for<'a> FunctionArgs<'a>, { diff --git a/minijinja/src/filters.rs b/minijinja/src/filters.rs index c21e78d2..5713f673 100644 --- a/minijinja/src/filters.rs +++ b/minijinja/src/filters.rs @@ -61,11 +61,11 @@ use crate::defaults::escape_formatter; use crate::error::Error; use crate::output::Output; use crate::utils::SealedMarker; -use crate::value::{ArgType, FunctionArgs, FunctionResult, Value}; +use crate::value::{FunctionArgs, FunctionResult, Value}; use crate::vm::State; use crate::AutoEscape; -type FilterFunc = dyn Fn(&State, &Value, &[Value]) -> Result + Sync + Send + 'static; +type FilterFunc = dyn Fn(&State, &[Value]) -> Result + Sync + Send + 'static; #[derive(Clone)] pub(crate) struct BoxedFilter(Arc); @@ -147,24 +147,25 @@ pub(crate) struct BoxedFilter(Arc); /// ```jinja /// {{ "|".join(1, 2, 3) }} -> 1|2|3 /// ``` -pub trait Filter: Send + Sync + 'static { +pub trait Filter: Send + Sync + 'static { /// Applies a filter to value with the given arguments. + /// + /// The value is always the first argument. #[doc(hidden)] - fn apply_to(&self, state: &State, value: V, args: Args, _: SealedMarker) -> Rv; + fn apply_to(&self, state: &State, args: Args, _: SealedMarker) -> Rv; } macro_rules! tuple_impls { ( $( $name:ident )* ) => { - impl Filter for Func + impl Filter for Func where - Func: Fn(&State, V, $($name),*) -> Rv + Send + Sync + 'static, - V: for<'a> ArgType<'a>, + Func: Fn(&State, $($name),*) -> Rv + Send + Sync + 'static, Rv: FunctionResult, { - fn apply_to(&self, state: &State, value: V, args: ($($name,)*), _: SealedMarker) -> Rv { + fn apply_to(&self, state: &State, args: ($($name,)*), _: SealedMarker) -> Rv { #[allow(non_snake_case)] let ($($name,)*) = args; - (self)(state, value, $($name,)*) + (self)(state, $($name,)*) } } }; @@ -178,30 +179,21 @@ tuple_impls! { A B C D } impl BoxedFilter { /// Creates a new boxed filter. - pub fn new(f: F) -> BoxedFilter + pub fn new(f: F) -> BoxedFilter where - F: Filter - + for<'a> Filter>::Output, >::Output>, + F: Filter + for<'a> Filter>::Output>, Rv: FunctionResult, - V: for<'a> ArgType<'a>, Args: for<'a> FunctionArgs<'a>, { - BoxedFilter(Arc::new( - move |state, value, args| -> Result { - f.apply_to( - state, - V::from_value(Some(value))?, - Args::from_values(args)?, - SealedMarker, - ) + BoxedFilter(Arc::new(move |state, args| -> Result { + f.apply_to(state, Args::from_values(args)?, SealedMarker) .into_result() - }, - )) + })) } /// Applies the filter to a value and argument. - pub fn apply_to(&self, state: &State, value: &Value, args: &[Value]) -> Result { - (self.0)(state, value, args) + pub fn apply_to(&self, state: &State, args: &[Value]) -> Result { + (self.0)(state, args) } } @@ -809,7 +801,7 @@ mod builtins { State::with_dummy(&env, |state| { let bx = BoxedFilter::new(test); assert_eq!( - bx.apply_to(state, &Value::from(23), &[Value::from(42)][..]) + bx.apply_to(state, &[Value::from(23), Value::from(42)][..]) .unwrap(), Value::from(65) ); @@ -828,8 +820,12 @@ mod builtins { assert_eq!( bx.apply_to( state, - &Value::from(1), - &[Value::from(2), Value::from(3), Value::from(4)][..] + &[ + Value::from(1), + Value::from(2), + Value::from(3), + Value::from(4) + ][..] ) .unwrap(), Value::from(1 + 2 + 3 + 4) @@ -837,23 +833,11 @@ mod builtins { }); } - #[test] - #[should_panic = "cannot collect remaining arguments in this argument position"] - fn test_incorrect_rest_args() { - fn sum(_: &State, _: crate::value::Rest) -> u32 { - panic!("should never happen"); - } - - let env = crate::Environment::new(); - State::with_dummy(&env, |state| { - let bx = BoxedFilter::new(sum); - bx.apply_to(state, &Value::from(1), &[]).unwrap(); - }); - } - #[test] fn test_optional_args() { fn add(_: &State, val: u32, a: u32, b: Option) -> Result { + // ensure we really get our value as first argument + assert_eq!(val, 23); let mut sum = val + a; if let Some(b) = b { sum += b; @@ -865,15 +849,14 @@ mod builtins { State::with_dummy(&env, |state| { let bx = BoxedFilter::new(add); assert_eq!( - bx.apply_to(state, &Value::from(23), &[Value::from(42)][..]) + bx.apply_to(state, &[Value::from(23), Value::from(42)][..]) .unwrap(), Value::from(65) ); assert_eq!( bx.apply_to( state, - &Value::from(23), - &[Value::from(42), Value::UNDEFINED][..] + &[Value::from(23), Value::from(42), Value::UNDEFINED][..] ) .unwrap(), Value::from(65) @@ -881,8 +864,7 @@ mod builtins { assert_eq!( bx.apply_to( state, - &Value::from(23), - &[Value::from(42), Value::from(1)][..] + &[Value::from(23), Value::from(42), Value::from(1)][..] ) .unwrap(), Value::from(66) diff --git a/minijinja/src/value/argtypes.rs b/minijinja/src/value/argtypes.rs index 6287c128..a599672e 100644 --- a/minijinja/src/value/argtypes.rs +++ b/minijinja/src/value/argtypes.rs @@ -446,10 +446,12 @@ impl DerefMut for Rest { impl<'a, T: ArgType<'a, Output = T>> ArgType<'a> for Rest { type Output = Self; - fn from_value(_value: Option<&'a Value>) -> Result { - Err(Error::new( - ErrorKind::ImpossibleOperation, - "cannot collect remaining arguments in this argument position", + fn from_value(value: Option<&'a Value>) -> Result { + Ok(Rest( + value + .iter() + .map(|v| T::from_value(Some(v))) + .collect::>()?, )) } diff --git a/minijinja/src/vm/mod.rs b/minijinja/src/vm/mod.rs index 7762a8f1..c46ec01f 100644 --- a/minijinja/src/vm/mod.rs +++ b/minijinja/src/vm/mod.rs @@ -314,8 +314,7 @@ impl<'env> Vm<'env> { Instruction::ApplyFilter(name) => { let top = stack.pop(); let args = try_ctx!(top.as_slice()); - let value = stack.pop(); - stack.push(try_ctx!(state.apply_filter(name, &value, args))); + stack.push(try_ctx!(state.apply_filter(name, args))); } Instruction::PerformTest(name) => { let top = stack.pop(); diff --git a/minijinja/src/vm/state.rs b/minijinja/src/vm/state.rs index fdbbc91f..a2097d91 100644 --- a/minijinja/src/vm/state.rs +++ b/minijinja/src/vm/state.rs @@ -80,14 +80,9 @@ impl<'vm, 'env, 'out, 'buf> State<'vm, 'env, 'out, 'buf> { }) } - pub(crate) fn apply_filter( - &self, - name: &str, - value: &Value, - args: &[Value], - ) -> Result { + pub(crate) fn apply_filter(&self, name: &str, args: &[Value]) -> Result { if let Some(filter) = self.env.get_filter(name) { - filter.apply_to(self, value, args) + filter.apply_to(self, args) } else { Err(Error::new( ErrorKind::UnknownFilter, From 7c8cc6fc4767e74d4e7354bcc12a4ea940e9e8ec Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 14 Sep 2022 10:52:16 +0200 Subject: [PATCH 09/10] Bump to 1.61.0 --- .github/workflows/tests.yml | 38 +++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 52282688..dfb1385b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -17,32 +17,34 @@ jobs: - name: Test run: make test - build-old-stable: - name: Build on 1.45.0 - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1 - - uses: actions-rs/toolchain@v1 - with: - toolchain: 1.45.0 - profile: minimal - override: true - - name: Build - run: cargo build --features=unstable_machinery,builtins,source,json,urlencode,debug,internal_debug - working-directory: ./minijinja - env: - CARGO_NET_GIT_FETCH_WITH_CLI: "true" - CARGO_HTTP_MULTIPLEXING: "false" + ## Bugs in older Rust versions for some of our borrowing logic for filters + ## No longer makes supporting this Rust version possible + # build-old-stable: + # name: Build on 1.45.0 + # runs-on: ubuntu-latest + # steps: + # - uses: actions/checkout@v1 + # - uses: actions-rs/toolchain@v1 + # with: + # toolchain: 1.45.0 + # profile: minimal + # override: true + # - name: Build + # run: cargo build --features=unstable_machinery,builtins,source,json,urlencode,debug,internal_debug + # working-directory: ./minijinja + # env: + # CARGO_NET_GIT_FETCH_WITH_CLI: "true" + # CARGO_HTTP_MULTIPLEXING: "false" test-stable: - name: Test on 1.56.1 + name: Test on 1.61.0 runs-on: ubuntu-latest steps: - uses: actions/checkout@v1 - uses: actions-rs/toolchain@v1 with: - toolchain: 1.56.1 + toolchain: 1.61.0 profile: minimal override: true - name: Test From 9a86d00a1db95408724b19881af7f92b355c2746 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 14 Sep 2022 11:11:58 +0200 Subject: [PATCH 10/10] Mention new minimum version in readme --- README.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 699e35bb..f2fbf8d0 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ [![Build Status](https://github.com/mitsuhiko/minijinja/workflows/Tests/badge.svg?branch=main)](https://github.com/mitsuhiko/minijinja/actions?query=workflow%3ATests) [![License](https://img.shields.io/github/license/mitsuhiko/minijinja)](https://github.com/mitsuhiko/minijinja/blob/main/LICENSE) [![Crates.io](https://img.shields.io/crates/d/minijinja.svg)](https://crates.io/crates/minijinja) -[![rustc 1.45.0](https://img.shields.io/badge/rust-1.45%2B-orange.svg)](https://img.shields.io/badge/rust-1.45%2B-orange.svg) +[![rustc 1.61.0](https://img.shields.io/badge/rust-1.61%2B-orange.svg)](https://img.shields.io/badge/rust-1.61%2B-orange.svg) [![Documentation](https://docs.rs/minijinja/badge.svg)](https://docs.rs/minijinja) @@ -63,9 +63,12 @@ fn main() { ## Minimum Rust Version -MiniJinja supports Rust versions down to 1.45 at the moment. For the order -preservation feature Rust 1.49 is required as it uses the indexmap dependency -which no longer supports older Rust versions. +MiniJinja's development version requires Rust 1.61 due to limitations with +HRTBs in older Rust versions. + +MiniJinja 0.20 supports Rust versions down to 1.45. It is possible to write +code that is compatible with both 0.20 and newer versions of MiniJinja which +should make it possible to defer the upgrade to later. ## Sponsor