From c11507c2a7488e7b1f90a154eb89254c1a0094cb Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Wed, 9 Nov 2022 15:38:59 -0800 Subject: [PATCH 1/2] Remove 'Arc' around `u128` and `i128` in ValueRepr The largest variant is already at least 16 bytes, so the `Arc` only adds an allocation without gain --- minijinja/src/key/mod.rs | 4 ++-- minijinja/src/value/argtypes.rs | 8 ++++---- minijinja/src/value/mod.rs | 16 ++++++++-------- minijinja/src/value/ops.rs | 8 ++++---- minijinja/src/value/serialize.rs | 4 ++-- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/minijinja/src/key/mod.rs b/minijinja/src/key/mod.rs index c0fc3363..9f256194 100644 --- a/minijinja/src/key/mod.rs +++ b/minijinja/src/key/mod.rs @@ -82,11 +82,11 @@ impl<'a> Key<'a> { ValueRepr::U64(v) => TryFrom::try_from(v) .map(Key::I64) .map_err(|_| ErrorKind::NonKey.into()), - ValueRepr::U128(ref v) => TryFrom::try_from(**v) + ValueRepr::U128(v) => TryFrom::try_from(v) .map(Key::I64) .map_err(|_| ErrorKind::NonKey.into()), ValueRepr::I64(v) => Ok(Key::I64(v)), - ValueRepr::I128(ref v) => TryFrom::try_from(**v) + ValueRepr::I128(v) => TryFrom::try_from(v) .map(Key::I64) .map_err(|_| ErrorKind::NonKey.into()), ValueRepr::F64(x) => { diff --git a/minijinja/src/value/argtypes.rs b/minijinja/src/value/argtypes.rs index 9fec2f34..d147d970 100644 --- a/minijinja/src/value/argtypes.rs +++ b/minijinja/src/value/argtypes.rs @@ -234,14 +234,14 @@ impl From<()> for Value { impl From for Value { #[inline(always)] fn from(val: i128) -> Self { - ValueRepr::I128(Arc::new(val)).into() + ValueRepr::I128(val).into() } } impl From for Value { #[inline(always)] fn from(val: u128) -> Self { - ValueRepr::U128(Arc::new(val)).into() + ValueRepr::U128(val).into() } } @@ -339,8 +339,8 @@ macro_rules! primitive_int_try_from { ValueRepr::U64(val) => val, // for the intention here see Key::from_borrowed_value ValueRepr::F64(val) if (val as i64 as f64 == val) => val as i64, - ValueRepr::I128(ref val) => **val, - ValueRepr::U128(ref val) => **val, + ValueRepr::I128(val) => val, + ValueRepr::U128(val) => val, }); } } diff --git a/minijinja/src/value/mod.rs b/minijinja/src/value/mod.rs index 21506b32..20f1fe1a 100644 --- a/minijinja/src/value/mod.rs +++ b/minijinja/src/value/mod.rs @@ -230,8 +230,8 @@ pub(crate) enum ValueRepr { F64(f64), Char(char), None, - U128(Arc), - I128(Arc), + U128(u128), + I128(i128), String(Arc, StringType), Bytes(Arc>), Seq(Arc>), @@ -482,9 +482,9 @@ impl Value { match self.0 { ValueRepr::Bool(val) => val, ValueRepr::U64(x) => x != 0, - ValueRepr::U128(ref x) => **x != 0, + ValueRepr::U128(x) => x != 0, ValueRepr::I64(x) => x != 0, - ValueRepr::I128(ref x) => **x != 0, + ValueRepr::I128(x) => x != 0, ValueRepr::F64(x) => x != 0.0, ValueRepr::Char(x) => x != '\x00', ValueRepr::String(ref x, _) => !x.is_empty(), @@ -779,11 +779,11 @@ impl Value { ValueRepr::U64(v) => TryFrom::try_from(v) .map(Key::I64) .map_err(|_| ErrorKind::NonKey.into()), - ValueRepr::U128(ref v) => TryFrom::try_from(**v) + ValueRepr::U128(v) => TryFrom::try_from(v) .map(Key::I64) .map_err(|_| ErrorKind::NonKey.into()), ValueRepr::I64(v) => Ok(Key::I64(v)), - ValueRepr::I128(ref v) => TryFrom::try_from(**v) + ValueRepr::I128(v) => TryFrom::try_from(v) .map(Key::I64) .map_err(|_| ErrorKind::NonKey.into()), ValueRepr::Char(c) => Ok(Key::Char(c)), @@ -857,8 +857,8 @@ impl Serialize for Value { ValueRepr::Char(c) => serializer.serialize_char(c), ValueRepr::None => serializer.serialize_unit(), ValueRepr::Undefined => serializer.serialize_unit(), - ValueRepr::U128(ref u) => serializer.serialize_u128(**u), - ValueRepr::I128(ref i) => serializer.serialize_i128(**i), + ValueRepr::U128(u) => serializer.serialize_u128(u), + ValueRepr::I128(i) => serializer.serialize_i128(i), ValueRepr::String(ref s, _) => serializer.serialize_str(s), ValueRepr::Bytes(ref b) => serializer.serialize_bytes(b), ValueRepr::Seq(ref elements) => elements.serialize(serializer), diff --git a/minijinja/src/value/ops.rs b/minijinja/src/value/ops.rs index cae0c385..42d9f150 100644 --- a/minijinja/src/value/ops.rs +++ b/minijinja/src/value/ops.rs @@ -14,9 +14,9 @@ fn as_f64(value: &Value) -> Option { Some(match value.0 { ValueRepr::Bool(x) => x as i64 as f64, ValueRepr::U64(x) => x as f64, - ValueRepr::U128(ref x) => **x as f64, + ValueRepr::U128(x) => x as f64, ValueRepr::I64(x) => x as f64, - ValueRepr::I128(ref x) => **x as f64, + ValueRepr::I128(x) => x as f64, ValueRepr::F64(x) => x, _ => return None, }) @@ -27,13 +27,13 @@ pub fn coerce(a: &Value, b: &Value) -> Option { // equal mappings are trivial (ValueRepr::U64(a), ValueRepr::U64(b)) => Some(CoerceResult::I128(*a as i128, *b as i128)), (ValueRepr::U128(a), ValueRepr::U128(b)) => { - Some(CoerceResult::I128(**a as i128, **b as i128)) + Some(CoerceResult::I128(*a as i128, *b as i128)) } (ValueRepr::String(a, _), ValueRepr::String(b, _)) => { Some(CoerceResult::String(a.to_string(), b.to_string())) } (ValueRepr::I64(a), ValueRepr::I64(b)) => Some(CoerceResult::I128(*a as i128, *b as i128)), - (ValueRepr::I128(ref a), ValueRepr::I128(ref b)) => Some(CoerceResult::I128(**a, **b)), + (ValueRepr::I128(a), ValueRepr::I128(b)) => Some(CoerceResult::I128(*a, *b)), (ValueRepr::F64(a), ValueRepr::F64(b)) => Some(CoerceResult::F64(*a, *b)), // are floats involved? diff --git a/minijinja/src/value/serialize.rs b/minijinja/src/value/serialize.rs index 20976231..8e37a5b3 100644 --- a/minijinja/src/value/serialize.rs +++ b/minijinja/src/value/serialize.rs @@ -43,7 +43,7 @@ impl Serializer for ValueSerializer { } fn serialize_i128(self, v: i128) -> Result { - Ok(ValueRepr::I128(Arc::new(v)).into()) + Ok(ValueRepr::I128(v).into()) } fn serialize_u8(self, v: u8) -> Result { @@ -63,7 +63,7 @@ impl Serializer for ValueSerializer { } fn serialize_u128(self, v: u128) -> Result { - Ok(ValueRepr::U128(Arc::new(v)).into()) + Ok(ValueRepr::U128(v).into()) } fn serialize_f32(self, v: f32) -> Result { From 3b047bd93626b6f11b4e609636eb4d4613264dee Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Wed, 9 Nov 2022 15:54:18 -0800 Subject: [PATCH 2/2] Add 'FromIterator', change 'From' 'Value' impls Removes the now unnecessary `Value::from_arc_object`. Adds `FromIterator` impls for: * 'Value' for sequences (items of `V: Into`) * `Value` for maps (items of `(K: Into, V: Into)`) Adds 'From' impls to `Value` from: * `Arc` * `u128` * `i128` Finally documents the new functionality. --- minijinja/src/compiler/ast.rs | 13 ++++---- minijinja/src/value/argtypes.rs | 54 ++++++++++++++++++++------------- minijinja/src/value/mod.rs | 48 +++++++++++++++++++++-------- minijinja/src/vm/context.rs | 2 +- 4 files changed, 76 insertions(+), 41 deletions(-) diff --git a/minijinja/src/compiler/ast.rs b/minijinja/src/compiler/ast.rs index 165ada4a..a530970f 100644 --- a/minijinja/src/compiler/ast.rs +++ b/minijinja/src/compiler/ast.rs @@ -390,14 +390,13 @@ impl<'a> List<'a> { return None; } - let mut rv = Vec::new(); - for expr in &self.items { - if let Expr::Const(val) = expr { - rv.push(val.value.clone()); - } - } + let items = self.items.iter(); + let sequence = items.filter_map(|expr| match expr { + Expr::Const(v) => Some(v.value.clone()), + _ => None, + }); - Some(Value::from(rv)) + Some(sequence.collect()) } } diff --git a/minijinja/src/value/argtypes.rs b/minijinja/src/value/argtypes.rs index d147d970..a2104279 100644 --- a/minijinja/src/value/argtypes.rs +++ b/minijinja/src/value/argtypes.rs @@ -5,7 +5,7 @@ use std::ops::{Deref, DerefMut}; use crate::error::{Error, ErrorKind}; use crate::key::{Key, StaticKey}; -use crate::value::{Arc, MapType, StringType, Value, ValueKind, ValueRepr}; +use crate::value::{Arc, MapType, Object, StringType, Value, ValueKind, ValueRepr}; use crate::vm::State; /// A utility trait that represents the return value of functions and filters. @@ -231,20 +231,6 @@ impl From<()> for Value { } } -impl From for Value { - #[inline(always)] - fn from(val: i128) -> Self { - ValueRepr::I128(val).into() - } -} - -impl From for Value { - #[inline(always)] - fn from(val: u128) -> Self { - ValueRepr::U128(val).into() - } -} - impl<'a> From> for Value { fn from(val: Key) -> Self { match val { @@ -257,19 +243,40 @@ impl<'a> From> for Value { } } +impl> FromIterator for Value { + fn from_iter>(iter: T) -> Self { + let vec = iter.into_iter().map(|v| v.into()).collect(); + + ValueRepr::Seq(Arc::new(vec)).into() + } +} + +impl, V: Into> FromIterator<(K, V)> for Value { + fn from_iter>(iter: T) -> Self { + let map = iter + .into_iter() + .map(|(k, v)| (k.into(), v.into())) + .collect(); + + ValueRepr::Map(Arc::new(map), MapType::Normal).into() + } +} + impl, V: Into> From> for Value { fn from(val: BTreeMap) -> Self { - ValueRepr::Map( - Arc::new(val.into_iter().map(|(k, v)| (k.into(), v.into())).collect()), - MapType::Normal, - ) - .into() + val.into_iter().map(|(k, v)| (k.into(), v.into())).collect() } } impl> From> for Value { fn from(val: Vec) -> Self { - ValueRepr::Seq(Arc::new(val.into_iter().map(|x| x.into()).collect())).into() + val.into_iter().map(|v| v.into()).collect() + } +} + +impl From> for Value { + fn from(object: Arc) -> Self { + Value::from(object as Arc) } } @@ -289,13 +296,18 @@ value_from!(u8, U64); value_from!(u16, U64); value_from!(u32, U64); value_from!(u64, U64); +value_from!(u128, U128); value_from!(i8, I64); value_from!(i16, I64); value_from!(i32, I64); value_from!(i64, I64); +value_from!(i128, I128); value_from!(f32, F64); value_from!(f64, F64); value_from!(char, Char); +value_from!(Arc>, Bytes); +value_from!(Arc>, Seq); +value_from!(Arc, Dynamic); fn unsupported_conversion(kind: ValueKind, target: &str) -> Error { Error::new( diff --git a/minijinja/src/value/mod.rs b/minijinja/src/value/mod.rs index 20f1fe1a..82b07966 100644 --- a/minijinja/src/value/mod.rs +++ b/minijinja/src/value/mod.rs @@ -18,6 +18,17 @@ //! let value = Value::from(42); //! ``` //! +//! Or via the [`FromIterator`] trait: +//! +//! ``` +//! # use minijinja::value::Value; +//! // collection into a sequence +//! let value: Value = (1..10).into_iter().collect(); +//! +//! // collection into a map +//! let value: Value = [("key", "value")].into_iter().collect(); +//! ``` +//! //! MiniJinja will however create values via an indirection via [`serde`] when //! a template is rendered or an expression is evaluated. This can also be //! triggered manually by using the [`Value::from_serializable`] method: @@ -62,6 +73,28 @@ //! [`Object`] trait. These can be used to implement dynamic functionality such as //! stateful values and more. Dynamic objects are internally also used to implement //! the special `loop` variable or macros. +//! +//! To create a dynamic `Value` object, use [`Value::from_object()`] or the +//! `From>` implementations for `Value`: +//! +//! ```rust +//! # use std::sync::Arc; +//! # use minijinja::value::{Value, Object}; +//! #[derive(Debug)] +//! struct Foo; +//! +//! # impl std::fmt::Display for Foo { +//! # fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { Ok(()) } +//! # } +//! # +//! impl Object for Foo { +//! /* implementation */ +//! } +//! +//! let value = Value::from_object(Foo); +//! let value = Value::from(Arc::new(Foo)); +//! let value = Value::from(Arc::new(Foo) as Arc); +//! ``` // this module is based on the content module in insta which in turn is based // on the content module in serde::private::ser. @@ -422,17 +455,8 @@ impl Value { /// /// let val = Value::from_object(Thing { id: 42 }); /// ``` - pub fn from_object(value: T) -> Value { - Value::from_arc_object(Arc::new(value)) - } - - /// Creates a value from a reference counted dynamic object. - /// - /// As values are internally holding dynamic objects in an `Arc` it can be - /// convenient to hold on to it so that the ownership can be shared between - /// the boxed value and outside. - pub fn from_arc_object(value: Arc) -> Value { - ValueRepr::Dynamic(value as Arc).into() + pub fn from_object(value: T) -> Value { + Value::from(Arc::new(value) as Arc) } /// Creates a callable value from a function. @@ -991,7 +1015,7 @@ fn test_dynamic_object_roundtrip() { } let x = Arc::new(X(Default::default())); - let x_value = Value::from_arc_object(x.clone()); + let x_value = Value::from(x.clone()); x.0.fetch_add(42, atomic::Ordering::Relaxed); let x_clone = Value::from_serializable(&x_value); x.0.fetch_add(23, atomic::Ordering::Relaxed); diff --git a/minijinja/src/vm/context.rs b/minijinja/src/vm/context.rs index b866e959..ab1f0ca7 100644 --- a/minijinja/src/vm/context.rs +++ b/minijinja/src/vm/context.rs @@ -179,7 +179,7 @@ impl<'env> Context<'env> { // if we are a loop, check if we are looking up the special loop var. if let Some(ref l) = frame.current_loop { if l.with_loop_var && key == "loop" { - return Some(Value::from_arc_object(l.object.clone())); + return Some(Value::from(l.object.clone())); } }