Repository: ggml-org/llama.cpp
Pull request: jinja: correct member access rule

Bug spotted by @pwilkin 

If object contains a key that shadow the builtin function name, then depending on syntax:
- `obj.member` --> the builtin takes precedence over the defined value 
- `obj["member"]` --> the defined value takes precedence over the builtin

Example:

```
{{ obj.keys()|join(',') }} vs {{ obj['keys'] }} vs {{ obj.test }}
```

With input:

```json
{
  "obj": {
    "keys": "value",
    "test": "attr_value"
  }
}
```

Results in:

```
keys,test vs value vs attr_value
```

Changed files:
filename: common/jinja/runtime.cpp
patch: @@ -560,6 +560,7 @@ value for_statement::execute_impl(context & ctx) {
     for (size_t i = 0; i < filtered_items.size(); i++) {
         JJ_DEBUG("For loop iteration %zu/%zu", i + 1, filtered_items.size());
         value_object loop_obj = mk_val<value_object>();
+        loop_obj->has_builtins = false; // loop object has no builtins
         loop_obj->insert("index", mk_val<value_int>(i + 1));
         loop_obj->insert("index0", mk_val<value_int>(i));
         loop_obj->insert("revindex", mk_val<value_int>(filtered_items.size() - i));
@@ -717,6 +718,7 @@ value member_expression::execute_impl(context & ctx) {
 
     value property;
     if (this->computed) {
+        // syntax: obj[expr]
         JJ_DEBUG("Member expression, computing property type %s", this->property->type().c_str());
 
         int64_t arr_size = 0;
@@ -745,10 +747,24 @@ value member_expression::execute_impl(context & ctx) {
             property = this->property->execute(ctx);
         }
     } else {
+        // syntax: obj.prop
         if (!is_stmt<identifier>(this->property)) {
-            throw std::runtime_error("Non-computed member property must be an identifier");
+            throw std::runtime_error("Static member property must be an identifier");
         }
         property = mk_val<value_string>(cast_stmt<identifier>(this->property)->val);
+        std::string prop = property->as_string().str();
+        JJ_DEBUG("Member expression, object type %s, static property '%s'", object->type().c_str(), prop.c_str());
+
+        // behavior of jinja2: obj having prop as a built-in function AND 'prop', as an object key,
+        // then obj.prop returns the built-in function, not the property value.
+        // while obj['prop'] returns the property value.
+        // example: {"obj": {"items": 123}} -> obj.items is the built-in function, obj['items'] is 123
+
+        value val = try_builtin_func(ctx, prop, object, true);
+        if (!is_val<value_undefined>(val)) {
+            return val;
+        }
+        // else, fallthrough to normal property access below
     }
 
     JJ_DEBUG("Member expression on object type %s, property type %s", object->type().c_str(), property->type().c_str());

filename: common/jinja/runtime.h
patch: @@ -56,6 +56,7 @@ struct context {
     // src is optional, used for error reporting
     context(std::string src = "") : src(std::make_shared<std::string>(std::move(src))) {
         env = mk_val<value_object>();
+        env->has_builtins = false; // context object has no builtins
         env->insert("true",  mk_val<value_bool>(true));
         env->insert("True",  mk_val<value_bool>(true));
         env->insert("false", mk_val<value_bool>(false));
@@ -265,7 +266,7 @@ struct comment_statement : public statement {
 struct member_expression : public expression {
     statement_ptr object;
     statement_ptr property;
-    bool computed;
+    bool computed; // true if obj[expr] and false if obj.prop
 
     member_expression(statement_ptr && object, statement_ptr && property, bool computed)
         : object(std::move(object)), property(std::move(property)), computed(computed) {

filename: common/jinja/value.cpp
patch: @@ -888,6 +888,11 @@ const func_builtins & value_array_t::get_builtins() const {
 
 
 const func_builtins & value_object_t::get_builtins() const {
+    if (!has_builtins) {
+        static const func_builtins no_builtins = {};
+        return no_builtins;
+    }
+
     static const func_builtins builtins = {
         // {"default", default_value}, // cause issue with gpt-oss
         {"get", [](const func_args & args) -> value {

filename: common/jinja/value.h
patch: @@ -286,6 +286,7 @@ using value_array = std::shared_ptr<value_array_t>;
 
 
 struct value_object_t : public value_t {
+    bool has_builtins = true; // context and loop objects do not have builtins
     value_object_t() = default;
     value_object_t(value & v) {
         val_obj = v->val_obj;

filename: tests/test-jinja.cpp
patch: @@ -1063,6 +1063,18 @@ static void test_object_methods(testing & t) {
         {{"obj", {{"items", json::array({1, 2, 3})}}}},
         "{\"items\": [1, 2, 3]}"
     );
+
+    test_template(t, "object attribute and key access",
+        "{{ obj.keys()|join(',') }} vs {{ obj['keys'] }} vs {{ obj.test }}",
+        {{"obj", {{"keys", "value"}, {"test", "attr_value"}}}},
+        "keys,test vs value vs attr_value"
+    );
+
+    test_template(t, "env should not have object methods",
+        "{{ keys is undefined }} {{ obj.keys is defined }}",
+        {{"obj", {{"a", "b"}}}},
+        "True True"
+    );
 }
 
 static void test_template(testing & t, const std::string & name, const std::string & tmpl, const json & vars, const std::string & expect) {