wip: objects 2.0

.gitignore

@@ -1,3 +1,5 @@
 target
 .vscode/*.log
 profile.json
+**/snapshots/*.new
+**/*.pending-snap

minijinja/src/compiler/ast.rs

@@ -4,7 +4,7 @@ use std::ops::Deref;
 use std::fmt;
 
 use crate::compiler::tokens::Span;
-use crate::value::{value_map_with_capacity, KeyRef, MapType, Value, ValueRepr};
+use crate::value::{value_map_with_capacity, Value};
 
 /// Container for nodes with location info.
 ///
@@ -479,7 +479,7 @@ impl<'a> List<'a> {
             _ => None,
         });
 
-        Some(sequence.collect())
+        Some(Value::from(sequence.collect::<Vec<_>>()))
     }
 }
 
@@ -499,11 +499,11 @@ impl<'a> Kwargs<'a> {
         let mut rv = value_map_with_capacity(self.pairs.len());
         for (key, value) in &self.pairs {
             if let Expr::Const(value) = value {
-                rv.insert(KeyRef::Value(Value::from(*key)), value.value.clone());
+                rv.insert(Value::from(*key), value.value.clone());
             }
         }
 
-        Some(Value(ValueRepr::Map(rv.into(), MapType::Kwargs)))
+        Some(Value::kwargs(rv))
     }
 }
 
@@ -526,11 +526,11 @@ impl<'a> Map<'a> {
         let mut rv = value_map_with_capacity(self.keys.len());
         for (key, value) in self.keys.iter().zip(self.values.iter()) {
             if let (Expr::Const(maybe_key), Expr::Const(value)) = (key, value) {
-                rv.insert(KeyRef::Value(maybe_key.value.clone()), value.value.clone());
+                rv.insert(maybe_key.value.clone(), value.value.clone());
             }
         }
 
-        Some(Value(ValueRepr::Map(rv.into(), MapType::Normal)))
+        Some(Value::from_object(rv))
     }
 }
 

minijinja/src/compiler/codegen.rs

