Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/tasks/0212-objects-as-keys-and-through-callables.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,8 @@ a JSON value viewed through `jinja2::Reflect` and read twice gives two accessors
through one read misses the other. The six `0212` lines in
`test/parity/divergences/reflect.txt` (the 0322 pass) are these cases; they go when a view has
a stable identity.

**Plan (2026-11-03).** Architect plan with decisions D1-D8 for Ruslan:
`/mnt/project-files/release-2.0/0212-plan.md`. 0370 already makes a JSON object unpack its keys
(it answers `HasAttributes() == false` through the `IJsonNode` marker). Macros passed to a user
callable no longer arrive as None (0300); what remains is the way back (`ident(m)()`).
14 changes: 13 additions & 1 deletion docs/tasks/0370-json-view-is-a-dict.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
status: open
status: in-progress
priority: medium
area: parity
depends: [0049]
Expand Down Expand Up @@ -35,3 +35,15 @@ land the marker here with only the dict answer.
**Done when.** The six `reflect.*` lines naming 0370 in `test/parity/divergences/reflect.txt`
are gone, the conformance suite (`test/binding/conformance.h`) has a row for a key shadowing
a dict method on every binding, and a reflected struct keeps its keys-first lookup.

**As built (parity batch 3, group E).** `include/jinja2cpp/json_adapter.h` adds the exported
marker `jinja2::IJsonNode` (empty for now), which `json::ObjectAccessor` and
`json::ArrayAccessor` derive from. `GenericMapAdapter` detects it once, in its constructor
(`dynamic_cast`), and then answers `MethodsFirst` and `HasAttributes() == false`; any other host
map, such as a reflected struct, keeps `KeysFirst`. Everything that asks those two questions
follows: `is sequence`, `attr()`, `map('attr')`, the `dict` type name, and tuple unpacking
(`{% set x, y = j %}` now unpacks the keys, the third bullet of 0212). Rows: the six
`reflect.*` lines are gone, `DictMethods` in `test/binding/conformance.h` (every binding, all
four modes, including `unpack_keys`), and `ExpressionTest.ReflectedStructFieldComesBeforeADictMethod`.
Whether the marker gets 0212's `NodeIdentity()` virtual before this merges is decision D6 of
`/mnt/project-files/release-2.0/0212-plan.md`.
15 changes: 15 additions & 0 deletions include/jinja2cpp/json_adapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// that library's value type.

#include <jinja2cpp/big_int.h>
#include <jinja2cpp/config.h>
#include <jinja2cpp/error_info.h>
#include <jinja2cpp/generic_list.h>
#include <jinja2cpp/reflected_value.h>
Expand Down Expand Up @@ -126,6 +127,18 @@ struct JsonCopyLimits
std::uint64_t maxSequenceSize = 0;
};

/*!
* \brief Marks the accessors of the JSON views that \ref Reflect makes (docs/tasks/0370)
*
* The engine tells a JSON object from a reflected struct by this base: a JSON object is a dict to the template, as
* Python's `json.loads` gives one (`d.items` is the dict method even with a key `items`, `attr()` does not see the
* keys, `is sequence` holds), while the fields of a reflected struct are its attributes. The accessors of
* \ref Reflect for a type with \ref JsonTraits derive from it; other accessors do not need to.
*/
struct JINJA2CPP_EXPORT IJsonNode
{
};

