From 7edb3d4685aff00880906873db4eb9f649a077d9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Kijewski?= <rene.kijewski@fu-berlin.de>
Date: Sat, 17 Sep 2022 03:48:10 +0200
Subject: [PATCH] Add Expr::is_cachable()

---
 askama_derive/src/generator.rs |  6 ++----
 askama_derive/src/parser.rs    | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/askama_derive/src/generator.rs b/askama_derive/src/generator.rs
index e8d15f6a5..7077cfd4b 100644
--- a/askama_derive/src/generator.rs
+++ b/askama_derive/src/generator.rs
@@ -7,7 +7,7 @@ use crate::CompileError;
 use proc_macro::TokenStream;
 use quote::{quote, ToTokens};
 
-use std::collections::HashMap;
+use std::collections::hash_map::{Entry, HashMap};
 use std::path::{Path, PathBuf};
 use std::{cmp, hash, mem, str};
 
@@ -1196,11 +1196,9 @@ impl<'a> Generator<'a> {
                             expr_buf.buf, self.input.escaper
                         ),
                     };
-                    let is_cacheable = !matches!(s, Expr::Call(..));
 
-                    use std::collections::hash_map::Entry;
                     let id = match expr_cache.entry(expression.clone()) {
-                        Entry::Occupied(e) if is_cacheable => *e.get(),
+                        Entry::Occupied(e) if s.is_cachable() => *e.get(),
                         e => {
                             let id = self.named;
                             self.named += 1;
diff --git a/askama_derive/src/parser.rs b/askama_derive/src/parser.rs
index efcad7369..dc163fae7 100644
--- a/askama_derive/src/parser.rs
+++ b/askama_derive/src/parser.rs
@@ -106,6 +106,39 @@ impl 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_cachable(&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_cachable()),
+            Expr::Attr(lhs, _) => lhs.is_cachable(),
+            Expr::Index(lhs, rhs) => lhs.is_cachable() && rhs.is_cachable(),
+            Expr::Filter(_, args) => args.iter().all(|arg| arg.is_cachable()),
+            Expr::Unary(_, arg) => arg.is_cachable(),
+            Expr::BinOp(_, lhs, rhs) => lhs.is_cachable() && rhs.is_cachable(),
+            Expr::Range(_, lhs, rhs) => {
+                lhs.as_ref().map_or(true, |v| v.is_cachable())
+                    && rhs.as_ref().map_or(true, |v| v.is_cachable())
+            }
+            Expr::Group(arg) => arg.is_cachable(),
+            Expr::Tuple(args) => args.iter().all(|arg| arg.is_cachable()),
+            // We have too little information to tell if the expression is pure:
+            Expr::Call(_, _) => false,
+            Expr::RustMacro(_, _) => false,
+            Expr::Try(_) => false,
+        }
+    }
 }
 
 pub(crate) type When<'a> = (Ws, Target<'a>, Vec<Node<'a>>);