Repository navigation
Print builtin classes, loop bound methods and imported modules as Jinja2 does (0305, 0309) - #721
Conversation
range, dict, cycler, joiner and namespace print as their Python class
(<class 'range'>), in {{ }}, xmlattr, pprint and every str()/repr() site.
loop.cycle and loop.changed print as bound methods with their loop
(<bound method LoopContext.cycle of <LoopContext 1/2>>) through a cell
they share with the loop, which does not keep the loop alive. Files 0366
for the length after a break out of a lazily filtered loop.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7mSUNFU4HfnrEazAXX
{% import 'lib' as m %} binds an object of class TemplateModule: str()
is the module's top-level output (unescaped under autoescape, its
__html__), repr() is <TemplateModule 'lib'>, and its exports stay
attributes. pprint of a builtin class prints the class (0305). Files
0367 for m is escaped (testers.cpp, another group's file).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7mSUNFU4HfnrEazAXX
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7mSUNFU4HfnrEazAXX
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7mSUNFU4HfnrEazAXX
…l (0309) Found by the verifier: two imports of one shared module compared unequal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7mSUNFU4HfnrEazAXX
|
I checked this branch against current master (b4249a6) on a local merge, which was not pushed.
Generated by Claude Code |
|
CI status on 118d961:
@codex review 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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 118d961870
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
…309) Codex P2: forceescape returned the module's output unescaped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7mSUNFU4HfnrEazAXX
|
4d9e639 fixes the forceescape finding with new logic in the escaping path, so I'm asking for another pass. The security review I asked for earlier never started. @codex review Generated by Claude Code |
|
Codex Review: Didn't find any major issues. Chef's kiss. 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". |
Carries #721 (parity batch 2 group B, 0305/0309). Perf approved on this head (Render/dict_ops +0.60% named item). Verifier 1 round, Codex 1 P2 fixed; the requested security review never started.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DTQTriEsFrrBgcYNKDUUF4
Resolve the divergence allow-lists: #721 removed the 0305 lines and #728 removes the 0306/0310 lines. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DTQTriEsFrrBgcYNKDUUF4
Requested by Ruslan · project thread
Parity batch 2, group B.
Problem
range,dict,cycler,joinerandnamespaceare classes. Python prints them as<class 'range'>and<class 'jinja2.utils.Cycler'>, andxmlattrkeeps them. Jinja2C++ printed nothing for any callable except a macro. The same gap coveredloop.cycleandloop.changed, which Python prints as<bound method LoopContext.cycle of <LoopContext 1/2>>.pprintof a global printed<callable>.{% import 'lib' as m %}bound a plain dict, so{{ m }}andpprintprinted{'x': 1}. In Jinja2,str(m)is the module's output andrepr(m)is<TemplateModule 'lib'>.Resulting behaviour
Callable::SetReprgives each of the five globals its static class text (MakeClassin global_functions.cpp). It is not an attribute, sorange.namestays undefined.ValueRendererBaseprints the text, so it shows wherever a str or repr is taken:{{ }},~,%,format,pprint, inside containers, and inxmlattr.lipsumis a function in Python, whose repr names an address, so it still prints nothing.loop.cycleandloop.changedprint as bound methods, throughCallable::SetBoundMethod(self, method, receiver)with anIBoundReceiver.LoopCycleCellthat replacesLoopState::cyclePosition. It holds the position and a pointer to the loop while the loop's frame holds it. The cell is detached inRecycleand~LoopFrame, and keeps the last position and length after that.statements.loop_cycle_stored_filtered_break).loopdoes.import ... as mbinds aModuleMapAdapter(internal_value.cpp), an object of classTemplateModule.IMapAccessor::GetStrValue()holds the module's top-level output, whichImportedModule::Rendernow keeps.{{ m }}prints that output, unescaped under autoescape, as Jinja2's__html__gives it (markup.h).pprint,[m]andstringgive<TemplateModule 'lib'>.m|length,for k in m,m|list,m.items(),m|dictsortandm|tojsonnow fail, as they do in Jinja2.ImportedModule::MakeObject), so two imports of a shared module compare equal andsameas.calls.global_class_str,calls.global_class_xmlattr,statements.loop_cycle_string,filters.pprint_global_and_module.loader.module_object_*).ForLoopTest.BoundMethodAndClassRepr(narrow and wide) andImportTest.ModuleObjectPrintsAsTemplateModule(narrow, wide, autoescape).Filed follow-ups (each pinned by a divergence line)
loop.cyclekept past abreakout of a lazily filtered loop prints its length as?. Jinja2 iterates the rest of the loop to count it.m is escapedis False where Jinja2 says True, andjoinandxmlattrtakestr()of a module where Jinja2 takes__html__. The fix belongs in testers.cpp and filters.cpp, which group A owns.Overlaps
ImportStatement::ImportNamesin statements.cpp now takesconst ImportedModule&. Group C's 0299 uses the same signature but different lines in that function, so the train needs one manual merge there: C's namespace branch should end with= module.MakeObject(std::move(importedNs)).string_converter_filter.cpp(group A) gets one 4-line branch, in thesafe/striptags/forceescape__html__block. The allow-listscalls.txtandstatements.txtare shared with groups A and C; only this PR's own lines change.Validation
scripts/preflight.sh --perf: clang-format, clang-tidy 22.1.8, null comparisons, use_count, CodeQL pattern and task files all pass. The instruction gate passes; the largest change is Render/macros +0.32%, and Render/for_loop_vars is +0.03%.Process
forceescapeof a module returned its output unescaped; fixed instring_converter_filter.cpp's__html__branch, which is group A's file, a 4-line region next to the host-object branch). Round 2 on 4d9e639 was clean. The requested security review never started.Generated by Claude Code