namespace detail::json
{
template<typename T>
Expand Down Expand Up @@ -401,6 +414,7 @@ inline std::optional<Value> BigIntValue(std::string_view text)
template<typename J>
class ObjectAccessor final : public IMapItemAccessor
, public detail::OwnedAccessor
, public IJsonNode
{
public:
explicit ObjectAccessor(Ref<J> ref)
Expand Down Expand Up @@ -493,6 +507,7 @@ template<typename J>
class ArrayAccessor final : public IListItemAccessor
, public IIndexBasedAccessor
, public detail::OwnedAccessor
, public IJsonNode
{
public:
explicit ArrayAccessor(Ref<J> ref)
Expand Down
40 changes: 38 additions & 2 deletions src/internal_value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <jinja2cpp/exception.h>
#include <jinja2cpp/big_int.h>
#include <jinja2cpp/generic_list.h>
#include <jinja2cpp/json_adapter.h>
#include <jinja2cpp/markup.h>
#include <jinja2cpp/string_helpers.h>
#include <jinja2cpp/template.h>
Expand All @@ -43,6 +44,7 @@
#include <string_view>
#include <thread>
#include <type_traits>
#include <typeinfo>
#include <utility>
#include <variant>
#include <vector>
Expand Down Expand Up @@ -3152,6 +3154,35 @@ InternalValue Value2IntValue(Value&& val)
return std::visit(visitors::InputValueConvertor(true, false), val.GetData());
}

// Whether a host map is a JSON object (IJsonNode, docs/tasks/0370). A cross-cast costs tens of
// nanoseconds and every lent map item asks, so the answer is kept per most-derived type: the last
// type found to be a JSON node and the last found not to be. Each slot is a fact about a type, so
// a race between renders only costs another cast
namespace
{
bool IsJsonNode(const IMapItemAccessor* accessor)
{
if (!accessor)
{
return false;
}
static std::atomic<const std::type_info*> lastJson{ nullptr };
static std::atomic<const std::type_info*> lastOther{ nullptr };
const std::type_info* type = &typeid(*accessor);
if (type == lastJson.load(std::memory_order_relaxed))
{
return true;
}
if (type == lastOther.load(std::memory_order_relaxed))
{
return false;
}
const bool isJson = dynamic_cast<const IJsonNode*>(accessor) != nullptr;
(isJson ? lastJson : lastOther).store(type, std::memory_order_relaxed);
return isJson;
}
} // namespace

template<template<typename> class Holder>
class GenericMapAdapter : public MapAccessorImpl<GenericMapAdapter<Holder>>
{
Expand All @@ -3160,6 +3191,7 @@ class GenericMapAdapter : public MapAccessorImpl<GenericMapAdapter<Holder>>
template<typename U, typename = std::enable_if_t<!std::is_same_v<std::decay_t<U>, GenericMapAdapter>>>
explicit GenericMapAdapter(U&& values)
: m_values(std::forward<U>(values))
, m_isDict(IsJsonNode(m_values.Get().GetAccessor()))
{
// As OwnedAdapter
this->SetTraversable();
Expand Down Expand Up @@ -3236,7 +3268,9 @@ class GenericMapAdapter : public MapAccessorImpl<GenericMapAdapter<Holder>>
return self;
}
}
[[nodiscard]] bool HasAttributes() const override { return true; }
// A JSON object is a dict (IJsonNode, docs/tasks/0370); the names of any other host map, such as a
// reflected struct, are its attributes
[[nodiscard]] bool HasAttributes() const override { return !m_isDict; }
Comment thread
rmorozov marked this conversation as resolved.
Resolved
[[nodiscard]] const void* GetIdentity() const override { return m_values.Get().GetAccessor(); }
[[nodiscard]] TemplateDict* GetMutableItems() const override
{
Expand All @@ -3249,7 +3283,7 @@ class GenericMapAdapter : public MapAccessorImpl<GenericMapAdapter<Holder>>
return m_link.Adopt(self, GetIdentity(), false, copy, this->IsGlobalData(), this->IsInSnapshot(), this->IsGlobalData() ? std::weak_ptr<const void>() : AdaptersOwner(m_values));
}
[[nodiscard]] bool MayBeLentAgain() const noexcept override { return AccessorHasOtherOwners(m_values.Get()); }
[[nodiscard]] MapAttrPolicy GetAttrPolicy() const override { return MapAttrPolicy::KeysFirst; }
[[nodiscard]] MapAttrPolicy GetAttrPolicy() const override { return m_isDict ? MapAttrPolicy::MethodsFirst : MapAttrPolicy::KeysFirst; }
[[nodiscard]] const CellLink* GetCellLink() const override { return &m_link; }
[[nodiscard]] GenericMap CreateGenericMap() const override
{
Expand Down Expand Up @@ -3299,6 +3333,8 @@ class GenericMapAdapter : public MapAccessorImpl<GenericMapAdapter<Holder>>
}

Holder<GenericMap> m_values;
// A JSON object (IJsonNode), which looks names up as a dict (docs/tasks/0370)
bool m_isDict;
// The render's copy of the map once the render wrote to it (docs/tasks/0049)
AutoCellLink m_link;
};
Expand Down
2 changes: 1 addition & 1 deletion src/internal_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ enum class MapAttrPolicy
// A Python dict (dict literals, kwargs, context mappings): dict methods first, as
// Python's getattr finds them before the items
MethodsFirst,
// A host object (reflected structs, JSON bindings): its keys, then dict methods
// A host object (reflected structs; a JSON view is a dict, MethodsFirst): its keys, then dict methods
KeysFirst,
// An object of a host language (IMapAccessor::GetHostObject, host_object.h): the object's
// own attributes, then its items, as Python's getattr; the engine adds no methods
Expand Down
27 changes: 26 additions & 1 deletion test/binding/conformance.h
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,25 @@ inline const std::vector<Row>& EqualityRows()
return rows;
}

