Repository navigation
typedef parity: code/function/method/cell/property/class/staticmethod (3.14) - #610
Conversation
![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
https://github.com/youknowone/pyre/blob/ccd23eb93ee51321a7cc86f3cec588e4391a7564/pyre/pyre-interpreter/src/pycode.rs#L970
When co_argcount (and the adjacent positional/kw-only count fields) is overridden, code_replace accepts the new value without re-running the constructor invariants such as posonly <= argcount and argcount + kwonly <= len(co_varnames). A call like f.__code__.replace(co_argcount=f.__code__.co_nlocals + 1) can therefore create an internally inconsistent code object; later function-call error handling indexes code_ref.varnames[i] for each argument slot and can panic instead of raising the ValueError that CodeType/CPython reject at replacement time.
https://github.com/youknowone/pyre/blob/ccd23eb93ee51321a7cc86f3cec588e4391a7564/pyre/pyre-interpreter/src/pycode.rs#L1032
Replacing co_linetable only swaps the raw bytes while leaving code.locations cloned from the original code object. For inputs such as func.__code__.replace(co_linetable=b'') (or a changed first line), the public co_positions(), co_lines(), co_lnotab, and traceback/debug-range data still report the old decoded locations even though co_linetable says otherwise; this should decode the new linetable using the effective first line and instruction length before reboxing.
ℹ️ 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".
| && crate::pyframe::code_constants(a) | ||
| .iter() | ||
| .zip(crate::pyframe::code_constants(b).iter()) | ||
| .all(|(left, right)| constant_strong_equal(left, right)) |
There was a problem hiding this comment.
code_data_equal returns true after comparing constants but never compares linetable or exceptiontable, and code_hash similarly omits them. Python treats code objects that differ only by co_linetable or co_exceptiontable as unequal (and hashes them differently), so code.replace(co_linetable=b'') == code and dictionary/set behavior will be wrong for generated or instrumented code objects.
Useful? React with 👍 / 👎.
WalkthroughThe PR adds Python 3.14 parity tests and benchmarks, updates descriptor and object-space behavior, removes implicit cell dereferencing, extends function and code-object metadata, and adds runtime invalidation checks to the WASM backend. ChangesProtocol parity and runtime integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
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 |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit fa67ef5). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patch
3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptations
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5299f6ec70
ℹ️ 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 stacksize = stacksize_value as u32; | ||
| let flags_bits = flags_value as u32; | ||
| let instructions = unsafe { read_code_units(args[7])? }; | ||
| let constants = unsafe { read_code_consts(args[8])? }; |
There was a problem hiding this comment.
When the new code.__new__ path reconstructs a code object from an existing co_consts tuple, read_code_consts rejects Python objects for constants that pyre already emits and reads back, such as frozenset and complex (pyobject_from_constant handles both). As a result, cloning ordinary code with a set-membership or complex literal via type(code)(..., code.co_consts, ...) raises ValueError instead of producing an equivalent code object.
Useful? React with 👍 / 👎.
| index -= code.varnames.len() as i64; | ||
| if let Some(name) = code.cellvars.get(index as usize) { | ||
| return Ok(w_str_new(name)); | ||
| } | ||
| index -= code.cellvars.len() as i64; |
There was a problem hiding this comment.
For functions where a cell variable is also a local or argument, CPython's localsplus table marks that existing local slot as a cell and only appends pure cellvars after co_varnames. This implementation indexes through every co_cellvars entry after the locals, so e.g. an outer function whose argument is captured reports that alias at an out-of-range oparg and shifts any following freevars; _varname_from_oparg should follow localsplus/pure-cell ordering rather than co_varnames + co_cellvars + co_freevars.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/check.py`:
- Around line 925-948: Preserve unrounded baseline timing values through the
performance-gate comparisons in the relevant retry and ratio-check flows,
including the logic around _retry_performance_gate and the affected baseline
assignments. Move two-decimal formatting to the reporting/display boundary only,
so _exec_time receives raw t_cpython/t_pypy floats and the 5 ms startup floor is
not distorted.
In `@pyre/pyre-interpreter/src/baseobjspace.rs`:
- Around line 10906-10910: Validate that the `__delete__` call receives a second
argument before accessing it in the surrounding property deletion
implementation. Update the `__delete__` registration to require arity 2, or
safely retrieve the object via `args.get(1)` and return the expected TypeError
when absent; preserve the existing deleter validation for valid calls.
In `@pyre/pyre-interpreter/src/function.rs`:
- Around line 2059-2149: The layout-specific builtins read method, range, or
function fields without validating their receivers. In
pyre/pyre-interpreter/src/function.rs:2059-2149, add method-receiver checks
before every w_method_get_* access in descr_method_eq, descr_method_ne,
descr_method_repr, descr_method_getattribute, descr_method_hash, and
descr_method__reduce__; in pyre/pyre-interpreter/src/typedef.rs:2364-2383,
validate args[0] is a range before w_range_eq; and in
pyre/pyre-interpreter/src/typedef.rs:8553-8560, validate args[0] is a function
before function_get_qualname, returning the established type error for invalid
receivers.
In `@pyre/pyre-interpreter/src/pycode.rs`:
- Around line 823-840: Update code_varname_from_oparg so the cellvar lookup uses
the non-overlapping cellvar count represented by npure_cellvars, or filters
cellvars against code.varnames before indexing. Preserve the existing
varname-first resolution while ensuring captured parameters are not counted
twice and subsequent cellvar/freevar slots resolve correctly.
- Line 1044: Update the replacement-code flow before
box_code_constant_with_firstlineno so code.locations is recomputed from the
updated co_linetable and instruction count after cloning or modifying the code
object. Ensure code.positions(), code.lines(), and co_lnotab observe the rebuilt
locations while preserving the existing co_firstlineno_raw update.
- Around line 891-913: Update the conditional-branch handling in co_branches()
to derive target from the decoded jump target rather than converting
read_arg(index) directly. Reuse the existing jump-target decoder from
pyopcode.rs, while preserving the current fallthrough and branch-row
construction.
In `@pyre/pyre-interpreter/src/typedef.rs`:
- Around line 10003-10011: Update staticmethod_descr_repr and the corresponding
classmethod descriptor repr path to use the Python 3.14 function repr
implementation instead of display::py_repr for non-null functions. Ensure exact
function descriptors include the function name and address-style suffix, while
preserving the existing <NULL> handling and outer descriptor formatting.
🪄 Autofix (Beta)
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
Run ID: 0f708640-fb59-48dd-97e8-71d2cb01288f
📒 Files selected for processing (31)
pyre/bench/synth/classmethod_protocol_hot.pypyre/bench/synth/property_protocol_hot.pypyre/check.pypyre/extra_tests/parity_tests/classmethod_protocol.pypyre/extra_tests/parity_tests/closure_cell_protocol.pypyre/extra_tests/parity_tests/code_python314.pypyre/extra_tests/parity_tests/function_python314.pypyre/extra_tests/parity_tests/mappingproxy_python314.pypyre/extra_tests/parity_tests/method_python314.pypyre/extra_tests/parity_tests/property_protocol.pypyre/extra_tests/parity_tests/range_python314.pypyre/extra_tests/parity_tests/staticmethod_protocol.pypyre/pyre-interpreter/src/baseobjspace.rspyre/pyre-interpreter/src/builtins.rspyre/pyre-interpreter/src/call.rspyre/pyre-interpreter/src/display.rspyre/pyre-interpreter/src/eval.rspyre/pyre-interpreter/src/function.rspyre/pyre-interpreter/src/module/_codecs/mod.rspyre/pyre-interpreter/src/module/_weakref/interp__weakref.rspyre/pyre-interpreter/src/objspace/descroperation.rspyre/pyre-interpreter/src/opcode_ops.rspyre/pyre-interpreter/src/pycode.rspyre/pyre-interpreter/src/pyframe.rspyre/pyre-interpreter/src/pyopcode.rspyre/pyre-interpreter/src/shared_opcode.rspyre/pyre-interpreter/src/typedef.rspyre/pyre-jit-trace/src/trace_opcode.rspyre/pyre-jit/src/call_jit.rspyre/pyre-object/src/descriptor.rspyre/pyre-object/src/function.rs
💤 Files with no reviewable changes (2)
- pyre/pyre-jit/src/call_jit.rs
- pyre/pyre-interpreter/src/opcode_ops.rs
| baseline_cmd, expected_output, baseline_key, | ||
| ): | ||
| """Check one performance ratio, retrying a failure by median.""" | ||
| if elapsed <= baseline_time * limit + BENCH_COMPARE_BUFFER_S: | ||
| """Check one performance ratio, retrying a failure by median. | ||
|
|
||
| The pass/fail decision compares execution-only (startup-subtracted) | ||
| times; the returned elapsed/baseline stay raw for reporting. | ||
| """ | ||
| if ( | ||
| self._exec_time(backend, elapsed) | ||
| <= self._exec_time(baseline_key, baseline_time) * limit | ||
| + BENCH_COMPARE_BUFFER_S | ||
| ): | ||
| return True, elapsed, baseline_time, "" | ||
|
|
||
| retry = self._retry_performance_gate( | ||
| backend, script, timeout, baseline_cmd, expected_output, | ||
| ) | ||
| if retry is not None: | ||
| median_elapsed, median_baseline = retry | ||
| if median_elapsed <= median_baseline * limit + BENCH_COMPARE_BUFFER_S: | ||
| if ( | ||
| self._exec_time(backend, median_elapsed) | ||
| <= self._exec_time(baseline_key, median_baseline) * limit | ||
| + BENCH_COMPARE_BUFFER_S | ||
| ): |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve raw baseline precision before subtracting startup.
These paths subtract startup from t_cpython/t_pypy, but those values were already rounded to two decimals at Lines 1106 and 1115. For short benchmarks, that rounding can move the adjusted baseline to the 5 ms floor or double it, producing incorrect ratios and gate results. Keep raw baseline floats through comparison and format only for display.
Also applies to: 1004-1055
🤖 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/check.py` around lines 925 - 948, Preserve unrounded baseline timing
values through the performance-gate comparisons in the relevant retry and
ratio-check flows, including the logic around _retry_performance_gate and the
affected baseline assignments. Move two-decimal formatting to the
reporting/display boundary only, so _exec_time receives raw t_cpython/t_pypy
floats and the 5 ms startup floor is not distorted.
| let prop = property_require_obj(args.first().copied().unwrap_or(PY_NULL), "__delete__")?; | ||
| let obj = args[1]; | ||
| let fdel = w_property_get_fdel(prop); | ||
| if fdel.is_null() || is_none(fdel) { | ||
| return Err(property_no_accessor(prop, obj, "deleter")); | ||
| return Err(property_no_accessor(prop, obj, "deleter")?); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Validate the __delete__ target before indexing.
__delete__ is registered without fixed arity, so property.__delete__(prop) reaches args[1] and panics instead of raising TypeError. Register arity 2 or use args.get(1).
🤖 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/baseobjspace.rs` around lines 10906 - 10910,
Validate that the `__delete__` call receives a second argument before accessing
it in the surrounding property deletion implementation. Update the `__delete__`
registration to require arity 2, or safely retrieve the object via `args.get(1)`
and return the expected TypeError when absent; preserve the existing deleter
validation for valid calls.
| pub unsafe fn code_varname_from_oparg( | ||
| obj: PyObjectRef, | ||
| index: PyObjectRef, | ||
| ) -> Result<PyObjectRef, crate::PyError> { | ||
| let code = unsafe { require_code(obj, "_varname_from_oparg")? }; | ||
| let mut index = unsafe { crate::builtins::space_index_w(index)? }; | ||
| if index >= 0 { | ||
| if let Some(name) = code.varnames.get(index as usize) { | ||
| return Ok(w_str_new(name)); | ||
| } | ||
| index -= code.varnames.len() as i64; | ||
| if let Some(name) = code.cellvars.get(index as usize) { | ||
| return Ok(w_str_new(name)); | ||
| } | ||
| index -= code.cellvars.len() as i64; | ||
| if let Some(name) = code.freevars.get(index as usize) { | ||
| return Ok(w_str_new(name)); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section and nearby helpers.
sed -n '760,900p' pyre/pyre-interpreter/src/pycode.rs
echo
echo '--- search for locals-plus construction and varname lookup ---'
rg -n "varnames|cellvars|freevars|localsplus|locals_plus|co_varnames|co_cellvars|_varname_from_oparg" pyre/pyre-interpreter/src -S
echo
echo '--- inspect any code that builds these vectors ---'
rg -n "cellvars.*varnames|varnames.*cellvars|localsplus" pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 33332
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- pyframe slot-layout helpers ---'
sed -n '1350,1455p' pyre/pyre-interpreter/src/pyframe.rs
echo
echo '--- code object construction around localspluskinds ---'
sed -n '610,665p' pyre/pyre-interpreter/src/pycode.rs
echo
echo '--- _varname_from_oparg registration / docs ---'
sed -n '9290,9325p' pyre/pyre-interpreter/src/typedef.rs
echo
echo '--- direct callers of _varname_from_oparg or related lookup ---'
rg -n "_varname_from_oparg|code_varname_from_oparg|varname_from_oparg" pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 7769
🌐 Web query:
CPython _varname_from_oparg co_cellvars co_varnames semantics
💡 Result:
In CPython, the resolution of bytecode arguments to variable names is managed through specific structures within code objects, most notably via the interaction of co_varnames, co_cellvars, and co_freevars [1][2][3]. _varname_from_oparg The symbol _varname_from_oparg is not a core CPython VM instruction or internal C structure, but rather a helper function found in the Python standard library's dis (disassembler) module [4][5]. It serves as a callback or utility during disassembly to map a numeric opcode argument (oparg) to its corresponding human-readable variable name [4][5]. co_varnames and co_cellvars Semantics These attributes of a code object define how the Python virtual machine manages local and non-local variable storage [6][7]: 1. co_varnames: A tuple containing the names of local variables, including function arguments [1][3][8]. These are accessed by "FAST" instructions (e.g., LOAD_FAST, STORE_FAST) which use the oparg as an index into an array of local variables [6][7][9]. 2. co_cellvars: A tuple containing the names of local variables that are captured and referenced by nested functions (closures) [1][3][8]. 3. co_freevars: A tuple containing the names of variables referenced in the current scope that were defined in an enclosing scope [3][6][8]. Internal Mapping: localsplus Modern CPython (since version 3.11) optimizes this by consolidating these into a single contiguous array, often referred to internally as localsplus [2][6]. The code object maintains co_localsplusnames, a tuple that combines the names from co_varnames, co_cellvars, and co_freevars [2][6]. When the VM executes bytecode, the oparg for "FAST" and "DEREF" instructions acts as an index into this unified localsplus array to efficiently access both local variables and closure cells [6][10]. The dis module uses the logic encapsulated by _varname_from_oparg to reconstruct this mapping during analysis by referring to these underlying code object attributes [4][5].
Citations:
- 1: https://docs.python.org/3/c-api/code.html
- 2: https://github.com/python/cpython/blob/main/Include/cpython/code.h
- 3: https://stackoverflow.com/questions/61550012/why-doesnt-co-varnames-return-list-of-all-the-variable-names
- 4: https://github.com/python/cpython/blob/main/Lib/dis.py
- 5: https://github.com/python/cpython/blob/3.13/Lib/dis.py
- 6: https://python-code-data.readthedocs.io/en/stable/learnings.html
- 7: https://tenthousandmeters.com/blog/python-behind-the-scenes-5-how-variables-are-implemented-in-cpython/
- 8: https://stackoverflow.com/questions/32221063/where-does-python-store-the-name-binding-of-function-closure
- 9: https://github.com/python/cpython/blob/ff8ad0a576c6cf375e68276e614e551b3b759254/Python/ceval.c
- 10: https://github.com/python/cpython/blob/v3.14.5/Python/bytecodes.c
🌐 Web query:
site:github.com python _varname_from_oparg code object co_cellvars co_varnames
💡 Result:
In Python's CPython implementation, co_varnames and co_cellvars are attributes of a code object (PyCodeObject) that map variable indices used in bytecode instructions to their corresponding names [1][2]. co_varnames This attribute is a tuple containing the names of local variables and arguments [2]. Bytecode instructions like LOAD_FAST, STORE_FAST, and DELETE_FAST use an operand (oparg) as an index into this tuple to access the specified local variable [3]. co_cellvars This attribute is a tuple containing the names of local variables that are referenced by nested functions (i.e., variables captured by closures) [4][2]. When a variable is accessed via a closure, the LOAD_CLOSURE instruction uses an index that spans both co_cellvars and co_freevars [3]. Relationship to Opargs and Internal Mapping While co_varnames, co_cellvars, and co_freevars remain part of the public Python API, recent CPython versions (starting from 3.11) have optimized how the virtual machine handles these variables internally [4][5][6]. The interpreter now uses a unified "locals plus" storage mechanism [7][6]. Internally, the code object tracks these variables using combined structures (such as co_localsplusnames and co_localspluskinds), which map the "locals plus" array—where locals, cells, and free variables are stored together—to their respective names and kinds [1][7][6]. Accessing these fields in C code should be done using the official C API functions PyCode_GetVarnames and PyCode_GetCellvars, as direct access to the underlying fields in the PyCodeObject struct is no longer supported or recommended [1][4][5].
Citations:
- 1: https://github.com/python/cpython/blob/main/Include/cpython/code.h
- 2: https://github.com/python/cpython/blob/main/Lib/inspect.py
- 3: https://github.com/python/cpython/blob/12e7cd8a51956a5ce373aac692ae6366c5f86584/Doc/library/dis.rst
- 4: https://github.com/python/cpython/blob/3.11/Doc/c-api/code.rst
- 5: Public C API for accessing code object fields python/cpython#94936
- 6: bpo-43693: Add new internal code objects fields: co_fastlocalnames and co_fastlocalkinds. python/cpython#26388
- 7: https://github.com/python/cpython/blob/3.11/Objects/codeobject.c
Skip overlapping cellvars in _varname_from_oparg.
Use npure_cellvars here, or filter co_cellvars against co_varnames; otherwise captured parameters are counted twice and later cell/freevar names resolve to the wrong slot.
🤖 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/pycode.rs` around lines 823 - 840, Update
code_varname_from_oparg so the cellvar lookup uses the non-overlapping cellvar
count represented by npure_cellvars, or filters cellvars against code.varnames
before indexing. Preserve the existing varname-first resolution while ensuring
captured parameters are not counted twice and subsequent cellvar/freevar slots
resolve correctly.
| while index < code.instructions.len() { | ||
| let op = code.instructions.read_op(index).deoptimize(); | ||
| let next = index + 1 + op.cache_entries(); | ||
| if op.has_jump() && !op.is_unconditional_jump() { | ||
| let delta = u8::from(code.instructions.read_arg(index)) as usize; | ||
| let target = next.saturating_add(delta); | ||
| // Python 3.14 inserts NOT_TAKEN at the fallthrough edge so branch | ||
| // instrumentation can distinguish the untaken path. `co_branches` | ||
| // reports the first real instruction after that marker. | ||
| let fallthrough = if next < code.instructions.len() | ||
| && matches!( | ||
| code.instructions.read_op(next).deoptimize(), | ||
| crate::bytecode::Instruction::NotTaken | ||
| ) { | ||
| next + 1 | ||
| } else { | ||
| next | ||
| }; | ||
| rows.push(w_tuple_new(vec![ | ||
| w_int_new((index * 2) as i64), | ||
| w_int_new((fallthrough * 2) as i64), | ||
| w_int_new((target * 2) as i64), | ||
| ])); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map relevant symbols in pycode.rs first.
ast-grep outline pyre/pyre-interpreter/src/pycode.rs --view expanded > /tmp/pycode_outline.txt
sed -n '1,220p' /tmp/pycode_outline.txt
# Inspect the relevant section around the comment.
nl -ba pyre/pyre-interpreter/src/pycode.rs | sed -n '860,940p'
# Search for bytecode jump decoding helpers and EXTENDED_ARG handling.
rg -n "EXTENDED_ARG|extended arg|jump.*target|target.*jump|deoptimize\(\)|cache_entries\(\)|read_arg\(" pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 6486
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect bytecode instruction definitions that may already handle jump decoding.
fd -a ".*(bytecode|instruction).*" pyre/pyre-interpreter/src
rg -n "enum Instruction|struct Instruction|fn .*target|jump" pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 15511
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If the repository has tests or examples around co_branches, locate them.
rg -n "co_branches|co_lines|co_positions|NotTaken|jump target" pyre -SRepository: youknowone/pyre
Length of output: 5652
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the branch generator and surrounding helper functions.
sed -n '887,920p' pyre/pyre-interpreter/src/pycode.rs
echo '--- pyopcode jump helpers ---'
sed -n '1598,1655p' pyre/pyre-interpreter/src/pyopcode.rs
echo '--- raw jump target helpers ---'
sed -n '3518,3545p' pyre/pyre-interpreter/src/pyopcode.rs
echo '--- instruction/argument readers ---'
rg -n "fn read_arg|fn read_op|struct CodeUnits|impl .*CodeUnits|cache_entries\(\)" pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 4641
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the bytecode instruction types and code-unit accessors.
sed -n '1,260p' pyre/pyre-interpreter/src/bytecode.rs
echo '--- codeunits accessors ---'
sed -n '1,220p' pyre/pyre-interpreter/src/bytecode.rs | catRepository: youknowone/pyre
Length of output: 234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focus on the exact branch generator and the helper implementations it relies on.
sed -n '887,918p' pyre/pyre-interpreter/src/pycode.rs
echo '---'
sed -n '1608,1650p' pyre/pyre-interpreter/src/pyopcode.rs
echo '---'
sed -n '3520,3536p' pyre/pyre-interpreter/src/pyopcode.rs
echo '---'
rg -n "struct CodeUnits|impl CodeUnits|read_op\(|read_arg\(" pyre/pyre-interpreter/src/bytecode.rs pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 3981
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the bytecode representation to understand whether read_arg already includes EXTENDED_ARG.
rg -n "read_arg|read_op|EXTENDED_ARG|cache_entries" pyre/pyre-interpreter/src/bytecode.rs pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 1536
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the bytecode accessors and the tests/parity expectations around co_branches.
sed -n '1,260p' pyre/pyre-interpreter/src/bytecode.rs
echo '--- pycode tests around co_branches ---'
sed -n '1740,1845p' pyre/pyre-interpreter/src/pycode.rs
echo '--- parity test references ---'
sed -n '1,170p' pyre/extra_tests/parity_tests/code_python314.pyRepository: youknowone/pyre
Length of output: 234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find jump-classification helpers on Instruction.
rg -n "is_backward_jump|is_forward_jump|is_unconditional_jump|has_jump|has_arg" pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 612
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether this opcode set actually has conditional backward jump variants.
rg -n "BackwardIf|BACKWARD_IF|POP_JUMP_BACKWARD|JUMP_BACKWARD_IF|is_backward_jump" pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the bytecode source file and inspect read_arg/read_op semantics.
fd -a "bytecode.rs|mod.rs" pyre/pyre-interpreter/src
echo '--- read_arg/read_op definitions ---'
rg -n "fn read_arg|fn read_op|struct CodeUnits|impl .*CodeUnits|EXTENDED_ARG|cache_entries" pyre/pyre-interpreter/src -S
echo '--- co_branches tests/parity expectations ---'
sed -n '1740,1845p' pyre/pyre-interpreter/src/pycode.rs
echo '--- relevant pyframe/eval comments about EXTENDED_ARG ---'
sed -n '1176,1210p' pyre/pyre-interpreter/src/pyframe.rs
sed -n '1358,1370p' pyre/pyre-interpreter/src/eval.rsRepository: youknowone/pyre
Length of output: 11268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the actual bytecode source file and inspect the relevant APIs.
fd -a "bytecode.rs" pyre/pyre-interpreter/src
fd -a "mod.rs" pyre/pyre-interpreter/src | rg "/bytecode/|/bytecode\.rs$|/bytecode/"
# Search for read_arg/read_op and instruction decoding in the source tree.
rg -n "read_arg\(|read_op\(|decode_instruction_at|EXTENDED_ARG|ExtendedArg" pyre/pyre-interpreter/src -S
# Inspect the code_branches neighborhood again with line numbers.
nl -ba pyre/pyre-interpreter/src/pycode.rs | sed -n '887,910p'Repository: youknowone/pyre
Length of output: 153
Use the decoded jump target here. read_arg(index) only sees the raw oparg byte, so co_branches() can misreport targets for branches that need EXTENDED_ARG. Reuse the bytecode jump-target decoder already used in pyopcode.rs.
🤖 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/pycode.rs` around lines 891 - 913, Update the
conditional-branch handling in co_branches() to derive target from the decoded
jump target rather than converting read_arg(index) directly. Reuse the existing
jump-target decoder from pyopcode.rs, while preserving the current fallthrough
and branch-row construction.
| } | ||
|
|
||
| Ok(box_code_constant(&code)) | ||
| Ok(box_code_constant_with_firstlineno(&code, firstlineno_raw)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file around the referenced line and nearby helpers.
FILE=pyre/pyre-interpreter/src/pycode.rs
wc -l "$FILE"
sed -n '980,1085p' "$FILE"
# Find the helper definition and any uses of locations/linetable-related fields.
rg -n "box_code_constant_with_firstlineno|locations|linetable|co_positions|co_lines|co_lnotab|firstlineno_raw" "$FILE" pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 13309
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=pyre/pyre-interpreter/src/pycode.rs
echo "=== file size ==="
wc -l "$FILE"
echo "=== relevant region ==="
sed -n '1000,1085p' "$FILE"
echo "=== symbol search ==="
rg -n "box_code_constant_with_firstlineno|locations|linetable|co_positions|co_lines|co_lnotab|firstlineno_raw" "$FILE" pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 12588
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrow inspection of the helper and related code-object methods.
FILE=pyre/pyre-interpreter/src/pycode.rs
sed -n '1,220p' "$FILE"
echo
sed -n '220,420p' "$FILE"Repository: youknowone/pyre
Length of output: 17758
🌐 Web query:
RustPython code.locations co_positions co_lines lnotab linetable_to_locations
💡 Result:
In RustPython, code object location information is primarily managed through the co_linetable attribute, which follows the CPython 3.11+ compact location format [1][2]. This format stores source position data (line, end line, column, and end column) for bytecode instructions [1][3][2]. Key attributes and methods in RustPython include: co_linetable: A bytes object containing the compact representation of source locations [1][4][5]. It is not to be confused with the deprecated co_lnotab [1][2]. In RustPython, co_lnotab is intentionally not implemented as it was deprecated in CPython 3.12 and removed [1]. co_positions: A method on the code object that returns an iterator yielding tuples of (line, end_line, column, end_column) for each instruction [1][4]. It processes the co_linetable internally, often using a reader to interpret the compact binary format [1][6]. co_lines: A method that returns an iterator over bytecode ranges, yielding (start, end, line) tuples [4][2]. This is more compact than co_positions but does not provide column or end-line information [2]. linetable_to_locations: While not a singular public function by this exact name in the public API, RustPython's internal logic (such as in crates/vm/src/builtins/code.rs) handles the conversion from the raw co_linetable bytes to usable location information [1][6]. This involves reading the header of each location entry to determine the instruction coverage and the location kind (short form, one-line form, no columns, or long form) [1][3]. The compiler is responsible for generating this table during bytecode assembly [7]. RustPython's implementation mirrors the CPython specification for location tables, ensuring that source-to-bytecode mapping is accurate for debugging and introspection [3][2][8]. Tools interacting with RustPython code objects should prefer these modern methods (co_positions, co_lines) over relying on legacy structures like co_lnotab [9][2].
Citations:
- 1: https://github.com/RustPython/RustPython/blob/885cf5c2/crates/vm/src/builtins/code.rs
- 2: https://github.com/python/cpython/blob/36e4ffc1/InternalDocs/code_objects.md
- 3: https://github.com/python/cpython/blob/944fffee8916cb94321fa33cd3a43f4108717746/Objects/locations.md
- 4: https://docs.rs/rustpython-vm/latest/rustpython_vm/builtins/struct.PyCode.html
- 5: https://github.com/RustPython/RustPython/blob/885cf5c2/crates/compiler-core/src/bytecode.rs
- 6: https://docs.rs/rustpython-vm/latest/src/rustpython_vm/builtins/code.rs.html
- 7: https://github.com/RustPython/RustPython/blob/885cf5c2/crates/codegen/src/ir.rs
- 8: https://github.com/RustPython/RustPython/blob/1e7a4903/Lib/test/test_code.py
- 9: https://peps.python.org/pep-0626/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=pyre/pyre-interpreter/src/pycode.rs
echo "=== helper ==="
sed -n '420,445p' "$FILE"
echo "=== constructor path around locations ==="
sed -n '620,670p' "$FILE"
echo "=== CodeObject definition / locations field ==="
rg -n "struct CodeObject|pub locations|locations:" pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 2709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=pyre/pyre-interpreter/src/pycode.rs
echo "=== co_lnotab / legacy_lnotab region ==="
sed -n '500,565p' "$FILE"
echo "=== co_positions / co_lines region ==="
sed -n '840,885p' "$FILE"
echo "=== code_data_equal / replace-adjacent region ==="
sed -n '700,790p' "$FILE"Repository: youknowone/pyre
Length of output: 7911
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where crate::CodeObject is defined and inspect the fields.
rg -n "struct CodeObject" pyre/pyre-interpreter/src -S
rg -n "locations:|linetable:|instructions:" pyre/pyre-interpreter/src -SRepository: youknowone/pyre
Length of output: 153
Rebuild decoded locations before boxing the replacement
code.positions(), code.lines(), and co_lnotab read code.locations, so cloning the code object and only updating co_firstlineno_raw leaves those views stale after co_firstlineno, co_linetable, or co_code changes. Recompute locations from the updated linetable and instruction count before boxing.
🤖 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/pycode.rs` at line 1044, Update the
replacement-code flow before box_code_constant_with_firstlineno so
code.locations is recomputed from the updated co_linetable and instruction count
after cloning or modifying the code object. Ensure code.positions(),
code.lines(), and co_lnotab observe the rebuilt locations while preserving the
existing co_firstlineno_raw update.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38966bbc65
ℹ️ 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".
| w_kw_defs: PY_NULL, | ||
| w_module: PY_NULL, | ||
| w_func_globals_obj, | ||
| w_builtins, |
There was a problem hiding this comment.
When constructing a function whose globals require builtin resolution, pick_builtin_obj and the later stable allocation can run GC while w_builtins is only protected through the shadow root pushed above. This store still uses the pre-GC local rather than re-reading the pinned root, so a forwarded func_builtins object can leave func.__builtins__ pointing at the old address; mirror the allocation patterns that re-read shadow-stack roots after try_gc_alloc_stable_raw before writing the Function payload.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5577c43a2
ℹ️ 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".
| if w_klass.is_null() || unsafe { pyre_object::is_none(w_klass) } { | ||
| w_klass = r#type(w_obj).unwrap_or(PY_NULL); |
There was a problem hiding this comment.
When classmethod.__get__ is called directly with obj as Python None and type omitted or also None, CPython raises TypeError: __get__(None, None) is invalid; this branch instead infers type(None) and returns a method bound to NoneType, so classmethod(f).__get__(None, None)() passes the wrong class rather than rejecting the invalid descriptor call. Please explicitly reject the obj is None + missing/None type case before falling back to r#type(w_obj).
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/pyre-interpreter/src/typedef.rs`:
- Around line 9889-9924: Update the __repr__ and __reduce__ callbacks in the
MemberDescriptorType definition to validate their receiver with the existing
is_member check before calling member_descriptor_repr, w_member_get_cls, or
w_member_get_name. Preserve the established invalid-receiver behavior used by
__name__, __objclass__, and __qualname__, and only perform unsafe member field
access after validation.
🪄 Autofix (Beta)
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
Run ID: b041120e-ebca-4227-8d79-3cb4930ac1c1
📒 Files selected for processing (9)
pyre/extra_tests/parity_tests/function_member_descriptors_python314.pypyre/extra_tests/parity_tests/getset_descriptor_python314.pypyre/pyre-interpreter/src/baseobjspace.rspyre/pyre-interpreter/src/display.rspyre/pyre-interpreter/src/eval.rspyre/pyre-interpreter/src/function.rspyre/pyre-interpreter/src/objspace/std/mapdict.rspyre/pyre-interpreter/src/typedef.rspyre/pyre-object/src/typedef.rs
| unsafe { | ||
| pyre_object::dictmultiobject::w_dict_setitem_str_no_proxy( | ||
| ns, | ||
| "__repr__", | ||
| make_builtin_function_with_arity( | ||
| "__repr__", | ||
| |args| { | ||
| let member = args.first().copied().unwrap_or(pyre_object::PY_NULL); | ||
| Ok(pyre_object::w_str_new(&unsafe { | ||
| member_descriptor_repr(member) | ||
| })) | ||
| }, | ||
| 1, | ||
| ), | ||
| ) | ||
| }; | ||
|
|
||
| unsafe { | ||
| pyre_object::dictmultiobject::w_dict_setitem_str_no_proxy( | ||
| ns, | ||
| "__reduce__", | ||
| make_builtin_function_with_arity( | ||
| "__reduce__", | ||
| |args| { | ||
| let member = args.first().copied().unwrap_or(pyre_object::PY_NULL); | ||
| let owner = unsafe { pyre_object::w_member_get_cls(member) }; | ||
| let name = | ||
| pyre_object::w_str_new(unsafe { pyre_object::w_member_get_name(member) }); | ||
| Ok(pyre_object::w_tuple_new(vec![ | ||
| crate::baseobjspace::builtin_callable("getattr"), | ||
| pyre_object::w_tuple_new(vec![owner, name]), | ||
| ])) | ||
| }, | ||
| 1, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="$(fd -a '^typedef\.rs$' pyre/pyre-interpreter/src | head -n1)"
ast-grep outline "$file" --items all \
--match 'init_member_descriptor_type|member_descriptor_repr'
sed -n '9889,9924p' "$file"
rg -n -C3 \
'MemberDescriptorType\.__(repr|reduce)__\s*\(' \
pyre/extra_tests/parity_testsRepository: youknowone/pyre
Length of output: 1664
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="pyre/pyre-interpreter/src/typedef.rs"
# Inspect the member-descriptor setup and helper implementations.
sed -n '9627,9985p' "$file"
# Find the builtin-callable/arity wrapper to see whether it binds or validates receivers.
rg -n -C4 'make_builtin_function_with_arity|builtin_callable\("getattr"\)|w_member_get_(cls|name)|member_descriptor_repr' pyre/pyre-interpreter/srcRepository: youknowone/pyre
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the builtin wrapper implementation and the member helpers.
rg -n -C4 'fn make_builtin_function_with_arity|make_builtin_function_with_arity\(|pub\(crate\) unsafe fn member_descriptor_repr|w_member_get_(cls|name)|is_member\(' pyre/pyre-interpreter/src
# Read only the relevant slices with line numbers.
sed -n '1,220p' pyre/pyre-interpreter/src/typedef.rsRepository: youknowone/pyre
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the builtin-function wrapper implementation.
rg -n -C6 'fn make_builtin_function_with_arity|struct .*BuiltinFunction|impl .*BuiltinFunction|BuiltinFunction' pyre/pyre-interpreter/src
# Read the member-descriptor section only.
sed -n '9786,9945p' pyre/pyre-interpreter/src/typedef.rsRepository: youknowone/pyre
Length of output: 47485
Guard MemberDescriptorType.__repr__ and __reduce__ against non-member receivers
types.MemberDescriptorType.__repr__(1) and .__reduce__(1) can reach w_member_get_cls / w_member_get_name with an arbitrary object. Add the same is_member check used by __name__, __objclass__, and __qualname__ before any unsafe field access.
🤖 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 9889 - 9924, Update the
__repr__ and __reduce__ callbacks in the MemberDescriptorType definition to
validate their receiver with the existing is_member check before calling
member_descriptor_repr, w_member_get_cls, or w_member_get_name. Preserve the
established invalid-receiver behavior used by __name__, __objclass__, and
__qualname__, and only perform unsafe member field access after validation.
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/90b20005e6095344d0389f87967327b28201a12b/pyre/pyre-interpreter/src/pycode.rs#L1033-L1034
When code.replace(co_linetable=...), co_firstlineno, or a different-length co_code is used, this only updates the raw bytes/instructions and leaves code.locations cloned from the original object. The new object's co_positions(), co_lines(), and derived co_lnotab therefore continue to report the old source positions (and can have the wrong number of rows when the bytecode length changes) instead of reflecting the replacement, so generated/instrumented code objects expose stale line metadata.
https://github.com/youknowone/pyre/blob/90b20005e6095344d0389f87967327b28201a12b/pyre/pyre-interpreter/src/pycode.rs#L1024-L1025
If co_varnames or an argument-count field is overridden, code_replace does not re-run the constructor checks (len(co_varnames) == co_nlocals and argcount + kwonlyargcount <= nlocals) and even accepts co_nlocals while ignoring it. For example, f.__code__.replace(co_varnames=()) can now create a code object with co_argcount == 1 and no local names; later calls and diagnostics index code.varnames[0], so pyre can panic or mis-execute instead of raising ValueError at replacement time.
ℹ️ 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: 40d5328deb
ℹ️ 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".
| obj: PyObjectRef, | ||
| name: PyObjectRef, | ||
| ) -> Result<PyObjectRef, crate::PyError> { | ||
| let Some(name) = (unsafe { pyre_object::w_str_get_value_opt(name) }) else { |
There was a problem hiding this comment.
When a bound method's __getattribute__ is invoked directly with a non-string name, this calls w_str_get_value_opt before checking the object's type; that helper assumes the pointer is a W_UnicodeObject and dereferences it as one. Inputs such as (1).__add__.__getattribute__(123) can therefore crash/read invalid memory instead of raising TypeError; guard with is_str/text_w before extracting the string contents.
Useful? React with 👍 / 👎.
| // CPython 3.14 `function.__type_params__`: tuple-only and not deletable. | ||
| let typeparams_getter = make_builtin_function("__type_params__", |args| { | ||
| let function = args.get(1).copied().unwrap_or(pyre_object::PY_NULL); | ||
| Ok(unsafe { crate::function::function_get_typeparams(function) }) |
There was a problem hiding this comment.
When the new __type_params__ descriptor is called directly with a non-function receiver, make_getset_property has no required-class guard, so this forwards arbitrary args[1] into function_get_typeparams, which blindly casts it to Function. For example, type(f).__dict__['__type_params__'].__get__(42, int) can read invalid fields or crash instead of raising the usual descriptor TypeError; use a class-checked getset or validate is_function before the cast.
Useful? React with 👍 / 👎.
| let op = code.instructions.read_op(index).deoptimize(); | ||
| let next = index + 1 + op.cache_entries(); | ||
| if op.has_jump() && !op.is_unconditional_jump() { | ||
| let delta = u8::from(code.instructions.read_arg(index)) as usize; |
There was a problem hiding this comment.
For conditional jumps whose offset needs an EXTENDED_ARG prefix, read_arg(index) only returns this instruction's low byte, while the interpreter executes the fully decoded oparg via the normal decode path. Large if/loop bodies with branch deltas above 255 will therefore make co_branches() report a target near the fallthrough instead of the real branch target, breaking coverage/introspection for those code objects.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pyre/pyre-interpreter/src/pycode.rs (1)
980-1032: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftRebuild locals-plus metadata in
code.replace
code.replacemutatesvarnames/cellvars/freevarson a clonedCodeObject, but it never recomputeslocalspluskindsor re-runs thecode_newarg-count checks. That can return a code object whose frame layout no longer matches its metadata.🤖 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/pycode.rs` around lines 980 - 1032, Update the code.replace construction flow around the varnames, cellvars, and freevars assignments to rebuild locals-plus metadata after these fields change. Reuse the same locals-plus derivation and code_new argument-count validation so the cloned CodeObject’s frame layout and metadata remain consistent, and propagate any validation error instead of returning the invalid object.pyre/pyre-jit/src/call_jit.rs (1)
1468-1478: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winStale frame pointer in
fail_valuesleads to use-after-free.The current assignment bypasses applying
ca_resume_framewheneverca_resume_deadframeisSome. If the GC moved the frame (settingca_resume_frame != 0),fail_values[0]retains the stale frame pointer from the deadframe snapshot. This leads to a use-after-free whenactual_green_keyrecovery or the blackhole fallback reads it.Restructure this to mirror the
raw_deadframelogic, applyingca_resume_frameuniformly regardless of where the baseline array originated.🐛 Proposed fix
- let fail_values_raw = unsafe { std::slice::from_raw_parts(fail_values_ptr, num_fail_values) }; - let mut fail_values_owned; - let fail_values = if let Some(values) = ca_resume_deadframe.as_deref() { - values - } else if ca_resume_frame != 0 { - fail_values_owned = fail_values_raw.to_vec(); - fail_values_owned[0] = ca_resume_frame as i64; - fail_values_owned.as_slice() - } else { - fail_values_raw - }; + let fail_values_raw = if let Some(values) = ca_resume_deadframe.as_deref() { + values + } else { + unsafe { std::slice::from_raw_parts(fail_values_ptr, num_fail_values) } + }; + let mut fail_values_owned; + let fail_values = if ca_resume_frame != 0 && !fail_values_raw.is_empty() { + fail_values_owned = fail_values_raw.to_vec(); + fail_values_owned[0] = ca_resume_frame as i64; + fail_values_owned.as_slice() + } else { + fail_values_raw + };🤖 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/src/call_jit.rs` around lines 1468 - 1478, Update the fail_values construction around ca_resume_deadframe and ca_resume_frame to apply a nonzero ca_resume_frame replacement uniformly, including when the baseline comes from ca_resume_deadframe. Mirror the existing raw_deadframe handling: select the appropriate source values, copy them when relocation is required, replace fail_values[0], and otherwise preserve the original slice without copying.
🤖 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/closure_cell_protocol.py`:
- Around line 63-109: Replace each `assert False, ...` fallback in the CellType
protocol checks with an explicit `raise AssertionError(...)`, preserving the
existing failure messages and exception-handling flow so these tests remain
effective under optimized Python execution.
In `@pyre/extra_tests/parity_tests/function_python314.py`:
- Around line 63-65: Rename the annotate function’s format parameter to a
non-shadowing name, and update its assertion accordingly while preserving the
existing return value and behavior.
In `@pyre/pyre-interpreter/src/baseobjspace.rs`:
- Around line 4990-5026: Update the property fast path in the is_property branch
of getattribute to catch AttributeError from each relevant get(descr, obj,
w_type) call and route it through instance_getattr_hook_or_err before returning.
Preserve successful descriptor results and propagate non-AttributeError failures
unchanged so property subclasses retain their __getattr__ fallback.
In `@pyre/pyre-interpreter/src/call.rs`:
- Line 1681: Update the TypeError construction in the callable handling branch
to use the runtime type name of the classmethod object, so subclasses report
“'<type>' object is not callable” consistently with the dynamic fallback path
instead of hardcoding “classmethod”.
- Around line 1669-1675: Update the staticmethod branch in the call dispatch
logic to unwrap and forward directly only when callable is exactly the builtin
staticmethod type. For staticmethod subclasses, skip w_staticmethod_get_func and
dispatch through the subclass’s own __call__ slot so overrides are honored,
while preserving the existing argument forwarding for exact builtin instances.
In `@pyre/pyre-interpreter/src/pycode.rs`:
- Around line 461-475: Update require_code to validate the code-body pointer’s
alignment before constructing a reference with &*ptr. Reuse the same
alignment-checking logic or helper used by w_code_new_with_hidden_applevel,
while preserving the existing null-pointer error handling and rejecting
unaligned test stubs with an appropriate error.
- Around line 575-588: Update code-object construction and replacement paths,
including read_code_u32 and the co_firstlineno handling, to validate integer
values against the supported C int/u32 range before casting. Return the
appropriate error on overflow or underflow instead of allowing wrapping,
truncation, or clamping; preserve existing behavior for values within range.
In `@pyre/pyre-interpreter/src/typedef.rs`:
- Around line 1639-1656: Update module_descr_init to validate that positional
contains the required self argument before indexing it. Handle an empty
positional slice by returning the appropriate TypeError, while preserving the
existing name validation and initialization flow when positional[0] is present;
follow the sibling staticmethod_descr_init, classmethod_descr_init, and
property_descr_init guards.
---
Outside diff comments:
In `@pyre/pyre-interpreter/src/pycode.rs`:
- Around line 980-1032: Update the code.replace construction flow around the
varnames, cellvars, and freevars assignments to rebuild locals-plus metadata
after these fields change. Reuse the same locals-plus derivation and code_new
argument-count validation so the cloned CodeObject’s frame layout and metadata
remain consistent, and propagate any validation error instead of returning the
invalid object.
In `@pyre/pyre-jit/src/call_jit.rs`:
- Around line 1468-1478: Update the fail_values construction around
ca_resume_deadframe and ca_resume_frame to apply a nonzero ca_resume_frame
replacement uniformly, including when the baseline comes from
ca_resume_deadframe. Mirror the existing raw_deadframe handling: select the
appropriate source values, copy them when relocation is required, replace
fail_values[0], and otherwise preserve the original slice without copying.
🪄 Autofix (Beta)
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
Run ID: 3f5a679a-3b05-40e5-91d5-0c26e689a3fa
📒 Files selected for processing (35)
pyre/bench/synth/classmethod_protocol_hot.pypyre/bench/synth/property_protocol_hot.pypyre/extra_tests/parity_tests/classmethod_protocol.pypyre/extra_tests/parity_tests/closure_cell_protocol.pypyre/extra_tests/parity_tests/code_python314.pypyre/extra_tests/parity_tests/function_member_descriptors_python314.pypyre/extra_tests/parity_tests/function_python314.pypyre/extra_tests/parity_tests/getset_descriptor_python314.pypyre/extra_tests/parity_tests/mappingproxy_python314.pypyre/extra_tests/parity_tests/method_python314.pypyre/extra_tests/parity_tests/module_type_python314.pypyre/extra_tests/parity_tests/property_protocol.pypyre/extra_tests/parity_tests/range_python314.pypyre/extra_tests/parity_tests/staticmethod_protocol.pypyre/pyre-interpreter/src/baseobjspace.rspyre/pyre-interpreter/src/builtins.rspyre/pyre-interpreter/src/call.rspyre/pyre-interpreter/src/display.rspyre/pyre-interpreter/src/eval.rspyre/pyre-interpreter/src/function.rspyre/pyre-interpreter/src/module/_codecs/mod.rspyre/pyre-interpreter/src/module/_weakref/interp__weakref.rspyre/pyre-interpreter/src/objspace/descroperation.rspyre/pyre-interpreter/src/objspace/std/mapdict.rspyre/pyre-interpreter/src/opcode_ops.rspyre/pyre-interpreter/src/pycode.rspyre/pyre-interpreter/src/pyframe.rspyre/pyre-interpreter/src/pyopcode.rspyre/pyre-interpreter/src/shared_opcode.rspyre/pyre-interpreter/src/typedef.rspyre/pyre-jit-trace/src/trace_opcode.rspyre/pyre-jit/src/call_jit.rspyre/pyre-object/src/descriptor.rspyre/pyre-object/src/function.rspyre/pyre-object/src/typedef.rs
💤 Files with no reviewable changes (1)
- pyre/pyre-interpreter/src/opcode_ops.rs
| try: | ||
| hash(one) | ||
| except TypeError: | ||
| pass | ||
| else: | ||
| assert False, "cell must be unhashable" | ||
|
|
||
| try: | ||
| CellType(contents=1) | ||
| except TypeError: | ||
| pass | ||
| else: | ||
| assert False, "cell constructor arguments are positional-only" | ||
|
|
||
| try: | ||
| CellType(1, 2) | ||
| except TypeError: | ||
| pass | ||
| else: | ||
| assert False, "cell accepts at most one content argument" | ||
|
|
||
| try: | ||
| type("CellSubclass", (CellType,), {}) | ||
| except TypeError: | ||
| pass | ||
| else: | ||
| assert False, "cell is not an acceptable base type" | ||
|
|
||
| # A cell is an ordinary visible Python object. Storing one in another cell | ||
| # must not transparently unwrap the inner cell at an object-space boundary. | ||
| nested = CellType(one) | ||
| assert nested.cell_contents is one | ||
| assert nested != one | ||
| assert bool(empty) and bool(one) | ||
| assert not isinstance(one, int) | ||
| box = [None] | ||
| box[0] = one | ||
| assert box[0] is one | ||
| mapping = {"cell": one} | ||
| assert mapping["cell"] is one | ||
| try: | ||
| one + 1 | ||
| except TypeError: | ||
| pass | ||
| else: | ||
| assert False, "cell arithmetic must not operate on its contents" | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Replace assert False with raise AssertionError.
assert False statements are entirely removed when Python is run with optimizations (-O), which would silently allow these tests to pass even if the expected exception is not raised. Use raise AssertionError(...) instead to ensure the test fails reliably, matching the pattern you used in other files like code_python314.py.
♻️ Proposed fixes
try:
hash(one)
except TypeError:
pass
else:
- assert False, "cell must be unhashable"
+ raise AssertionError("cell must be unhashable")
try:
CellType(contents=1)
except TypeError:
pass
else:
- assert False, "cell constructor arguments are positional-only"
+ raise AssertionError("cell constructor arguments are positional-only")
try:
CellType(1, 2)
except TypeError:
pass
else:
- assert False, "cell accepts at most one content argument"
+ raise AssertionError("cell accepts at most one content argument")
try:
type("CellSubclass", (CellType,), {})
except TypeError:
pass
else:
- assert False, "cell is not an acceptable base type"
+ raise AssertionError("cell is not an acceptable base type")
# A cell is an ordinary visible Python object. Storing one in another cell
# must not transparently unwrap the inner cell at an object-space boundary.
nested = CellType(one)
assert nested.cell_contents is one
assert nested != one
assert bool(empty) and bool(one)
assert not isinstance(one, int)
box = [None]
box[0] = one
assert box[0] is one
mapping = {"cell": one}
assert mapping["cell"] is one
try:
one + 1
except TypeError:
pass
else:
- assert False, "cell arithmetic must not operate on its contents"
+ raise AssertionError("cell arithmetic must not operate on its contents")📝 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.
| try: | |
| hash(one) | |
| except TypeError: | |
| pass | |
| else: | |
| assert False, "cell must be unhashable" | |
| try: | |
| CellType(contents=1) | |
| except TypeError: | |
| pass | |
| else: | |
| assert False, "cell constructor arguments are positional-only" | |
| try: | |
| CellType(1, 2) | |
| except TypeError: | |
| pass | |
| else: | |
| assert False, "cell accepts at most one content argument" | |
| try: | |
| type("CellSubclass", (CellType,), {}) | |
| except TypeError: | |
| pass | |
| else: | |
| assert False, "cell is not an acceptable base type" | |
| # A cell is an ordinary visible Python object. Storing one in another cell | |
| # must not transparently unwrap the inner cell at an object-space boundary. | |
| nested = CellType(one) | |
| assert nested.cell_contents is one | |
| assert nested != one | |
| assert bool(empty) and bool(one) | |
| assert not isinstance(one, int) | |
| box = [None] | |
| box[0] = one | |
| assert box[0] is one | |
| mapping = {"cell": one} | |
| assert mapping["cell"] is one | |
| try: | |
| one + 1 | |
| except TypeError: | |
| pass | |
| else: | |
| assert False, "cell arithmetic must not operate on its contents" | |
| try: | |
| hash(one) | |
| except TypeError: | |
| pass | |
| else: | |
| raise AssertionError("cell must be unhashable") | |
| try: | |
| CellType(contents=1) | |
| except TypeError: | |
| pass | |
| else: | |
| raise AssertionError("cell constructor arguments are positional-only") | |
| try: | |
| CellType(1, 2) | |
| except TypeError: | |
| pass | |
| else: | |
| raise AssertionError("cell accepts at most one content argument") | |
| try: | |
| type("CellSubclass", (CellType,), {}) | |
| except TypeError: | |
| pass | |
| else: | |
| raise AssertionError("cell is not an acceptable base type") | |
| # A cell is an ordinary visible Python object. Storing one in another cell | |
| # must not transparently unwrap the inner cell at an object-space boundary. | |
| nested = CellType(one) | |
| assert nested.cell_contents is one | |
| assert nested != one | |
| assert bool(empty) and bool(one) | |
| assert not isinstance(one, int) | |
| box = [None] | |
| box[0] = one | |
| assert box[0] is one | |
| mapping = {"cell": one} | |
| assert mapping["cell"] is one | |
| try: | |
| one + 1 | |
| except TypeError: | |
| pass | |
| else: | |
| raise AssertionError("cell arithmetic must not operate on its contents") |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 68-68: Assertion always fails, replace with pytest.fail()
(PT015)
[warning] 68-68: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
[warning] 75-75: Assertion always fails, replace with pytest.fail()
(PT015)
[warning] 75-75: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
[warning] 82-82: Assertion always fails, replace with pytest.fail()
(PT015)
[warning] 82-82: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
[warning] 89-89: Assertion always fails, replace with pytest.fail()
(PT015)
[warning] 89-89: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
[warning] 96-96: Assertion should be broken down into multiple parts
Break down assertion into multiple parts
(PT018)
[warning] 108-108: Assertion always fails, replace with pytest.fail()
(PT015)
[warning] 108-108: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
🤖 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/closure_cell_protocol.py` around lines 63 -
109, Replace each `assert False, ...` fallback in the CellType protocol checks
with an explicit `raise AssertionError(...)`, preserving the existing failure
messages and exception-handling flow so these tests remain effective under
optimized Python execution.
| def annotate(format): | ||
| assert format == 1 | ||
| return {"value": int} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the protocol parameter to satisfy Ruff.
format shadows the builtin and triggers the configured A002 error.
Proposed fix
-def annotate(format):
- assert format == 1
+def annotate(annotation_format):
+ assert annotation_format == 1📝 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 annotate(format): | |
| assert format == 1 | |
| return {"value": int} | |
| def annotate(annotation_format): | |
| assert annotation_format == 1 | |
| return {"value": int} |
🧰 Tools
🪛 Ruff (0.15.21)
[error] 63-63: Function argument format is shadowing a Python builtin
(A002)
🤖 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/function_python314.py` around lines 63 - 65,
Rename the annotate function’s format parameter to a non-shadowing name, and
update its assertion accordingly while preserving the existing return value and
behavior.
Source: Linters/SAST tools
| // W_Property is a native W_Root allocation, but a property subclass is a | ||
| // normal heap type with an instance dict. Run the same five-step | ||
| // descriptor protocol as `W_ObjectObject` before the generic builtin | ||
| // TypeDef shortcut below: data descriptor, instance dict, non-data | ||
| // descriptor. This is the structural consequence of PyPy's | ||
| // `W_Property(W_Root)` + `getclass(space)`, and is what lets the subclass | ||
| // class's `__doc__ = None` entry be shadowed by the doc value that | ||
| // property.__init__ writes into the subclass instance dict. | ||
| unsafe { | ||
| if is_property(obj) { | ||
| if let Some(w_type) = crate::typedef::r#type(obj) { | ||
| let w_descr = lookup_in_type_where(w_type, name); | ||
| if let Some(descr) = w_descr { | ||
| if is_data_descr(descr) { | ||
| if let Some(result) = get(descr, obj, w_type)? { | ||
| return Ok(result); | ||
| } | ||
| } | ||
| } | ||
| let w_dict = getdict_backing(obj); | ||
| if !w_dict.is_null() { | ||
| if let Some(value) = pyre_object::w_dict_getitem_str(w_dict, name) { | ||
| return Ok(value); | ||
| } | ||
| } | ||
| if let Some(descr) = w_descr { | ||
| if crate::is_function(descr) { | ||
| return Ok(pyre_object::w_method_new(descr, obj, w_type)); | ||
| } | ||
| if let Some(result) = get(descr, obj, w_type)? { | ||
| return Ok(result); | ||
| } | ||
| return Ok(descr); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline pyre/pyre-interpreter/src/baseobjspace.rs \
--items all --match 'getattr_str_impl|instance_getattr_hook_or_err|getattribute_if_not_from_object'
rg -n -C4 \
'if is_property\(obj\)|getattribute_if_not_from_object|instance_getattr_hook_or_err' \
pyre/pyre-interpreter/src/baseobjspace.rsRepository: youknowone/pyre
Length of output: 6945
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '4950,5045p' pyre/pyre-interpreter/src/baseobjspace.rs | cat -nRepository: youknowone/pyre
Length of output: 4856
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline pyre/pyre-interpreter/src/baseobjspace.rs --match 'fn getattr_str_impl'
sed -n '3527,5065p' pyre/pyre-interpreter/src/baseobjspace.rs | cat -n | sed -n '1,260p'Repository: youknowone/pyre
Length of output: 14908
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '3970,4105p' pyre/pyre-interpreter/src/baseobjspace.rs | cat -nRepository: youknowone/pyre
Length of output: 7638
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'fn get\(|match .*PyErrorKind::AttributeError|instance_getattr_hook_or_err\(' pyre/pyre-interpreter/src/baseobjspace.rsRepository: youknowone/pyre
Length of output: 2324
Honor the existing __getattr__ fallback in this property fast path pyre/pyre-interpreter/src/baseobjspace.rs:4999-5026
get(descr, ...) can raise AttributeError here, but unlike the main instance path this branch returns it directly. Route that error through instance_getattr_hook_or_err so property subclasses still see __getattr__.
🤖 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/baseobjspace.rs` around lines 4990 - 5026, Update
the property fast path in the is_property branch of getattribute to catch
AttributeError from each relevant get(descr, obj, w_type) call and route it
through instance_getattr_hook_or_err before returning. Preserve successful
descriptor results and propagate non-AttributeError failures unchanged so
property subclasses retain their __getattr__ fallback.
| // function.py:712-713 StaticMethod.descr_call — the wrapper contributes | ||
| // no implicit argument; forward the original positional and keyword | ||
| // collections unchanged to its w_function. | ||
| if unsafe { pyre_object::is_staticmethod(callable) } { | ||
| let func = unsafe { pyre_object::w_staticmethod_get_func(callable) }; | ||
| return call_with_kwargs(frame, func, pos_args, kwargs); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate existing staticmethod subtype-call parity coverage and dispatch helpers.
rg -n -C3 'class .*\(staticmethod\)|staticmethod.*__call__|is_staticmethod|staticmethod_call_override' \
pyre/extra_tests pyre/pyre-interpreter/srcRepository: youknowone/pyre
Length of output: 12941
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the surrounding dispatch in call.rs and the helper/type semantics.
sed -n '1610,1705p' pyre/pyre-interpreter/src/call.rs
printf '\n--- typedef staticmethod call helper ---\n'
sed -n '10610,10640p' pyre/pyre-interpreter/src/typedef.rs
printf '\n--- staticmethod protocol tests around subclassing ---\n'
sed -n '130,190p' pyre/extra_tests/parity_tests/staticmethod_protocol.py
printf '\n--- exact helper definitions/uses ---\n'
rg -n -C2 'fn is_staticmethod|struct StaticMethod|staticmethod_descr_call|descr_call|__call__' pyre/pyre-interpreter/srcRepository: youknowone/pyre
Length of output: 48280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the local helper definition and whether the parity tests exercise subclass __call__.
rg -n -C3 'fn is_staticmethod|pub fn is_staticmethod|StaticMethodSubclass|__call__' pyre/pyre-interpreter/src pyre/extra_tests/parity_tests/staticmethod_protocol.py
printf '\n--- CPython semantics probe ---\n'
python3 - <<'PY'
class S(staticmethod):
def __call__(self, *a, **kw):
print("subclass __call__", a, kw)
return "subclass"
def f(*a, **kw):
print("wrapped f", a, kw)
return "wrapped"
s = S(f)
print(type(s).__name__)
print("call result:", s(1, x=2))
PYRepository: youknowone/pyre
Length of output: 36696
🌐 Web query:
CPython staticmethod subclass __call__ override semantics callable subclass of staticmethod
💡 Result:
In CPython, the semantics regarding the callability of staticmethod objects changed significantly in Python 3.10 [1][2]. Prior to Python 3.10, staticmethod objects were not themselves callable [2][3]; while they acted as descriptors that returned the underlying function when accessed via a class or instance, calling the staticmethod object directly (e.g., via its reference in the class dictionary) would raise a TypeError [1][4][5]. Since Python 3.10, staticmethod objects are natively callable [1][2][3]. This was implemented by adding a call method to the C implementation of staticmethod, which simply forwards the arguments to the underlying wrapped callable [2]. Consequently, instances of staticmethod now behave as wrappers that are both descriptors (for integration into the attribute lookup process) and callable objects (acting as direct proxies for the wrapped function) [2][3]. Regarding subclassing and overriding: 1. Polymorphism and Inheritance: Static methods do not participate in standard polymorphism [6]. When you define a static method in a subclass with the same name as one in a base class, you are performing name shadowing [6]. Because static methods do not receive implicit self or cls arguments and do not utilize the descriptor get protocol to bind to the caller, there is no automatic dynamic dispatch [6]. 2. Overriding call: While you can create a subclass of staticmethod, overriding call to customize behavior is generally ineffective for the standard usage of static methods. When a class is accessed, the descriptor protocol's get method is invoked [7][8]. The default staticmethod.get returns the underlying wrapped object (the function), effectively bypassing your custom subclass's call logic unless you explicitly invoke that descriptor manually [3][7]. If you require polymorphic behavior or complex call-time logic, it is recommended to use standard instance methods, class methods, or descriptor objects that explicitly implement the desired get or call semantics, rather than subclassing the staticmethod descriptor itself [6][5].
Citations:
- 1: Make static methods created by @staticmethod callable python/cpython#87848
- 2: python/cpython@553ee27
- 3: https://docs.python.org/3/reference/datamodel.html
- 4: https://stackoverflow.com/questions/45375944/python-static-method-is-not-always-callable
- 5: https://stackoverflow.com/questions/38364980/why-are-python-static-class-method-not-callable
- 6: https://www.digitalocean.com/community/tutorials/python-static-method
- 7: https://docs.python.org/3.11/howto/descriptor.html
- 8: https://docs.python.org/3/howto/descriptor.html
🌐 Web query:
site:github.com/python/cpython staticmethod subclass __call__ tp_call
💡 Result:
Starting in Python 3.10, objects created by the staticmethod decorator are callable [1][2]. This change was implemented by adding a tp_call slot to the staticmethod type in the CPython source code [1]. Before Python 3.10, staticmethod objects were not callable, and attempting to call one directly would result in a TypeError [2][3]. The implementation of this feature involved adding the sm_call function to the PyStaticMethod_Type definition, which serves as the handler for the tp_call slot [1][4]. The implementation of sm_call is straightforward: static PyObject* sm_call(PyObject *callable, PyObject *args, PyObject *kwargs) { staticmethod *sm = (staticmethod *)callable; return PyObject_Call(sm->sm_callable, args, kwargs); } When a staticmethod object is called, the tp_call slot (sm_call) is invoked, which in turn delegates the call to the underlying wrapped callable (sm->sm_callable) using PyObject_Call [1][4]. Subclassing staticmethod does not change this behavior; the subclass will inherit the same tp_call implementation unless it explicitly overrides it, ensuring that instances of the subclass remain callable as well.
Citations:
- 1: python/cpython@553ee27
- 2: Make static methods created by @staticmethod callable python/cpython#87848
- 3: Not all method descriptors are callable python/cpython#64508
- 4: https://github.com/python/cpython/blob/master/Objects/funcobject.c
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect nearby classmethod handling to compare the dispatch strategy.
sed -n '1640,1688p' pyre/pyre-interpreter/src/call.rs
printf '\n--- classmethod-related call sites ---\n'
rg -n -C2 'is_classmethod|classmethod' pyre/pyre-interpreter/src/call.rs pyre/pyre-interpreter/src/typedef.rs pyre/extra_tests/parity_testsRepository: youknowone/pyre
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the exact predicate implementation for staticmethod and related wrappers.
rg -n -C3 'fn is_staticmethod|pub fn is_staticmethod|is_staticmethod\(' pyre/pyre-interpreter/srcRepository: youknowone/pyre
Length of output: 10823
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any parity coverage that exercises the staticmethod subclass.
rg -n -C4 'StaticMethodSubclass|staticmethod subclass|subclass.*staticmethod|__call__' pyre/extra_testsRepository: youknowone/pyre
Length of output: 11366
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the staticmethod predicate and type layout directly.
rg -n -C4 'pub unsafe fn is_staticmethod|fn is_staticmethod|STATICMETHOD_TYPE|w_staticmethod_new|typedef.*staticmethod' \
pyre/pyre-interpreter/src/function.rs pyre/pyre-interpreter/src/typedef.rs pyre/pyre-interpreter/src/baseobjspace.rs
printf '\n--- surrounding function helpers ---\n'
sed -n '1,220p' pyre/pyre-interpreter/src/function.rsRepository: youknowone/pyre
Length of output: 16300
Honor staticmethod subclass __call__ overrides before unwrapping. staticmethod subclasses can override __call__; this fast path unwraps first and skips that override. Keep the unwrap fast path only for the exact builtin type, and dispatch subclasses through their own call slot.
🤖 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/call.rs` around lines 1669 - 1675, Update the
staticmethod branch in the call dispatch logic to unwrap and forward directly
only when callable is exactly the builtin staticmethod type. For staticmethod
subclasses, skip w_staticmethod_get_func and dispatch through the subclass’s own
__call__ slot so overrides are honored, while preserving the existing argument
forwarding for exact builtin instances.
| if let Some(bound) = classmethod_call_override(callable)? { | ||
| return call_with_kwargs(frame, bound, pos_args, kwargs); | ||
| } | ||
| return Err(PyError::type_error("'classmethod' object is not callable")); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant regions in call.rs
sed -n '1640,1705p' pyre/pyre-interpreter/src/call.rs
printf '\n----\n'
sed -n '2325,2360p' pyre/pyre-interpreter/src/call.rs
printf '\n---- SEARCH ----\n'
rg -n "classmethod|__name__|type\\(callable\\)|not callable" pyre/pyre-interpreter/src/call.rsRepository: youknowone/pyre
Length of output: 5925
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1640,1705p' pyre/pyre-interpreter/src/call.rs
printf '\n----\n'
sed -n '2325,2360p' pyre/pyre-interpreter/src/call.rs
printf '\n---- SEARCH ----\n'
rg -n "classmethod|__name__|type\\(callable\\)|not callable" pyre/pyre-interpreter/src/call.rsRepository: youknowone/pyre
Length of output: 5925
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=pyre/pyre-interpreter/src/call.rs
# Print the relevant line ranges with numbers.
nl -ba "$FILE" | sed -n '1665,1695p'
printf '\n----\n'
nl -ba "$FILE" | sed -n '2338,2355p'
printf '\n----\n'
# Show all occurrences of classmethod-related callable errors in this file.
rg -n "'classmethod' object is not callable|not callable|__name__" "$FILE"Repository: youknowone/pyre
Length of output: 193
🌐 Web query:
Python classmethod subclass TypeError "object is not callable" subclass name
💡 Result:
The TypeError: 'classmethod' object is not callable occurs because you are attempting to directly call a classmethod object before it has been processed by Python's descriptor protocol [1][2][3]. In Python, the @classmethod decorator does not return a standard, callable function [4][3]. Instead, it returns a descriptor object (an instance of the classmethod type) [2][3]. This object is specifically designed to handle attribute access; when you access the method via a class (e.g., MyClass.my_method), the descriptor's get method is triggered [1][2][3]. This process "extracts" the underlying function and binds it to the class, returning a callable bound method [1][3][5]. If you bypass this lookup process—such as by accessing the method directly from the class dict or attempting to call it before the class is fully constructed—you are left with the raw, non-callable classmethod descriptor [2][6][3]. Common scenarios where this error arises include: 1. Accessing a classmethod directly from the class dict: Using MyClass.dict['my_method'] returns the raw descriptor, not the callable method [3]. 2. Calling it during class construction: Attempting to call a classmethod inside the body of the class definition (before the class object exists) fails because the descriptor has not yet been bound to a class [2]. 3. Decorator order: If you apply a custom decorator to a method, applying the custom decorator before @classmethod can cause the custom decorator to wrap the raw classmethod object instead of the bound function [3]. To resolve this, ensure you always access the method through the class (e.g., MyClass.my_method) so that the descriptor protocol is invoked, or access the underlying function directly via the func attribute if you must bypass the binding [1][4][7].
Citations:
- 1: https://stackoverflow.com/questions/75064142/defered-invocation-of-a-python-classmethod-obtained-in-a-decorator
- 2: https://stackoverflow.com/questions/37569798/how-can-one-use-a-class-method-in-a-class-attribute-definition
- 3: https://blog.peterlamut.com/2017/10/17/applying-a-decorator-to-a-class-method-results-in-an-error/
- 4: https://stackoverflow.com/questions/40593183/what-does-classmethod-do-except-changing-self-to-cls
- 5: https://stackoverflow.com/questions/76382044/factory-class-in-python-with-a-mapping-dictionary-returns-typeerror
- 6: https://stackoverflow.com/questions/11058686/various-errors-in-code-that-tries-to-call-classmethods
- 7: https://bugs.python.org/issue20309
🏁 Script executed:
python3 - <<'PY'
class C(classmethod):
pass
def f(cls):
return cls
x = C(f)
print(type(x).__name__)
try:
x()
except Exception as e:
print(type(e).__name__, e)
PYRepository: youknowone/pyre
Length of output: 192
Use the runtime type name here. classmethod subclasses can reach this branch too, and the error should match the dynamic fallback path: TypeError: '<type>' object is not callable rather than always saying 'classmethod' object is not callable'.
🤖 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/call.rs` at line 1681, Update the TypeError
construction in the callable handling branch to use the runtime type name of the
classmethod object, so subclasses report “'<type>' object is not callable”
consistently with the dynamic fallback path instead of hardcoding “classmethod”.
| #[inline] | ||
| unsafe fn require_code( | ||
| obj: PyObjectRef, | ||
| descriptor: &str, | ||
| ) -> Result<&'static crate::CodeObject, crate::PyError> { | ||
| if obj.is_null() || !unsafe { is_code(obj) } { | ||
| return Err(crate::PyError::type_error(format!( | ||
| "descriptor '{descriptor}' requires a 'code' object" | ||
| ))); | ||
| } | ||
| let ptr = unsafe { w_code_get_ptr(obj) } as *const crate::CodeObject; | ||
| if ptr.is_null() { | ||
| return Err(crate::PyError::type_error("code object has no code body")); | ||
| } | ||
| Ok(unsafe { &*ptr }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject unaligned code-body pointers before dereferencing.
w_code_new_with_hidden_applevel intentionally permits unaligned test stubs, but require_code only rejects null before forming &*ptr. Reuse the constructor’s alignment check to avoid undefined behavior when a descriptor receives such a stub.
🤖 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/pycode.rs` around lines 461 - 475, Update
require_code to validate the code-body pointer’s alignment before constructing a
reference with &*ptr. Reuse the same alignment-checking logic or helper used by
w_code_new_with_hidden_applevel, while preserving the existing null-pointer
error handling and rejecting unaligned test stubs with an appropriate error.
| let argcount = unsafe { read_code_u32(args[1], "argcount")? }; | ||
| let posonly = unsafe { read_code_u32(args[2], "posonlyargcount")? }; | ||
| let kwonly = unsafe { read_code_u32(args[3], "kwonlyargcount")? }; | ||
| let nlocals = unsafe { read_code_u32(args[4], "nlocals")? }; | ||
| let stacksize_value = unsafe { crate::builtins::space_index_w(args[5])? }; | ||
| let flags_value = unsafe { crate::builtins::space_index_w(args[6])? }; | ||
| if stacksize_value < 0 || flags_value < 0 { | ||
| return Err(crate::PyError::new( | ||
| crate::PyErrorKind::SystemError, | ||
| "Objects/codeobject.c: bad argument to internal function", | ||
| )); | ||
| } | ||
| let stacksize = stacksize_value as u32; | ||
| let flags_bits = flags_value as u32; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 \
'space_index_w\(.*\)|as u32|clamp\(i32::MIN|read_code_u32' \
pyre/pyre-interpreter/src/pycode.rs
curl -fsSL https://raw.githubusercontent.com/python/cpython/3.14/Objects/codeobject.c |
grep -E 'code_new_impl|code_replace_impl' | headRepository: youknowone/pyre
Length of output: 6115
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact Rust conversion paths around the constructor and replacement helpers.
sed -n '560,690p' pyre/pyre-interpreter/src/pycode.rs
printf '\n---\n'
sed -n '960,1065p' pyre/pyre-interpreter/src/pycode.rs
printf '\n=== CPython codeobject signatures and clinic text ===\n'
python3 - <<'PY'
import urllib.request, re
url = "https://raw.githubusercontent.com/python/cpython/3.14/Objects/codeobject.c"
text = urllib.request.urlopen(url, timeout=20).read().decode()
for pat in [
r'code_new_impl\(PyTypeObject \*type, int argcount, int posonlyargcount,',
r'code_replace_impl\(PyCodeObject \*self, int co_argcount,',
r'co_stacksize',
r'co_flags',
r'co_firstlineno',
]:
print(f'-- {pat} --')
for m in re.finditer(pat, text):
start = max(0, m.start()-250)
end = min(len(text), m.end()+600)
print(text[start:end])
print()
break
PYRepository: youknowone/pyre
Length of output: 12166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== Rust helper ranges ===\n'
rg -n -C2 'fn space_index_w|read_code_u32|as u32|clamp\(i32::MIN' pyre/pyre-interpreter/src
printf '\n=== CPython codeobject signatures (raw source, no cert verify) ===\n'
python3 - <<'PY'
import urllib.request, ssl, re
ctx = ssl._create_unverified_context()
url = "https://raw.githubusercontent.com/python/cpython/3.14/Objects/codeobject.c"
text = urllib.request.urlopen(url, context=ctx, timeout=20).read().decode()
for name in ("code_new_impl", "code_replace_impl"):
m = re.search(rf'{name}\((.*?)\)\s*{{', text, re.S)
if not m:
print(f"missing {name}")
continue
sig = m.group(1).replace("\n", " ")
print(f"{name}({sig[:500]})")
print("\n=== relevant occurrences ===")
for pat in ("co_stacksize", "co_flags", "co_firstlineno"):
print(f'-- {pat} --')
for m in re.finditer(pat, text):
start = max(0, m.start()-180)
end = min(len(text), m.end()+300)
print(text[start:end])
print()
break
PYRepository: youknowone/pyre
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== Rust helper ranges ===\n'
rg -n -C2 'fn space_index_w|read_code_u32|as u32|clamp\(i32::MIN' pyre/pyre-interpreter/src
printf '\n=== CPython codeobject signatures (raw source, no cert verify) ===\n'
python3 - <<'PY'
import urllib.request, ssl, re
ctx = ssl._create_unverified_context()
url = "https://raw.githubusercontent.com/python/cpython/3.14/Objects/codeobject.c"
text = urllib.request.urlopen(url, context=ctx, timeout=20).read().decode()
for name in ("code_new_impl", "code_replace_impl"):
m = re.search(rf'{name}\((.*?)\)\s*{{', text, re.S)
if not m:
print(f"missing {name}")
continue
sig = " ".join(m.group(1).split())
print(f"{name}({sig[:700]})")
print("\n=== relevant occurrences ===")
for pat in ("co_stacksize", "co_flags", "co_firstlineno"):
print(f'-- {pat} --')
for m in re.finditer(pat, text):
start = max(0, m.start()-180)
end = min(len(text), m.end()+300)
print(text[start:end])
print()
break
PYRepository: youknowone/pyre
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== CPython codeobject signatures and checks ===\n'
curl -k -fsSL https://raw.githubusercontent.com/python/cpython/3.14/Objects/codeobject.c |
awk '
/code_new_impl\(PyTypeObject \*type, int argcount, int posonlyargcount,/ {show=1}
/code_replace_impl\(PyCodeObject \*self, int co_argcount,/ {show=1}
show {print}
/}/ && show {exit}
'
printf '\n=== Overflow / range-related fields in pycode.rs ===\n'
sed -n '575,668p' pyre/pyre-interpreter/src/pycode.rs
sed -n '1049,1061p' pyre/pyre-interpreter/src/pycode.rsRepository: youknowone/pyre
Length of output: 1153
Reject oversized int inputs before casting
space_index_w(...)? as u32 and the co_firstlineno clamp silently truncate values outside the C int range. code.__new__ and code.replace() take int parameters in CPython, so these paths should fail on overflow instead of wrapping/clamping. The same check is needed in read_code_u32.
🤖 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/pycode.rs` around lines 575 - 588, Update
code-object construction and replacement paths, including read_code_u32 and the
co_firstlineno handling, to validate integer values against the supported C
int/u32 range before casting. Return the appropriate error on overflow or
underflow instead of allowing wrapping, truncation, or clamping; preserve
existing behavior for values within range.
| let Some(w_name) = w_name else { | ||
| return Err(crate::PyError::type_error( | ||
| "module.__init__() missing required argument 'name' (pos 1)".to_string(), | ||
| "module() missing required argument 'name' (pos 1)", | ||
| )); | ||
| } | ||
| }; | ||
| let self_ = positional[0]; | ||
| let w_name = positional[1]; | ||
| let w_doc = positional | ||
| .get(2) | ||
| .copied() | ||
| .unwrap_or_else(pyre_object::w_none); | ||
| let name = crate::baseobjspace::text_w(w_name)?; | ||
| if !unsafe { pyre_object::is_str(w_name) } { | ||
| let received = if unsafe { pyre_object::is_none(w_name) } { | ||
| "None".to_string() | ||
| } else { | ||
| crate::typedef::r#type(w_name) | ||
| .map(|tp| unsafe { pyre_object::w_type_get_name(tp) }.to_string()) | ||
| .unwrap_or_else(|| unsafe { (*(*w_name).ob_type).name }.to_string()) | ||
| }; | ||
| return Err(crate::PyError::type_error(format!( | ||
| "module() argument 'name' must be str, not {received}" | ||
| ))); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
positional[0] panics when module()/module.__init__ is called with no positional args. After split_builtin_kwargs, a keyword-only invocation such as types.ModuleType.__init__(name='x') yields positional == []; given is 0, the name kwarg satisfies the let Some(w_name) check, and execution reaches let self_ = positional[0], indexing an empty slice → panic. CPython raises a TypeError here instead. The sibling staticmethod_descr_init/classmethod_descr_init/property_descr_init all guard this with positional.first()/empty-checks; module_descr_init should too.
🛡️ Proposed fix
- let self_ = positional[0];
+ let Some(self_) = positional.first().copied() else {
+ return Err(crate::PyError::type_error(
+ "descriptor '__init__' of 'module' object needs an argument",
+ ));
+ };📝 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.
| let Some(w_name) = w_name else { | |
| return Err(crate::PyError::type_error( | |
| "module.__init__() missing required argument 'name' (pos 1)".to_string(), | |
| "module() missing required argument 'name' (pos 1)", | |
| )); | |
| } | |
| }; | |
| let self_ = positional[0]; | |
| let w_name = positional[1]; | |
| let w_doc = positional | |
| .get(2) | |
| .copied() | |
| .unwrap_or_else(pyre_object::w_none); | |
| let name = crate::baseobjspace::text_w(w_name)?; | |
| if !unsafe { pyre_object::is_str(w_name) } { | |
| let received = if unsafe { pyre_object::is_none(w_name) } { | |
| "None".to_string() | |
| } else { | |
| crate::typedef::r#type(w_name) | |
| .map(|tp| unsafe { pyre_object::w_type_get_name(tp) }.to_string()) | |
| .unwrap_or_else(|| unsafe { (*(*w_name).ob_type).name }.to_string()) | |
| }; | |
| return Err(crate::PyError::type_error(format!( | |
| "module() argument 'name' must be str, not {received}" | |
| ))); | |
| } | |
| let Some(w_name) = w_name else { | |
| return Err(crate::PyError::type_error( | |
| "module() missing required argument 'name' (pos 1)", | |
| )); | |
| }; | |
| let Some(self_) = positional.first().copied() else { | |
| return Err(crate::PyError::type_error( | |
| "descriptor '__init__' of 'module' object needs an argument", | |
| )); | |
| }; | |
| if !unsafe { pyre_object::is_str(w_name) } { | |
| let received = if unsafe { pyre_object::is_none(w_name) } { | |
| "None".to_string() | |
| } else { | |
| crate::typedef::r#type(w_name) | |
| .map(|tp| unsafe { pyre_object::w_type_get_name(tp) }.to_string()) | |
| .unwrap_or_else(|| unsafe { (*(*w_name).ob_type).name }.to_string()) | |
| }; | |
| return Err(crate::PyError::type_error(format!( | |
| "module() argument 'name' must be str, not {received}" | |
| ))); | |
| } |
🤖 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 1639 - 1656, Update
module_descr_init to validate that positional contains the required self
argument before indexing it. Handle an empty positional slice by returning the
appropriate TypeError, while preserving the existing name validation and
initialization flow when positional[0] is present; follow the sibling
staticmethod_descr_init, classmethod_descr_init, and property_descr_init guards.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53dc79bd5c
ℹ️ 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".
| pyre_object::dictmultiobject::w_dict_setitem_str_no_proxy( | ||
| ns, | ||
| "__annotate__", | ||
| make_getset_property(annotate_getter, annotate_setter, annotate_deleter), |
There was a problem hiding this comment.
When function.__dict__['__annotate__'] is invoked directly with a non-function receiver, this reqcls-less make_getset_property forwards arbitrary args[1] into function_get_annotate/function_set_annotate, which cast it to Function. For example, type(f).__dict__['__annotate__'].__get__(42, int) can read invalid fields instead of raising the descriptor TypeError; add the same Function receiver guard/reqcls stamping used for descriptors that access typed payloads.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/fa67ef5d6af75b8ef46f827bbecf75df92aa0825/pyre/pyre-interpreter/src/function.rs#L1128-L1131
When a function's PEP 649 __annotate__ callable returns anything other than a dict, Python 3.14 raises TypeError and leaves __annotations__ unset. This path accepts any non-null result and stores it in w_ann, so f.__annotate__ = lambda fmt: 1; f.__annotations__ returns and permanently caches an int, breaking callers that rely on annotations being a mapping.
ℹ️ 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".
| fn module_descr_dir(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError> { | ||
| let module = args[0]; | ||
| let w_dict = unsafe { pyre_object::w_module_get_w_dict(module) }; | ||
| if w_dict.is_null() |
There was a problem hiding this comment.
For direct descriptor calls with a foreign receiver, such as types.ModuleType.__dir__(42), this reaches w_module_get_w_dict without first proving the receiver is a module. That helper reads the Module payload layout, so arbitrary objects can be mis-cast or crash instead of raising the descriptor TypeError; add the same module receiver guard used for the new module getsets before loading the dict.
Useful? React with 👍 / 👎.
| |args| { | ||
| Ok(pyre_object::w_str_new(&unsafe { | ||
| crate::function::function_get_qualname(args[0]) | ||
| })) |
There was a problem hiding this comment.
For built-in functions that have a non-None __self__ (for example type-bound carriers like int.__new__/dict.fromkeys that this code stamps separately), returning only the qualname is not enough to reconstruct the object. Python 3.14 reduces these as getattr(__self__, __name__); with the current string-only result, pickling/copying those bound builtins will resolve the wrong global name or fail instead of preserving the bound receiver.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyre/pyre-interpreter/src/call.rs (1)
328-340: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClamp oversized
__defaults__to the trailing positional window
Atpyre/pyre-interpreter/src/call.rs:328-340, 371-383, 1364-1380, 1432-1445, 1916-1932, 1952-1968,__defaults__needs to be treated as the last N positional defaults. Oversized tuples currently bind the wrong values in the fill paths and can underflow the too-many-args message.🤖 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/call.rs` around lines 328 - 340, The positional-default handling in the call argument binding and arity-message paths must clamp oversized __defaults__ to the trailing positional parameter window. Update the ndefaults calculations and corresponding fill paths in the affected call logic so only the last N defaults are used, preventing incorrect bindings and underflow in the too-many-arguments message; preserve behavior for tuples within the valid window.
♻️ Duplicate comments (3)
pyre/pyre-interpreter/src/baseobjspace.rs (2)
10919-10925: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winValidate the
__delete__target before indexing.
property.__delete__(prop)can still reachargs[1]and panic. Require arity two at registration or retrieve the target safely and returnTypeError.Based on the prior review finding, which remains present in the current code.
🤖 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/baseobjspace.rs` around lines 10919 - 10925, Update property_descr_delete_impl to validate that args contains both the property and target object before indexing args[1]. Enforce the two-argument arity at registration or safely retrieve the target and return the established TypeError for missing arguments, while preserving the existing deleter validation flow.
4990-5022: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRoute property-subclass lookup through normal descriptor binding.
Line 5016 wraps every function as a bound method before calling
get(). A builtin function stored on a property subclass must remain unbound, but this path binds it. Theget()calls also still propagateAttributeErrorinstead of invoking the subclass’s__getattr__, as previously reported.Mirror the instance path: call
get()first, routeAttributeErrorthroughinstance_getattr_hook_or_err, and only use the user-function fallback when needed.Based on the supplied descriptor-dispatch context and the prior unresolved review finding.
🤖 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/baseobjspace.rs` around lines 4990 - 5022, Update the property-subclass branch identified by is_property to mirror normal instance descriptor lookup: call get() before any function-specific fallback, preserve builtin functions as unbound, and handle AttributeError from descriptor access through instance_getattr_hook_or_err. Retain the user-function binding fallback only when get() does not resolve the descriptor, and ensure unresolved attributes invoke the subclass’s __getattr__ path.pyre/pyre-interpreter/src/function.rs (1)
2097-2233: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winValidate method receivers before unsafe layout reads.
These public type methods accept arbitrary explicit receivers.
descr_method_callcan dispatch a null receiver, while equality, repr, attribute, hash, and reduce operations castthisdirectly to the method layout. Calls such astypes.MethodType.__repr__(1)can therefore crash instead of raisingTypeError.Use a shared
require_methodcheck before everyw_method_get_*access. This is the same receiver-validation issue previously marked addressed, but it remains in the current code.🤖 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/function.rs` around lines 2097 - 2233, Introduce and reuse a shared require_method validation helper in the method descriptor functions. Validate the explicit receiver before dispatch in descr_method_call and before every w_method_get_* access in descr_method_eq, descr_method_ne, descr_method_repr, descr_method_getattribute, descr_method_hash, and descr_method__reduce__, returning TypeError for non-method or null receivers while preserving normal behavior for valid method objects.
🤖 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/module_type_python314.py`:
- Around line 124-126: Rename the annotation-format parameter in annotate and
the corresponding parameter at the other reported use from format to
annotation_format or _annotation_format as appropriate, updating references
within each function to match and avoid shadowing the built-in while satisfying
Ruff A002, A006, and ARG005.
In `@pyre/pyre-interpreter/src/executioncontext.rs`:
- Around line 1048-1053: Replace the w_dict_str_entries iteration in the
builtins-module binding block with the GC-safe pattern used by importing.rs:
snapshot only string keys before allocating module, then allocate module and
iterate those keys while fetching each current value from the dictionary in a
non-allocating loop. Ensure module is rooted or pinned across the allocation and
pass freshly fetched values to builtin_function_set_module_obj, avoiding
retained raw value pointers from the initial snapshot.
In `@pyre/pyre-interpreter/src/function.rs`:
- Around line 134-138: Update walk_raw_function_roots to visit func.w_moduleobj
alongside the other function root fields, including for immortal builtin
functions created through the builtin-allocation path. Preserve the existing
handling of GC-managed Function allocations and ensure the module object is
traced for relocation and collection.
In `@pyre/pyre-interpreter/src/importing.rs`:
- Around line 864-872: Merge the new module-binding loop into the preceding loop
that already iterates over the same keys, eliminating the second
w_dict_getitem_str lookup. Allocate module immediately before the merged loop,
then apply both builtin_function_set_module and builtin_function_set_module_obj
operations to each retrieved value while preserving their existing behavior.
In `@pyre/pyre-interpreter/src/pycode.rs`:
- Around line 619-636: Update code_replace to rebuild the derived
localspluskinds layout after applying replacement co_varnames, co_cellvars, and
co_freevars, rather than retaining the original CodeObject values. Match the
construction used in code_new, including cell aliases, pure cells, and trailing
free-variable slots, so hidden_local() and frame indexing remain aligned.
---
Outside diff comments:
In `@pyre/pyre-interpreter/src/call.rs`:
- Around line 328-340: The positional-default handling in the call argument
binding and arity-message paths must clamp oversized __defaults__ to the
trailing positional parameter window. Update the ndefaults calculations and
corresponding fill paths in the affected call logic so only the last N defaults
are used, preventing incorrect bindings and underflow in the too-many-arguments
message; preserve behavior for tuples within the valid window.
---
Duplicate comments:
In `@pyre/pyre-interpreter/src/baseobjspace.rs`:
- Around line 10919-10925: Update property_descr_delete_impl to validate that
args contains both the property and target object before indexing args[1].
Enforce the two-argument arity at registration or safely retrieve the target and
return the established TypeError for missing arguments, while preserving the
existing deleter validation flow.
- Around line 4990-5022: Update the property-subclass branch identified by
is_property to mirror normal instance descriptor lookup: call get() before any
function-specific fallback, preserve builtin functions as unbound, and handle
AttributeError from descriptor access through instance_getattr_hook_or_err.
Retain the user-function binding fallback only when get() does not resolve the
descriptor, and ensure unresolved attributes invoke the subclass’s __getattr__
path.
In `@pyre/pyre-interpreter/src/function.rs`:
- Around line 2097-2233: Introduce and reuse a shared require_method validation
helper in the method descriptor functions. Validate the explicit receiver before
dispatch in descr_method_call and before every w_method_get_* access in
descr_method_eq, descr_method_ne, descr_method_repr, descr_method_getattribute,
descr_method_hash, and descr_method__reduce__, returning TypeError for
non-method or null receivers while preserving normal behavior for valid method
objects.
🪄 Autofix (Beta)
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
Run ID: 292b59c7-b9f3-4c0c-b09a-a0189adbb69d
📒 Files selected for processing (41)
majit/majit-backend-wasm/src/codegen.rsmajit/majit-backend-wasm/src/lib.rsmajit/majit-backend-wasm/tests/codegen_test.rspyre/bench/synth/classmethod_protocol_hot.pypyre/bench/synth/property_protocol_hot.pypyre/extra_tests/parity_tests/builtin_function_python314.pypyre/extra_tests/parity_tests/classmethod_protocol.pypyre/extra_tests/parity_tests/closure_cell_protocol.pypyre/extra_tests/parity_tests/code_python314.pypyre/extra_tests/parity_tests/function_member_descriptors_python314.pypyre/extra_tests/parity_tests/function_python314.pypyre/extra_tests/parity_tests/getset_descriptor_python314.pypyre/extra_tests/parity_tests/mappingproxy_python314.pypyre/extra_tests/parity_tests/method_python314.pypyre/extra_tests/parity_tests/module_type_python314.pypyre/extra_tests/parity_tests/property_protocol.pypyre/extra_tests/parity_tests/range_python314.pypyre/extra_tests/parity_tests/staticmethod_protocol.pypyre/pyre-interpreter/src/baseobjspace.rspyre/pyre-interpreter/src/builtins.rspyre/pyre-interpreter/src/call.rspyre/pyre-interpreter/src/display.rspyre/pyre-interpreter/src/eval.rspyre/pyre-interpreter/src/executioncontext.rspyre/pyre-interpreter/src/function.rspyre/pyre-interpreter/src/importing.rspyre/pyre-interpreter/src/module/_codecs/mod.rspyre/pyre-interpreter/src/module/_weakref/interp__weakref.rspyre/pyre-interpreter/src/objspace/descroperation.rspyre/pyre-interpreter/src/objspace/std/mapdict.rspyre/pyre-interpreter/src/opcode_ops.rspyre/pyre-interpreter/src/pycode.rspyre/pyre-interpreter/src/pyframe.rspyre/pyre-interpreter/src/pyopcode.rspyre/pyre-interpreter/src/shared_opcode.rspyre/pyre-interpreter/src/typedef.rspyre/pyre-jit-trace/src/trace_opcode.rspyre/pyre-jit/src/call_jit.rspyre/pyre-object/src/descriptor.rspyre/pyre-object/src/function.rspyre/pyre-object/src/typedef.rs
💤 Files with no reviewable changes (2)
- pyre/pyre-interpreter/src/opcode_ops.rs
- pyre/pyre-jit/src/call_jit.rs
| def annotate(format): | ||
| calls.append(format) | ||
| return {"answer": format} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the annotation-format parameters.
Both uses of format shadow the built-in and trigger configured Ruff errors; use annotation_format and _annotation_format.
Proposed fix
-def annotate(format):
- calls.append(format)
- return {"answer": format}
+def annotate(annotation_format):
+ calls.append(annotation_format)
+ return {"answer": annotation_format}
...
-module.__annotate__ = lambda format: 1
+module.__annotate__ = lambda _annotation_format: 1Based on static analysis: Ruff A002, A006, and ARG005.
Also applies to: 148-148
🧰 Tools
🪛 Ruff (0.15.21)
[error] 124-124: Function argument format is shadowing a Python builtin
(A002)
🤖 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/module_type_python314.py` around lines 124 -
126, Rename the annotation-format parameter in annotate and the corresponding
parameter at the other reported use from format to annotation_format or
_annotation_format as appropriate, updating references within each function to
match and avoid shadowing the built-in while satisfying Ruff A002, A006, and
ARG005.
Source: Linters/SAST tools
| // function.py:797-815 BuiltinFunction.w_moduleobj. The defining | ||
| // module object does not exist while `new_builtin_module_dict` fills | ||
| // the namespace, so bind each builtin's `__self__` now. | ||
| for (_, value) in unsafe { pyre_object::w_dict_str_entries(self.builtins_module) } { | ||
| unsafe { crate::function::builtin_function_set_module_obj(value, module) }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Fix critical GC vulnerabilities caused by unrooted module and stale value pointers.
w_dict_str_entries calls w_dict_items, which allocates w_str_new for string keys. Because Pyre's GC can trigger during these allocations:
- The unpinned
moduleallocated just before the loop can be collected, becoming a dangling pointer. - The raw
PyObjectRefvalues collected into the RustVecearlier in the iteration can become stale dangling pointers if a subsequentw_str_newtriggers a GC.
To fix this, snapshot only the string keys (which are GC-safe) before allocating module, then fetch the fresh value pointers inside a non-allocating loop. This mirrors the safe pattern used in importing.rs.
🔒️ Proposed fix to ensure GC safety
- let module = pyre_object::w_module_new_aliasing_dict("builtins", self.builtins_module);
// function.py:797-815 BuiltinFunction.w_moduleobj. The defining
// module object does not exist while `new_builtin_module_dict` fills
// the namespace, so bind each builtin's `__self__` now.
- for (_, value) in unsafe { pyre_object::w_dict_str_entries(self.builtins_module) } {
- unsafe { crate::function::builtin_function_set_module_obj(value, module) };
- }
+ let keys: Vec<String> = unsafe { pyre_object::w_dict_str_entries(self.builtins_module) }
+ .into_iter()
+ .map(|(k, _)| k)
+ .collect();
+ let module = pyre_object::w_module_new_aliasing_dict("builtins", self.builtins_module);
+ for key in &keys {
+ if let Some(value) = unsafe { pyre_object::w_dict_getitem_str(self.builtins_module, key) } {
+ unsafe { crate::function::builtin_function_set_module_obj(value, module) };
+ }
+ }📝 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.
| // function.py:797-815 BuiltinFunction.w_moduleobj. The defining | |
| // module object does not exist while `new_builtin_module_dict` fills | |
| // the namespace, so bind each builtin's `__self__` now. | |
| for (_, value) in unsafe { pyre_object::w_dict_str_entries(self.builtins_module) } { | |
| unsafe { crate::function::builtin_function_set_module_obj(value, module) }; | |
| } | |
| // function.py:797-815 BuiltinFunction.w_moduleobj. The defining | |
| // module object does not exist while `new_builtin_module_dict` fills | |
| // the namespace, so bind each builtin's `__self__` now. | |
| let keys: Vec<String> = unsafe { pyre_object::w_dict_str_entries(self.builtins_module) } | |
| .into_iter() | |
| .map(|(k, _)| k) | |
| .collect(); | |
| let module = pyre_object::w_module_new_aliasing_dict("builtins", self.builtins_module); | |
| for key in &keys { | |
| if let Some(value) = unsafe { pyre_object::w_dict_getitem_str(self.builtins_module, key) } { | |
| unsafe { crate::function::builtin_function_set_module_obj(value, module) }; | |
| } | |
| } |
🤖 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/executioncontext.rs` around lines 1048 - 1053,
Replace the w_dict_str_entries iteration in the builtins-module binding block
with the GC-safe pattern used by importing.rs: snapshot only string keys before
allocating module, then allocate module and iterate those keys while fetching
each current value from the dictionary in a non-allocating loop. Ensure module
is rooted or pinned across the allocation and pass freshly fetched values to
builtin_function_set_module_obj, avoiding retained raw value pointers from the
initial snapshot.
| /// function.py:797-815 `BuiltinFunction.w_moduleobj` — the module object | ||
| /// bound as `__self__` for an interp-level module function. PyPy stores | ||
| /// this on each BuiltinFunction; CPython 3.14 likewise exposes the | ||
| /// defining module object from `PyCFunction_GET_SELF`. | ||
| pub w_moduleobj: PyObjectRef, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Trace w_moduleobj from immortal builtin functions.
FUNCTION_GC_PTR_OFFSETS only covers GC-managed Function allocations. Builtins use the immortal allocation path, then store a movable module in w_moduleobj; the supplied walk_raw_function_roots visits the other new fields but omits this one, allowing collection or relocation to leave __self__ dangling.
Add func.w_moduleobj to walk_raw_function_roots.
Required cross-file change
visitor(&mut *(&mut func.w_text_signature as *mut PyObjectRef as *mut majit_ir::GcRef));
+visitor(&mut *(&mut func.w_moduleobj as *mut PyObjectRef as *mut majit_ir::GcRef));Based on the supplied walk_raw_function_roots and builtin-allocation context.
Also applies to: 300-301, 616-629
🤖 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/function.rs` around lines 134 - 138, Update
walk_raw_function_roots to visit func.w_moduleobj alongside the other function
root fields, including for immortal builtin functions created through the
builtin-allocation path. Preserve the existing handling of GC-managed Function
allocations and ensure the module object is traced for relocation and
collection.
| // function.py:797-815 BuiltinFunction.w_moduleobj — MixedModule binds | ||
| // every interp-level function to the live defining module object. | ||
| for key in &keys { | ||
| if let Some(value) = | ||
| unsafe { pyre_object::dictmultiobject::w_dict_getitem_str(w_dict, key) } | ||
| { | ||
| unsafe { crate::function::builtin_function_set_module_obj(value, module) }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Consolidate loops to eliminate redundant dictionary lookups.
The newly added iteration over keys can be safely merged into the preceding loop. By allocating module immediately before the loop, you halve the number of w_dict_getitem_str lookups, which benefits the startup time optimizations targeted by this PR.
Because neither builtin_function_set_module nor builtin_function_set_module_obj allocate or trigger GC, the unpinned module pointer remains perfectly safe inside the merged loop.
♻️ Proposed fix to merge the loops (applies to lines 851-872)
- for key in &keys {
- if let Some(value) =
- unsafe { pyre_object::dictmultiobject::w_dict_getitem_str(w_dict, key) }
- {
- unsafe {
- crate::function::builtin_function_set_module(
- value,
- pyre_object::gc_roots::shadow_stack_get(save_point + 1),
- );
- }
- }
- }
let module = pyre_object::w_module_new_aliasing_dict(name, w_dict);
// function.py:797-815 BuiltinFunction.w_moduleobj — MixedModule binds
// every interp-level function to the live defining module object.
for key in &keys {
if let Some(value) =
unsafe { pyre_object::dictmultiobject::w_dict_getitem_str(w_dict, key) }
{
unsafe {
+ crate::function::builtin_function_set_module(
+ value,
+ pyre_object::gc_roots::shadow_stack_get(save_point + 1),
+ );
crate::function::builtin_function_set_module_obj(value, module);
}
}
}🤖 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/importing.rs` around lines 864 - 872, Merge the new
module-binding loop into the preceding loop that already iterates over the same
keys, eliminating the second w_dict_getitem_str lookup. Allocate module
immediately before the merged loop, then apply both builtin_function_set_module
and builtin_function_set_module_obj operations to each retrieved value while
preserving their existing behavior.
| if argcount + kwonly > nlocals || posonly > argcount { | ||
| return Err(crate::PyError::value_error("code: invalid argument count")); | ||
| } | ||
|
|
||
| // CPython's localsplus table stores cell aliases on the local slot and | ||
| // appends only pure cells, followed by free variables. | ||
| let mut localspluskinds = vec![crate::bytecode::CO_FAST_LOCAL; varnames.len()]; | ||
| for cell in cellvars.iter() { | ||
| if let Some(index) = varnames.iter().position(|name| name == cell) { | ||
| localspluskinds[index] |= crate::bytecode::CO_FAST_CELL; | ||
| } else { | ||
| localspluskinds.push(crate::bytecode::CO_FAST_CELL); | ||
| } | ||
| } | ||
| localspluskinds.extend(std::iter::repeat_n( | ||
| crate::bytecode::CO_FAST_FREE, | ||
| freevars.len(), | ||
| )); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and locate the constructor/replacement paths.
git ls-files pyre/pyre-interpreter/src/pycode.rs
ast-grep outline pyre/pyre-interpreter/src/pycode.rs --view expanded || true
# Show the relevant sections with line numbers.
sed -n '520,760p' pyre/pyre-interpreter/src/pycode.rs
sed -n '920,1060p' pyre/pyre-interpreter/src/pycode.rsRepository: youknowone/pyre
Length of output: 21842
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect where localspluskinds and CO_FAST_HIDDEN are defined/used.
rg -n "localspluskinds|CO_FAST_HIDDEN|CO_FAST_LOCAL|CO_FAST_CELL|CO_FAST_FREE|npure_cellvars|hidden_applevel" pyre/pyre-interpreter/src/pycode.rs pyre/pyre-interpreter/src -g '!**/target/**'
# Show the CodeObject definition and any helpers around boxing/validation.
sed -n '1,260p' pyre/pyre-interpreter/src/pycode.rs
sed -n '260,460p' pyre/pyre-interpreter/src/pycode.rsRepository: youknowone/pyre
Length of output: 29956
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the frame-side layout consumers around localspluskinds.
sed -n '1360,1465p' pyre/pyre-interpreter/src/pyframe.rs
sed -n '1560,1605p' pyre/pyre-interpreter/src/pyframe.rs
sed -n '2930,3045p' pyre/pyre-interpreter/src/pyframe.rs
sed -n '3260,3295p' pyre/pyre-interpreter/src/pyframe.rs
sed -n '1565,1595p' pyre/pyre-interpreter/src/eval.rsRepository: youknowone/pyre
Length of output: 13817
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for any other localspluskinds construction or recomputation.
rg -n "localspluskinds|CO_FAST_HIDDEN|npure_cellvars\(|hidden_local\(" pyre/pyre-interpreter/src pyre -g '!**/target/**'
# Inspect any codeobject-related helpers/tests that might cover replacement semantics.
sed -n '1,220p' pyre/pyre-interpreter/src/pyframe.rs
sed -n '1720,1775p' pyre/pyre-interpreter/src/pycode.rsRepository: youknowone/pyre
Length of output: 18122
Rebuild localspluskinds in code_replace.
code_new already validates the constructor counts, but code_replace can change co_varnames/co_cellvars/co_freevars and then re-box a CodeObject with the old localspluskinds. Recompute the derived locals-plus layout there so hidden_local() and frame slot indexing stay aligned.
🤖 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/pycode.rs` around lines 619 - 636, Update
code_replace to rebuild the derived localspluskinds layout after applying
replacement co_varnames, co_cellvars, and co_freevars, rather than retaining the
original CodeObject values. Match the construction used in code_new, including
cell aliases, pure cells, and trailing free-variable slots, so hidden_local()
and frame indexing remain aligned.
Source: Coding guidelines
Completes typedef parity for the callable/descriptor family and the code
object against Python 3.14, and adds parity tests for each.
Type objects
pycode.rs/typedef.rs: the 3.14codetype layout, members,and methods (
replace,co_*attributes) with a GC offset coverage test fix.function.rs/pyre-object/src/function.rs: function typedefmembers and
__dict__/__weakref__exposure.check.py
Measures each interpreter's empty-program startup time and subtracts it from
the reported perf ratios and gates.
Tests
New parity tests under
pyre/extra_tests/parity_tests/(
code_python314.py,function_python314.py,method_python314.py,property_protocol.py,classmethod_protocol.py,staticmethod_protocol.py,updated
closure_cell_protocol.py,range_python314.py) and syntheticbenchmarks for the property/classmethod protocols.
🤖 Generated with Claude Code
Summary by CodeRabbit
firstlinenopreservation and relatedreplace()behavior.