From 348f853b7adc7374a4dec989750eaa6ea563535e Mon Sep 17 00:00:00 2001
From: Xuan-Son Nguyen <son@huggingface.co>
Date: Tue, 22 Sep 2026 15:27:59 +0200
Subject: [PATCH] jinja: use const for statement::execute and ::visit (#29271)

---
 common/jinja/runtime.cpp |  53 +++++++++---------
 common/jinja/runtime.h   | 117 ++++++++++++++++++---------------------
 2 files changed, 80 insertions(+), 90 deletions(-)

diff --git a/common/jinja/runtime.cpp b/common/jinja/runtime.cpp
index 252ab55de2..227f6c094a 100644
--- a/common/jinja/runtime.cpp
+++ b/common/jinja/runtime.cpp
@@ -51,7 +51,7 @@ static void ensure_key_type_allowed(const value & val) {
 }
 
 // execute with error handling
-value statement::execute(context & ctx) {
+value statement::execute(context & ctx) const {
     try {
         return execute_impl(ctx);
     } catch (const continue_statement::signal & /* ex */) {
@@ -80,7 +80,7 @@ value statement::execute(context & ctx) {
     }
 }
 
-value identifier::execute_impl(context & ctx) {
+value identifier::execute_impl(context & ctx) const {
     auto it = ctx.get_val(val);
     auto builtins = global_builtins();
     if (!it->is_undefined()) {
@@ -98,7 +98,7 @@ value identifier::execute_impl(context & ctx) {
     }
 }
 
-value object_literal::execute_impl(context & ctx) {
+value object_literal::execute_impl(context & ctx) const {
     auto obj = mk_val<value_object>();
     for (const auto & pair : val) {
         value key = pair.first->execute(ctx);
@@ -109,7 +109,7 @@ value object_literal::execute_impl(context & ctx) {
     return obj;
 }
 
-value binary_expression::execute_impl(context & ctx) {
+value binary_expression::execute_impl(context & ctx) const {
     value left_val = left->execute(ctx);
 
     // Logical operators
@@ -317,9 +317,7 @@ static value try_builtin_func(context & ctx, const std::string & name, value & i
     throw std::runtime_error("Unknown (built-in) filter '" + name + "' for type " + input->type());
 }
 
-value filter_expression::execute_impl(context & ctx) {
-    value input = operand ? operand->execute(ctx) : val;
-
+static value apply_filter(context & ctx, const statement_ptr & filter, value input) {
     JJ_DEBUG("Applying filter to %s", input->type().c_str());
 
     auto set_filter_alias = [](auto & filter_id) {
@@ -375,22 +373,21 @@ value filter_expression::execute_impl(context & ctx) {
     }
 }
 
-value filter_statement::execute_impl(context & ctx) {
+value filter_expression::execute_impl(context & ctx) const {
+    return apply_filter(ctx, filter, operand->execute(ctx));
+}
+
+value filter_statement::execute_impl(context & ctx) const {
     // eval body as string, then apply filter
     auto body_val = exec_statements(body, ctx);
     value_string parts = mk_val<value_string>();
     gather_string_parts_recursive(body_val, parts);
 
     JJ_DEBUG("FilterStatement: applying filter to body string of length %zu", parts->val_str.length());
-    filter_expression filter_expr(std::move(parts), std::move(filter));
-    value out = filter_expr.execute(ctx);
-
-    // this node can be reused later, make sure filter is preserved
-    this->filter = std::move(filter_expr.filter);
-    return out;
+    return apply_filter(ctx, filter, parts);
 }
 
-value test_expression::execute_impl(context & ctx) {
+value test_expression::execute_impl(context & ctx) const {
     // NOTE: "value is something" translates to function call "test_is_something(value)"
     const auto & builtins = global_builtins();
 
@@ -439,7 +436,7 @@ value test_expression::execute_impl(context & ctx) {
     }
 }
 
-value unary_expression::execute_impl(context & ctx) {
+value unary_expression::execute_impl(context & ctx) const {
     value operand_val = argument->execute(ctx);
     JJ_DEBUG("Executing unary expression with operator '%s'", op.value.c_str());
 
@@ -458,7 +455,7 @@ value unary_expression::execute_impl(context & ctx) {
     throw std::runtime_error("Unknown unary operator '" + op.value + "'");
 }
 
-value if_statement::execute_impl(context & ctx) {
+value if_statement::execute_impl(context & ctx) const {
     value test_val = test->execute(ctx);
 
     auto out = mk_val<value_array>();
@@ -479,17 +476,17 @@ value if_statement::execute_impl(context & ctx) {
     return str;
 }
 
-value for_statement::execute_impl(context & ctx) {
+value for_statement::execute_impl(context & ctx) const {
     context scope(ctx); // new scope for loop variables
 
-    jinja::select_expression * select_expr = cast_stmt<select_expression>(iterable);
+    const jinja::select_expression * select_expr = cast_stmt<select_expression>(iterable);
     statement_ptr test_expr_nullptr;
 
-    statement_ptr & iter_expr = [&]() -> statement_ptr & {
+    const statement_ptr & iter_expr = [&]() -> const statement_ptr & {
         auto tmp = cast_stmt<select_expression>(iterable);
         return tmp ? tmp->lhs : iterable;
     }();
-    statement_ptr & test_expr = [&]() -> statement_ptr & {
+    const statement_ptr & test_expr = [&]() -> const statement_ptr & {
         auto tmp = cast_stmt<select_expression>(iterable);
         return tmp ? tmp->test : test_expr_nullptr;
     }();
@@ -645,7 +642,7 @@ value for_statement::execute_impl(context & ctx) {
     return str;
 }
 
-value set_statement::execute_impl(context & ctx) {
+value set_statement::execute_impl(context & ctx) const {
     auto rhs = val ? val->execute(ctx) : exec_statements(body, ctx);
 
     if (is_stmt<identifier>(assignee)) {
@@ -744,7 +741,7 @@ static inline void bind_parameters(const std::string & name, const statements &
     }
 }
 
-value macro_statement::execute_impl(context & ctx) {
+value macro_statement::execute_impl(context & ctx) const {
     if (!is_stmt<identifier>(this->name)) {
         throw std::runtime_error("Macro name must be an identifier");
     }
@@ -767,7 +764,7 @@ value macro_statement::execute_impl(context & ctx) {
     return mk_val<value_undefined>();
 }
 
-value call_statement::execute_impl(context & ctx) {
+value call_statement::execute_impl(context & ctx) const {
     auto call_expr = cast_stmt<call_expression>(this->call);
     if (!call_expr) {
         throw std::runtime_error("Call statement requires a valid call expression");
@@ -807,7 +804,7 @@ value call_statement::execute_impl(context & ctx) {
     return callee_func->invoke(args);
 }
 
-value member_expression::execute_impl(context & ctx) {
+value member_expression::execute_impl(context & ctx) const {
     value object = this->object->execute(ctx);
 
     value property;
@@ -940,7 +937,7 @@ value member_expression::execute_impl(context & ctx) {
     return val;
 }
 
-value call_expression::execute_impl(context & ctx) {
+value call_expression::execute_impl(context & ctx) const {
     // gather arguments
     func_args args(ctx);
     for (auto & arg_stmt : this->args) {
@@ -958,7 +955,7 @@ value call_expression::execute_impl(context & ctx) {
     return callee_func->invoke(args);
 }
 
-value keyword_argument_expression::execute_impl(context & ctx) {
+value keyword_argument_expression::execute_impl(context & ctx) const {
     if (!is_stmt<identifier>(key)) {
         throw std::runtime_error("Keyword argument key must be identifiers");
     }
@@ -982,7 +979,7 @@ std::string runtime::debug_dump_program(const program & prog, const std::string
         return std::string(lvl * 2, ' ');
     };
 
-    ctx.visitor = [&](bool is_leaf, statement * node, std::vector<visitor_pair> children) {
+    ctx.visitor = [&](bool is_leaf, const statement * node, std::vector<visitor_pair> children) {
         oss << indent(lvl) << node->type() << ":\n";
         lvl++;
         if (is_leaf) {
diff --git a/common/jinja/runtime.h b/common/jinja/runtime.h
index 69bd683c68..bbd0c5caea 100644
--- a/common/jinja/runtime.h
+++ b/common/jinja/runtime.h
@@ -48,9 +48,9 @@ const T * cast_stmt(const statement_ptr & ptr) {
 void enable_debug(bool enable);
 
 // for visiting AST nodes
-// function signature: void(bool is_leaf, statement * node, pair of <label, children>)
-using visitor_pair = std::pair<std::string, std::vector<statement *>>;
-using visitor_fn = std::function<void(bool, statement *, std::vector<visitor_pair>)>;
+// function signature: void(bool is_leaf, const statement * node, pair of <label, children>)
+using visitor_pair = std::pair<std::string, std::vector<const statement *>>;
+using visitor_fn = std::function<void(bool, const statement *, std::vector<visitor_pair>)>;
 
 struct context {
     std::shared_ptr<std::string> src; // for debugging; use shared_ptr to avoid copying on scope creation
@@ -107,8 +107,8 @@ private:
 };
 
 // utils for visiting AST nodes
-static std::vector<statement *> stmts_to_ptr(const statements & stmts) {
-    std::vector<statement *> children;
+static std::vector<const statement *> stmts_to_ptr(const statements & stmts) {
+    std::vector<const statement *> children;
     for (const auto & stmt : stmts) {
         children.push_back(stmt.get());
     }
@@ -117,17 +117,18 @@ static std::vector<statement *> stmts_to_ptr(const statements & stmts) {
 
 /**
  * Base class for all nodes in the AST.
+ * The AST is shared between threads, so visit and execute must be const.
  */
 struct statement {
     size_t pos; // position in source, for debugging
     virtual ~statement() = default;
     virtual std::string type() const { return "Statement"; }
-    virtual void visit(context & ctx) { ctx.visitor(true, this, {}); }
+    virtual void visit(context & ctx) const { ctx.visitor(true, this, {}); }
 
     // execute_impl must be overridden by derived classes
-    virtual value execute_impl(context &) { throw_exec_error(); }
+    virtual value execute_impl(context &) const { throw_exec_error(); }
     // execute is the public method to execute a statement with error handling
-    value execute(context &);
+    value execute(context &) const;
 
 private:
     [[noreturn]] void throw_exec_error() const {
@@ -166,7 +167,7 @@ struct program : public statement {
     program() = default;
     explicit program(statements && body) : body(std::move(body)) {}
     std::string type() const override { return "Program"; }
-    [[noreturn]] value execute_impl(context &) override {
+    [[noreturn]] value execute_impl(context &) const override {
         throw std::runtime_error("Cannot execute program directly, use jinja::runtime instead");
     }
 };
@@ -182,8 +183,8 @@ struct if_statement : public statement {
     }
 
     std::string type() const override { return "If"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"test", {test.get()}},
             {"body", stmts_to_ptr(body)},
@@ -213,8 +214,8 @@ struct for_statement : public statement {
     }
 
     std::string type() const override { return "For"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"loopvar", {loopvar.get()}},
             {"iterable", {iterable.get()}},
@@ -233,7 +234,7 @@ struct break_statement : public statement {
         }
     };
 
-    [[noreturn]] value execute_impl(context &) override {
+    [[noreturn]] value execute_impl(context &) const override {
         throw break_statement::signal();
     }
 };
@@ -247,7 +248,7 @@ struct continue_statement : public statement {
         }
     };
 
-    [[noreturn]] value execute_impl(context &) override {
+    [[noreturn]] value execute_impl(context &) const override {
         throw continue_statement::signal();
     }
 };
@@ -255,7 +256,7 @@ struct continue_statement : public statement {
 // do nothing
 struct noop_statement : public statement {
     std::string type() const override { return "Noop"; }
-    value execute_impl(context &) override {
+    value execute_impl(context &) const override {
         return mk_val<value_undefined>();
     }
 };
@@ -272,8 +273,8 @@ struct set_statement : public statement {
     }
 
     std::string type() const override { return "Set"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"assignee", {assignee.get()}},
             {"value", {val.get()}},
@@ -294,8 +295,8 @@ struct macro_statement : public statement {
     }
 
     std::string type() const override { return "Macro"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"name", {name.get()}},
             {"args", stmts_to_ptr(args)},
@@ -308,7 +309,7 @@ struct comment_statement : public statement {
     std::string val;
     explicit comment_statement(const std::string & v) : val(v) {}
     std::string type() const override { return "Comment"; }
-    value execute_impl(context &) override {
+    value execute_impl(context &) const override {
         return mk_val<value_undefined>();
     }
 };
@@ -318,7 +319,7 @@ struct comment_statement : public statement {
 // Represents an omitted expression in a computed member, e.g. `a[]`.
 struct blank_expression : public expression {
     std::string type() const override { return "BlankExpression"; }
-    value execute_impl(context &) override {
+    value execute_impl(context &) const override {
         return mk_val<value_undefined>();
     }
 };
@@ -334,8 +335,8 @@ struct member_expression : public expression {
         chk_type<expression>(this->property);
     }
     std::string type() const override { return "MemberExpression"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"object", {object.get()}},
             {"property", {property.get()}}
@@ -353,8 +354,8 @@ struct call_expression : public expression {
         for (const auto& arg : this->args) chk_type<expression>(arg);
     }
     std::string type() const override { return "CallExpression"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"callee", {callee.get()}},
             {"args", stmts_to_ptr(args)}
@@ -369,7 +370,7 @@ struct identifier : public expression {
     std::string val;
     explicit identifier(const std::string & val) : val(val) {}
     std::string type() const override { return "Identifier"; }
-    value execute_impl(context & ctx) override;
+    value execute_impl(context & ctx) const override;
 };
 
 // Literals
@@ -378,7 +379,7 @@ struct integer_literal : public expression {
     int64_t val;
     explicit integer_literal(int64_t val) : val(val) {}
     std::string type() const override { return "IntegerLiteral"; }
-    value execute_impl(context &) override {
+    value execute_impl(context &) const override {
         return mk_val<value_int>(val);
     }
 };
@@ -387,7 +388,7 @@ struct float_literal : public expression {
     double val;
     explicit float_literal(double val) : val(val) {}
     std::string type() const override { return "FloatLiteral"; }
-    value execute_impl(context &) override {
+    value execute_impl(context &) const override {
         return mk_val<value_float>(val);
     }
 };
@@ -396,7 +397,7 @@ struct string_literal : public expression {
     std::string val;
     explicit string_literal(const std::string & val) : val(val) {}
     std::string type() const override { return "StringLiteral"; }
-    value execute_impl(context &) override {
+    value execute_impl(context &) const override {
         return mk_val<value_string>(val);
     }
 };
@@ -407,7 +408,7 @@ struct array_literal : public expression {
         for (const auto& item : this->val) chk_type<expression>(item);
     }
     std::string type() const override { return "ArrayLiteral"; }
-    value execute_impl(context & ctx) override {
+    value execute_impl(context & ctx) const override {
         auto arr = mk_val<value_array>();
         for (const auto & item_stmt : val) {
             arr->push_back(item_stmt->execute(ctx));
@@ -422,7 +423,7 @@ struct tuple_literal : public expression {
         for (const auto& item : this->val) chk_type<expression>(item);
     }
     std::string type() const override { return "TupleLiteral"; }
-    value execute_impl(context & ctx) override {
+    value execute_impl(context & ctx) const override {
         auto arr = mk_val<value_array>();
         for (const auto & item_stmt : val) {
             arr->push_back(item_stmt->execute(ctx));
@@ -441,7 +442,7 @@ struct object_literal : public expression {
         }
     }
     std::string type() const override { return "ObjectLiteral"; }
-    value execute_impl(context & ctx) override;
+    value execute_impl(context & ctx) const override;
 };
 
 // Complex Expressions
@@ -462,8 +463,8 @@ struct binary_expression : public expression {
         chk_type<expression>(this->right);
     }
     std::string type() const override { return "BinaryExpression"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"left", {left.get()}},
             {"right", {right.get()}}
@@ -476,10 +477,7 @@ struct binary_expression : public expression {
  * Operator precedence: https://github.com/pallets/jinja/issues/379#issuecomment-168076202
  */
 struct filter_expression : public expression {
-    // either an expression or a value is allowed
     statement_ptr operand;
-    value_string val; // will be set by filter_statement
-
     statement_ptr filter;
 
     filter_expression(statement_ptr && operand, statement_ptr && filter)
@@ -488,14 +486,9 @@ struct filter_expression : public expression {
         chk_type<identifier, call_expression>(this->filter);
     }
 
-    filter_expression(value_string && val, statement_ptr && filter)
-        : val(std::move(val)), filter(std::move(filter)) {
-        chk_type<identifier, call_expression>(this->filter);
-    }
-
     std::string type() const override { return "FilterExpression"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"operand", {operand.get()}},
             {"filter", {filter.get()}}
@@ -512,8 +505,8 @@ struct filter_statement : public statement {
         chk_type<identifier, call_expression>(this->filter);
     }
     std::string type() const override { return "FilterStatement"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"filter", {filter.get()}},
             {"body", stmts_to_ptr(body)}
@@ -537,14 +530,14 @@ struct select_expression : public expression {
         chk_type<expression>(this->test);
     }
     std::string type() const override { return "SelectExpression"; }
-    value execute_impl(context & ctx) override {
+    value execute_impl(context & ctx) const override {
         auto predicate = test->execute_impl(ctx);
         if (!predicate->as_bool()) {
             return mk_val<value_undefined>();
         }
         return lhs->execute_impl(ctx);
     }
-    void visit(context & ctx) override {
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"lhs", {lhs.get()}},
             {"test", {test.get()}}
@@ -567,8 +560,8 @@ struct test_expression : public expression {
         chk_type<identifier, call_expression>(this->test);
     }
     std::string type() const override { return "TestExpression"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"operand", {operand.get()}},
             {"test", {test.get()}}
@@ -588,8 +581,8 @@ struct unary_expression : public expression {
         chk_type<expression>(this->argument);
     }
     std::string type() const override { return "UnaryExpression"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"argument", {argument.get()}}
         });
@@ -608,10 +601,10 @@ struct slice_expression : public expression {
         chk_type<expression>(this->step_expr);
     }
     std::string type() const override { return "SliceExpression"; }
-    [[noreturn]] value execute_impl(context &) override {
+    [[noreturn]] value execute_impl(context &) const override {
         throw std::runtime_error("must be handled by MemberExpression");
     }
-    void visit(context & ctx) override {
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"start_expr", {start_expr.get()}},
             {"stop_expr", {stop_expr.get()}},
@@ -630,8 +623,8 @@ struct keyword_argument_expression : public expression {
         chk_type<expression>(this->val);
     }
     std::string type() const override { return "KeywordArgumentExpression"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"key", {key.get()}},
             {"val", {val.get()}}
@@ -645,7 +638,7 @@ struct spread_expression : public expression {
         chk_type<expression>(this->argument);
     }
     std::string type() const override { return "SpreadExpression"; }
-    void visit(context & ctx) override {
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"argument", {argument.get()}}
         });
@@ -663,8 +656,8 @@ struct call_statement : public statement {
         for (const auto & arg : this->caller_args) chk_type<expression>(arg);
     }
     std::string type() const override { return "CallStatement"; }
-    value execute_impl(context & ctx) override;
-    void visit(context & ctx) override {
+    value execute_impl(context & ctx) const override;
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"call", {call.get()}},
             {"caller_args", stmts_to_ptr(caller_args)},
@@ -685,7 +678,7 @@ struct ternary_expression : public expression {
         chk_type<expression>(this->false_expr);
     }
     std::string type() const override { return "Ternary"; }
-    value execute_impl(context & ctx) override {
+    value execute_impl(context & ctx) const override {
         value cond_val = condition->execute(ctx);
         if (cond_val->as_bool()) {
             return true_expr->execute(ctx);
@@ -693,7 +686,7 @@ struct ternary_expression : public expression {
             return false_expr->execute(ctx);
         }
     }
-    void visit(context & ctx) override {
+    void visit(context & ctx) const override {
         ctx.visitor(false, this, {
             {"condition", {condition.get()}},
             {"true_expr", {true_expr.get()}},