inline const std::vector<Row>& DictMethodsRows()
{
static const std::vector<Row> rows = {
{ "shadowed_method", R"J2({"items":"own","keys":1,"name":"bob","o":{"get":"g"},"l":[{"name":"x"},{"v":1}]})J2",
R"J2({{ j.items is callable }}|{{ j.items()|list }}|{{ j['items'] }}|{{ j.o.get('get') }}|{{ j.keys()|list }})J2",
R"J2(True|[('items', 'own'), ('keys', 1), ('name', 'bob'), ('o', {'get': 'g'}), ('l', [{'name': 'x'}, {'v': 1}])]|own|g|['items', 'keys', 'name', 'o', 'l'])J2", R"J2(True|[('items', 'own'), ('keys', 1), ('l', [{'name': 'x'}, {'v': 1}]), ('name', 'bob'), ('o', {'get': 'g'})]|own|g|['items', 'keys', 'l', 'name', 'o'])J2", nullptr },
{ "attr_filter", R"J2({"items":"own","keys":1,"name":"bob","o":{"get":"g"},"l":[{"name":"x"},{"v":1}]})J2",
R"J2({{ j|attr('items') is callable }}|[{{ j|attr('name') }}]|{{ j.l|map('attr', 'name')|select('defined')|list }}|{{ j.l|map(attribute='name')|select('defined')|list }})J2",
R"J2(True|[]|[]|['x'])J2", nullptr, nullptr },
{ "tests", R"J2({"items":"own","keys":1,"name":"bob","o":{"get":"g"},"l":[{"name":"x"},{"v":1}]})J2",
R"J2({{ j is mapping }}|{{ j is sequence }}|{{ j is iterable }}|{{ j.o is sequence }})J2",
R"J2(True|True|True|True)J2", nullptr, nullptr },
{ "unpack_keys", R"J2({"p":{"x":1,"y":2}})J2",
R"J2({% set a, b = j.p %}{{ a }}{{ b }}|{% for k, v in j.p|dictsort %}{{ k }}{{ v }}{% endfor %})J2",
R"J2(xy|x1y2)J2", nullptr, nullptr },
};
return rows;
}

inline const std::vector<Row>& LifetimeRows()
{
static const std::vector<Row> rows = {
Expand Down Expand Up @@ -722,6 +741,12 @@ TYPED_TEST_P(JsonConformance, Equality)
}
}

// Task 0370: a JSON object is a dict in every mode, not a host object whose keys come before the dict methods
TYPED_TEST_P(JsonConformance, DictMethods)
{
jc::RunRows<TypeParam>(jc::DictMethodsRows(), jc::kAllModes);
}

