Python 3.14 text signatures, sizeof, and SyntaxError offsets - #1149
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change adds Python 3.14 parity coverage and runtime support for builtin signatures, syntax-error locations, memoryview ordering, frame locals, ChangesTranslator pointer types
Python 3.14 runtime parity
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant memoryview_tobytes
participant BufferView
Caller->>memoryview_tobytes: call tobytes(order)
memoryview_tobytes->>memoryview_tobytes: validate C, F, or A
memoryview_tobytes->>BufferView: gather_order(fortran)
BufferView-->>memoryview_tobytes: ordered bytes
memoryview_tobytes-->>Caller: bytes result
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`run.py` passes a script only when its last non-empty stdout line equals "OK"; a script that exits 0 with any other final line is reported as "exited 0 without a final 'OK' line". The 25 scripts added on this branch each closed with a descriptive sentence instead, so all three runners (CPython, dynasm, cranelift) failed them. Assisted-by: Claude
![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: 548eda3313
ℹ️ 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".
| // the Python-3.14 surface additions where the project's stated version | ||
| // target takes precedence over the PyPy TypeDef. | ||
| let mut table_size = 8usize; | ||
| while (table_size << 1) / 3 < items.len() { |
There was a problem hiding this comment.
When entries are deleted without calling clear(), the allocated dictionary table is retained, but this loop reconstructs its size solely from the current number of live items. For example, CPython 3.14 keeps d.__sizeof__() at 36936 after creating 1,000 entries and popping 999 of them, whereas this implementation reports the single-entry size; an emptied dictionary reached through repeated pop is also incorrectly handled as the shared empty-table case above. Track the backing table's allocation state rather than deriving it from items.len().
Useful? React with 👍 / 👎.
| // is four times `used` (twice above 50,000), rounded up to a power of two. | ||
| let mut table_size = 8usize; | ||
| let mut threshold = ((table_size - 1) * 3).div_ceil(5); | ||
| while len >= threshold { |
There was a problem hiding this comment.
For a set or frozenset that once contained many elements and was then reduced with remove()/pop(), CPython retains the large allocated table, while this calculation chooses a table using only the current length. For example, reducing a 1,000-element set to one element leaves __sizeof__() at 32968 in CPython 3.14, but this code reports the one-element inline-table size of 200. The implementation needs the backing set's retained capacity/history rather than w_set_len() alone.
Useful? React with 👍 / 👎.
| if bytes[prefix..quote] | ||
| .iter() | ||
| .any(|byte| byte.eq_ignore_ascii_case(&b'f')) | ||
| && (prefix..=token_head_end).contains(&raw_index) |
There was a problem hiding this comment.
When an unterminated ordinary string immediately follows an alphabetic name containing f, this predicate misclassifies the name as an f-string prefix. For example, compile('foo"', '<x>', 'exec') should report unterminated string literal, but the raw location is at the token head boundary and foo satisfies this any('f') test, causing the message below to become unterminated f-string literal. Accept only valid complete f-string prefixes such as f, fr, and rf instead of searching an arbitrary identifier run.
Useful? React with 👍 / 👎.
| // PyPy's W_TextIOWrapper.isatty_w delegates to its live buffer, which in | ||
| // turn delegates to the raw descriptor. Do not install an instance | ||
| // override here: after forkpty changes fd 0 into the slave terminal, the | ||
| // type method must observe that new descriptor state. |
There was a problem hiding this comment.
When make_std_stream cannot open the binary descriptor—most notably in the sandbox path documented earlier in this function—it deliberately stores None as buffer; removing the instance override then makes TextIOWrapper.isatty() delegate to None.isatty() and raise AttributeError instead of returning False. Ensure these retained standard-stream objects have a valid buffer/delegation owner before relying exclusively on the type method.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 238629c). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptations
|
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/ae7d5b244bba2c6e05e0582278d4a59d32189990/pyre-interpreter/src/pyframe.rs#L2574-L2576
When an optimized frame's locals proxy is read, w_dict_new() is a GC allocation after getdictscope() returns the raw w_locals pointer. If that allocation triggers the moving collector, the pointer passed to dict_update_value is stale; the newly created snapshot is also not pinned for the copy. This can corrupt or crash frame.f_locals operations under GC pressure, so pin and reload both objects as the equivalent snapshot paths immediately above and below do.
https://github.com/youknowone/pyre/blob/ae7d5b244bba2c6e05e0582278d4a59d32189990/pyre-interpreter/src/pyframe.rs#L2594-L2599
In a module/class comprehension whose iteration variable is captured, such as [(locals().copy(), lambda: x) for x in range(1)], MAKE_CELL leaves the hidden slot holding a cell while the comprehension runs. This overlay inserts that cell object itself, so locals()['x'] exposes a cell rather than 0; read w_cell_get for hidden cell slots just as fast2locals does.
AGENTS.md reference: AGENTS.md:L231-L233
https://github.com/youknowone/pyre/blob/ae7d5b244bba2c6e05e0582278d4a59d32189990/pyre-interpreter/src/builtins.rs#L11560-L11562
Because malformed_unicode_name_escape scans the entire source for every compile failure, a malformed escape after an earlier syntax error replaces the earlier diagnostic. For example, compiling 1 +\nx = "\\N"\n should report invalid syntax on line 1, but this branch changes the message to the line-2 Unicode error while retaining the line-1 location. Only apply the rewrite when the parser's selected error span is the malformed string token.
AGENTS.md reference: AGENTS.md:L231-L233
https://github.com/youknowone/pyre/blob/ae7d5b244bba2c6e05e0582278d4a59d32189990/pyre-interpreter/src/builtins.rs#L10863-L10866
When the reported nonlocal error occurs inside an async def, this scope search does not descend into AsyncFunctionDef, and the visitor below also does not stop at its body. Thus def outer(): global x; async def inner(): nonlocal x is incorrectly rewritten as name 'x' is nonlocal and global at the outer declaration; Python 3.14 reports no binding for nonlocal 'x' found at the inner declaration. Handle async functions wherever synchronous function scopes are handled.
AGENTS.md reference: AGENTS.md:L231-L233
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/extra_tests/parity_tests/complex_text_signatures_python314.py`:
- Around line 6-20: Replace the VALUE_BINARY and SELF_ONLY constant-valued dict
comprehensions with dict.fromkeys, preserving their existing key collections and
mapped signature strings.
In `@pyre/extra_tests/parity_tests/dict_set_sizeof_python314.py`:
- Around line 7-16: Guard the size assertions in
pyre/extra_tests/parity_tests/dict_set_sizeof_python314.py lines 7-16 with a
sys.maxsize > 2**32 check, since all eight expected values are 64-bit-specific.
In pyre/extra_tests/parity_tests/tuple_text_signatures_python314.py lines 33-35,
replace the hardcoded 64-bit maximum with an f-string that uses sys.maxsize when
constructing the expected signature.
In `@pyre/extra_tests/parity_tests/memoryview_tobytes_order_python314.py`:
- Around line 45-50: Update the tobytes duplicate-argument handling exercised by
memoryview.tobytes so a positional argument plus the keyword order raises
“argument for tobytes() given by name ('order') and position (1)” before
clinic_arity performs its arity check. Adjust the fixture assertion in the
duplicate-order case while preserving the failure expectation.
In `@pyre/extra_tests/parity_tests/script_source_encoding_startup.py`:
- Around line 15-22: Set PYTHONIOENCODING to UTF-8 in the subprocess environment
used by the run around subprocess.run, preserving the existing environment
variables, so result.stdout.decode() consistently validates the intended output
independent of the runner locale.
In `@pyre/extra_tests/parity_tests/syntax_error_python314_offsets.py`:
- Around line 95-102: Replace the mojibake literal in the syntax_error case with
six literal ¢ characters, matching the upstream CPython input. Recompute the
expected error offset tuple for the corrected character count while preserving
the existing syntax-error assertion and character-column coverage.
In `@pyre/pyre-interpreter/src/builtins.rs`:
- Around line 11560-11646: Refactor the diagnostic selection flow around the
visible unicode, comment, delimiter, f-string, scope, generator, assignment, and
prefix detectors so each replacement message is stored together with its
corresponding span. Select one winning detector result according to explicit
precedence, then derive both the final msg and diagnostic_span from that same
result, preserving the existing message transformations while preventing
unrelated span/message combinations.
- Around line 11560-11569: In compile_err_to_syntax_error_maybe_incomplete,
couple each diagnostic message with its corresponding span and select one
detector result as a single (message, span) winner, eliminating the independent
precedence chains around malformed_unicode_name_escape and
fstring_mismatched_delimiter. At lines 11560-11569, either gate the
malformed_unicode_name_escape message override on the parser error shape or
return its literal byte range; at lines 11640-11646, use the selected pair for
both the reported message and diagnostic_span.
- Around line 10756-10767: Update source_byte_location to clamp byte_index not
only to source.len() but also to the nearest preceding UTF-8 character boundary
before slicing source. Preserve the existing line and column calculations while
ensuring an interior multi-byte index cannot panic.
- Around line 2596-2606: Update the classmethod setup for __class_getitem__ and
_from_flags so their __text_signature__ is forwarded through the classmethod
wrapper, not stored only on __func__. Modify the relevant
w_classmethod_new/classmethod type behavior and preserve the signatures already
assigned by fset_func_text_signature, ensuring parity lookups on the wrapped
methods do not raise AttributeError.
In `@pyre/pyre-interpreter/src/error.rs`:
- Around line 638-642: Update decode_source_bytes and the surrounding
error-position handling so a genuine end_offset of 0 is preserved while
location-less codec failures remain absent. Represent the end position with a
distinct optional value, and convert only None to the Python None object; do not
apply wrap_pos to both zero-valued cases. Keep the existing numeric handling for
start offsets unchanged.
In `@pyre/pyre-interpreter/src/module/posix/interp_posix.rs`:
- Around line 5970-5976: Update the cfg attribute guarding the real forkpty
registration in register_module to require both Unix targets and a non-sandbox
build, using cfg(all(unix, not(feature = "sandbox"))). Leave the earlier noop
registration unchanged.
In `@pyre/pyre-interpreter/src/module/sys/vm.rs`:
- Around line 3205-3208: Update the stream initialization logic around
builtin_open and W_TextIOWrapper so that instances with buffer set to None
receive an isatty override returning False, while buffered streams retain
type-method delegation to the live buffer. Preserve forkpty descriptor updates
by avoiding the override whenever a buffer exists.
In `@pyre/pyre-interpreter/src/pyframe.rs`:
- Around line 108-158: Root all raw PyObjectRef values that cross allocation or
user-code calls in the frame-locals proxy paths. In
pyre/pyre-interpreter/src/pyframe.rs:108-158, update fast_local_index and
setitem_value to pin key, reload it after w_str_new and eq_w, and reload
key/value before slot writes and backing setitem; in
pyre/pyre-interpreter/src/pyframe.rs:280-293, pin incoming across call_method
and reload each key/value before setitem_value; in
pyre/pyre-interpreter/src/pyframe.rs:2572-2578, use push_roots for w_locals and
the snapshot dict and access them through shadow_stack_get around
dict_update_value.
- Line 215: The FrameLocalsProxy removal paths misclassify bound hidden locals
as backing-namespace keys. In pyre/pyre-interpreter/src/pyframe.rs:215-215,
update __delitem__ to reject bound hidden locals with the same “cannot remove
local variables from FrameLocalsProxy” error as fast locals; in
pyre/pyre-interpreter/src/pyframe.rs:324-331, apply the identical classification
in pop before invoking the backing mapping’s pop, reusing the shared
hidden-local detection logic.
In `@pyre/pyre-interpreter/src/typedef.rs`:
- Around line 6275-6289: Validate both __sizeof__ receiver paths before reading
storage: at pyre/pyre-interpreter/src/typedef.rs:6275-6289, replace the
debug_assert! around resolve_dict_backing with a real null check that returns
basicsize before w_dict_items; at
pyre/pyre-interpreter/src/typedef.rs:25142-25150, call require_set_receiver or
require_frozenset_receiver before w_set_len unless setlike_method_gateways!
demonstrably performs that validation.
- Around line 11783-11803: Remove the __instancecheck__ and __subclasscheck__
entries from the signature-stamping loop in init_type_type, and delete the
earlier duplicate carrier definitions and signature stamps for these names.
Preserve the replacement carriers created later with their existing
parameter-name spelling so each signature is installed only once.
- Around line 4353-4365: Update make_functional_method so __setstate__ is
created without a text signature, matching its METH_O definition; preserve the
existing signatures for other functional methods and do not merely rename the
object parameter.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c59f63e1-5ad8-44b5-bd8a-a0a43c91efdc
📒 Files selected for processing (39)
majit/majit-translate/src/codewriter/assembler.rsmajit/majit-translate/src/codewriter/call.rspyre/extra_tests/parity_tests/bool_text_signatures_python314.pypyre/extra_tests/parity_tests/builtin_text_signatures_python314.pypyre/extra_tests/parity_tests/bytearray_text_signatures_python314.pypyre/extra_tests/parity_tests/bytes_text_signatures_python314.pypyre/extra_tests/parity_tests/complex_text_signatures_python314.pypyre/extra_tests/parity_tests/dict_set_sizeof_python314.pypyre/extra_tests/parity_tests/dict_text_signatures_python314.pypyre/extra_tests/parity_tests/float_text_signatures_python314.pypyre/extra_tests/parity_tests/functional_iterator_text_signatures_python314.pypyre/extra_tests/parity_tests/int_text_signatures_python314.pypyre/extra_tests/parity_tests/list_text_signatures_python314.pypyre/extra_tests/parity_tests/memoryview_text_signatures_python314.pypyre/extra_tests/parity_tests/memoryview_tobytes_order_python314.pypyre/extra_tests/parity_tests/method_wrapper_text_signatures_python314.pypyre/extra_tests/parity_tests/ordered_dict_python314.pypyre/extra_tests/parity_tests/property_text_signatures_python314.pypyre/extra_tests/parity_tests/range_text_signatures_python314.pypyre/extra_tests/parity_tests/script_source_encoding_startup.pypyre/extra_tests/parity_tests/set_text_signatures_python314.pypyre/extra_tests/parity_tests/slice_text_signatures_python314.pypyre/extra_tests/parity_tests/str_text_signatures_python314.pypyre/extra_tests/parity_tests/super_text_signatures_python314.pypyre/extra_tests/parity_tests/syntax_error_python314_offsets.pypyre/extra_tests/parity_tests/tuple_text_signatures_python314.pypyre/extra_tests/parity_tests/type_text_signatures_python314.pypyre/pyre-interpreter/src/builtins.rspyre/pyre-interpreter/src/compile.rspyre/pyre-interpreter/src/error.rspyre/pyre-interpreter/src/module/_collections/app_odict.pypyre/pyre-interpreter/src/module/posix/interp_posix.rspyre/pyre-interpreter/src/module/sys/vm.rspyre/pyre-interpreter/src/pycode.rspyre/pyre-interpreter/src/pyframe.rspyre/pyre-interpreter/src/type_methods.rspyre/pyre-interpreter/src/typedef.rspyre/pyre-object/src/bufferview.rspyre/pyrex/src/lib.rs
| VALUE_BINARY = { | ||
| name: "($self, value, /)" | ||
| for name in ( | ||
| "__lt__", "__le__", "__eq__", "__ne__", "__gt__", "__ge__", | ||
| "__add__", "__radd__", "__sub__", "__rsub__", "__mul__", "__rmul__", | ||
| "__truediv__", "__rtruediv__", | ||
| ) | ||
| } | ||
| SELF_ONLY = { | ||
| name: "($self, /)" | ||
| for name in ( | ||
| "__repr__", "__hash__", "__neg__", "__pos__", "__abs__", "__bool__", | ||
| "conjugate", "__complex__", "__getnewargs__", | ||
| ) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Replace the constant-valued dict comprehensions with dict.fromkeys.
Ruff reports C420 on both comprehensions. Each maps every key to the same constant string.
♻️ Proposed change
-VALUE_BINARY = {
- name: "($self, value, /)"
- for name in (
+VALUE_BINARY = dict.fromkeys(
+ (
"__lt__", "__le__", "__eq__", "__ne__", "__gt__", "__ge__",
"__add__", "__radd__", "__sub__", "__rsub__", "__mul__", "__rmul__",
"__truediv__", "__rtruediv__",
- )
-}
-SELF_ONLY = {
- name: "($self, /)"
- for name in (
+ ),
+ "($self, value, /)",
+)
+SELF_ONLY = dict.fromkeys(
+ (
"__repr__", "__hash__", "__neg__", "__pos__", "__abs__", "__bool__",
"conjugate", "__complex__", "__getnewargs__",
- )
-}
+ ),
+ "($self, /)",
+)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| VALUE_BINARY = { | |
| name: "($self, value, /)" | |
| for name in ( | |
| "__lt__", "__le__", "__eq__", "__ne__", "__gt__", "__ge__", | |
| "__add__", "__radd__", "__sub__", "__rsub__", "__mul__", "__rmul__", | |
| "__truediv__", "__rtruediv__", | |
| ) | |
| } | |
| SELF_ONLY = { | |
| name: "($self, /)" | |
| for name in ( | |
| "__repr__", "__hash__", "__neg__", "__pos__", "__abs__", "__bool__", | |
| "conjugate", "__complex__", "__getnewargs__", | |
| ) | |
| } | |
| VALUE_BINARY = dict.fromkeys( | |
| ( | |
| "__lt__", "__le__", "__eq__", "__ne__", "__gt__", "__ge__", | |
| "__add__", "__radd__", "__sub__", "__rsub__", "__mul__", "__rmul__", | |
| "__truediv__", "__rtruediv__", | |
| ), | |
| "($self, value, /)", | |
| ) | |
| SELF_ONLY = dict.fromkeys( | |
| ( | |
| "__repr__", "__hash__", "__neg__", "__pos__", "__abs__", "__bool__", | |
| "conjugate", "__complex__", "__getnewargs__", | |
| ), | |
| "($self, /)", | |
| ) |
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 6-13: Unnecessary dict comprehension for iterable; use dict.fromkeys instead
Replace with dict.fromkeys(iterable))
(C420)
[warning] 14-20: Unnecessary dict comprehension for iterable; use dict.fromkeys instead
Replace with dict.fromkeys(iterable))
(C420)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/extra_tests/parity_tests/complex_text_signatures_python314.py` around
lines 6 - 20, Replace the VALUE_BINARY and SELF_ONLY constant-valued dict
comprehensions with dict.fromkeys, preserving their existing key collections and
mapped signature strings.
Source: Linters/SAST tools
| assert dict().__sizeof__() == 48 | ||
| assert {0: None}.__sizeof__() == 208 | ||
| assert {str(i): None for i in range(6)}.__sizeof__() == 256 | ||
| assert dict.fromkeys(range(11)).__sizeof__() == 616 | ||
|
|
||
| for typ in (set, frozenset): | ||
| assert typ().__sizeof__() == 200 | ||
| assert typ(range(4)).__sizeof__() == 200 | ||
| assert typ(range(5)).__sizeof__() == 712 | ||
| assert typ(range(19)).__sizeof__() == 2248 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Two parity fixtures hardcode 64-bit-specific integers. Both encode values that depend on the pointer width, so they fail on a 32-bit build and the runner attributes the failure to pyre rather than to the platform.
pyre/extra_tests/parity_tests/dict_set_sizeof_python314.py#L7-L16: the eight byte counts derive fromsize_of::<usize>() == 8; guard the block with asys.maxsize > 2**32check.pyre/extra_tests/parity_tests/tuple_text_signatures_python314.py#L33-L35:9223372036854775807is the 64-bitsys.maxsize; build the expected string with an f-string oversys.maxsize.
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 7-7: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
📍 Affects 2 files
pyre/extra_tests/parity_tests/dict_set_sizeof_python314.py#L7-L16(this comment)pyre/extra_tests/parity_tests/tuple_text_signatures_python314.py#L33-L35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/extra_tests/parity_tests/dict_set_sizeof_python314.py` around lines 7 -
16, Guard the size assertions in
pyre/extra_tests/parity_tests/dict_set_sizeof_python314.py lines 7-16 with a
sys.maxsize > 2**32 check, since all eight expected values are 64-bit-specific.
In pyre/extra_tests/parity_tests/tuple_text_signatures_python314.py lines 33-35,
replace the hardcoded 64-bit maximum with an f-string that uses sys.maxsize when
constructing the expected signature.
| try: | ||
| view.tobytes("C", order="F") | ||
| except TypeError as error: | ||
| assert str(error) == "tobytes() takes at most 1 argument (2 given)" | ||
| else: | ||
| raise AssertionError("duplicate order accepted") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
CPython Argument Clinic error "given by name" and "position" _PyArg_UnpackKeywords duplicate argument message
💡 Result:
The error message "duplicate argument" or "given by name and position" in the context of CPython's Argument Clinic and the underlying _PyArg_UnpackKeywords function indicates a violation of Python's argument passing rules [1][2]. Specifically, this occurs when a function is called with a value for the same parameter both positionally and as a keyword, or when a keyword is provided multiple times [2]. _PyArg_UnpackKeywords is a low-level helper used by Argument Clinic for functions with METH_FASTCALL | METH_KEYWORDS signatures [1]. It is designed to map positional and keyword arguments into a flat, linearized array of values corresponding to the function's parameters [1]. The internal logic of this function follows a strict three-pass structure to guarantee correctness: 1. Positional arguments are assigned to the leading slots in the array [1]. 2. Keyword arguments fill the remaining named slots in the array [1]. 3. Any provided keyword that does not match the function's signature (or if a duplicate is detected during the mapping process) results in a TypeError [3][1]. When you encounter this error in code generated by Argument Clinic, it typically means the input provided by the Python-level caller is invalid [2]. If you believe the error is being raised incorrectly by your own C extension or CPython modification, ensure that: - The argument list (kwnames) passed to _PyArg_UnpackKeywords does not contain duplicates [1]. - The kwlist (the array of expected argument names) is correctly defined and matches the function's parameters [1]. - There is no overlap between the positional arguments (defined by min_pos and max_pos) and the keyword arguments provided by the caller [1]. For debugging purposes, recent versions of CPython have improved error reporting for these cases, often providing clearer diagnostic messages when an unexpected or duplicate keyword argument is encountered [3]. If you are developing an extension, double-check that your Argument Clinic configuration (e.g., in your .c file or the clinic/ directory) accurately reflects the intended positional/keyword constraints [4][5].
Citations:
- 1: https://gopy.tamnd.com/docs/annotations/python/getargs
- 2: https://deepsource.com/directory/python/issues/PYL-E1132
- 3: fedora-python/cpython@17689e3
- 4: https://peps.python.org/pep-0436/
- 5: https://devguide.python.org/development-tools/clinic/howto/
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- fixture ---'
cat -n pyre/extra_tests/parity_tests/memoryview_tobytes_order_python314.py
printf '%s\n' '--- related implementation and tests ---'
rg -n -C 5 'memoryview_tobytes|clinic_arity|tobytes\(\).*takes at most|given by name|duplicate order' .
printf '%s\n' '--- repository files mentioning CPython 3.14 or memoryview.tobytes ---'
rg -n -C 3 'memoryview\.tobytes|tobytes\("C"|python314|Python 3\.14' pyre .github 2>/dev/null || trueRepository: youknowone/pyre
Length of output: 50372
🌐 Web query:
site:github.com/python/cpython "argument for tobytes() given by name ('order') and position (1)"
💡 Result:
The tobytes method exists in multiple Python objects, most notably memoryview and array.array, but they have different signatures regarding the 'order' parameter. 1. memoryview.tobytes: As of Python 3.8, memoryview.tobytes accepts an optional 'order' parameter [1][2]. The signature is tobytes(order='C', /) [1][3]. This parameter controls the memory layout of the returned byte string, accepting values 'C' (C-style, row-major), 'F' (Fortran-style, column-major), or 'A' (retain existing order if contiguous, otherwise use 'C') [1][4][3]. Because the 'order' parameter is defined as positional-or-keyword in the method signature (indicated by the slash / in the signature tobytes(order='C', /)), it can indeed be provided as a positional argument [1][3]. In this context, 'order' is at position 1 [1][3]. 2. array.array.tobytes: In contrast, the tobytes method for array.array objects does not accept any arguments [5][6]. Its signature is simply tobytes [5]. Consequently, attempting to pass an 'order' argument—either by name or by position—to array.array.tobytes will raise a TypeError. In summary, the argument 'order' at position 1 is a valid feature specifically for memoryview.tobytes [1][3], whereas it is not a valid parameter for array.array.tobytes [5][6].
Citations:
- 1: https://github.com/python/cpython/blob/0274d830/Objects/clinic/memoryobject.c.h
- 2: https://github.com/python/cpython/blob/master/Misc/NEWS.d/3.8.0a1.rst
- 3: python/cpython@80a5036
- 4: https://github.com/python/cpython/blob/master/Lib/test/test_buffer.py
- 5: https://github.com/python/cpython/blob/3.11/Doc/library/array.rst
- 6: https://github.com/python/cpython/blob/main/Doc/library/array.rst
🌐 Web query:
site:github.com/python/cpython memoryview tobytes Argument Clinic 3.14 clinic_arity
💡 Result:
In CPython, memoryview.tobytes was converted to use Argument Clinic to improve argument parsing performance and code maintainability [1][2]. Regarding your query about clinic_arity: Argument Clinic does not typically expose a direct parameter or attribute named clinic_arity for user-facing configuration in the generated code [3][4]. Instead, Argument Clinic automatically determines the arity (number of arguments) of a function based on the parameter definitions provided in the clinic block (e.g., [clinic start... clinic end]) [1][5][4]. It then generates the appropriate C code—such as METH_FASTCALL or METH_KEYWORDS method definitions—and handles the parsing logic, including minimum and maximum argument counts, internally [1][4]. For memoryview.tobytes, the conversion resulted in code that correctly handles the optional order argument using Argument Clinic's generated parsing logic [6][1]. The implementation uses METH_FASTCALL|METH_KEYWORDS, and Argument Clinic handles the extraction and validation of the order argument (str or None) before calling the implementation function memoryview_tobytes_impl [6][1].
Citations:
- 1: python/cpython@80a5036
- 2: Convert memoryview to Argument Clinic python/cpython#85434
- 3: https://github.com/python/cpython/blob/3.10/Doc/howto/clinic.rst
- 4: https://github.com/python/cpython/blob/36e4ffc1/Tools/clinic/libclinic/parse_args.py
- 5: https://github.com/python/cpython/blob/0274d830/Tools/clinic/libclinic/function.py
- 6: https://github.com/python/cpython/blob/0274d830/Objects/clinic/memoryobject.c.h
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target fixture ---'
cat -n pyre/extra_tests/parity_tests/memoryview_tobytes_order_python314.py | sed -n '1,90p'
printf '%s\n' '--- exact repository matches ---'
rg -n -C 8 --glob '*.rs' --glob '*.py' --glob '*.c' --glob '*.h' \
'memoryview_tobytes|clinic_arity|given by name|duplicate order|tobytes\(\) takes at most' . \
| head -n 300Repository: youknowone/pyre
Length of output: 27174
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import sys
try:
view = memoryview(b"abc")
for call in (
lambda: view.tobytes("C", order="F"),
lambda: view.tobytes(order="F"),
lambda: view.tobytes("F"),
):
try:
print("result:", call())
except BaseException as exc:
print(type(exc).__name__ + ":", str(exc))
except BaseException as exc:
print("runtime setup:", type(exc).__name__ + ":", str(exc))
print("python:", sys.version)
PYRepository: youknowone/pyre
Length of output: 325
🏁 Script executed:
#!/bin/bash
set -eu
for ref in 3.14 main; do
url="https://raw.githubusercontent.com/python/cpython/${ref}/Objects/clinic/memoryobject.c.h"
echo "--- ${url} ---"
curl -fsSL "$url" | rg -n -C 18 'memoryview_tobytes|tobytes\(' | head -n 120
doneRepository: youknowone/pyre
Length of output: 6995
🌐 Web query:
CPython 3.14 memoryview.tobytes("C", order="F") exact TypeError
💡 Result:
In CPython, the memoryview.tobytes() method accepts an optional order argument, which can be one of three values: 'C' (row-major), 'F' (column-major), or 'A' (as-is/physical) [1]. If you receive a TypeError when calling memoryview.tobytes("C", order="F"), it is because the method signature for tobytes only accepts a single optional argument for order [2][1]. Providing both a positional argument "C" and a keyword argument order="F" constitutes passing two values for the same parameter, which is invalid [2][1]. To fix this, pass only the desired order as a single argument: # Correct usage mv.tobytes("F") # or mv.tobytes(order="F") The order parameter was introduced to allow conversion of data into specific layouts [3][1]. When using 'C' or 'F', the data is converted to the specified order; 'A' returns an exact copy of the physical memory for contiguous views [1]. If order is not provided, it defaults to 'C' [4][1]. Passing conflicting or redundant arguments to this method will trigger a TypeError due to the incorrect number of arguments provided to the underlying implementation [2][1].
Citations:
- 1: pablogsal/cpython@d08ea70
- 2: python/cpython@80a5036
- 3: Can't read a F-contiguous memoryview in physical order python/cpython#80026
- 4: hugovk/cpython@b7bb54f
🏁 Script executed:
#!/bin/bash
set -eu
url='https://raw.githubusercontent.com/python/cpython/3.14/Objects/clinic/memoryobject.c.h'
curl -fsSL "$url" | sed -n '255,345p'
printf '%s\n' '--- CPython source references for the duplicate-argument diagnostic ---'
for file in Python/getargs.c Include/internal/pycore_arguments.h; do
echo "--- ${file} ---"
curl -fsSL "https://raw.githubusercontent.com/python/cpython/3.14/${file}" \
| rg -n -C 8 'given by name|UnpackKeywords|CheckPositional' || true
doneRepository: youknowone/pyre
Length of output: 9007
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- local memoryview.tobytes implementation ---'
sed -n '1325,1395p' pyre/pyre-interpreter/src/builtins.rs
printf '%s\n' '--- local clinic_arity implementation ---'
sed -n '4728,4795p' pyre/pyre-interpreter/src/builtins.rs
printf '%s\n' '--- local keyword helpers ---'
sed -n '4655,4730p' pyre/pyre-interpreter/src/builtins.rsRepository: youknowone/pyre
Length of output: 8674
🌐 Web query:
site:github.com/python/cpython/blob/3.14/Python/getargs.c "_PyArg_UnpackKeywords" "given by name"
💡 Result:
No exact match was found. In CPython’s 3.14/Python/getargs.c, neither "_PyArg_UnpackKeywords" nor "given by name" appears. (github.com)
The referenced file is here: python/cpython → 3.14 → Python/getargs.c. (github.com)
Citations:
- 1: https://github.com/python/cpython/blob/3.14/Python/getargs.c
- 2: https://github.com/python/cpython/blob/3.14/Python/getargs.c
Handle duplicate order before the arity check.
CPython 3.14 raises TypeError: argument for tobytes() given by name ('order') and position (1). Detect the duplicate before clinic_arity so the fixture does not expect the incorrect arity message.
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 48-48: Found assertion on exception error in except block, use pytest.raises() instead
(PT017)
[warning] 50-50: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/extra_tests/parity_tests/memoryview_tobytes_order_python314.py` around
lines 45 - 50, Update the tobytes duplicate-argument handling exercised by
memoryview.tobytes so a positional argument plus the keyword order raises
“argument for tobytes() given by name ('order') and position (1)” before
clinic_arity performs its arity check. Adjust the fixture assertion in the
duplicate-order case while preserving the failure expectation.
| result = subprocess.run( | ||
| [sys.executable, path], | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| check=False, | ||
| ) | ||
| assert result.returncode == 0, result.stderr | ||
| assert result.stdout.decode().strip() == "¢", result.stdout |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Pin the child stdout encoding so the assertion does not depend on the runner locale.
Line 22 decodes the child output as UTF-8. The child selects its stdout encoding from its own locale, so a runner with a non-UTF-8 locale makes this fixture fail for an environment reason instead of a parity reason. Set PYTHONIOENCODING for the subprocess.
🔧 Proposed fix to pin the child encoding
+ env = dict(os.environ, PYTHONIOENCODING="utf-8")
result = subprocess.run(
[sys.executable, path],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
check=False,
+ env=env,
)
assert result.returncode == 0, result.stderr
assert result.stdout.decode().strip() == "┬ó", result.stdout📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| result = subprocess.run( | |
| [sys.executable, path], | |
| stdout=subprocess.PIPE, | |
| stderr=subprocess.PIPE, | |
| check=False, | |
| ) | |
| assert result.returncode == 0, result.stderr | |
| assert result.stdout.decode().strip() == "¢", result.stdout | |
| env = dict(os.environ, PYTHONIOENCODING="utf-8") | |
| result = subprocess.run( | |
| [sys.executable, path], | |
| stdout=subprocess.PIPE, | |
| stderr=subprocess.PIPE, | |
| check=False, | |
| env=env, | |
| ) | |
| assert result.returncode == 0, result.stderr | |
| assert result.stdout.decode().strip() == "¢", result.stdout |
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 15-20: Prefer capture_output over sending stdout and stderr to PIPE
Replace with capture_output keyword argument
(UP022)
[error] 15-15: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/extra_tests/parity_tests/script_source_encoding_startup.py` around lines
15 - 22, Set PYTHONIOENCODING to UTF-8 in the subprocess environment used by the
run around subprocess.run, preserving the existing environment variables, so
result.stdout.decode() consistently validates the intended output independent of
the runner locale.
| error = syntax_error('"¢¢¢¢¢¢" + f(4, x for x in range(1))') | ||
| assert error.msg == "invalid syntax", error.msg | ||
| assert (error.lineno, error.offset, error.end_lineno, error.end_offset) == (1, 25, 1, 28), ( | ||
| error.lineno, | ||
| error.offset, | ||
| error.end_lineno, | ||
| error.end_offset, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Check the literal on line 95 for a mojibake round trip.
The string contains ┬ó repeated six times. ┬ó is the UTF-8 encoding of ¢ (U+00A2) re-read as latin-1. The upstream CPython case this mirrors uses ¢¢¢¢¢¢, which is six characters, not twelve.
The asserted offsets (1, 25, 1, 28) are consistent with the twelve-character form, so the test passes as written. It does not exercise the intended input.
The file docstring states the suite covers "character, not UTF-8 byte, columns". A doubly-encoded literal weakens that coverage, because each source character is now itself ASCII-range in the corrupted form.
Restore the intended literal and recompute the expected offsets.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/extra_tests/parity_tests/syntax_error_python314_offsets.py` around lines
95 - 102, Replace the mojibake literal in the syntax_error case with six literal
¢ characters, matching the upstream CPython input. Recompute the expected error
offset tuple for the corrected character count while preserving the existing
syntax-error assertion and character-column coverage.
| fn fast_local_index(&self, key: PyObjectRef) -> Result<Option<usize>, crate::PyError> { | ||
| let frame = self.frame(); | ||
| let code = frame.code(); | ||
| for (index, name) in code.varnames.iter().enumerate() { | ||
| if hidden_local(code, index) { | ||
| continue; | ||
| } | ||
| if crate::baseobjspace::eq_w(pyre_object::w_str_new(name.as_ref()), key)? { | ||
| return Ok(true); | ||
| return Ok(Some(index)); | ||
| } | ||
| } | ||
| let mut index = code.varnames.len(); | ||
| for name in code.cellvars.iter() { | ||
| if code.varnames.iter().any(|local| local == name) { | ||
| continue; | ||
| } | ||
| if crate::baseobjspace::eq_w(pyre_object::w_str_new(name.as_ref()), key)? { | ||
| return Ok(Some(index)); | ||
| } | ||
| index += 1; | ||
| } | ||
| for name in code.freevars.iter() { | ||
| if crate::baseobjspace::eq_w(pyre_object::w_str_new(name.as_ref()), key)? { | ||
| return Ok(Some(index)); | ||
| } | ||
| index += 1; | ||
| } | ||
| Ok(None) | ||
| } | ||
|
|
||
| fn setitem_value( | ||
| &mut self, | ||
| key: PyObjectRef, | ||
| value: PyObjectRef, | ||
| ) -> Result<(), crate::PyError> { | ||
| if let Some(index) = self.fast_local_index(key)? { | ||
| let frame = self.frame(); | ||
| let slot = locals_w!(frame)[index]; | ||
| if !slot.is_null() && unsafe { pyre_object::is_cell(slot) } { | ||
| unsafe { pyre_object::w_cell_set(slot, value) }; | ||
| } else { | ||
| frame.set_locals_w(index, value); | ||
| } | ||
| return Ok(()); | ||
| } | ||
| Ok(false) | ||
| let backing = self.frame().get_or_create_w_locals(); | ||
| crate::baseobjspace::setitem(backing, key, value).map(|_| ()) | ||
| } | ||
|
|
||
| fn key_is_fast_local(&self, key: PyObjectRef) -> Result<bool, crate::PyError> { | ||
| Ok(self.fast_local_index(key)?.is_some()) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Unrooted raw object references cross collection points in the new frame-locals proxy paths. Each of these three additions holds a raw PyObjectRef local across a call that allocates or runs user code, without publishing it on the shadow stack. frame_locals_snapshot at Lines 2545-2556 documents the required discipline for exactly this situation, and the non-optimized branch of frame_locals_proxy_snapshot follows it. A moving collection at any of these points leaves a stale pointer behind.
pyre/pyre-interpreter/src/pyframe.rs#L108-L158: pinkeybefore the comparison loop, reload it after eachw_str_newallocation and eacheq_wcall, and reloadkeyandvalueinsetitem_valuebefore the slot write and before the backing-mappingsetitem.pyre/pyre-interpreter/src/pyframe.rs#L280-L293: pinincomingbeforecall_method(incoming, "update", &[other])and reload it afterwards; reload each key and value from a root before passing the pair tosetitem_value.pyre/pyre-interpreter/src/pyframe.rs#L2572-L2578: wrap the optimized branch inpush_roots, pinw_localsand the new snapshot dict, and read both back throughshadow_stack_getarounddict_update_value.
📍 Affects 1 file
pyre/pyre-interpreter/src/pyframe.rs#L108-L158(this comment)pyre/pyre-interpreter/src/pyframe.rs#L280-L293pyre/pyre-interpreter/src/pyframe.rs#L2572-L2578
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-interpreter/src/pyframe.rs` around lines 108 - 158, Root all raw
PyObjectRef values that cross allocation or user-code calls in the frame-locals
proxy paths. In pyre/pyre-interpreter/src/pyframe.rs:108-158, update
fast_local_index and setitem_value to pin key, reload it after w_str_new and
eq_w, and reload key/value before slot writes and backing setitem; in
pyre/pyre-interpreter/src/pyframe.rs:280-293, pin incoming across call_method
and reload each key/value before setitem_value; in
pyre/pyre-interpreter/src/pyframe.rs:2572-2578, use push_roots for w_locals and
the snapshot dict and access them through shadow_stack_get around
dict_update_value.
Source: Learnings
| )); | ||
| } | ||
| crate::baseobjspace::delitem(mapping, key) | ||
| crate::baseobjspace::delitem(self.frame().get_or_create_w_locals(), key) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Bound hidden locals are visible to lookup but absent from the removal target. frame_locals_proxy_snapshot overlays currently-bound hidden comprehension locals onto the snapshot (Lines 2590-2601), while key_is_fast_local skips hidden varnames (Lines 112-114). Both removal methods therefore accept such a key at the lookup step, decline to reject it as a fast local, and then operate on the real module/class namespace, which never received the hidden binding. The user observes a key that __contains__ and __getitem__ report as present but that del and pop report as missing. Decide the intended classification once and apply it to both call sites.
pyre/pyre-interpreter/src/pyframe.rs#L215-L215: reject a bound hidden local in__delitem__with the samecannot remove local variables from FrameLocalsProxyerror the fast-local guard raises, instead of forwarding the delete to the backing mapping.pyre/pyre-interpreter/src/pyframe.rs#L324-L331: apply the identical classification inpopbefore calling the backing mapping'spop.
📍 Affects 1 file
pyre/pyre-interpreter/src/pyframe.rs#L215-L215(this comment)pyre/pyre-interpreter/src/pyframe.rs#L324-L331
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-interpreter/src/pyframe.rs` at line 215, The FrameLocalsProxy
removal paths misclassify bound hidden locals as backing-namespace keys. In
pyre/pyre-interpreter/src/pyframe.rs:215-215, update __delitem__ to reject bound
hidden locals with the same “cannot remove local variables from
FrameLocalsProxy” error as fast locals; in
pyre/pyre-interpreter/src/pyframe.rs:324-331, apply the identical classification
in pop before invoking the backing mapping’s pop, reusing the shared
hidden-local detection logic.
| fn make_functional_method(name: &'static str, function: DunderFn, arity: u16) -> PyObjectRef { | ||
| let text_signature = if arity == 1 { | ||
| "($self, /)" | ||
| } else { | ||
| "($self, object, /)" | ||
| }; | ||
| crate::gateway::make_builtin_function_with_arity_and_text_signature( | ||
| name, | ||
| function, | ||
| arity, | ||
| text_signature, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
CPython 3.14 reversed.__setstate__ __text_signature__ value
💡 Result:
In CPython 3.14, the text_signature attribute remains an internal, undocumented mechanism primarily used to provide introspection information for built-in functions and types defined in C [1][2][3]. The reversed built-in function does not have a setstate method because it is a type (iterator type), not a stateful object that supports pickling in the manner of user-defined classes [1]. Regarding the text_signature attribute: 1. Nature and Purpose: It is not a public API [4][3]. Its value is typically a string generated by the Argument Clinic, which the inspect module uses to reconstruct a Signature object [1]. It acts as a bridge for C-implemented callables to expose their arguments, default values, and annotations to Python-level introspection tools [1][3]. 2. Structure: There is no single, globally constant text_signature value for all types. Instead, it is dynamically managed and populated for specific callables where signature metadata is available [1][5]. 3. Implementation Detail: The CPython internal implementation of signature parsing has seen iterative changes, such as the removal of unnecessary processing (e.g., stripping / separators) [4][6] and the addition of support for basic annotations [3]. Because it is an implementation detail, its exact format is subject to change without warning, and it is not intended for use in custom user code [7][2]. If you are attempting to inspect the reversed object, note that it is an iterator type. Attempting to access reversed.setstate will raise an AttributeError because that method is not defined on the reversed type.
Citations:
- 1: https://stackoverflow.com/questions/25847035/what-are-signature-and-text-signature-used-for-in-python-3-4
- 2: __text_signature__ in custom code python/cpython#93865
- 3: gh-81677: basic support for annotations in __text_signature__'s python/cpython#101872
- 4: gh-102378: don't bother stripping
/from __text_signature__ python/cpython#102379 - 5: 10 built-in functions need non-None .__text_signature__ python/cpython#89465
- 6: ericsnowcurrently/cpython@71cf7c3
- 7: https://discuss.python.org/t/remove-extra-processing-from-signature-attribute/66906
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- relevant registrations and helper ---'
rg -n -C 5 'make_functional_method|__setstate__|reversed|map|zip' pyre/pyre-interpreter/src/typedef.rs
printf '%s\n' '--- local Python runtime ---'
python3 - <<'PY'
import inspect
for typ in (reversed, map, zip):
method = getattr(typ, "__setstate__", None)
print(typ.__name__, method, getattr(method, "__text_signature__", None))
if method is not None:
try:
print("signature:", inspect.signature(method))
except Exception as exc:
print("signature-error:", type(exc).__name__, str(exc))
PYRepository: youknowone/pyre
Length of output: 50374
🌐 Web query:
site:github.com/python/cpython "reversed_setstate" "__setstate__" Argument Clinic
💡 Result:
In the CPython codebase, reversed_setstate is a static C function defined in Objects/enumobject.c [1][2][3]. It implements the setstate method for the built-in reversed iterator type (PyReversed_Type), allowing the iterator's state to be restored during unpickling [1][2][4]. As of the current CPython implementation, reversed_setstate is not defined using Argument Clinic [1]. While many built-in methods in CPython have been converted to use Argument Clinic to simplify argument parsing and documentation generation [5][6], the reversed iterator's setstate method remains manually implemented using the legacy PyMethodDef structure [1][2]. Specifically, it is defined in the reversediter_methods array with a METH_O flag, meaning it takes exactly one argument (the state) [1][2]. For contrast, Argument Clinic-converted methods typically contain a /[clinic input]/ block and a corresponding /[clinic start generated code]/ section, as seen in other modules like Objects/exceptions.c [7]. The reversed_setstate implementation remains a standard static C function [1].
Citations:
- 1: https://github.com/python/cpython/blob/main/Objects/enumobject.c
- 2: https://github.com/python/cpython/blob/0ee2d77331f2362fcaab20cc678530b18e467e3c/Objects/enumobject.c
- 3: https://github.com/python/cpython/blob/c8ba47b5518f83b5766fefe6f68557b5033e1d70/Objects/enumobject.c
- 4: https://github.com/python/cpython/blob/v3.6.5/Objects/enumobject.c
- 5: https://github.com/python/cpython/blob/main/Doc/whatsnew/3.8.rst?plain=1
- 6: Derby #5: Convert 50 sites to Argument Clinic across 3 files python/cpython#64373
- 7: https://github.com/python/cpython/blob/main/Objects/exceptions.c
🌐 Web query:
site:github.com/python/cpython "map.__setstate__" "state"
💡 Result:
In Python, setstate is a special method used in the pickling (serialization) process [1][2]. It is not a method of the map object itself; rather, it is a method that a class can define to control how its instance is restored from a pickled state [1][3]. When an object is being unpickled, the pickle module checks if the object's class defines a setstate method [1][4]. If it does, pickle calls this method, passing it the state (data) that was saved during the pickling process [1][3]. This allows the object to manually handle the restoration of its internal attributes or perform any necessary setup, such as reopening files or database connections [1][5]. If a class does not define setstate, the pickle module expects the pickled state to be a dictionary, and it will automatically update the object's dict with the contents of that dictionary [1][3]. While some internal objects or standard library types might have pickling support that uses setstate internally (such as functools.partial [6]), map is a built-in iterator type in Python and is not designed to be pickled. Summary of setstate behavior: - Purpose: Used during unpickling to restore an object's state [1][3]. - Trigger: Called by the pickle module if defined by the class [1][4]. - Mechanism: Receives the serialized state object as an argument [1][3]. - Requirement: If not defined, the state is assumed to be a dictionary and applied to the object's dict [1][3].
Citations:
- 1: https://github.com/python/cpython/blob/master/Doc/library/pickle.rst
- 2: https://github.com/python/cpython/blob/3.9/Doc/library/pickle.rst
- 3: https://github.com/python/cpython/blob/3.10/Doc/library/pickle.rst
- 4: https://github.com/python/cpython/blob/3.11/Lib/pickle.py
- 5: https://github.com/python/cpython/blob/3.7/Doc/library/pickle.rst
- 6: https://github.com/python/cpython/blob/master/Lib/test/test_functools.py
🌐 Web query:
site:docs.python.org/3.14/library/functions.html reversed __setstate__
💡 Result:
The Python 3.14 documentation defines reversed() as:
Returns a reverse iterator. The argument must implement
__reversed__()or support the sequence protocol via__len__()and integer-indexed__getitem__(). [1]
__setstate__ is not documented on the built-in functions page; it is typically a pickle protocol method used to restore object state. [1]
🏁 Script executed:
#!/bin/bash
set -eu
log=/tmp/coderabbit-shell-logs/shell-output-kfIWDH
printf '%s\n' '--- local runtime probe ---'
rg -n -A 8 -B 2 'local Python runtime|^reversed |^map |^zip |signature' "$log" || true
printf '%s\n' '--- focused source registrations ---'
sed -n '4414,4512p' pyre/pyre-interpreter/src/typedef.rs
printf '%s\n' '--- method implementation declarations ---'
rg -n -C 3 'fn (reversed|map|zip)_setstate_method|setstate_method' pyre/pyre-interpreter/srcRepository: youknowone/pyre
Length of output: 26343
🏁 Script executed:
#!/bin/bash
set -eu
tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' EXIT
for spec in \
'v3.14.0:Objects/enumobject.c' \
'v3.14.0:Objects/itertoolsmodule.c' \
'v3.14.0:Python/bltinmodule.c'; do
tag=${spec%%:*}; path=${spec#*:}
file="$tmp/$(echo "$path" | tr / _)"
curl --fail --silent --show-error \
"https://raw.githubusercontent.com/python/cpython/$tag/$path" > "$file"
printf '\n--- %s ---\n' "$path"
rg -n -C 8 '__setstate__|setstate|text_signature|PyMethodDef' "$file" || true
doneRepository: youknowone/pyre
Length of output: 2359
🏁 Script executed:
#!/bin/bash
set -eu
tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' EXIT
for path in Modules/itertoolsmodule.c Python/bltinmodule.c; do
file="$tmp/$(echo "$path" | tr / _)"
if curl --fail --silent --show-error \
"https://raw.githubusercontent.com/python/cpython/v3.14.0/$path" > "$file"; then
printf '\n--- %s ---\n' "$path"
rg -n -C 10 'map.*setstate|zip.*setstate|__setstate__|setstate_doc|PyMethodDef' "$file" || true
else
printf '\n--- unavailable: %s ---\n' "$path"
fi
done
printf '\n--- CPython 3.14 branch source search via GitHub API ---\n'
curl --fail --silent --show-error \
'https://api.github.com/repos/python/cpython/git/trees/v3.14.0?recursive=1' |
jq -r '.tree[].path' |
rg '(^|/)(itertoolsmodule|bltinmodule|enumobject)\.c$'Repository: youknowone/pyre
Length of output: 9722
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository tests and signature expectations ---'
rg -n -C 3 '__setstate__|text_signature|inspect.signature' \
--glob '*.py' --glob '*.rs' --glob '*.toml' . |
rg -n 'setstate|text_signature|signature' | head -200
printf '%s\n' '--- CPython 3.14 method docstrings and signatures in source ---'
curl --fail --silent --show-error \
https://raw.githubusercontent.com/python/cpython/v3.14.0/Python/bltinmodule.c |
sed -n '1584,1607p;3260,3282p'
curl --fail --silent --show-error \
https://raw.githubusercontent.com/python/cpython/v3.14.0/Objects/enumobject.c |
sed -n '505,541p'Repository: youknowone/pyre
Length of output: 24211
Do not expose a text signature for __setstate__. CPython 3.14 defines these methods with METH_O and no text signature. Remove the generated ($self, object, /) signature instead of renaming the parameter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-interpreter/src/typedef.rs` around lines 4353 - 4365, Update
make_functional_method so __setstate__ is created without a text signature,
matching its METH_O definition; preserve the existing signatures for other
functional methods and do not merely rename the object parameter.
| let receiver = args[0]; | ||
| let dict = crate::type_methods::resolve_dict_backing(receiver); | ||
| debug_assert!(!dict.is_null()); | ||
| let w_type = crate::typedef::r#type(receiver) | ||
| .expect("every dict has a type") | ||
| .as_ptr(); | ||
| let basicsize = cpython_type_layout(w_type) | ||
| .expect("dict and its subclasses have CPython layout metadata") | ||
| .0 as usize; | ||
| let items = unsafe { pyre_object::dictmultiobject::w_dict_items(dict) }; | ||
| if items.is_empty() { | ||
| // CPython's empty dict shares the immortal empty-keys table, whose | ||
| // storage is consequently not charged to the instance. | ||
| return Ok(w_int_new(basicsize as i64)); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Both new __sizeof__ implementations read the payload without a proven-valid receiver. The sibling methods in this file validate the receiver before touching the storage, but these two new functions rely on an assertion that compiles out or on an unverified gateway guarantee.
pyre/pyre-interpreter/src/typedef.rs#L6275-L6289: replacedebug_assert!(!dict.is_null())with a real check, or return the basicsize whenresolve_dict_backingyields null, before callingw_dict_items(dict).pyre/pyre-interpreter/src/typedef.rs#L25142-L25150: callrequire_set_receiverorrequire_frozenset_receiverbeforew_set_len(args[0]), unlesssetlike_method_gateways!is confirmed to inject that check.
📍 Affects 1 file
pyre/pyre-interpreter/src/typedef.rs#L6275-L6289(this comment)pyre/pyre-interpreter/src/typedef.rs#L25142-L25150
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-interpreter/src/typedef.rs` around lines 6275 - 6289, Validate both
__sizeof__ receiver paths before reading storage: at
pyre/pyre-interpreter/src/typedef.rs:6275-6289, replace the debug_assert! around
resolve_dict_backing with a real null check that returns basicsize before
w_dict_items; at pyre/pyre-interpreter/src/typedef.rs:25142-25150, call
require_set_receiver or require_frozenset_receiver before w_set_len unless
setlike_method_gateways! demonstrably performs that validation.
| for (name, text_signature) in [ | ||
| ("__new__", "($type, *args, **kwargs)"), | ||
| ("__repr__", "($self, /)"), | ||
| ("__call__", "($self, /, *args, **kwargs)"), | ||
| ("__getattribute__", "($self, name, /)"), | ||
| ("__setattr__", "($self, name, value, /)"), | ||
| ("__delattr__", "($self, name, /)"), | ||
| ("__init__", "($self, /, *args, **kwargs)"), | ||
| ("__or__", "($self, value, /)"), | ||
| ("__ror__", "($self, value, /)"), | ||
| ("mro", "($self, /)"), | ||
| ("__subclasses__", "($self, /)"), | ||
| ("__instancecheck__", "($self, instance, /)"), | ||
| ("__subclasscheck__", "($self, subclass, /)"), | ||
| ("__dir__", "($self, /)"), | ||
| ("__sizeof__", "($self, /)"), | ||
| ] { | ||
| let function = unsafe { pyre_object::w_dict_getitem_str(ns, name) } | ||
| .expect("type TypeDef callable was just installed"); | ||
| unsafe { crate::function::fset_func_text_signature(function, w_str_new(text_signature)) }; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the duplicated __instancecheck__ and __subclasscheck__ signature stamps.
These two names are stamped three times in init_type_type:
- Lines 11071-11074 and 11095-11098 stamp
"($self, instance, /)"and"($self, subclass, /)"on carriers that lines 11532-11543 later replace in the namespace. - Lines 11510-11531 build replacement carriers with
"($self, inst, /)"and"($self, sub, /)". - This loop overwrites both again with
"($self, instance, /)"and"($self, subclass, /)".
Only step 3 is observable. Steps 1 and 2 are dead work and they disagree on the parameter names. Keep one spelling and delete the other stamps.
♻️ Proposed cleanup
let instancecheck_method = crate::gateway::make_builtin_function_with_arity_and_text_signature(
"__instancecheck__",
|args| {
crate::type_methods::arity_exact(args, "__instancecheck__", 1)?;
let matched =
unsafe { crate::baseobjspace::p_recursive_isinstance_type_w(args[1], args[0])? };
Ok(pyre_object::w_bool_from(matched))
},
2,
- "($self, inst, /)",
+ "($self, instance, /)",
);Then drop ("__instancecheck__", ...) and ("__subclasscheck__", ...) from the loop at lines 11795-11796, and delete the earlier carriers at lines 11056-11099.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-interpreter/src/typedef.rs` around lines 11783 - 11803, Remove the
__instancecheck__ and __subclasscheck__ entries from the signature-stamping loop
in init_type_type, and delete the earlier duplicate carrier definitions and
signature stamps for these names. Preserve the replacement carriers created
later with their existing parameter-name spelling so each signature is installed
only once.
Restore the per-frame MIFrame operand-stack image when an authoritative walk aborts in a conditional expression, preventing replay from skipping or duplicating already-applied effects.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs`:
- Around line 5989-6000: Thread the single existing WalkSession from
drive_bridge_carrier_walk through full_body_walk_trace into run_perfn_walk,
instead of creating a nested session. Update the FBW_BRANCH_ABORT_STACK latch,
take, and reset operations to access the session-owned state, then remove the
TLS-based carrier storage while preserving the existing root-epilogue handoff
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0cc8b697-da10-4d89-a63d-d610661091a6
📒 Files selected for processing (7)
pyre/extra_tests/parity_tests/script_source_encoding_startup.pypyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rspyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rspyre/pyre-jit-trace/src/jitcode_dispatch/mod.rspyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rspyre/pyre-jit-trace/src/state.rspyre/pyre-jit-trace/src/trace.rs
| /// Kept-stack branch-abort resume carrier: `(py_pc, complete operand-stack | ||
| /// OpRef mirror)` captured from the live MIFrame-equivalent walk context | ||
| /// before it is dropped. | ||
| /// | ||
| /// Like the quasiimmutable carrier above, this is needed only for a | ||
| /// mid-expression abort: the virtualizable array is authoritative at merge | ||
| /// points, while RPython resumes a blackhole from the MIFrame register | ||
| /// image at the exact abort point (`blackhole.py:1711-1727`). The trace | ||
| /// epilogue accepts this carrier only when its Python coordinate exactly | ||
| /// matches the decoded abort resume pc. | ||
| static FBW_BRANCH_ABORT_STACK: std::cell::RefCell<Option<(usize, Vec<OpRef>)>> = | ||
| const { std::cell::RefCell::new(None) }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline pyre/pyre-jit-trace/src/jitcode_dispatch --items all --match 'WalkSession|FBW_BRANCH_ABORT_STACK'
rg -n -C 4 'FBW_BRANCH_ABORT_STACK|fbw_branch_abort_stack_(latch|take)|struct WalkSession' pyre/pyre-jit-trace/srcRepository: youknowone/pyre
Length of output: 6771
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- WalkSession definition and construction/use ---'
sed -n '460,570p' pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs
rg -n -C 6 'WalkSession\s*\{|WalkSession::|walk_session|WalkContext|fbw_branch_abort_stack_(latch|take)' \
pyre/pyre-jit-trace/src/jitcode_dispatch pyre/pyre-jit-trace/src/trace.rs
printf '%s\n' '--- State declarations and reset/lifecycle ---'
sed -n '350,420p' pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs
sed -n '1215,1260p' pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs
sed -n '620,665p' pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs
sed -n '4540,4595p' pyre/pyre-jit-trace/src/trace.rs
printf '%s\n' '--- All related TLS declarations ---'
rg -n -C 3 'thread_local|FBW_[A-Z_]+:' pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rsRepository: youknowone/pyre
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- WalkSession and WalkContext ---'
sed -n '470,555p' pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs
rg -n -C 8 'pub struct WalkContext|struct WalkContext|session:' pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs pyre/pyre-jit-trace/src/jitcode_dispatch/*.rs | head -n 240
printf '%s\n' '--- Exact branch-abort paths ---'
sed -n '630,660p' pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs
sed -n '1235,1255p' pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs
sed -n '4555,4585p' pyre/pyre-jit-trace/src/trace.rs
sed -n '380,410p' pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs
printf '%s\n' '--- Walk driver boundaries and nested subwalk calls ---'
rg -n -C 10 'WalkContext\s*\{|WalkSession::default|bridge_subwalk|full_body_walk|walk.*subwalk|subwalk.*walk' \
pyre/pyre-jit-trace/src/jitcode_dispatch pyre/pyre-jit-trace/src/trace.rs | head -n 400Repository: youknowone/pyre
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Full-body walk session lifetime ---'
rg -n -C 12 'fn full_body_walk_trace|fn run_perfn_walk|WalkSession::default|fbw_branch_abort_stack_take|bool_box_truth_reset|fbw_store_journal_reset' \
pyre/pyre-jit-trace/src/trace.rs pyre/pyre-jit-trace/src/jitcode_dispatch/*.rs
printf '%s\n' '--- Session access at the branch-abort epilogue ---'
sed -n '4380,4610p' pyre/pyre-jit-trace/src/trace.rs
rg -n -C 8 'ACTIVE_WALK_SESSION|active_session|InlineFrameGuard::enter|InlineFrameGuard::exit' \
pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs pyre/pyre-jit-trace/src/jitcode_dispatch/*.rs
printf '%s\n' '--- All branch-abort symbol references, excluding tests ---'
rg -n 'FBW_BRANCH_ABORT_STACK|fbw_branch_abort_stack_(latch|take)' pyre/pyre-jit-trace/src \
-g '!tests.rs'
printf '%s\n' '--- Read-only structural check of relevant function regions ---'
python3 - <<'PY'
from pathlib import Path
for p in [
Path("pyre/pyre-jit-trace/src/trace.rs"),
Path("pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs"),
Path("pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs"),
]:
text = p.read_text()
needles = ("fn full_body_walk_trace", "fbw_branch_abort_stack_take",
"fbw_branch_abort_stack_latch", "fbw_store_journal_reset")
print(p)
for n in needles:
starts = [i for i in range(len(text)) if text.startswith(n, i)]
print(f" {n}: {len(starts)} occurrence(s), lines {[text.count(chr(10), 0, i)+1 for i in starts]}")
PYRepository: youknowone/pyre
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Function declarations and session creation (production files only) ---'
rg -n '^(pub )?(pub\(crate\) )?fn (full_body_walk_trace|run_perfn_walk|trace_bytecode|drive_bridge_carrier_walk)|WalkSession::default|fbw_branch_abort_stack_(latch|take)|fbw_store_journal_reset' \
pyre/pyre-jit-trace/src/trace.rs \
pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs \
pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs \
pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs \
pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs
printf '%s\n' '--- Full-body walk declaration and setup ---'
sed -n '1300,1385p' pyre/pyre-jit-trace/src/trace.rs
sed -n '1385,1475p' pyre/pyre-jit-trace/src/trace.rs
printf '%s\n' '--- Branch-abort epilogue and enclosing locals ---'
sed -n '4470,4590p' pyre/pyre-jit-trace/src/trace.rs
sed -n '4590,4645p' pyre/pyre-jit-trace/src/trace.rs
printf '%s\n' '--- Production branch-abort references only ---'
rg -n -C 4 'FBW_BRANCH_ABORT_STACK|fbw_branch_abort_stack_(latch|take)' \
pyre/pyre-jit-trace/src/trace.rs \
pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs \
pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs \
pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rsRepository: youknowone/pyre
Length of output: 26567
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- run_perfn_walk and full_body_walk_trace ---'
sed -n '3245,3335p' pyre/pyre-jit-trace/src/trace.rs
sed -n '4860,4910p' pyre/pyre-jit-trace/src/trace.rs
sed -n '5480,5605p' pyre/pyre-jit-trace/src/trace.rs
printf '%s\n' '--- Session passed through the dispatch and epilogue ---'
rg -n -C 10 'dispatch_perfn_frame\(|dispatch_via_miframe\(|drive_bridge_frame_subwalk\(|fbw_branch_abort_stack_take\(' \
pyre/pyre-jit-trace/src/trace.rs \
pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs
printf '%s\n' '--- Read-only function-range verifier ---'
python3 - <<'PY'
from pathlib import Path
def functions(path, names):
text = Path(path).read_text()
out = []
for name in names:
needle = f"fn {name}"
pos = 0
while True:
pos = text.find(needle, pos)
if pos < 0:
break
brace = text.find("{", pos)
if brace < 0:
break
depth = 0
i = brace
in_str = None
esc = False
while i < len(text):
c = text[i]
if in_str:
if esc:
esc = False
elif c == "\\":
esc = True
elif c == in_str:
in_str = None
elif c in ('"', "'"):
in_str = c
elif c == "{":
depth += 1
elif c == "}":
depth -= 1
if depth == 0:
break
i += 1
start_line = text.count("\n", 0, pos) + 1
end_line = text.count("\n", 0, i) + 1
body = text[brace:i+1]
refs = [r for r in ("WalkSession::default", "dispatch_via_miframe",
"fbw_branch_abort_stack_latch",
"fbw_branch_abort_stack_take")
if r in body]
out.append((name, start_line, end_line, refs))
pos += len(needle)
return out
for row in functions("pyre/pyre-jit-trace/src/trace.rs",
["run_perfn_walk", "full_body_walk_trace",
"drive_bridge_carrier_walk"]):
print(row)
PYRepository: youknowone/pyre
Length of output: 23627
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Bridge-carrier session handoff ---'
sed -n '1680,1915p' pyre/pyre-jit-trace/src/trace.rs
printf '%s\n' '--- run_perfn_walk dispatch and branch-abort epilogue ---'
sed -n '3640,3875p' pyre/pyre-jit-trace/src/trace.rs
sed -n '4490,4645p' pyre/pyre-jit-trace/src/trace.rs
printf '%s\n' '--- WalkSession arguments in bridge-carrier paths ---'
rg -n -C 6 'session: &|&session|full_body_walk_trace\(|run_perfn_walk\(' \
pyre/pyre-jit-trace/src/trace.rs \
pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs \
pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs
printf '%s\n' '--- Static handoff verifier ---'
python3 - <<'PY'
from pathlib import Path
p = Path("pyre/pyre-jit-trace/src/trace.rs")
s = p.read_text()
checks = {
"drive_bridge_carrier_walk creates session": "fn drive_bridge_carrier_walk" in s and
"let session = std::cell::RefCell::new(crate::jitcode_dispatch::WalkSession::default());" in s[s.index("fn drive_bridge_carrier_walk"):s.index("fn drive_bridge_carrier_walk")+500],
"drive_bridge_carrier_walk calls full_body_walk_trace": "full_body_walk_trace(" in s[s.index("fn drive_bridge_carrier_walk"):s.index("fn drive_bridge_carrier_walk")+30000],
"full_body_walk_trace accepts session": "fn full_body_walk_trace" in s and
"session:" in s[s.index("fn full_body_walk_trace"):s.index("fn full_body_walk_trace")+500],
"run_perfn_walk creates session": "fn run_perfn_walk" in s and
"let session = std::cell::RefCell::new(crate::jitcode_dispatch::WalkSession::default());" in s[s.index("fn run_perfn_walk"):s.index("fn run_perfn_walk")+500],
}
for k, v in checks.items():
print(f"{k}: {v}")
PYRepository: youknowone/pyre
Length of output: 47574
Thread the existing WalkSession through the branch-abort handoff.
drive_bridge_carrier_walk creates one WalkSession, but full_body_walk_trace calls run_perfn_walk, which creates another. Pass the existing session through both functions before moving FBW_BRANCH_ABORT_STACK into WalkSession; otherwise a sub-walk can latch a carrier that the root epilogue cannot read.
Update the latch, take, and reset paths to use that session. Do not use TLS for this interpreter-owned state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs` around lines 5989 - 6000,
Thread the single existing WalkSession from drive_bridge_carrier_walk through
full_body_walk_trace into run_perfn_walk, instead of creating a nested session.
Update the FBW_BRANCH_ABORT_STACK latch, take, and reset operations to access
the session-owned state, then remove the TLS-based carrier storage while
preserving the existing root-epilogue handoff behavior.
Source: Coding guidelines
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/4959bb5fec8ed35046d59934be6b3249f597d24b/pyre-interpreter/src/builtins.rs#L11560-L11561
When the source has an earlier parser error and a later malformed named escape, such as if:\n'\\N{', CPython 3.14 reports invalid syntax on line 1. This unconditional whole-source scan instead finds the later literal, overwrites the message with a Unicode escape error, and retains the first error's location, producing a mismatched diagnostic. Apply this recovery only when the underlying error points to the malformed literal, or preserve tokenizer ordering rather than rescanning arbitrary later source.
AGENTS.md reference: AGENTS.md:L252-L254
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bd88f909f
ℹ️ 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".
| let w_locals = self.getdictscope()?; | ||
| let snapshot = unsafe { pyre_object::w_dict_new() }; |
There was a problem hiding this comment.
When an optimized frame has locals and w_dict_new() triggers a nursery collection, the w_locals pointer captured immediately beforehand can be relocated because it is not pinned or reloaded. dict_update_value then reads the stale address, risking corrupted f_locals results or invalid memory access; root both dictionaries and reload them as the non-optimized branch below already does.
Useful? React with 👍 / 👎.
| let child = match statement { | ||
| ast::Stmt::FunctionDef(node) => Some(node.body.as_slice()), | ||
| ast::Stmt::ClassDef(node) => Some(node.body.as_slice()), | ||
| _ => None, |
There was a problem hiding this comment.
When the failing nonlocal is inside an async def, this scope search does not descend into the function, while the visitor below also does not stop at AsyncFunctionDef; declarations from unrelated async functions are therefore combined at module scope. For example, a global x in one async function followed by nonlocal x in another should report no binding for nonlocal 'x' found at the latter declaration, but this helper rewrites it as a global/nonlocal conflict at the former. Include async functions in both scope-boundary matches.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/extra_tests/parity_tests/jit_recursive_closure_live_set.py`:
- Around line 10-18: Update the nested visit function to declare a None return
type with -> None, preserving its existing recursive behavior and all return
paths.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 38ab17fd-3caa-4336-868f-1e4840130709
📒 Files selected for processing (3)
pyre/extra_tests/parity_tests/jit_recursive_closure_live_set.pypyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rspyre/pyre-jit-trace/src/trace.rs
| def visit(w): | ||
| if w <= limit or w in seen: | ||
| return | ||
| seen.add(w) | ||
| lo = w >> 1 | ||
| hi = w - lo | ||
| need.add(hi if need_hi else lo) | ||
| visit(lo) | ||
| visit(hi) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required return annotation.
visit returns None on all paths. Add -> None to satisfy ANN202.
Proposed fix
- def visit(w):
+ def visit(w) -> None:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def visit(w): | |
| if w <= limit or w in seen: | |
| return | |
| seen.add(w) | |
| lo = w >> 1 | |
| hi = w - lo | |
| need.add(hi if need_hi else lo) | |
| visit(lo) | |
| visit(hi) | |
| def visit(w) -> None: | |
| if w <= limit or w in seen: | |
| return | |
| seen.add(w) | |
| lo = w >> 1 | |
| hi = w - lo | |
| need.add(hi if need_hi else lo) | |
| visit(lo) | |
| visit(hi) |
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 10-10: Missing return type annotation for private function visit
Add return type annotation: None
(ANN202)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/extra_tests/parity_tests/jit_recursive_closure_live_set.py` around lines
10 - 18, Update the nested visit function to declare a None return type with ->
None, preserving its existing recursive behavior and all return paths.
Source: Linters/SAST tools
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/0ef4a58e599855e13956b271ef3de8a9a46f2064/pyre-interpreter/src/builtins.rs#L11319-L11322
This separate scanner still treats any alphabetic run containing f as an f-string prefix. For example, compiling foo"{(]}" should produce CPython 3.14's ordinary invalid syntax, but foo passes this predicate, so the unconditional delimiter rewrite reports a mismatched parenthesis from inside an ordinary string. Restrict this check to complete valid prefixes such as f, fr, and rf.
AGENTS.md reference: AGENTS.md:L231-L233
https://github.com/youknowone/pyre/blob/0ef4a58e599855e13956b271ef3de8a9a46f2064/pyre-interpreter/src/module/posix/interp_posix.rs#L6018-L6020
With the moving GC enabled, both w_int_new calls can allocate, so the second call may collect and relocate the unrooted pid object produced by the first call before w_tuple_new gets a chance to pin its inputs. The resulting tuple can therefore receive a stale pointer, causing corruption or an invalid access on os.forkpty() in the parent. Allocate and pin/reload both result objects before constructing the tuple.
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/ccdf0ccd19a5f2bb820b3c0cffdfe9667ba81f91/pyre-interpreter/src/pyframe.rs#L283-L284
When other is a large or user-defined mapping whose iteration, hashing, or item lookup triggers a nursery collection, the newly allocated incoming dictionary can be relocated during call_method. The local pointer is not registered as a GC root, so the subsequent w_dict_items(incoming) dereferences its stale pre-collection address, risking corrupted proxy updates or invalid memory access; pin the dictionary and reload it after the call.
https://github.com/youknowone/pyre/blob/ccdf0ccd19a5f2bb820b3c0cffdfe9667ba81f91/pyre-interpreter/src/builtins.rs#L11560
This source-wide scan runs for every compilation failure, so a malformed Unicode escape later in the file can replace an earlier, unrelated parser diagnostic. For example, compile("if:\n pass\n'\\N{BROKEN'", '<x>', 'exec') should report the invalid if syntax on line 1, but this call replaces its message with the Unicode codec error while retaining the unrelated location. Apply the decoder-first rewrite only when the malformed literal corresponds to the parser's failing token rather than scanning the entire source unconditionally.
AGENTS.md reference: AGENTS.md:L231-L233
ℹ️ 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".
1 participant
40 commits rebased onto
f705ef075e1. Working tree clean,cargo fmt --all --checkclean.__text_signature__for the builtin types (20 commits)bool,builtins,functional(map/filter/zip/enumerate/reversed),property,super,slice,range,tuple,type,list,dict,set,int,float,complex,str,bytes,bytearray,memoryview, and the descriptor wrappers now carry the 3.14 signature text, soinspect.signatureandhelp()resolve against them instead of falling back to(*args, **kwargs).Each type ships its own parity fixture (
*_text_signatures_python314.py), 20 files, so a regression on one type does not hide behind another.Sizing
dict.__sizeof__/set.__sizeof__added (dict_set_sizeof_python314.py).sys.getsizeof's pre-header term rebuilt as the two independent components_PyType_PreHeaderSizeadds: a two-word GC header for tracked objects, plus a two-word managed dict/weakref prefix where the instance type requests it. The GC term is a runtimegc_hook::try_gc_owns_objectquery rather than a hand-maintained type list.object.__sizeof__'snitemsnow branches on the layout (int → digit count, tuple →w_tuple_len, bytes →w_bytes_len, memoryview →3 * ndim, otherwise 0) instead of reading a fixed slot.SyntaxErroroffsets and f-string diagnostics (11 commits)exceptions: port Python 3.14 syntax error offsetslands the 3.14 offset model; the ten commits on top of it fix the individual selections that model exposed — indentation priority,global/nonlocalconflict range, dict-comprehension assignment targets, generator-for-token in a binary call, non-ASCII bytes literal token, incompatible string prefixes, f-string brace/comment/delimiter/unterminated-token cases, and malformed unicode-name escapes decoded before the surrounding diagnostic. Covered bysyntax_error_python314_offsets.py.Other
collections:OrderedDict's public method set aligned with 3.14 (ordered_dict_python314.py).memoryview.tobytes(order=…)ported (memoryview_tobytes_order_python314.py).posix.forkptyimplemented, including the child-side lifecycle.script_source_encoding_startup.py).pyframe: locals of an inlined comprehension are preserved rather than dropped with the inlined frame.majit-translatecodewriter: a raw shape pointer is classified as an integer word, not a GC reference.27 new parity fixtures in total.
Summary by CodeRabbit
New Features
memoryview.tobytes()now supportsC,F, andAordering.posix.forkpty()support.OrderedDictmethods and iterator behavior.Bug Fixes
isatty()results.Tests