Skip to content

jinja : implement mixed type object keys #18955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
CISC merged 17 commits into from
Jan 27, 2026
Merged

jinja : implement mixed type object keys #18955

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
0cb1d45
implement mixed type object keys
CISC Jan 20, 2026
7c17e55
add tests
CISC Jan 20, 2026
b3fa217
refactor
CISC Jan 20, 2026
c929773
minor fixes
CISC Jan 20, 2026
b04e1fd
massive refactor
CISC Jan 25, 2026
eb8428a
add more tests
CISC Jan 25, 2026
0712934
forgotten tuples
CISC Jan 25, 2026
2c1c417
fix array/object is_hashable
CISC Jan 25, 2026
a620d88
correct (albeit broken) jinja responses
CISC Jan 25, 2026
bf83cd4
Merge branch 'master' into cisc/jinja-mixed-type-object-keys
CISC Jan 25, 2026
be80c62
improved hashing and equality
CISC Jan 26, 2026
77e51ec
refactor hash function
ngxson Jan 26, 2026
91774bf
more exhausive test case
ngxson Jan 26, 2026
fa1ef3f
clean up
ngxson Jan 27, 2026
80d0899
cont
ngxson Jan 27, 2026
8eccc41
cont (2)
ngxson Jan 27, 2026
2160c0b
missing cstring
ngxson Jan 27, 2026
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
42 changes: 11 additions & 31 deletions common/jinja/runtime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,20 +95,10 @@ value identifier::execute_impl(context & ctx) {
value object_literal::execute_impl(context & ctx) {
auto obj = mk_val<value_object>();
for (const auto & pair : val) {
value key_val = pair.first->execute(ctx);
if (!is_val<value_string>(key_val) && !is_val<value_int>(key_val)) {
throw std::runtime_error("Object literal: keys must be string or int values, got " + key_val->type());
}
std::string key = key_val->as_string().str();
value key = pair.first->execute(ctx);
value val = pair.second->execute(ctx);
JJ_DEBUG("Object literal: setting key '%s' with value type %s", key.c_str(), val->type().c_str());
JJ_DEBUG("Object literal: setting key '%s' with value type %s", key->as_string().str().c_str(), val->type().c_str());
obj->insert(key, val);

if (is_val<value_int>(key_val)) {
obj->val_obj.is_key_numeric = true;
} else if (obj->val_obj.is_key_numeric) {
throw std::runtime_error("Object literal: cannot mix numeric and non-numeric keys");
}
}
return obj;
}
Expand All @@ -127,9 +117,9 @@ value binary_expression::execute_impl(context & ctx) {
value right_val = right->execute(ctx);
JJ_DEBUG("Executing binary expression %s '%s' %s", left_val->type().c_str(), op.value.c_str(), right_val->type().c_str());
if (op.value == "==") {
return mk_val<value_bool>(value_compare(left_val, right_val, value_compare_op::eq));
return mk_val<value_bool>(*left_val == *right_val);
} else if (op.value == "!=") {
return mk_val<value_bool>(!value_compare(left_val, right_val, value_compare_op::eq));
return mk_val<value_bool>(!(*left_val == *right_val));
}

auto workaround_concat_null_with_str = [&](value & res) -> bool {
Expand Down Expand Up @@ -230,7 +220,7 @@ value binary_expression::execute_impl(context & ctx) {
auto & arr = right_val->as_array();
bool member = false;
for (const auto & item : arr) {
if (value_compare(left_val, item, value_compare_op::eq)) {
if (*left_val == *item) {
member = true;
break;
}
Expand Down Expand Up @@ -265,10 +255,9 @@ value binary_expression::execute_impl(context & ctx) {
}
}

// String in object
if (is_val<value_string>(left_val) && is_val<value_object>(right_val)) {
auto key = left_val->as_string().str();
bool has_key = right_val->has_key(key);
// Value key in object
if (is_val<value_object>(right_val)) {
bool has_key = right_val->has_key(left_val);
if (op.value == "in") {
return mk_val<value_bool>(has_key);
} else if (op.value == "not in") {
Expand Down Expand Up @@ -465,14 +454,8 @@ value for_statement::execute_impl(context & ctx) {
JJ_DEBUG("%s", "For loop over object keys");
auto & obj = iterable_val->as_ordered_object();
for (auto & p : obj) {
auto tuple = mk_val<value_array>();
if (iterable_val->val_obj.is_key_numeric) {
tuple->push_back(mk_val<value_int>(std::stoll(p.first)));
} else {
tuple->push_back(mk_val<value_string>(p.first));
}
tuple->push_back(p.second);
items.push_back(tuple);
auto tuple = mk_val<value_tuple>(p);
items.push_back(std::move(tuple));
}
if (ctx.is_get_stats) {
iterable_val->stats.used = true;
Expand Down Expand Up @@ -774,11 +757,8 @@ value member_expression::execute_impl(context & ctx) {
JJ_DEBUG("%s", "Accessing property on undefined object, returning undefined");
return val;
} else if (is_val<value_object>(object)) {
if (!is_val<value_string>(property)) {
throw std::runtime_error("Cannot access object with non-string: got " + property->type());
}
auto key = property->as_string().str();
val = object->at(key, val);
val = object->at(property, val);
if (is_val<value_undefined>(val)) {
val = try_builtin_func(ctx, key, object, true);
}
Expand Down
26 changes: 18 additions & 8 deletions common/jinja/runtime.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,18 @@ struct context {
}

value get_val(const std::string & name) {
auto it = env->val_obj.unordered.find(name);
if (it != env->val_obj.unordered.end()) {
return it->second;
} else {
return mk_val<value_undefined>(name);
}
value default_val = mk_val<value_undefined>(name);
return env->at(name, default_val);
}

void set_val(const std::string & name, const value & val) {
env->insert(name, val);
}

void set_val(const value & name, const value & val) {
env->insert(name, val);
}

void print_vars() const {
printf("Context Variables:\n%s\n", value_to_json(env, 2).c_str());
}
Expand Down Expand Up @@ -344,9 +344,19 @@ struct array_literal : public expression {
}
};

struct tuple_literal : public array_literal {
explicit tuple_literal(statements && val) : array_literal(std::move(val)) {}
struct tuple_literal : public expression {
statements val;
explicit tuple_literal(statements && val) : val(std::move(val)) {
for (const auto& item : this->val) chk_type<expression>(item);
}
std::string type() const override { return "TupleLiteral"; }
value execute_impl(context & ctx) override {
auto arr = mk_val<value_array>();
for (const auto & item_stmt : val) {
arr->push_back(item_stmt->execute(ctx));
}
return mk_val<value_tuple>(std::move(arr->as_array()));
}
};

struct object_literal : public expression {
Expand Down
17 changes: 17 additions & 0 deletions common/jinja/string.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,23 @@ size_t string::length() const {
return len;
}

size_t string::hash(std::optional<size_t> seed) const noexcept {
if (parts.empty()) {
return seed ? *seed : 0;
}
size_t hash;
if (seed) {
hash = *seed;
} else {
const auto & first = parts.at(0);
hash = hash_bytes(first.val.data(), first.val.length());
}
for (auto it = parts.begin() + 1; it != parts.end(); ++it) {
hash = hash_bytes(hash, it->val.data(), it->val.length());
}
return hash;
}

bool string::all_parts_are_input() const {
for (const auto & part : parts) {
if (!part.is_input) {
Expand Down
1 change: 1 addition & 0 deletions common/jinja/string.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ struct string {

std::string str() const;
size_t length() const;
size_t hash(std::optional<size_t> seed) const noexcept;
bool all_parts_are_input() const;
bool is_uppercase() const;
bool is_lowercase() const;
Expand Down
35 changes: 35 additions & 0 deletions common/jinja/utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <string>
#include <sstream>
#include <algorithm>
#include <limits>
Comment thread
CISC marked this conversation as resolved.
Resolved

namespace jinja {

Expand Down Expand Up @@ -46,4 +47,38 @@ static std::string fmt_error_with_source(const std::string & tag, const std::str
return oss.str();
}

// FNV-1a hash function that takes initial seed hash
// No need to worry about the whole hash_combine drama
// NOTE: **Never** pass a seed that is not a hash value originating from this function
static constexpr auto size_t_digits = std::numeric_limits<size_t>::digits;
static_assert(size_t_digits == 64 || size_t_digits == 32);

template <typename... Args>
static size_t hash_bytes(size_t seed, void const * bytes, size_t len, Args&&... args) noexcept
{
static_assert(sizeof...(args) % 2 == 0);
static constexpr size_t prime = size_t_digits == 64 ? 0x100000001b3 : 0x01000193;

unsigned char const * c = static_cast<unsigned char const *>(bytes);
unsigned char const * const e = c + len;

for (; c < e; ++c) {
seed = (seed ^ *c) * prime;
}

if constexpr (sizeof...(args) > 0) {
seed = hash_bytes(seed, std::forward<Args>(args)...);
}

return seed;
}

@ngxson ngxson Jan 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just thinking out loud in math terms: if we consider hash_bytes(seed, data, size) as a (surjective) function f(s, x) with s = seed and x = tuple(data, size)

given input data x0, x1, x2:

s = initial_seed
hash = f(f(f(s, x0), x1), x2)

and your "convenient" function hash_bytes(x0, x1, ...) can be defined as g:

g() = s
g(x0) = f(s, x0)
g(x0, x1) = f(f(s, x0), x1)
...

therefore, g is still surjective and g(x0, x1) != g(x1, x0) which is so far so good.

but now, consider: f(x0 ~ x1) == g(x0, x1) with ~ the "concatenation" operation: as long as x0 ~ x1 == x2 ~ x3, then g(x0, x1) == g(x2, x3), which is expected when calculating hash of string parts (where each part is different but concatenated version is the same). technically says, this make g no longer a good hash function, since we have a known set of collisions. but in our context this is acceptable. FNV-1a is not that good anyway.

(note: the property f(x0 ~ x1) == g(x0, x1) can be derived from the implementation and can be considered as a postulation in this context)

in practice, to prevent this, most hash functions use some kind of internal state such that: hash(s, x) = output(s, mix(x)), so hash(x0 ~ x1) != hash(x1 ~ x0). we can implement this easily but it's not really necessary. we are not doing cryptographic hash anyway.

end of the math part. now, in term of speed, what I think can be nice & fun to make hash_bytes to operate on blocks of data instead of one byte at a time. because one update depends on the prior state, CPU will have hard time with out-of-order execution. a simple vectorization should help a lot. just the case of x0 ~ x1 will be a bit complicated. I'll see how to do that.

@CISC CISC Jan 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FNV-1a certainly has its flaws, it was mainly chosen for its simplicity and chainability, it performs well enough in terms of quality and speed here I think.

Feel free to improve upon it if you wish though. :)

@ngxson ngxson Jan 26, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I ended up refactor this into a new struct called hasher(), it is inspired by nodejs's crypto.createHash()

// Old notion:
size_t seed = hash_bytes(data0, len0);
       seed = hash_bytes(data1, len1);
       // ...
size_t output = hash_bytes(seed, dataN, lenN);

// New notion
hasher hash = hasher().update(data0, len0).update(data1, len1); // ...
size_t output = hash.digest();

With this type of notion, we can in theory implement the notion of "internal state", though I won't add it because it's unnecessary (just mentioning here for completeness)

The new notion should have the exact same mathematical properties as the old one (reflected by the new test cases in test-jinja)

The result will be different from the old one though, because we are processing block of N bytes at once (on 64-bit system, we process 64/8 = 8 bytes). I expect most compilers will know how to vectorize it. If data is not a multiple of the block size, they will be buffered.

I'll do another pass tomorrow to see if there are any other clean up needed. In the meantime, feel free to review my last commit and lmk if anything seems off to you.


template <typename... Args>
static size_t hash_bytes(void const * bytes, size_t len, Args&&... args) noexcept
{
static constexpr size_t seed = size_t_digits == 64 ? 0xcbf29ce484222325 : 0x811c9dc5;

return hash_bytes(seed, bytes, len, std::forward<Args>(args)...);
}

} // namespace jinja
Loading
Loading