TYPED_TEST_P(JsonConformance, LifetimeInTemplates)
{
// Task 0125: a callable returns a reflected document that nothing else holds; its items outlive the call
Expand Down Expand Up @@ -940,6 +965,6 @@ struct JsonBindingNames
}
};

REGISTER_TYPED_TEST_SUITE_P(JsonConformance, Scalars, BigIntegers, Nesting, KeyOrder, MissingKeysAndIndexes, EmptyContainers, NullMemberAndNullItem, Strings, IterationAndFilters, EnumeratorPastTheEnd, Equality, LifetimeInTemplates, LifetimeOfItemsWithoutEngine, ConcurrentRenders, CopyLimits, RepeatedKeys, DeepViewRendersOrFailsCleanly);
REGISTER_TYPED_TEST_SUITE_P(JsonConformance, Scalars, BigIntegers, Nesting, KeyOrder, MissingKeysAndIndexes, EmptyContainers, NullMemberAndNullItem, Strings, IterationAndFilters, EnumeratorPastTheEnd, Equality, DictMethods, LifetimeInTemplates, LifetimeOfItemsWithoutEngine, ConcurrentRenders, CopyLimits, RepeatedKeys, DeepViewRendersOrFailsCleanly);

#endif // JINJA2CPP_TEST_BINDING_CONFORMANCE_H
9 changes: 9 additions & 0 deletions test/binding/conformance_expect.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
EMPTY = '{"e":{},"a":[],"s":""}'
NULLS = '{"x":null,"l":[1,null,3],"o":{"n":null}}'
ITER = '{"l":[3,1,2],"w":["b","a","c"],"o":[{"n":"x","v":2},{"n":"y","v":1},{"n":"z","v":3}],"f":[1.5,0.5],"e":[]}'
DICT = '{"items":"own","keys":1,"name":"bob","o":{"get":"g"},"l":[{"name":"x"},{"v":1}]}'
EQUAL = '{"a":{"x":[1,2],"y":{"z":null}},"b":{"x":[1,2],"y":{"z":null}},"c":{"x":[1,3]},"l":[1,2]}'

# group -> [(row name, JSON document, template)]
Expand Down Expand Up @@ -126,6 +127,14 @@
("list", EQUAL, "{{ j.l == [1, 2] }}|{{ j.l == [2, 1] }}|{{ j.l == j.a.x }}|{{ j.a.x in [j.b.x] }}|{{ j.l != [1] }}"),
("literal", EQUAL, "{{ j.c == {'x': [1, 3]} }}|{{ j.a.y == {'z': none} }}"),
],
# A JSON object is a dict (docs/tasks/0370): its methods come before its keys, and getattr does not see the keys
"DictMethods": [
("shadowed_method", DICT, "{{ j.items is callable }}|{{ j.items()|list }}|{{ j['items'] }}|{{ j.o.get('get') }}|{{ j.keys()|list }}"),
("attr_filter", DICT,
"{{ j|attr('items') is callable }}|[{{ j|attr('name') }}]|{{ j.l|map('attr', 'name')|select('defined')|list }}|{{ j.l|map(attribute='name')|select('defined')|list }}"),
("tests", DICT, "{{ j is mapping }}|{{ j is sequence }}|{{ j is iterable }}|{{ j.o is sequence }}"),
("unpack_keys", '{"p":{"x":1,"y":2}}', "{% set a, b = j.p %}{{ a }}{{ b }}|{% for k, v in j.p|dictsort %}{{ k }}{{ v }}{% endfor %}"),
],
"Lifetime": [
("sort", LIFETIME_DOC, "{% set r = rjson().list | sort %}{{ r }}"),
("items_tojson", LIFETIME_DOC, "{{ rjson().obj.items()|list|tojson }}"),
Expand Down
38 changes: 38 additions & 0 deletions test/expressions_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,37 @@
#include <iostream>
#include <optional>
#include <string>
#include <unordered_map>
#include <vector>

using namespace jinja2;

namespace
{
// A reflected struct with fields named like dict methods (docs/tasks/0370)
struct ShadowingStruct
{
std::string items = "field";
std::string name = "bob";
};
} // namespace

namespace jinja2
{
template<>
struct TypeReflection<ShadowingStruct> : TypeReflected<ShadowingStruct>
{
static auto& GetAccessors()
{
static std::unordered_map<std::string, FieldAccessor> accessors = {
{ "items", [](const ShadowingStruct& obj) { return Reflect(obj.items); } },
{ "name", [](const ShadowingStruct& obj) { return Reflect(obj.name); } },
};
return accessors;
}
};
} // namespace jinja2

using ExpressionsMultiStrTest = BasicTemplateRenderer;

// clang-format off
Expand Down Expand Up @@ -289,6 +316,17 @@ TEST(ExpressionTest, MapKeyNamedLikeADictMethodUsesTheMethod)
EXPECT_EQ("True|True|1|2|3|get,items,keys|True|3", tpl.RenderAsString(params).value());
}