@@ -368,7 +368,6 @@ impl<'source> CodeGenerator<'source> {
     #[cfg(feature = "macros")]
     fn compile_macro_expression(&mut self, macro_decl: &ast::Spanned<ast::Macro<'source>>) {
         use crate::compiler::instructions::MACRO_CALLER;
-        use crate::value::ValueRepr;
         self.set_line_from_span(macro_decl.span());
         let instr = self.add(Instruction::Jump(!0));
         let mut defaults_iter = macro_decl.defaults.iter().rev();
@@ -394,17 +393,16 @@ impl<'source> CodeGenerator<'source> {
             self.add(Instruction::Enclose(name));
         }
         self.add(Instruction::GetClosure);
-        self.add(Instruction::LoadConst(Value::from(ValueRepr::Seq(
+        self.add(Instruction::LoadConst(Value::from_object(
             macro_decl
                 .args
                 .iter()
                 .map(|x| match x {
                     ast::Expr::Var(var) => Value::from(var.id),
                     _ => unreachable!(),
                 })
-                .collect::<Vec<_>>()
-                .into(),
-        ))));
+                .collect::<Vec<Value>>(),
+        )));
         let mut flags = 0;
         if caller_reference {
             flags |= MACRO_CALLER;
@@ -476,26 +474,35 @@ impl<'source> CodeGenerator<'source> {
     }
 
     fn compile_for_loop(&mut self, for_loop: &ast::Spanned<ast::ForLoop<'source>>) {
+        // #[cfg(feature = "internal_debug")]
+        // println!("compiling {:?}", &**for_loop);
+
         self.set_line_from_span(for_loop.span());
+
+        // filter expressions work like a nested for loop without
+        // the special loop variable. in one loop, the condition is checked and
+        // passing items accumlated into a list. in the second, that list is
+        // iterated over normally
         if let Some(ref filter_expr) = for_loop.filter_expr {
-            // filter expressions work like a nested for loop without
-            // the special loop variable that append into a new list
-            // just outside of the loop.
-            self.add(Instruction::BuildList(0));
+            self.add(Instruction::LoadConst(Value::from(0usize)));
             self.compile_expr(&for_loop.iter);
             self.start_for_loop(false, false);
             self.add(Instruction::DupTop);
             self.compile_assignment(&for_loop.target);
             self.compile_expr(filter_expr);
             self.start_if();
-            self.add(Instruction::ListAppend);
+            self.add(Instruction::Swap);
+            self.add(Instruction::LoadConst(Value::from(1usize)));
+            self.add(Instruction::Add);
             self.start_else();
             self.add(Instruction::DiscardTop);
             self.end_if();
             self.end_for_loop(false);
+            self.add(Instruction::BuildList(None));
         } else {
             self.compile_expr(&for_loop.iter);
         }
+
         self.start_for_loop(true, for_loop.recursive);
         self.compile_assignment(&for_loop.target);
         for node in &for_loop.body {
@@ -650,7 +657,7 @@ impl<'source> CodeGenerator<'source> {
                     for item in &l.items {
                         self.compile_expr(item);
                     }
-                    self.add(Instruction::BuildList(l.items.len()));
+                    self.add(Instruction::BuildList(Some(l.items.len())));
                 }
             }
             ast::Expr::Map(m) => {

minijinja/src/compiler/instructions.rs

@@ -61,14 +61,11 @@ pub enum Instruction<'source> {
     BuildKwargs(usize),
 
     /// Builds a list of the last n pairs on the stack.
-    BuildList(usize),
+    BuildList(Option<usize>),
 
     /// Unpacks a list into N stack items.
     UnpackList(usize),
 
-    /// Appends to the list.
-    ListAppend,
-
     /// Add the top two values
     Add,
 
@@ -198,6 +195,9 @@ pub enum Instruction<'source> {
     /// A fast loop recurse instruction without intermediate capturing.
     FastRecurse,
 
+    /// Swaps the top two items in the stack.
+    Swap,
+
     /// Call into a block.
     #[cfg(feature = "multi_template")]
     CallBlock(&'source str),

minijinja/src/environment.rs

@@ -765,8 +765,6 @@ impl<'source> Environment<'source> {
 
 #[cfg(not(feature = "loader"))]
 mod basic_store {
-    use crate::template::TemplateConfig;
-
     use super::*;
 
     #[derive(Clone)]

minijinja/src/error.rs

@@ -3,6 +3,8 @@ use std::fmt;
 
 use crate::compiler::tokens::Span;
 
+pub type Result<T, E = Error> = std::result::Result<T, E>;
+
 /// Represents template errors.
 ///
 /// If debug mode is enabled a template error contains additional debug
@@ -390,7 +392,7 @@ impl From<fmt::Error> for Error {
     }
 }
 
-pub fn attach_basic_debug_info<T>(rv: Result<T, Error>, source: &str) -> Result<T, Error> {
+pub fn attach_basic_debug_info<T>(rv: Result<T>, source: &str) -> Result<T> {
     #[cfg(feature = "debug")]
     {
         match rv {

minijinja/src/filters.rs

@@ -505,8 +505,10 @@ mod builtins {
     pub fn reverse(v: Value) -> Result<Value, Error> {
         if let Some(s) = v.as_str() {
             Ok(Value::from(s.chars().rev().collect::<String>()))
-        } else if let Some(seq) = v.as_seq() {
-            Ok(Value::from(seq.iter().rev().collect::<Vec<_>>()))
+        } else if let Some(obj) = v.as_object() {
+            Ok(Value::from_object_iter(obj, |this| {
+                Box::new(this.values().rev())
+            }))
         } else {
             Err(Error::new(
                 ErrorKind::InvalidOperation,
@@ -545,9 +547,9 @@ mod builtins {
                 rv.push(c);
             }
             Ok(rv)
-        } else if let Some(seq) = val.as_seq() {
+        } else if let Some(obj) = val.as_object() {
             let mut rv = String::new();
-            for item in seq.iter() {
+            for item in obj.values() {
                 if !rv.is_empty() {
                     rv.push_str(joiner);
                 }
@@ -636,10 +638,7 @@ mod builtins {
                 .parse::<i128>()
                 .map(Value::from)
                 .map_err(|err| Error::new(ErrorKind::InvalidOperation, err.to_string())),
-            ValueRepr::Bytes(_)
-            | ValueRepr::Seq(_)
-            | ValueRepr::Map(_, _)
-            | ValueRepr::Dynamic(_) => Err(Error::new(
+            ValueRepr::Bytes(_) | ValueRepr::Object(_) => Err(Error::new(
                 ErrorKind::InvalidOperation,
                 format!("cannot convert {} to integer", value.kind()),
             )),
@@ -731,8 +730,9 @@ mod builtins {
     pub fn first(value: Value) -> Result<Value, Error> {
         if let Some(s) = value.as_str() {
             Ok(s.chars().next().map_or(Value::UNDEFINED, Value::from))
-        } else if let Some(s) = value.as_seq() {
-            Ok(s.get_item(0).unwrap_or(Value::UNDEFINED))
+        } else if let Some(s) = value.as_object() {
+            // FIXME: Seq only?
+            Ok(s.values().next().unwrap_or(Value::UNDEFINED))
         } else {
             Err(Error::new(
                 ErrorKind::InvalidOperation,
@@ -760,8 +760,9 @@ mod builtins {
     pub fn last(value: Value) -> Result<Value, Error> {
         if let Some(s) = value.as_str() {
             Ok(s.chars().next_back().map_or(Value::UNDEFINED, Value::from))
-        } else if let Some(seq) = value.as_seq() {
-            Ok(seq.iter().last().unwrap_or(Value::UNDEFINED))
+        } else if let Some(obj) = value.as_object() {
+            // FIXME: Seq only?
+            Ok(obj.values().next_back().unwrap_or(Value::UNDEFINED))
         } else {
             Err(Error::new(
                 ErrorKind::InvalidOperation,

minijinja/src/functions.rs

@@ -99,7 +99,7 @@ env.add_function("is_adult", is_adult);
 use std::fmt;
 use std::sync::Arc;
 
-use crate::error::Error;
+use crate::error::{Error, ErrorKind};
 use crate::utils::SealedMarker;
 use crate::value::{ArgType, FunctionArgs, FunctionResult, Object, Value};
 use crate::vm::State;
@@ -254,7 +254,19 @@ impl fmt::Display for BoxedFunction {
 }
 
 impl Object for BoxedFunction {
-    fn call(&self, state: &State, args: &[Value]) -> Result<Value, Error> {
+    fn call(
+        self: &Arc<Self>,
+        state: &State,
+        method: Option<&str>,
+        args: &[Value],
+    ) -> Result<Value, Error> {
+        if method.is_some() {
+            return Err(Error::new(
+                ErrorKind::InvalidOperation,
+                "cannot call method on function",
+            ));
+        }
+
         self.invoke(state, args)
     }
 }
@@ -263,8 +275,7 @@ impl Object for BoxedFunction {
 mod builtins {
     use super::*;
 
-    use crate::error::ErrorKind;
-    use crate::value::{MapType, ObjectKind, Rest, ValueKind, ValueMap, ValueRepr};
+    use crate::value::{ObjectRepr, Rest, ValueMap, ValueRepr};
 
     /// Returns a range.
     ///
@@ -334,39 +345,24 @@ mod builtins {
     #[cfg_attr(docsrs, doc(cfg(feature = "builtins")))]
     pub fn dict(value: Option<Value>, update_with: crate::value::Kwargs) -> Result<Value, Error> {
         let mut rv = match value {
-            None => Arc::new(ValueMap::default()),
+            None => ValueMap::default(),
             Some(value) => match value.0 {
-                ValueRepr::Undefined => Arc::new(ValueMap::default()),
-                ValueRepr::Map(map, _) => map,
-                ValueRepr::Dynamic(ref dynamic) => match dynamic.kind() {
-                    ObjectKind::Plain => Arc::new(ValueMap::default()),
-                    ObjectKind::Seq(_) | ObjectKind::Iterator(_) => {
-                        return Err(Error::from(ErrorKind::InvalidOperation))
-                    }
-                    ObjectKind::Struct(s) => {
-                        let mut rv = ValueMap::default();
-                        for field in s.fields() {
-                            if let Some(value) = s.get_field(&field) {
-                                rv.insert(crate::value::KeyRef::Value(Value::from(field)), value);
-                            }
-                        }
-                        Arc::new(rv)
-                    }
-                },
+                ValueRepr::Undefined => ValueMap::default(),
+                ValueRepr::Object(obj) if obj.repr() == ObjectRepr::Map => obj.iter().collect(),
                 _ => return Err(Error::from(ErrorKind::InvalidOperation)),
             },
         };
 
         if !update_with.values.is_empty() {
-            Arc::make_mut(&mut rv).extend(
+            rv.extend(
                 update_with
                     .values
                     .iter()
                     .map(|(k, v)| (k.clone(), v.clone())),
             );
         }
 
-        Ok(Value(ValueRepr::Map(rv, MapType::Normal)))
+        Ok(Value::from_object(rv))
     }
 
     /// Outputs the current context or the arguments stringified.
@@ -406,10 +402,10 @@ mod builtins {
     pub fn namespace(defaults: Option<Value>) -> Result<Value, Error> {
         let ns = crate::value::namespace_object::Namespace::default();
         if let Some(defaults) = defaults {
-            if defaults.kind() == ValueKind::Map {
-                for key in ok!(defaults.try_iter()) {
+            if let Some(obj) = defaults.as_object() {
+                for (key, value) in obj.iter() {
                     if let Some(key) = key.as_str() {
-                        ns.set_field(key, ok!(defaults.get_attr(key)));
+                        ns.set_field(key, value);
                     }
                 }
             } else {
@@ -422,7 +418,7 @@ mod builtins {
                 ));
             }
         }
-        Ok(Value::from_struct_object(ns))
+        Ok(Value::from_object(ns))
     }
 }
 

minijinja/src/lib.rs

@@ -196,8 +196,9 @@
 #![allow(clippy::cognitive_complexity)]
 #![allow(clippy::get_first)]
 #![allow(clippy::default_constructed_unit_structs)]
+#![allow(where_clauses_object_safety)]
 #![cfg_attr(docsrs, feature(doc_cfg))]
-#![deny(missing_docs)]
+// #![deny(missing_docs)]
 #![doc(html_logo_url = "https://github.com/mitsuhiko/minijinja/raw/main/artwork/logo-square.png")]
 
 #[macro_use]