derive: move generator-specific methods out of Expr

2 files changed, 71 insertions, 73 deletions

diff --git a/askama_derive/src/generator.rs b/askama_derive/src/generator.rs
index f17b6f6..e2d657f 100644
--- a/askama_derive/src/generator.rs
+++ b/askama_derive/src/generator.rs
@@ -1277,7 +1277,7 @@ impl<'a> Generator<'a> {
                     };
 
                     let id = match expr_cache.entry(expression.clone()) {
-                        Entry::Occupied(e) if s.is_cacheable() => *e.get(),
+                        Entry::Occupied(e) if is_cacheable(s) => *e.get(),
                         e => {
                             let id = self.named;
                             self.named += 1;
@@ -1596,7 +1596,7 @@ impl<'a> Generator<'a> {
                 buf.write(", ");
             }
 
-            let borrow = !arg.is_copyable();
+            let borrow = !is_copyable(arg);
             if borrow {
                 buf.write("&(");
             }
@@ -2124,6 +2124,75 @@ impl MapChain<'_, &str, LocalMeta> {
     }
 }
 
+/// Returns `true` if enough assumptions can be made,
+/// to determine that `self` is copyable.
+fn is_copyable(expr: &Expr<'_>) -> bool {
+    is_copyable_within_op(expr, false)
+}
+
+fn is_copyable_within_op(expr: &Expr<'_>, within_op: bool) -> bool {
+    use Expr::*;
+    match expr {
+        BoolLit(_) | NumLit(_) | StrLit(_) | CharLit(_) => true,
+        Unary(.., expr) => is_copyable_within_op(expr, true),
+        BinOp(_, lhs, rhs) => is_copyable_within_op(lhs, true) && is_copyable_within_op(rhs, true),
+        Range(..) => true,
+        // The result of a call likely doesn't need to be borrowed,
+        // as in that case the call is more likely to return a
+        // reference in the first place then.
+        Call(..) | Path(..) => true,
+        // If the `expr` is within a `Unary` or `BinOp` then
+        // an assumption can be made that the operand is copy.
+        // If not, then the value is moved and adding `.clone()`
+        // will solve that issue. However, if the operand is
+        // implicitly borrowed, then it's likely not even possible
+        // to get the template to compile.
+        _ => within_op && is_attr_self(expr),
+    }
+}
+
+/// Returns `true` if this is an `Attr` where the `obj` is `"self"`.
+pub(crate) fn is_attr_self(expr: &Expr<'_>) -> bool {
+    match expr {
+        Expr::Attr(obj, _) if matches!(obj.as_ref(), Expr::Var("self")) => true,
+        Expr::Attr(obj, _) if matches!(obj.as_ref(), Expr::Attr(..)) => is_attr_self(expr),
+        _ => false,
+    }
+}
+
+/// Returns `true` if the outcome of this expression may be used multiple times in the same
+/// `write!()` call, without evaluating the expression again, i.e. the expression should be
+/// side-effect free.
+pub(crate) fn is_cacheable(expr: &Expr<'_>) -> bool {
+    match expr {
+        // Literals are the definition of pure:
+        Expr::BoolLit(_) => true,
+        Expr::NumLit(_) => true,
+        Expr::StrLit(_) => true,
+        Expr::CharLit(_) => true,
+        // fmt::Display should have no effects:
+        Expr::Var(_) => true,
+        Expr::Path(_) => true,
+        // Check recursively:
+        Expr::Array(args) => args.iter().all(is_cacheable),
+        Expr::Attr(lhs, _) => is_cacheable(lhs),
+        Expr::Index(lhs, rhs) => is_cacheable(lhs) && is_cacheable(rhs),
+        Expr::Filter(_, args) => args.iter().all(is_cacheable),
+        Expr::Unary(_, arg) => is_cacheable(arg),
+        Expr::BinOp(_, lhs, rhs) => is_cacheable(lhs) && is_cacheable(rhs),
+        Expr::Range(_, lhs, rhs) => {
+            lhs.as_ref().map_or(true, |v| is_cacheable(v))
+                && rhs.as_ref().map_or(true, |v| is_cacheable(v))
+        }
+        Expr::Group(arg) => is_cacheable(arg),
+        Expr::Tuple(args) => args.iter().all(is_cacheable),
+        // We have too little information to tell if the expression is pure:
+        Expr::Call(_, _) => false,
+        Expr::RustMacro(_, _) => false,
+        Expr::Try(_) => false,
+    }
+}
+
 fn median(sizes: &mut [usize]) -> usize {
     sizes.sort_unstable();
     if sizes.len() % 2 == 1 {
diff --git a/askama_derive/src/parser/expr.rs b/askama_derive/src/parser/expr.rs
index 1d4ea89..deefb48 100644
--- a/askama_derive/src/parser/expr.rs
+++ b/askama_derive/src/parser/expr.rs
@@ -43,77 +43,6 @@ impl Expr<'_> {
     pub(super) fn parse_arguments(i: &str) -> IResult<&str, Vec<Expr<'_>>> {
         arguments(i)
     }
-
-    /// Returns `true` if enough assumptions can be made,
-    /// to determine that `self` is copyable.
-    pub(crate) fn is_copyable(&self) -> bool {
-        self.is_copyable_within_op(false)
-    }
-
-    fn is_copyable_within_op(&self, within_op: bool) -> bool {
-        use Expr::*;
-        match self {
-            BoolLit(_) | NumLit(_) | StrLit(_) | CharLit(_) => true,
-            Unary(.., expr) => expr.is_copyable_within_op(true),
-            BinOp(_, lhs, rhs) => {
-                lhs.is_copyable_within_op(true) && rhs.is_copyable_within_op(true)
-            }
-            Range(..) => true,
-            // The result of a call likely doesn't need to be borrowed,
-            // as in that case the call is more likely to return a
-            // reference in the first place then.
-            Call(..) | Path(..) => true,
-            // If the `expr` is within a `Unary` or `BinOp` then
-            // an assumption can be made that the operand is copy.
-            // If not, then the value is moved and adding `.clone()`
-            // will solve that issue. However, if the operand is
-            // implicitly borrowed, then it's likely not even possible
-            // to get the template to compile.
-            _ => within_op && self.is_attr_self(),
-        }
-    }
-
-    /// Returns `true` if this is an `Attr` where the `obj` is `"self"`.
-    pub(crate) fn is_attr_self(&self) -> bool {
-        match self {
-            Expr::Attr(obj, _) if matches!(obj.as_ref(), Expr::Var("self")) => true,
-            Expr::Attr(obj, _) if matches!(obj.as_ref(), Expr::Attr(..)) => obj.is_attr_self(),
-            _ => false,
-        }
-    }
-
-    /// Returns `true` if the outcome of this expression may be used multiple times in the same
-    /// `write!()` call, without evaluating the expression again, i.e. the expression should be
-    /// side-effect free.
-    pub(crate) fn is_cacheable(&self) -> bool {
-        match self {
-            // Literals are the definition of pure:
-            Expr::BoolLit(_) => true,
-            Expr::NumLit(_) => true,
-            Expr::StrLit(_) => true,
-            Expr::CharLit(_) => true,
-            // fmt::Display should have no effects:
-            Expr::Var(_) => true,
-            Expr::Path(_) => true,
-            // Check recursively:
-            Expr::Array(args) => args.iter().all(|arg| arg.is_cacheable()),
-            Expr::Attr(lhs, _) => lhs.is_cacheable(),
-            Expr::Index(lhs, rhs) => lhs.is_cacheable() && rhs.is_cacheable(),
-            Expr::Filter(_, args) => args.iter().all(|arg| arg.is_cacheable()),
-            Expr::Unary(_, arg) => arg.is_cacheable(),
-            Expr::BinOp(_, lhs, rhs) => lhs.is_cacheable() && rhs.is_cacheable(),
-            Expr::Range(_, lhs, rhs) => {
-                lhs.as_ref().map_or(true, |v| v.is_cacheable())
-                    && rhs.as_ref().map_or(true, |v| v.is_cacheable())
-            }
-            Expr::Group(arg) => arg.is_cacheable(),
-            Expr::Tuple(args) => args.iter().all(|arg| arg.is_cacheable()),
-            // We have too little information to tell if the expression is pure:
-            Expr::Call(_, _) => false,
-            Expr::RustMacro(_, _) => false,
-            Expr::Try(_) => false,
-        }
-    }
 }
 
 fn expr_bool_lit(i: &str) -> IResult<&str, Expr<'_>> {