TEST(ExpressionTest, ReflectedStructFieldComesBeforeADictMethod)
{
// The fields of a reflected struct are its attributes, so they come before the dict methods; a JSON object viewed
// through Reflect is a dict instead (test/binding/conformance.h, DictMethods)
Template tpl;
ASSERT_TRUE(tpl.Load("{{ s.items }}|{{ s.items is callable }}|{{ s|attr('name') }}|{{ s is sequence }}"));
const ShadowingStruct data;
ValuesMap params = { { "s", Reflect(data) } };
EXPECT_EQ("field|False|bob|False", tpl.RenderAsString(params).value());
}

// A namespace, dict or list that holds itself is a reference cycle, which the render frees
// when it ends (docs/tasks/0110): the sanitizer configuration fails this test on a leak
TEST(ExpressionTest, SelfContainingContainers)
Expand Down
8 changes: 0 additions & 8 deletions test/parity/divergences/reflect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,10 @@
# (jinja2::ToValue). The host's data is never written, so cases that mutate the context through
# two reads of one JSON value diverge until a view has a stable identity (0049 P5, moved to 0212).
# Ids are "reflect.<case id>"; kinds are relative to Python. Format: test/parity/README.md.
# The 0370 lines are not mutation cases: a JSON object viewed through Reflect is looked up
# keys-first, as a reflected struct, where Python (and the copy) treat it as a dict.

reflect.filters.attr output 0370 a JSON view is keys-first: attr('name') finds the key, Python's getattr does not see keys
reflect.filters.map_attr_filter output 0370 a JSON view is keys-first: map('attr', 'name') finds the keys
reflect.methods.alias_item_then_owner output 0212 a JSON value read twice gives two accessors until 0049 P5's stable identity (moved to 0212): the write through messages[0] misses m
reflect.methods.alias_loop_var_kept output 0212 a JSON value read twice gives two accessors until 0049 P5's stable identity (moved to 0212): the write through the loop item misses messages
reflect.methods.alias_loop_var_update output 0212 a JSON value read twice gives two accessors until 0049 P5's stable identity (moved to 0212): the write through the loop item misses messages
reflect.methods.alias_owner_then_item output 0212 a JSON value read twice gives two accessors until 0049 P5's stable identity (moved to 0212): the write through m misses messages
reflect.methods.alias_set_item_kept output 0212 a JSON value read twice gives two accessors until 0049 P5's stable identity (moved to 0212): the write through nd['k'] misses a
reflect.methods.attr_dict_method_before_key output 0370 a JSON view is keys-first: the key items shadows the dict method for attr()
reflect.methods.dict_method_before_key rejects 0370 a JSON view is keys-first: dd.items is the key's string, so dd.items() is not callable
reflect.sequences.mapping_is_sequence output 0370 a JSON view is not a dict to `is sequence` (only methods-first maps are)
reflect.statements.for_pop_context_list output 0212 a JSON value read twice gives two accessors until 0049 P5's stable identity (moved to 0212): l.pop() misses the list the loop iterates
reflect.subscripts.dot_shadows_method output 0370 a JSON view is keys-first: d.items finds the key before the dict method
Loading