Repository navigation
Parity batch 2, group A: *args/**kwargs in filters and tests, given undefined arguments, divisibleby of a str (0306, 0310, 0308) - #728
Conversation
…ivisibleby of a str 0306: a filter or test call with *args or **kwargs (a filter block too) is bound when it runs, as Python does: the parser keeps the call's arguments and UnpackingFilter/UnpackingTester make the filter or test from their values on each evaluation. FunctionBase binds either the Load-time expressions or those values (FunctionArgs, BoundArguments), and the filters that pass arguments on (map, select/reject, applymacro, format, environment filters and tests) keep them as ForwardedArgs. A call without unpacking is bound at Load as before. 0310: FunctionBase tells an argument passed as an undefined value from one left out (IsArgumentGiven, IsArgumentSet), and the filters that test an optional argument with `is not None` treat a given Undefined as Python does: trim, replace count, truncate end/leeway, urlize trim_url_limit, wordwrap wrapstring, the attribute of join/max/min/sort/sum/unique, sum's start and the fill_with of batch/slice. sum(start=None) now adds to None as Python does. 0308: `str is divisibleby n` formats the string with Python's %, raising its TypeErrors, and is False otherwise. Corpus: 40 new cases; six allow-list lines removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7oejcYgZdf6X4vtqUK
…t fixes
The verifier found that map('map', 'map', ...) nests a native call per argument with no
stack check; unpacked arguments made that a 40-byte template. Map::Filter now calls
CheckStack. truncate checks a StrictUndefined end where Python takes len(end), and reads
end once. FunctionBase::ParseParams no longer allocates a vector to check the bound
parameters; include and tidy fixes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7oejcYgZdf6X4vtqUK
CallArgsInfo extends CallParamsInfo, so a call that unpacks nothing is passed to the filter as it was parsed; the filter's arguments are parsed in place (assigning an expected swapped them); forwarded arguments are filled in place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7oejcYgZdf6X4vtqUK
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7oejcYgZdf6X4vtqUK
|
@codex review @codex security review (the change touches the expression parser and binds filter and test arguments at render time) Generated by Claude Code |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex security review Generated by Claude Code |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The perf track measured +1-2% on Render/filters, strings and config_file. - ForwardedArgs holds plain CallParamsInfo and the values of an unpacking call behind a pointer, so a call bound at Load visits no variant. - GetArgumentValue(name, context, given) reads whether the call passes the parameter in the same lookup as its value; IsArgumentSet takes that bit, so there is no second scan of the table. - ParseParams, and the Map and Tester constructors that Visit both kinds of arguments, move out of filters.cpp (function_base.cpp, filter_factories.cpp): there they changed GCC's inlining of Join and Map. Every Render row is within +0.2% of base in callgrind; Load is at or below. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7oejcYgZdf6X4vtqUK
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7oejcYgZdf6X4vtqUK
|
@codex review 696c314 reworks the render path for performance. ForwardedArgs no longer uses a variant, the given bit is read in the same lookup as the value, and ParseParams and the Map/Tester constructors move out of filters.cpp. Generated by Claude Code |
|
@codex review The previous run on 696c314 failed before it completed, so this is a retry. Generated by Claude Code |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DTQTriEsFrrBgcYNKDUUF4
Requested by Ruslan · project thread
Closes tasks 0306, 0310 and 0308 (left
in-progressfor the steward's train).Problem
*args/**kwargsin filter calls, test calls and{% filter %}blocks ('a'|replace(*['a','b'])), because it binds them at call time. Jinja2C++ binds filter and test arguments at Load, one expression node per parameter, so it rejected unpacking withUnexpected token.'xyx'|trim(nope)stripped whitespace, while Python raises a TypeError. A sweep of every optional argument of every builtin against Python 3.12 / Jinja2 3.1.6 found 15 such arguments.'a' is divisibleby 2answered False. Python raises, because the test isvalue % num == 0andstr % nis printf formatting.What changes
ParseCallArgs.filters::UnpackingFilter/testers::UnpackingTester. On each evaluation these evaluate the spliced arguments (helpers::EvaluateCallParams, which raises Python's errors for a non-iterable*xor a non-mapping**x) and build the filter or test from those values. A call that does not fit the parameters is reported the same way as a written-out call.FunctionBasebinds either the Load-time nodes or the values:FunctionArgsis a view overCallParamsInfoorCallParams.BoundArgumentshas a value mode.ForwardedArgsholds the extras thatmap,select/reject,applymacro,formatand environment filters and tests pass on.map(*['replace','a','z']),select('divisibleby', *[3])andformat(*args)work too.CallArgsInfonow derives fromCallParamsInfo, so the parsed arguments are passed through without a copy.FunctionBase::GetArgumentValue(name, context, given)reports whether the call passes the parameter, andIsArgumentSet(Python'sis not None) uses that. They cover:charsand replacecount: TypeError;end/leeway, urlizetrim_url_limitand wordwrapwrapstring: UndefinedError where Python uses the value;attributeof join/max/min/sort/sum/unique: becomes an[Undefined]key path;start: given values are used as is, sostart=Nonenow raises as in Python;fill_with: fills with the Undefined.PythonPercentFormat, which raises Python's TypeErrors, and otherwise answers False.map('map', 'map', …)nests one native call per argument with no stack check. Unpacking made that reachable from a 40-byte template or from context data.Map::Filternow callsCheckStack(), with a regression test inrecursion_limits_test.cpp.endwhere Python takeslen(end).Files outside the batch brief's ownership:
function_base.cpp(new) andCMakeLists.txtserialize_filters.cpp:StringFormatonly, soformattakesForwardedArgs.expression_evaluator.h:CallArgsInfonow derives fromCallParamsInfo.filters.handtesters.h.The batch lead and the steward know about these.
Validation
generate.py --checkclean). Six allow-list lines are removed (calls 4, filters 1, tests 1).ctestpasses 5/5.acc3129, which includes the chat-template suite from test: Hugging Face chat template suite against transformers (0360 phase 1) #720,ctestpasses 6/6,jinja2cpp_chat_templatesincluded. No chat-template divergence line went stale. That merge was not pushed.scripts/preflight.sh --perf: every check passes, including the instruction gate.Load/filtersandLoad/strings. After theCallArgsInfoand in-place changes, Load is at or below baseline.ForwardedArgsis now a plainCallParamsInfowith the unpacking values behind a pointer, so there is no variant visit. Whether an argument was given is read in the same lookup as its value, so the table is no longer scanned twice.ParseParamsand theMap/Testerconstructors moved out offilters.cpp, because there they had changed GCC's inlining ofJoinandMap(found with callgrind per-function diffs andnm -S).jinja2cpp_tests, 8378 tests, with no reports. About 340 Python-oracle probes, narrow and wide, under every undefined policy. The perf rework in 696c314 was not run through the verifier again. It changes no behaviour, andctest(corpus included) and preflight pass on it.Remaining limits
divisiblebywith a huge%width hits the existing 1 MiB formatting cap, whichformatshares, so it does not raise MemoryError.m is escapedof an imported module) is left for after B's module repr lands.Generated by Claude Code