builtins: layout-checked object.__new__, buffer request kinds, and six argument-handling sites - #1063
Conversation
|
Important Review skippedToo many files! This PR contains 1009 files, which is 909 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1009)
You can disable this status message by setting the WalkthroughThe PR updates interpreter compatibility behavior, buffer and argument validation, byte comparisons, JIT vararg inlining, tuple virtualization, parity tests, and benchmark statistics. ChangesInterpreter compatibility and validation
JIT tuple and vararg handling
Validation and benchmark baselines
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit f3cf797). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74b51f8ffb
ℹ️ 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".
| /// `bytes()` / `bytearray()` source this way, which is why | ||
| /// `bytes(memoryview(b'abcd')[::2])` is a copy and not a `BufferError`. | ||
| pub(crate) fn full_ro_buffer_bytes(obj: PyObjectRef) -> Result<Option<SimpleBufferBytes>, PyError> { | ||
| buffer_bytes(obj, false) |
There was a problem hiding this comment.
When the source implements PEP 688 __buffer__, this new constructor path still falls through to w_memoryview_new_with_flags(r_obj, 0), so bytes(x) and bytearray(x) pass PyBUF_SIMPLE to the exporter instead of the BUF_FULL_RO request this wrapper documents. An exporter that branches on the requested flags will observe the wrong contract even though PyPy's _convert_from_buffer_or_iterable calls space.buffer_w(..., space.BUF_FULL_RO); thread the full request flag through buffer_bytes rather than only disabling the contiguity check.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed on this branch after the review, in the commit
"baseobjspace: hand the buffer request kind's flags to a __buffer__ exporter":
buffer_bytes now takes the request kind and threads its flags into
w_memoryview_new_with_flags, so bytes(x) / bytearray(x) reach a PEP 688
exporter with BUF_FULL_RO (0x011c) rather than PyBUF_SIMPLE, matching
_convert_from_buffer_or_iterable (bytesobject.py:830).
— commented by Claude
| if pyre_object::memoryview::is_w_memoryview(obj) { | ||
| crate::builtins::memoryview_check_released(obj)?; | ||
| if !crate::builtins::memoryview_contiguity(obj).0 { |
There was a problem hiding this comment.
For a PickleBuffer wrapping a strided memoryview, this helper returns Ok(()) because the wrapper itself is not a memoryview; the caller then reaches buffer_as_bytes_like, which unwraps the forwarded exporter and gathers the strided bytes. That leaves BUF_SIMPLE operand paths such as strip, translate, and join accepting non-C-contiguous PickleBuffers instead of raising BufferError; recurse through forwarded_exporter here the way simple_buffer_bytes already does.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The shape is real; the predicted BufferError is not.
W_PickleBuffer.buffer_w forwards to self.buf.w_obj (interp_buffer.py:169-174),
and for a sliced memoryview that is the contiguous base object, so no
contiguity failure arises to report. check_buf_flags tests writability only
(baseobjspace.py:1646-1648). A BUF_SIMPLE operand path therefore does not raise
here upstream either.
— commented by Claude
|
Follow-up pushed (
The first was a silent wrong answer. The middle one was only visible through ordering — Two cases where the references disagree land on PyPy, which is what a
Also folded in: Verification (re-run in full on the new base)
Deliberately not changed
— commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8fc362e94
ℹ️ 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".
| // first arg is `self`; the count reported excludes it. | ||
| return Err(crate::PyError::type_error(format!( | ||
| "function takes exactly 4 arguments ({} given)", | ||
| args.len() - 1 |
There was a problem hiding this comment.
These __init__ methods are installed with make_builtin_function, so unbound calls such as UnicodeTranslateError.__init__() can reach this branch with args.len() == 0; the new args.len() - 1 subtraction then underflows (panics with overflow checks, or formats a huge count in release) instead of returning the normal TypeError. The same changed pattern is repeated in the UnicodeDecodeError and UnicodeEncodeError initializers below, so use a saturating count before formatting it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed on this branch after the review — all three initializers use
args.len().saturating_sub(1) for the reported count.
— commented by Claude
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/ada9c264b8e04bc3a7c12362b545e66476905707/pyre-jit-trace/src/jitcode_dispatch/specialize.rs#L3745-L3748
For every traced two-element BUILD_TUPLE whose backing-array length is cached, the reordered residual dispatch now reaches this arm first and constructs a canonical W_TupleObject, while the interpreter and upstream makespecialisedtuple2 construct Cls_ii/Cls_ff/Cls_oo. This intentionally replaces the upstream representation to reuse existing canonical-only consumer folds rather than porting those folds for specialized tuples, violating the repository's required line-by-line structural parity; retain the specialized shape and extend its consumer optimizations instead.
AGENTS.md reference: AGENTS.md:L231-L233
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3d151e924
ℹ️ 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 !unsafe { std::ptr::eq(pos[0], crate::typedef::w_type()) } { | ||
| return Err(crate::PyError::type_error(new_arity_message(pos[0]))); |
There was a problem hiding this comment.
For an invalid direct call such as type.__new__(42, 1), this branch passes the non-type first argument to new_arity_message, which unconditionally calls w_type_get_name and casts it to W_TypeObject. An integer or other ordinary object is therefore read using the wrong layout, potentially crashing the interpreter instead of raising the expected TypeError; reject or safely name a non-type metatype before formatting the arity error.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed on this branch after the review, in the commit
"builtins: check the metatype is a type before naming it in type.new":
new_arity_message no longer reads w_type_get_name off an unvalidated first
argument. Upstream decides arity first and names the metatype through the %N
operand, then runs _precheck_for_new (typeobject.py:886-911).
— commented by Claude
| /// Arity 2 is `makespecialisedtuple2` territory at runtime (`Cls_ii` / | ||
| /// `Cls_ff` / `Cls_oo`, `specialisedtupleobject.py`), and this arm builds the | ||
| /// canonical shape there instead. Representation is not observable — the |
There was a problem hiding this comment.
The specialized representation is observable for an exact-int pair: w_tuple_new normally creates Cls_ii, stores the integers unboxed, and w_tuple_getitem boxes them again, whereas this canonical tuple retains the original object references. Consequently hot code such as x = int('1000'); (x, x)[0] is x can change from false in the interpreter to true after tracing; leave length-two tuples on makespecialisedtuple2 or reproduce its exact representation and readers.
AGENTS.md reference: AGENTS.md:L16-L19
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The mechanism is right but the consequence does not occur, on pypy or here.
W_AbstractIntObject.is_w is bigint value equality and immutable_unique_id
is derived from the value (intobject.py:44-59); floats compare by bit pattern
(floatobject.py:196-204). So an int or float pair's representation is not
identity-observable — only bool is, and it keeps pointer identity.
Ran the suggested witness hot, 200000 iterations, collecting the set of results
so an interpreter/traced split would show as [False, True]:
pypy3 pyre-dynasm pyre PYRE_NO_JIT=1
tuple_pair_is [True] [True] [True]
float_pair_is [True] [True] [True]
bool_pair_is [True] [True] [True]
No divergence between the interpreter and the compiled trace, and none from
pypy.
— commented by Claude
| } | ||
| // A new allocation with no heap mutation, safe during the walk, and the | ||
| // same constructor `emit_object_tuple_inline` reproduces. | ||
| let concrete = pyre_object::w_tuple_new_array_backed(surplus_concretes); |
There was a problem hiding this comment.
When a newly inlined *args callee receives exactly two exact integers, _match_signature would create the tuple through the ordinary space.newtuple path and therefore use Cls_ii, but this code always constructs an array-backed tuple containing the original boxes. A callee that observes identity, for example def f(a, *args): return args[0] is a called hot as f(x, x, x), can therefore return a different result once inlined; either emit the normal specialized pair shape or decline this case.
AGENTS.md reference: AGENTS.md:L16-L19
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same answer as the BUILD_TUPLE comment: the premise about the representation is
accurate, the observable consequence is not.
def f(a, *args): return args[0] is a, called hot as f(x, x, x) with
x = int('1000'), 200000 iterations, results collected as a set:
pypy3 pyre-dynasm pyre PYRE_NO_JIT=1
vararg_is [True] [True] [True]
is on ints is value equality upstream (intobject.py:44-59), so the packed
tuple's element representation is not reachable through identity.
— commented by Claude
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/builtins.rs`:
- Around line 6671-6678: Update the argument-count formatting in
exc_unicode_translate_error_init, exc_unicode_decode_error_init, and
exc_unicode_encode_error_init to use args.len().saturating_sub(1) instead of
args.len() - 1, preventing underflow when these initializers receive no
arguments.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs`:
- Line 3887: Update the concrete argument construction near param_boxes so the
callee frame receives code.varnames.len() elements rather than limiting the
input to nparams. Ensure the seeded vararg slot is populated consistently with
PyFrame::new_for_call_with_closure_and_globals_obj, while preserving the
existing argument ordering and behavior for non-vararg calls.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 92253de9-0722-48cb-82ea-c04d41e18b3f
📒 Files selected for processing (100)
lib-python/3/test/test_descr.pypyre/bench/fannkuch.cranelift.jitstatspyre/bench/fannkuch.dynasm.jitstatspyre/bench/fannkuch.wasm.jitstatspyre/bench/fib_loop.cranelift.jitstatspyre/bench/fib_loop.dynasm.jitstatspyre/bench/fib_loop.wasm.jitstatspyre/bench/fib_recursive.cranelift.jitstatspyre/bench/fib_recursive.dynasm.jitstatspyre/bench/fib_recursive.wasm.jitstatspyre/bench/float_loop.cranelift.jitstatspyre/bench/float_loop.dynasm.jitstatspyre/bench/float_loop.wasm.jitstatspyre/bench/inline_helper.cranelift.jitstatspyre/bench/inline_helper.dynasm.jitstatspyre/bench/inline_helper.wasm.jitstatspyre/bench/int_loop.cranelift.jitstatspyre/bench/int_loop.dynasm.jitstatspyre/bench/int_loop.wasm.jitstatspyre/bench/nbody.cranelift.jitstatspyre/bench/nbody.dynasm.jitstatspyre/bench/nbody.wasm.jitstatspyre/bench/nested_loop.cranelift.jitstatspyre/bench/nested_loop.dynasm.jitstatspyre/bench/nested_loop.wasm.jitstatspyre/bench/raise_catch_loop.cranelift.jitstatspyre/bench/raise_catch_loop.dynasm.jitstatspyre/bench/raise_catch_loop.wasm.jitstatspyre/bench/spectral_norm.cranelift.jitstatspyre/bench/spectral_norm.dynasm.jitstatspyre/bench/spectral_norm.wasm.jitstatspyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstatspyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstatspyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstatspyre/bench/synth/comprehension_object_append_hot.cranelift.jitstatspyre/bench/synth/comprehension_object_append_hot.dynasm.jitstatspyre/bench/synth/comprehension_object_append_hot.wasm.jitstatspyre/bench/synth/const_arg_call_resume.cranelift.jitstatspyre/bench/synth/const_arg_call_resume.dynasm.jitstatspyre/bench/synth/const_arg_call_resume.wasm.jitstatspyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstatspyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstatspyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstatspyre/bench/synth/exception_catching_frame_tb_node.cranelift.jitstatspyre/bench/synth/exception_catching_frame_tb_node.dynasm.jitstatspyre/bench/synth/exception_catching_frame_tb_node.wasm.jitstatspyre/bench/synth/exception_inline_callee_tb_frames.cranelift.jitstatspyre/bench/synth/exception_inline_callee_tb_frames.dynasm.jitstatspyre/bench/synth/exception_inline_callee_tb_frames.wasm.jitstatspyre/bench/synth/exception_reentry_guard_finally_residual.cranelift.jitstatspyre/bench/synth/exception_reentry_guard_finally_residual.dynasm.jitstatspyre/bench/synth/exception_reentry_guard_finally_residual.wasm.jitstatspyre/bench/synth/exception_traceback_frame_lineno.cranelift.jitstatspyre/bench/synth/exception_traceback_frame_lineno.dynasm.jitstatspyre/bench/synth/exception_traceback_frame_lineno.wasm.jitstatspyre/bench/synth/exception_traceback_lineno_chain.cranelift.jitstatspyre/bench/synth/exception_traceback_lineno_chain.dynasm.jitstatspyre/bench/synth/exception_traceback_lineno_chain.wasm.jitstatspyre/bench/synth/gc_bug_bridge_flavor_traceback_names.cranelift.jitstatspyre/bench/synth/gc_bug_bridge_flavor_traceback_names.dynasm.jitstatspyre/bench/synth/gc_bug_bridge_flavor_traceback_names.wasm.jitstatspyre/bench/synth/getattribute_override_no_bind.cranelift.jitstatspyre/bench/synth/getattribute_override_no_bind.dynasm.jitstatspyre/bench/synth/getattribute_override_no_bind.wasm.jitstatspyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstatspyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstatspyre/bench/synth/list_append_write_barrier_gc.wasm.jitstatspyre/bench/synth/minmax_key_rooting.cranelift.jitstatspyre/bench/synth/minmax_key_rooting.dynasm.jitstatspyre/bench/synth/minmax_key_rooting.wasm.jitstatspyre/bench/synth/nested_list_comprehension_hot.cranelift.jitstatspyre/bench/synth/nested_list_comprehension_hot.dynasm.jitstatspyre/bench/synth/nested_list_comprehension_hot.wasm.jitstatspyre/bench/synth/pickle_terminal_raise_resume.cranelift.jitstatspyre/bench/synth/pickle_terminal_raise_resume.dynasm.jitstatspyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstatspyre/bench/synth/sre_pattern_methods.cranelift.jitstatspyre/bench/synth/sre_pattern_methods.dynasm.jitstatspyre/bench/synth/sre_pattern_methods.wasm.jitstatspyre/bench/synth/sre_wasm_min.cranelift.jitstatspyre/bench/synth/sre_wasm_min.dynasm.jitstatspyre/bench/synth/sre_wasm_min.wasm.jitstatspyre/bench/synth/sre_wasm_min1.cranelift.jitstatspyre/bench/synth/sre_wasm_min1.dynasm.jitstatspyre/bench/synth/sre_wasm_min1.wasm.jitstatspyre/bench/synth/str_index_bytes_iter_surface.cranelift.jitstatspyre/bench/synth/str_index_bytes_iter_surface.dynasm.jitstatspyre/bench/synth/str_index_bytes_iter_surface.wasm.jitstatspyre/extra_tests/parity_tests/vararg_callee_inline_shapes.pypyre/pyre-interpreter/src/baseobjspace.rspyre/pyre-interpreter/src/builtins.rspyre/pyre-interpreter/src/display.rspyre/pyre-interpreter/src/eval.rspyre/pyre-interpreter/src/gateway.rspyre/pyre-interpreter/src/objspace/descroperation.rspyre/pyre-interpreter/src/type_methods.rspyre/pyre-interpreter/src/typedef.rspyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rspyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rspyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82d10778c5
ℹ️ 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".
| // `getdata()` is read after the acquisition: a `__buffer__` slot is | ||
| // app-level code and may have resized the receiver. | ||
| let data = unsafe { pyre_object::bytesobject::bytes_like_data(a) }.to_vec(); |
There was a problem hiding this comment.
For ordering comparisons against a Python __buffer__ exporter, the exporter can mutate the left-hand bytearray before this post-acquisition copy is taken. PyPy's _comparison_helper snapshots self.getdata() before acquire_py_buffer (pypy/objspace/std/bytearrayobject.py:355-379), so an exporter that clears or replaces the receiver compares against its original contents; this implementation compares against the mutated contents and can reverse </> results. Keep the equality ordering as-is, but snapshot before acquisition for the four ordering operations.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real, and deliberately not fixed here — the faithful repair is larger than this
PR and the literal one is a regression.
Copying bytes_like_data(a) before acquisition, as suggested, regresses six
oracle rows: the length that decides the comparison must come from the
receiver's state after acquisition, not from a pre-acquisition copy. The
faithful edit reproduces RPython's list identity instead, which needs
bytearray_method_clear and bytearray_descr_init to install a fresh Vec in
W_BytearrayObject.data rather than clearing in place (mirroring
self._data = ... at bytearrayobject.py:251 and :557). Without that rebind the
captured pointer equals the current one and the capture is inert; with it, a
raw *mut Vec<u8> acquires a second owner, and a leak or a use-after-free is
the failure mode.
Trigger is an app-level __buffer__ that clears the left operand mid-comparison.
Filing it rather than attempting it inside a PR whose subject is the buffer
request kind.
— commented by Claude
| if !unsafe { pyre_object::is_str(args[2]) } { | ||
| let type_name = arg_type_name(args[2]); | ||
| return Err(crate::PyError::type_error(format!( | ||
| "{method}() argument 2 must be a single character" | ||
| "The fill character must be a unicode character, not {type_name}" | ||
| ))); |
There was a problem hiding this comment.
This shared rejection deliberately replaces two different upstream paths: center uses space.utf8_w, while ljust and rjust use convert_arg_to_w_unicode (pypy/objspace/std/unicodeobject.py:1098-1104,1351-1376). Consequently center(3, None) now reports the CPython-style fill-character error instead of the upstream typed-unwrapping error, and ljust/rjust still reject buffer operands that upstream decodes. Split these methods and port their respective conversion paths rather than normalizing them to one shortcut.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed for the wording, fixed in c390a3a and 567ea0d. Measured first
— pyre matched CPython on all seven refusals and pypy on none of them:
| case | pypy3 | pyre, before |
|---|---|---|
"ab".center(6, 1) |
expected str, got int object |
The fill character must be a unicode character, not int |
"ab".ljust(6, 1) |
decoding to str: a bytes-like object is required, not 'int' |
same shared string |
"ab".ljust(6, b"x") |
Can't convert 'bytes' object to str implicitly |
same shared string |
The three arms now follow the two converters: space.utf8_w for center
(unicodeobject.py:1101), and for ljust/rjust the bytes refusal at
unicodeobject.py:179-180 plus decode_object's "decoding to str: %S" wrapper
(unicodeobject.py:1727-1739), with None rendered unquoted where a type name is
quoted. All eight cases checked, plus the eight type names %T renders, now
print what pypy prints byte for byte.
The conversion half is deliberately not taken here. decode_object turns a
bytearray/memoryview/array fill into a fill char — "ab".ljust(6, bytearray(b"x"))
is 'abxxxx' on pypy — and pyre refuses it. That refusal is on main and
predates this PR, which only touched the message; importing the decode changes
three previously-failing inputs into successes and belongs in its own change.
The doc comment now states that as the remaining difference instead of
presenting it as a reason for a shared message.
— commented by Claude
|
Worked through all six review findings. Two were real and are fixed, one is FixedP1
The fix follows
P2 class Picky:
def __buffer__(self, flags):
if not (flags & 0x004): raise BufferError("format not requested")
return memoryview(b'wxyz')
def __release_buffer__(self, view): pass
bytes(Picky()) # cpython: b'wxyz' (flags 0x11c) / pyre before: BufferError (flags 0x0)
Hardened, but the reported path does not existP2 DeclinedP1 ×2 — arity-2 Measured, with values built at runtime far outside any cache:
P2
( — commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87fd52b910
ℹ️ 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".
| } else { | ||
| None | ||
| }; | ||
| let seeded_locals = nparams + usize::from(vararg_slot.is_some()); |
There was a problem hiding this comment.
When an admitted *args callee executes a residual operation that publishes or forces its concrete frame (for example sys._getframe() or traceback inspection), seeded_locals includes the packed tuple, but the concrete-frame builder at lines 3913-3918 still roots and passes only nparams entries. The frame therefore leaves the vararg local as PY_NULL, so frame readers during tracing can observe missing locals and bake a result that differs from the interpreted call; build concrete_args from all seeded_locals as well.
AGENTS.md reference: AGENTS.md:L32-L41
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 92df982.
Reproduced against the oracle before touching anything — the symbolic frame is
built from param_boxes, which spans seeded_locals, while the concrete frame
beside it took the first nparams entries only:
def g(a, *args):
L = sys._getframe().f_locals
return ('args' in L, L.get('args'), 'a' in L, L.get('a'))driven by a hot while loop, 200000 iterations:
| wrong iterations | |
|---|---|
| pyre-dynasm, before | 5 — e.g. (1040, (False, None, True, 1040)) |
| pypy3 | 0 |
| pyre-dynasm, after | 0 |
a was present and correct throughout; only args was missing, which is
exactly the nparams vs seeded_locals split. _match_signature writes the
vararg tuple into scope_w like any other local (argument.py:222-234), so both
halves must carry the same set. callee_arg_concretes already holds the tuple
at index nparams and the call is declined unless its length is
seeded_locals, so both bounds stay in range.
— commented by Claude
| // (stringmethods.py:200-201). This positional shape cannot skip the | ||
| // slot when `errors` follows it, so spell the default out: `decode` | ||
| // refuses a literal `None` there like any other non-str. | ||
| let mut decode_args = vec![src, w_encoding.unwrap_or_else(|| w_str_new("utf-8"))]; |
There was a problem hiding this comment.
For calls such as str(data, errors="ignore") where encoding is omitted, this now invokes w_str_new("utf-8") on every call. That constructor is explicitly the immortal/off-GC string path (pyre-object/src/unicodeobject.rs:130-165), so a hot loop permanently leaks one string header and backing buffer per iteration; preserve the omitted encoding through the decode path or use a shared rooted default instead of materializing a new immortal object.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 82d6aef, using the cell that already exists for this
shape rather than a new one: warn::PrebuiltText documents exactly this
reasoning (w_str_new is immortal, so the box is never swept, never relocated
and needs no root). bytes_method_decode only reads the encoding through
str_utf8_w, so sharing one object is not observable.
— commented by Claude
| let ty = match receiver { | ||
| Some(r) if unsafe { pyre_object::typeobject::is_type(r) } => { | ||
| unsafe { pyre_object::w_type_get_name(r) }.to_string() | ||
| } | ||
| Some(r) => crate::baseobjspace::object_functionstr_type_name(r), | ||
| None => owner.type_name.to_string(), | ||
| Some(r) if unsafe { pyre_object::typeobject::is_type(r) } => unsafe { | ||
| pyre_object::w_type_get_name(r) | ||
| }, | ||
| _ => owner.type_name, |
There was a problem hiding this comment.
When a builtin method is first bound from a subclass and then called with the wrong arity, this fallback now always uses the declaring owner. For example, checked on CPython 3.14, f = MyList().append; f() reports MyList.append(), consistent with the callable's __qualname__, whereas this branch reports list.append(); the preceding comment explicitly acknowledges the mismatch and normalizes both callable forms to the descriptor behavior instead of preserving their distinct semantics.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is what pypy prints, so there is nothing to preserve.
BuiltinCode.funcrun_obj passes func.qualname — the declaring type's name —
as the name source for the arity/keyword error, and the receiver w_obj is
never consulted for it (gateway.py:875). Ran the comment's own example on
pypy3 and the output matches this branch verbatim.
— commented by Claude
|
The What broke. Attribution and narrowing, all on the full module (550 tests):
So the Cost. The Verification on the new head. CPython gate 46/46 no regressions; One note on the cranelift run: The — commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/481d73eabfece0b0772cb46cfc81f7f8fa04c806/pyre-interpreter/src/eval.rs#L1532-L1534
When an invalid cause is used, such as raise ValueError() from 1, PyPy's OperationError._exception_getclass reports exception causes must derive from BaseException, not int (pypy/interpreter/error.py:255-260), but this replacement drops the , not <type> suffix and the updated test locks in that divergence. Include the rejected cause's class name when constructing this error to preserve the upstream behavior.
AGENTS.md reference: AGENTS.md:L231-L232
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/acadf40daeb450ffbe5ee948a6148879ff6ba6e8/pyre-interpreter/src/builtins.rs#L4951-L4955
For type.__new__(42, 1, (), {}), this new arm leaves w_metaclass null and validates only the trailing (name, bases, dict), producing type() argument 1 must be string instead of rejecting 42 as the metatype. PyPy's descr__new__ runs _precheck_for_new before _check_new_args, so validate pos[0] before dispatching this shape. The newly added four-positional branch is fresh evidence distinct from the earlier two-positional metatype report.
AGENTS.md reference: AGENTS.md:L231-L232
ℹ️ 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: 8653735c58
ℹ️ 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".
| } { | ||
| return crate::objspace::descroperation::compare_slot(a, b, op); | ||
| } | ||
| let buffer = match crate::baseobjspace::simple_buffer_bytes(b) { |
There was a problem hiding this comment.
When b implements Python-level __buffer__, simple_buffer_bytes(b) can execute arbitrary allocating code and trigger the moving GC, but a remains only an unrooted raw pointer and is dereferenced afterward at line 16638. An exporter that induces collection can therefore make the equality path read a stale receiver address, causing memory corruption or a crash; pin a before acquisition and reload it from the shadow stack for the required post-acquisition snapshot.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The code path is as described, but the ordering is upstream's own.
W_BytearrayObject.descr_eq acquires first and reads the receiver second —
view = space.acquire_py_buffer(w_other, space.BUF_SIMPLE) at
bytearrayobject.py:317, then value = self.getdata() at :318. bytearray_compare
reproduces that, which is why the receiver read sits after the acquisition.
Reordering it would diverge from upstream, not converge on it.
The related hazard for the ordering comparisons is tracked in the
Snapshot ordering receiver thread, where the faithful repair is described.
— commented by Claude
2a52f28 to
567ea0d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 567ea0d24e
ℹ️ 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 pos.len() == 4 { | ||
| if unsafe { pyre_object::is_type(pos[0]) } { | ||
| w_metaclass = pos[0]; | ||
| } | ||
| return type_descr_new_with_metaclass(&pos[1..], w_metaclass, kwargs); |
There was a problem hiding this comment.
For a direct call such as type.__new__(42, 1, (), {}), this newly added fallback leaves w_metaclass null and validates 1 as the class name, producing the argument-1 diagnostic instead of rejecting 42 as a non-type metatype. PyPy's descr__new__ runs _precheck_for_new after confirming the three public arguments are present but before _check_new_args, so call precheck_for_new(pos[0]) before dispatching this shape. Fresh evidence beyond the previously reported two-argument case is this new four-position branch, which bypasses that branch's precheck.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in f6e0e2f — precheck_for_new(pos[0])? now runs before the branch dispatches.
type.__new__(42, 1, (), {})
pypy3 TypeError: X is not a type object (int)
before TypeError: type() argument 1 must be string, not int
after TypeError: X is not a type object (int)
Checked the blast radius against pypy3 on an 18-case matrix: the five-argument super() shape and every call whose name argument is a str are taken by the scan above and never reach this branch, so M(1, (), {}), type.__new__(M, 1, (), {}) and type.__new__(M, 'A', (), {}) are unchanged and still match.
Worth recording separately: the same matrix shows five rows that diverge and are not from this branch — origin/main has a byte-identical scan loop, and this PR added no branch that reaches them.
| pypy3 | here | |
|---|---|---|
type.__new__(42, 'A', (), {}) |
X is not a type object (int) |
creates the class |
type.__new__(None, 'A', (), {}) |
X is not a type object (NoneType) |
creates the class |
type.__new__('s', 'A', (), {}) |
X is not a type object (str) |
type() takes 1 or 3 arguments |
type.__new__(int, 'A', (), {}) |
type.__new__(int): int is not a subtype of type |
metaclass conflict: ... |
type.__new__(42, 'A', ()) |
?.__new__() takes exactly 3 arguments (1 given) |
type() takes 1 or 3 arguments |
The first two are wrong answers rather than wrong wording. The cause is structural: descr__new__ is a plain switch on len(__args__.arguments_w) with _precheck_for_new after it (typeobject.py:886-911), whereas type_descr_new locates the triple by scanning for the first str, so the metatype is never validated on that path. Bolting a precheck onto the scan is not safe on its own, because the scan is also what absorbs the five-argument super() binding shape [self, metatype, name, bases, dict], where pos[0] is not the metatype. Porting the arity switch means changing how the receiver reaches this function, which is out of scope here.
Verified on this head: dynasm 391/391, cranelift 391/391, wasm 387/387, parity all pass, CPython gate 46 PASS / 0 FAIL / no regressions, cargo test --all 7495 passed 0 failed, cargo fmt --check clean, 0 LLBC STALE.
— commented by Claude
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78b45f07f2
ℹ️ 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".
| /// missing decode still shows. | ||
| /// | ||
| /// A str of the wrong length is a separate refusal, and the one upstream | ||
| /// words per method. | ||
| fn pad_fillchar(args: &[PyObjectRef], method: &str) -> Result<CodePoint, crate::PyError> { | ||
| if args.len() <= 2 { | ||
| return Ok(CodePoint::from_char(' ')); | ||
| } | ||
| if !unsafe { pyre_object::is_str(args[2]) } { |
There was a problem hiding this comment.
When the fill operand is a one-byte bytearray, memoryview, or array, this non-str branch raises TypeError, while PyPy's descr_ljust and descr_rjust pass it through convert_arg_to_w_unicode/decode_object and use the decoded character. Fresh evidence beyond the earlier review is the final follow-up's own comment immediately above this branch explicitly stating that the decode is still not imported; changing only the error wording therefore leaves the behavioral divergence intact. Split center from ljust/rjust and port their respective conversion paths.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
| /// Arity 2 is `makespecialisedtuple2` territory at runtime (`Cls_ii` / | ||
| /// `Cls_ff` / `Cls_oo`, `specialisedtupleobject.py`), and this arm builds the | ||
| /// canonical shape there instead. Representation is not observable — the |
There was a problem hiding this comment.
When one BUILD_TUPLE site alternates between traced and residual execution, this canonical object conflicts with the Cls_ii/Cls_ff/Cls_oo object the interpreter constructs, creating mixed-representation side exits. Fresh evidence beyond the earlier identity-based review is in the committed baselines: binary_int_overflow_local_resume rises from 647 to 686 guard failures and exc_bridge_entry_guard_not_removed from 809 to 1009, each gaining a bridge on all three backends, and the follow-up commit attributes these deltas to this representation mismatch. Keep arity two on the specialized shape and port its consumers instead of deliberately diverging from the interpreter.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
…x argument-handling sites
`object.__new__` now runs `check_user_subclass`, and `type` gets its own
Layout typedef (`TYPE_TYPE`) instead of sharing `object`'s — that identity is
what the check reads, so `object.__new__(int)` and `object.__new__(<metaclass>)`
are refused.
Buffer requests are split by kind: `bytes()` / `bytearray()` read their source
with `BUF_FULL_RO` (a strided memoryview is copied out, not refused), while
bytes-method operands — `replace`, `strip`, `join`, `translate`, the fill char —
require the C-contiguity `BUF_SIMPLE` carries.
`bytes.startswith` / `endswith` convert the operand before the
`start > len(value)` early-out, so an empty window no longer hides a prefix of
the wrong type.
A supplied `None` is a value, not an omitted argument, for `bytes.center` /
`ljust` / `rjust`'s fill char, `bytes.decode`'s encoding and errors,
`bytearray.pop`'s index, and `memoryview.cast`'s shape. `builtin_str` spells
the utf-8 default out where it previously passed `None` through.
Surplus positional arguments are rejected by `str.replace`, `bytes.center` /
`ljust` / `rjust`, `bytearray.remove` and `memoryview.cast`.
An unset `__slots__` read reports `%T` — the bare type name — through
`raiseattrerror`, matching the same miss taken through the descriptor's
`__get__`. `type(1, (), {})` names argument 1 instead of reporting an arity
error, and argument 1's message says `string` like its two siblings.
`SyntaxError.__str__` splits its filename with `ntpath` rules on windows.
Assisted-by: Claude
`cmp_guard_bytearray` admitted only bytes and bytearray, so
`bytearray(b'ab') == array.array('B', [97, 98])` answered `False` and
`bytearray(b'ab') < memoryview(b'b')` raised. `descr_eq` / `descr_ne` /
`_comparison_helper` (bytearrayobject.py) hand a non-bytes-like operand to
`space.acquire_py_buffer(w_other, space.BUF_SIMPLE)` and turn only the
TypeError that raises into `NotImplemented`; a released view's ValueError and
a strided view's BufferError propagate.
The six dunders are now built from `bytearray_compare`, which keeps the
by-layout `compare_slot` for the bytes-like arms and for any receiver the slot
was not meant for, and reads the receiver's data after the acquisition since a
`__buffer__` slot is app-level code.
`bytes` keeps the narrow guard: its comparisons never acquire a buffer, which
is what makes `b'ab' == array.array('B', [97, 98])` `False`.
`ordering_satisfies` replaces the two spellings of the `_memcmp`-result
mapping in `descroperation`.
`pad_fillchar`'s doc records why `str.ljust` / `rjust` keep refusing a buffer
fill char: `convert_arg_to_w_unicode` decodes one, but CPython refuses it for
all three methods and pyre follows CPython there.
Assisted-by: Claude
…Error `descr_member_get`'s miss reported `getfulltypename` before d8fc362 narrowed it to the bare `%T` name; `test.test_descr.test_slots` pins the `module.__qualname__` form and the cpython_tests runner drives that module through its dotted-identity driver, so the narrowing turned the gate red. The unit test's name and expectation go back with it. `test_bad_new` regains the `@support.impl_detail(cpython=False)` marker the 3.14.6 stdlib import replaced with CPython's `@unittest.expectedFailure`: the layout check added in fdcce06 makes the test pass here, and an unexpected success fails the module. Assisted-by: Claude
…rarg local `try_walker_specialize_newtuple_object` no longer declines arity 2. The canonical array-backed `W_TupleObject` is the shape `subscr_tuple`, `builtin_len`, `get_iter` and the array-backed arm of `unpack` already read, whereas a `makespecialisedtuple2` pair has an UNPACK fold and nothing else, so every other read of one forced it out of virtual state. The `spec_ii` arm stays as the fallback for a pair whose backing-array length never reached the heap-cache as a constant. Measured over an empty loop: `(i, i + 1)[1]` 258.6ns -> 0.1ns, `f()[1]` for a pair-returning `f` 1247.7ns -> 34.4ns, `d[(a, b)]` 1207.6ns -> 313.1ns. `try_walker_inline_resolved_user_call` accepts a `*args` callee and writes `newtuple(starargs_w)` into `scope_w[co_argcount]` (`argument.py:222-234 _match_signature`) instead of leaving the call residual. `**kwargs` and keyword-only callees stay residual, as does a zero-surplus call (the empty tuple is a singleton) and a bound method whose callee has no positional parameter to hold the receiver — its `callee_args[0]` is still the placeholder the resolved half replaces with `GetfieldGcR(Method.w_self)`. 300k calls: `f(*args)` 0.480s -> 0.000s, `c.m(*args)` 0.514s -> 0.000s, `f(a=i)` into `**kw` 0.254s -> 0.142s. jit-stats: the trace-built pair and a runtime-built specialised one meeting at one code location costs a side exit, so three fixtures gain a bridge (`binary_int_overflow_local_resume`, `exc_bridge_entry_guard_not_removed`, `list_append_write_barrier_gc`); `getattribute_override_no_bind` compiles one loop instead of two now that its `*args` callee inlines, and `pickle_ctor_args` sheds half its cranelift guard failures. The wasm baseline missing for `exception_escape_hot_callee_tb_node_once` is recorded. Assisted-by: Claude
15 synthetic fixtures move on all three backends: the arity-2 BUILD_TUPLE virtualization composes with the walker setfield_gc store and the FOR_ITER RETURN_VALUE admission from #1068 and the loop-perf folds from #1061, so the sre, exception-traceback and comprehension traces take fewer side exits — `nested_list_comprehension_hot` drops from 6 bridges / 1202 guard failures to 2 / 401, `sre_wasm_min` from 8 / 1849 to 5 / 1161. The 30 macro baselines only gain `field_pos_attached_misplaced` and `field_pos_spec_misplaced` at 0, the counters #1053 added to the binary without recording them here. Assisted-by: Claude
Each carries `# pyre-check: skip-cpython` followed by the measured cpython and pyre times, the way the directive requires. The directive itself is on the base; this only names the fixtures that claim it. Assisted-by: Claude
`specialised_pair_consumers.py` reads the `_ii` / `_ff` / `_oo` pair layouts through `len()`, subscription and unpacking, at a constant index, at an alternating index and off a nested pair, with accumulators that do not cancel a swapped or mis-represented slot. The specialisation folds themselves are already on the base. Assisted-by: Claude
`compare_slot`'s tuple arm walked both operands with `w_tuple_getitem`, which for a `W_SpecialisedTupleObject_ii` / `_ff` builds a fresh box per element because the payload is an inline machine word. `specialised_tuple_same_class_eq` reproduces `specialisedtupleobject.py:113-127 descr_eq`: when both operands are the same specialised class the value slots compare raw, with the float arm falling back to the bit pattern so the same NaN in both slots stays equal (`float2longlong` upstream) while `+0.0` / `-0.0` are caught by the value compare. `_oo` slots still go through `eq_w`. Eq/Ne only — ordering keeps the generic walk, as upstream does. A mixed pair (one specialised, one array-backed) falls through to the existing element walk. Measured `(1, 2) == (1, 2)` on two loop-invariant pairs: 252.9ns -> 151.1ns. The remainder is not the boxing: `_ff` barely moves and an arity-3 array-backed comparison is 29ns, so ~120ns of arity-2 comparison is upstream of this arm. Assisted-by: Claude
The baselines committed in e3d151e were recorded against a stale `build/llbc`: a `pyre-jit-trace` / `pyre-interpreter` edit invalidates the extraction fingerprint, and the JIT reads the function bodies it inlines out of those artefacts, so trace shape — not just field offsets — depends on them. The recorded counters therefore did not reproduce on CI, which extracts its own. `pyre/check.py (ubuntu-24.04)` failed with 49 jit-stats regressions across 19 benches on all three backends with identical numbers. Re-extracted `pyre-object pyre-interpreter pyre-jit`, rebuilt dynasm, cranelift and wasm with no `LLBC STALE` warning, and re-recorded. A local run now reproduces the CI numbers exactly, e.g. `nested_list_comprehension_hot` bridges 2 -> 6 and guard_failures 401 -> 1202. 84 counter values change across 21 benches (51 guard_failures, 33 bridges_compiled). 19 are the arity-2 tuple fold's mixed-representation side exits, which the ca7351f message under-reported for the same stale-artefact reason. Two are improvements from the specialised-pair subscript fold: `divmod_long_int_pair` guard_failures 9 -> 7 (its pair result now folds) and `exception_oserror_fields` 202 -> 201. The remaining 2218 added lines are `field_pos_attached_misplaced` / `field_pos_spec_misplaced`, counters #1053 added to the binary without recording them. check.py --synthetic-only: dynasm 371/371, cranelift 371/371, wasm 370/370. Assisted-by: Claude
`type_descr_new` reached `new_arity_message` with an unvalidated first
argument, and that read it through the `W_TypeObject` layout:
`type.__new__(42, 1)` segfaulted and `type.__new__('s', 1)` reported
`s.__new__() takes exactly 3 arguments (1 given)`, naming the str's own
bytes.
`descr__new__` (typeobject.py:886-911) decides the arity first and then
runs `_precheck_for_new` (typeobject.py:1001-1003), so the one-name form
now refuses a non-type with `X is not a type object (%T)` and the
no-name form names it through the `%N` operand spelling — `W_Root.getname`
(baseobjspace.py:90-94), which answers `?` when `__name__` is absent.
`type.__new__(42)` answered `<class 'int'>` and now raises.
Also folds the two `pos.len() == 1` arms, which had become the same
branch, and saturates the reported argument count in the three unicode
error initialisers; those are installed as `wrapper_descriptor`s that
reject a zero-argument call before the body runs, so the subtraction was
not reachable.
Assisted-by: Claude
…exporter `buffer_bytes` passed a literal `0` to `w_memoryview_new_with_flags` on every path, so a Python `__buffer__` saw `PyBUF_SIMPLE` even when the caller was `full_ro_buffer_bytes`, whose request is `BUF_FULL_RO`. An exporter that branches on the request observed the wrong one: `bytes(x)` on a `__buffer__` that requires `PyBUF_FORMAT` raised `BufferError` where cpython returns the bytes. `require_contiguous: bool` becomes a `BufferRequest` naming the two requests, and both the contiguity rule and the exporter flags are derived from it. `BUF_FULL_RO` moves next to it from `interp_buffer`, which already spelled the same constant. Assisted-by: Claude
`try_walker_specialize_subscr_specialised_pair` reaches
`W_SpecialisedTupleObject_oo.value0` / `value1` through
`walker_emit_specialised_pair_item`, which reads them with a `getfield_gc_r`.
That read is wrong code on this path. `test.test_datetime` holds
`self.lt = (array('q', ut), array('q', ut))` and reads `self.lt[dt.fold]`; with
the fold in place the next call in that frame comes out one positional argument
short, so `bisect.bisect_right(lt, timestamp)` raises `TypeError: bisect_right()
missing 1 required positional argument: 'x'` and the module goes `PASS -> FAIL`
on the CPython gate.
Measured on the full module, 550 tests: `PYRE_NO_JIT=1` passes while the JIT
fails one. Declining only the `Object` kind passes. `MAJIT_NO_BRIDGE=1` still
fails, so the exit is the main trace's and not a compiled bridge; executing the
residual for the object arm and recording its concrete result, dropping the
`replace_box`, and emitting the index guard through
`walker_emit_guard_with_snapshot` each leave it failing. What makes the
object-slot read itself wrong is not yet known.
The decline sits in the subscript entry point rather than in
`walker_emit_specialised_pair_item`, because UNPACK reaches the same slots
through that helper with no index operand and is sound. The `ii` and `ff` arms
share the class guard and the pinned index and keep their fold — over an empty
loop, `II[0]` 0.1ns and `II[i & 1]` 0.7ns against 169.3ns and 175.5ns with the
whole fold declined. `len()` on a pair is untouched. `OO[0]` returns to the
residual at 193.1ns from 35.9ns.
Assisted-by: Claude
…e measures The rebase carried this branch's earlier recording through without raising a conflict: bridges_compiled=4 and guard_failures=803. All three backends read 3 and 603 against the rebased tree, which is what main records. Assisted-by: Claude
The rebase resolved this file to main's side, which reads loops_compiled=2 and guard_failures=2. The tree measures 1 and 1 on wasm, matching the dynasm and cranelift baselines for the same fixture. The re-record also picks up the five counters added to the snapshot field set. Assisted-by: Claude
The symbolic frame is built from `param_boxes`, which spans `seeded_locals`
and so carries the packed `*args` tuple; the concrete frame beside it was
built from the first `nparams` entries only. That frame is published on the
interpreter frame chain for the whole sub-walk, so a residual running inside
an admitted `*args` callee read the vararg name as unbound:
def g(a, *args):
return 'args' in sys._getframe().f_locals
called in a hot `while` loop answered False on 5 of 200000 iterations, where
pypy answers True on all of them. `_match_signature` writes the vararg tuple
into `scope_w` like any other local (argument.py:222-234).
`callee_arg_concretes` already holds the tuple at index `nparams` and is
declined unless its length is `seeded_locals`, so both bounds stay in range.
Assisted-by: Claude
`center` converts with `space.utf8_w` and `ljust`/`rjust` with
`convert_arg_to_w_unicode` (unicodeobject.py:1101, 175-184), and the two
refuse in different words. Both arms carried one shared string that matched
neither:
"ab".center(6, 1) pypy: expected str, got int object
"ab".ljust(6, b"x") pypy: Can't convert 'bytes' object to str implicitly
pyre, both: The fill character must be a unicode character, not X
`arg_type_name` renders the same names `%T` does for all eight types checked.
`decode_object`, which turns a buffer operand into a fill char for
`ljust`/`rjust`, is still not imported; the doc comment now states that as the
remaining difference instead of as the reason for a shared message.
Assisted-by: Claude
`builtin_str` wrapped a fresh "utf-8" for every `str(b, errors=...)` call that omits the encoding. `w_str_new` is immortal, so each one stays allocated for the life of the process. `warn::PrebuiltText` is the existing cell for this shape; `bytes_method_decode` only reads the encoding through `str_utf8_w`. Assisted-by: Claude
`convert_arg_to_w_unicode` declines only `bytes` itself; every other non-str
operand reaches `decode_object`, which reports a failed conversion as
"decoding to str: %S" over the buffer error (unicodeobject.py:175-184,
1727-1739). The `ljust`/`rjust` arm now says that, with `None` rendered
unquoted where a type name is quoted:
"ab".ljust(6, 1) decoding to str: a bytes-like object is required, not 'int'
"ab".ljust(6, None) decoding to str: a bytes-like object is required, not None
"ab".ljust(6, b"x") Can't convert 'bytes' object to str implicitly
All eight cases checked now print what pypy prints, byte for byte.
Assisted-by: Claude
`type_descr_new` finds `(name, bases, dict)` by scanning for a str, so a
four-position call whose name is not a str falls past the scan. That branch
took `pos[0]` as the metatype only when it already was a type and otherwise
left it null, which sent `type.__new__(42, 1, (), {})` on to report argument
1. `descr__new__` runs `_precheck_for_new` once the count is settled and
before `_check_new_args` (typeobject.py:899), so the branch calls
`precheck_for_new` first:
type.__new__(42, 1, (), {})
before TypeError: type() argument 1 must be string, not int
after TypeError: X is not a type object (int)
The five-argument `super()` shape and every call whose name is a str are
taken by the scan above and do not reach this branch.
Assisted-by: Claude
`try_walker_specialize_newtuple_object` emitted a canonical `W_TupleObject`
virtual at every arity, including 2. At that arity the interpreter calls
`makespecialisedtuple2` (specialisedtupleobject.py:169-179) instead, so the
virtual is the one shape the runtime never builds: `Cls_ii` / `Cls_ff` /
`Cls_oo` hold `value0` / `value1` inline and carry no `wrappeditems` block.
The trace is self-consistent on its own, but a side exit puts a real pair in
front of a consumer the trace chose for the canonical layout, and
`try_walker_specialize_subscr_specialised_pair` reads a field that is not
there. A pair built inside the loop and subscripted at an alternating
non-negative index reaches it:
t = (i, BIG)
item = t[i & 1]
which segfaults, or returns whatever the stale pointer lands on — one run
answered `TypeError: unsupported operand type(s) for &: 'type' and 'int'`.
`extra_tests/parity_tests/subscr_specialised_pair_shapes.py` fails both ways.
Building the pair outside the loop, or indexing it only at a negative index,
does not reach it.
Arity 2 now falls to `try_walker_specialize_newtuple`, which builds the
specialised shape the runtime builds. Arity 1 and 3 up are unchanged.
Assisted-by: Claude
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3cf79760d
ℹ️ 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".
| } | ||
| // A new allocation with no heap mutation, safe during the walk, and the | ||
| // same constructor `emit_object_tuple_inline` reproduces. | ||
| let concrete = pyre_object::w_tuple_new_array_backed(surplus_concretes); |
There was a problem hiding this comment.
When tracing an eligible call with surplus *args, this GC-managed tuple is kept only as a raw pointer in callee_arg_concretes until it is stamped onto tuple_op roughly 900 lines later. The intervening admission and guard-emission code can allocate or encounter a concurrent collection; raw concrete shadows are not GC roots, so the tuple may be swept or its address may become stale before it is used to seed the callee frame. Pin the tuple across this interval or create and stamp its frontend op immediately.
Useful? React with 👍 / 👎.
…elines `9d2fff92649` (#1063) re-recorded 993 jit-stats baselines. All but four gained only the two new `field_pos_*_misplaced=0` keys; three changed a value: binary_int_overflow_local_resume bridges 5 -> 6 guards 647 -> 686 exc_bridge_entry_guard_not_removed bridges 4 -> 5 guards 809 -> 1009 list_append_write_barrier_gc bridges 5 -> 6 guards 1345 -> 1562 Five runs report the pre-#1063 values and none reports the recorded ones: dynasm, cranelift and wasm here, and `main`'s own CI on ubuntu-24.04 and macos-latest at 9d2fff9 -- run 31139317566, jobs 92747505633 and 92748753166, on a tree carrying no commit from this branch. The three benches fail identically on all three backends in each of them. Only those two keys are restored; #1063's two added keys stay. The fourth bench it revalued, getattribute_override_no_bind, is left as recorded: it passes here and in that CI run, so its new values do reproduce. Assisted-by: Claude
… on a non-measurement (#1095) * jit: stamp the qmut abort's own subwalk coordinate, and re-seed a live-NULL operand slot A walk that executed residual side effects and then fails to commit its end state falls back to the legacy replay from the traced region's entry, which runs those residuals a second time. Two shapes reached that fallback; both show up under `PYRE_FBW_CENSUS=1` as `committed=false effects>0`. `WalkSession::abort_in_subwalk` is sticky — `claim_abort_coordinate` only ever sets it — so an inline sub-walk abort the walk recovered from left it true for every later abort in the same trace attempt, and `flush_qmut_abort_state`'s gate then read a root-frame abort as a callee coordinate. The `ForceQuasiImmutable` raise in `dispatch_residual_call_iRd_kind` now stamps it from `fbw_mode.inline_subwalk` at the raise point, as the two kept-stack branch-guard raises already do. `reseed_vstack_from_shadow` rejected a NULL const-ptr shadow slot outright, because a NULL there can also mean a slot the portal never wrote. It now accepts one carrying the `virtualizable_live_null_slots` marker, which records that the last executed store into that slot wrote a NULL. PUSH_NULL's `self_or_null` sentinel is such a slot and stays live across the whole callable/args/kwargs build ahead of a CALL; the reorder region re-seeds the mirror in the middle of that build, and the rejected slot made `capture_vstack_mirror_image` refuse the image, leaving an escape inside the call with no blackhole resume. `capture_vstack_mirror_image`'s decline line gains the Python pc and the mirror boxes. The LoadName cell-fold gate comment is rewritten to the measured state: with the gate lifted the `bench/synth` corpus is output-correct, and what fails is `exception_reraise_tb_depth_jitstress` at 13.0x against its 4x pypy gate plus four benches' jit-stats. Measured with the gate lifted, in-place arms: `iter57/real_exception` 100003 -> 100000, `exception_reentry_guard_finally_residual` `leaked 4 reentry 2` -> `leaked 0 reentry 0`. Assisted-by: Claude * jit: record why reseed_vstack_from_callee_shadow keeps its NULL const-ptr rejection The callee-shadow reseed is the structural twin of `reseed_vstack_from_shadow` and rejects a NULL const-ptr the same way, but its source is a sparse `HashMap`, where a present key is already the write-witness the dense virtualizable array needed a per-slot side table to supply. So the clause discards a proven write whose value happens to be PUSH_NULL's `self_or_null`. Measured before writing this: dropping the clause leaves `check.py --backend dynasm` at 386/386 with no jit-stats movement and no baseline change, so the corpus does not distinguish the two behaviours. Behaviour unchanged; the comment records the asymmetry and the measurement. Assisted-by: Claude * rework.md: refresh the audit against the current tree The findings were measured on `pc-map` on 2026-07-05. Re-measured on `ec-wiring` at base 58fcd37, thirteen of the fifteen issues the document tracks are closed and the priority order has inverted. F1: gh#366/367/368/369 closed; `metadata.pc_map` and `resume_jitcode_pc_for` have zero hits and `resume::SnapshotFrame.pc` is the JitCode byte offset. The surviving `pc_map` matches are the compile-time exit-recovery `Vec<usize>` in jit/codewriter.rs and jit/flatten.rs, a different thing. Residue recorded: recorder.rs's SnapshotFrame doc still describes the deleted translation, py_pc is stored rather than derived, and build_state_field_snapshot stamps the JitCode offset into py_pc (unproven, needs a repro). F2: verified done — `is_full_body_walk`, `PYRE_FULL_BODY_WALK` and `OpcodeHandler for MIFrame` have zero hits each. F3: regressed to 15 registrations against MAX_EXTRA_ROOT_WALKERS = 16; the 16th caller hits `panic!("capacity exceeded")` at startup. F4: gh#346 and gh#373 closed, coverage still landing (#1065); abort_permanent unchanged in scale, but the exit criterion is the census, not a match count. F5: gate-triage.md now exists but the population grew from 119 matches to 245 distinct PYRE_* identifiers. Sequencing amended to WS3 > WS2 > WS1-residue > WS4. Assisted-by: Claude * rework.md: correct the F5 gate count to a reproducible measurement The refresh recorded 245 distinct `PYRE_*` identifiers against the audit's original 119. That figure does not reproduce: tracked `*.rs` holds 131 distinct identifiers, all tracked files 174, and 548 raw matches. The quantity comparable to the original "distinct `PYRE_*` env gates" is the set of names actually read from the environment, which is 126. The command is now stated in the document so the number can be re-derived, along with the three other counts it is easy to confuse it with. Assisted-by: Claude * check.py: do not fail a ratio gate whose baseline is clamped to the floor `_exec_time` clamps a startup-subtracted time to `EXEC_TIME_FLOOR_S` so ratios cannot divide by ~0. When the pypy baseline lands there, the ratio is `pyre_exec / EXEC_TIME_FLOOR_S` and the ceiling it is compared against is an absolute wall-clock budget of `ceiling * EXEC_TIME_FLOOR_S` seconds, fitted on whichever host wrote the header. The comparison table already marks those ratios `~` and prints "ratio is not a measurement"; the gate failed the run on them anyway. `failed_bound` now returns None whenever the baseline is clamped, instead of requiring the backend to be at the floor as well. Only the ceiling changes behaviour: the floor arms at `exec_baseline >= FLOOR_GATE_MIN_BASELINE_S`, which a clamped baseline is always under. The gate can therefore only pass more than before, never fail more. The `[... clamped to floor; ratio not a measurement]` suffix in `_gate_fail_detail` is unreachable once a clamped baseline returns no bound, and is removed; the `~` legend states the consequence instead. Three consecutive `main` runs failed this way on three different fixtures across two runners: global_cell_shortpreamble_hot 24.1x > 19x and class_reassign_hot 49.2x > 47x on ubuntu-24.04, reentrant_key_eq_mutation 10.3x > 5x on macos-latest (runs 31079972573, 31080288895). Discriminator, cranelift, `class_reassign_hot` with its ceiling temporarily set to 1: the previous check.py reports SLOWER "exec 0.13s > pypy 0.01s ratio 27.0x > gate 1x [pypy exec clamped to floor; ratio not a measurement]", this one reports PASS. With the same ceiling of 1 on seqiter_tuple_error_parity, whose pypy exec is a measurement, this check.py still reports SLOWER at 18.3x — the ceiling is untouched wherever the baseline is real. The three fixtures above pass with their own ceilings restored. Assisted-by: Claude * posix: correct which stat rejection precedes the platform's dir_fd check `stat_entry` consults `HAVE_FSTATAT` while unwrapping `dir_fd`, above the descriptor branch, so the descriptor+`dir_fd` conflict is unreachable where `fstatat` does not exist. The comment claimed both fd-conflict rejections come first. #1081 corrected the same claim in `extra_tests/parity_tests/os_stat_file_descriptor.py` and cites `_DirFD_Unavailable` (`interp_posix.py:285-292`) for it; this is the statement of it that sits next to the code. Assisted-by: Claude * bench: re-record the wasm jit-stats for exception_reused_object_tb_not_doubled `fbw_blackhole_adopted_single_frame` reads 3 where the baseline had no entry for it. `loops_compiled=4` and `bridges_compiled=3` are unchanged, so the trace shape is the same and what moved is that the walk now adopts the blackhole resume image instead of falling back to the replay from the traced region's entry. Attributed by measuring both arms with the same command, `check.py --backend wasm --synthetic-only --synthetic-pattern exception_reused_object_tb_not_doubled`: with `ff503b5d746` reverse-applied in place the bench reports ALL PASSED against the existing baseline, and with it restored it reports the 0 -> 3 change. The control arm took 2m32s against the treatment arm's 4s, which is the wasm module being relinked rather than reused. The counter arrived with #1064 and this bench's baselines were last recorded at `da5e6fb38c7` (#1059), so absence from the baseline did not by itself say which of the two it was. No CI job runs `--backend wasm`, so the wasm baselines are not gated there either. The other four keys the re-record adds -- fbw_blackhole_adopted_multi_frame, fbw_store_journal_rollback_failed, field_pos_attached_misplaced, field_pos_spec_misplaced -- are counters that did not exist at #1059 and are pinned at 0 here for the first time. The dynasm and cranelift baselines are not re-recorded: both backends still report ALL PASSED for this bench. Assisted-by: Claude * bench: restore bridges_compiled and guard_failures on three synth baselines `9d2fff92649` (#1063) re-recorded 993 jit-stats baselines. All but four gained only the two new `field_pos_*_misplaced=0` keys; three changed a value: binary_int_overflow_local_resume bridges 5 -> 6 guards 647 -> 686 exc_bridge_entry_guard_not_removed bridges 4 -> 5 guards 809 -> 1009 list_append_write_barrier_gc bridges 5 -> 6 guards 1345 -> 1562 Five runs report the pre-#1063 values and none reports the recorded ones: dynasm, cranelift and wasm here, and `main`'s own CI on ubuntu-24.04 and macos-latest at 9d2fff9 -- run 31139317566, jobs 92747505633 and 92748753166, on a tree carrying no commit from this branch. The three benches fail identically on all three backends in each of them. Only those two keys are restored; #1063's two added keys stay. The fourth bench it revalued, getattribute_override_no_bind, is left as recorded: it passes here and in that CI run, so its new values do reproduce. Assisted-by: Claude
…es no host produces `pyre/check.py` has been red on main for binary_int_overflow_local_resume, exc_bridge_entry_guard_not_removed and list_append_write_barrier_gc since 9d2fff9, on ubuntu-24.04, macos-latest AND windows-latest (run 31140634730), and locally on macOS across all three backends. Every one of those hosts observes bridges_compiled/guard_failures 5/647, 4/809 and 5/1345. Those are exactly the values that stood on main before 9d2fff9 (last written by #947 and #1059); 9d2fff9 recorded 6/686, 5/1009 and 6/1562, which reproduce nowhere. The re-record was taken against a base whose behaviour these fixtures no longer had, and the merge replayed it. Re-recorded on dynasm, cranelift and wasm. The counters land back on the pre-9d2fff92649 values; the `field_pos_*` fields 9d2fff9 added are kept. Assisted-by: Claude
…es that have main red on every OS (#1099) * jit: keep find_biggest_function's closed-frame result when the recorder is gone pyjitpl.py:3562 reads `self.history.get_trace_position()` unconditionally, so the `max_key` the closed-frame loop above produced always survives to the return. pyre's recorder is an `Option` and the port spelled that read as `self.tracing.as_ref()?`, which returns `None` for the whole function whenever tracing has ended with an unmatched open entry still in `portal_trace_positions`. Only the open frame is unmeasurable without a recorder, so only its measurement is skipped now. Not reachable from `blackhole_trace_too_long_slow`, which holds `self.tracing` as `Some`; `find_biggest_function` is `pub`. `find_biggest_function_keeps_a_closed_frame_when_the_recorder_is_gone` covers it and fails with the `?` put back. Also corrects blackhole_inlined_callee_local_after_escape_declined.py's second description block, which still called `sys._getframe()` the residual after the file's own header states it folds and the added `.f_locals` read is the force. Assisted-by: Claude * jit: emit sys._getframe's mark_as_escaped as a setfield, and carry the sized frame's jitdriver out of find_biggest_function vm.py:54 `f.mark_as_escaped()` is traced as an ordinary `setfield_gc` on the flag. The constant-depth fold emitted it as a void CallN into a Rust helper instead, which hides the update from the optimizer and its heap cache. Replaced with the read/or/store the `tb_frame` fold in the same file already uses (specialize.rs:2299-2313): getfield_gc_i(flags) + int_or(FLAG_ESCAPED) + setfield_gc + heapcache_setfield_cached. `jit_frame_mark_as_escaped` is deleted. pyjitpl.py:3575 returns `max_jdsd, max_key`, and pyjitpl.py:2821-2824 uses both -- the disable goes through the OWNING driver's warmstate and that driver is what `aborted_tracing_jitdriver` stores. The port dropped the jd_no its own log entries already carry and hardcoded driver 0. It now returns `Option<(usize, u64)>` and the caller stores the index it was given. pyre keeps one WarmEnterState on the MetaInterp rather than one per JitDriverStaticData, so `disable_noninlinable_function` still lands on that single state; the comment names it. No recorded counter moves on dynasm, cranelift or wasm. Assisted-by: Claude * bench: restore the three jit-stats baselines #1063 replaced with values no host produces `pyre/check.py` has been red on main for binary_int_overflow_local_resume, exc_bridge_entry_guard_not_removed and list_append_write_barrier_gc since 9d2fff9, on ubuntu-24.04, macos-latest AND windows-latest (run 31140634730), and locally on macOS across all three backends. Every one of those hosts observes bridges_compiled/guard_failures 5/647, 4/809 and 5/1345. Those are exactly the values that stood on main before 9d2fff9 (last written by #947 and #1059); 9d2fff9 recorded 6/686, 5/1009 and 6/1562, which reproduce nowhere. The re-record was taken against a base whose behaviour these fixtures no longer had, and the merge replayed it. Re-recorded on dynasm, cranelift and wasm. The counters land back on the pre-9d2fff92649 values; the `field_pos_*` fields 9d2fff9 added are kept. Assisted-by: Claude * jit: arm the deferred escape-flush undo when only the locals region flushed `flush_active_frame_escape`'s force arm has three outcomes. A committed full flush publishes a resume pc into `COMMITTED_FRAME_ESCAPE_PC`; an all-or-nothing decline discards the undo capture; the third -- the full flush declines and `flush_locals_region_to_frame` writes slots `0..nlocals` on their own -- did neither. That leg claims no resume pc, so `take_committed_frame_escape_pc` yields nothing and the walk-end block gated on it is skipped in its entirety, including the `restore_escape_flush_undo()` in its `else`. The capture stays armed, `LiveLastInstrGuard::drop` reads an armed capture as a flush owning the frame and declines to put `last_instr` back, and the legacy replay re-enters one opcode past the call on an operand stack no flush wrote: `value-stack underflow: depth=N base=N`, a JIT-only panic with no program output. `mark_escape_flush_undo_pending()` routes the leg to the walk-end deferred restore, which is already conditioned on no continuation having claimed the flushed frame -- so where the walk goes on to adopt a blackhole image the request is consumed without restoring and the adoption keeps the frame it claimed. Restoring earlier is not equivalent: making `LiveLastInstrGuard::drop` test the commit instead removes the crash and returns a stale caller line, because the walk goes on after the residual and nothing else advances `last_instr`. `bench/synth/handler_tb_frame_locals_after_declined_flush.py` reaches the leg: `'i' in tb.tb_frame.f_locals` forces the frame mid-expression, with the `seen.add` receiver and its bound method live below the value being computed. A/B on the cranelift binary that reproduced it: 10/10 panics without the change, 0/10 with it, output `[True]` matching `PYRE_NO_JIT=1`. Assisted-by: Claude * bench: survey a caller's f_lineno and f_lasti from two call sites A callee reading its caller's frame through `sys._getframe(1)` had no coverage of the resume coordinate: `bench/synth` holds ten `_getframe(1)` fixtures, one `f_lineno` fixture (a traceback frame) and no `f_lasti` fixture at all. Both fields resolve off `last_instr`, which compiled code does not store per opcode, so the value only reaches the frame if the force publishes it. Two call sites are what make that observable. One holds the caller's coordinate constant by construction, so a frozen read is indistinguishable from a live one. Surveying every iteration into a set rather than sampling the last one is the other half: the pre-compile iterations are correct, so a miss appears as a changed row count. `f_lasti` is a bytecode offset and so is not comparable against the pypy oracle; only its discrimination is printed. `f_lineno` is compared directly, relative to `co_firstlineno`. Measured by putting a defect back in: with the `flushed` test dropped from `LiveLastInstrGuard::drop`, so the guard restores at the residual's return instead of at walk end, the fixture reports ([(0, 3), (0, 8), (1, 3), (1, 6)], [0, 0, 1, 1], 3) against its ([(0, 8), (1, 6)], [0, 1], 2) -- the pre-call coordinate appears alongside the call-site one on both legs. cpython, pypy, `PYRE_NO_JIT=1`, dynasm, cranelift and wasm all print the latter. The walk-end epilogue gains the negative result measured while looking for a counter to gate the same defect: every walk reaching that point on this fixture reports `armed=false fb=true`, so a leak counter conditioned on the three adoption flags being false reads 0 whether or not the force arm arms its deferred restore. Assisted-by: Claude * check.py: fail the build on a stale LLBC instead of measuring through it `pyre-jit-trace/build.rs` compares each `build/llbc/*.ullbc` against what its crate's sources hash to now and reports a mismatch as `cargo::warning`, which cargo replays only when it re-runs the build script -- so a run whose crates were cached prints nothing at all. Every number check.py produces is read out of a binary whose field offsets come from those artefacts. Measured on this tree: four measurement runs -- a three-backend gate, two A/B arms and a base control -- carried the mismatch, and the string `LLBC STALE` appears in none of their logs, while `cargo check -p pyrex` on the same tree printed it for all three crates. check.py only ever tested for the artefacts being missing. It now exports `PYRE_LLBC_STRICT=1` before every backend build, the promotion build.rs documents for callers that want a gate, and names staleness in the build-failure diagnostics beside the missing-artefact branch. The cost is that a rebase which moves the LLBC crates stops the next check.py until a re-extraction; `PYRE_LLBC_SKIP_FINGERPRINT_CHECK=1` still opts out for an A/B whose only changed crate contributes no field offsets. First use found one: the wasm jit-stats fall on `exception_reused_object_tb_not_doubled` that four arms reproduced was an artefact of the stale artefacts, and the bench passes on all three backends after a re-extraction with nothing re-recorded. Assisted-by: Claude
1 participant
An audit of the builtin types against CPython 3.14.2 and PyPy 7.3.20, fixing the places where pyre agreed with neither.
object.__new__and the layout checkobject.__new__never rancheck_user_subclass, soobject.__new__(int),object.__new__(list)and user subclasses of them all allocated a bare instance missing the fields their own methods then read. The check itself already existed; only the call was absent.typewas a second, independent hole:check_user_subclassdecides safety purely bylayout.typedefpointer identity, andtypewas built withlayout_pytype = &INSTANCE_TYPE, so it reusedobject'sLayout. That made the identity hold fortypeand every metaclass, since heap types inheritbase_layout.typedef.typenow gets&TYPE_TYPE, matchingW_TypeObject.typedefupstream.object.__new__(type)did not fail at the allocation — it returned a live non-type, and the firstrepr()of it reporteddescriptor '__repr__' requires a 'type' object but received a 'type'.Buffer request kinds were inverted
Upstream spells C-contiguity in the request flag, not the operation:
BUF_SIMPLEBufferErrorreplace,strip,join,translate,find,startswith, fill charBUF_FULL_RObytes()/bytearray()constructorspyre had these exactly backwards, so
bytes(memoryview(b'abcd')[::2])raised where both references returnb'ac', whileb'abcd'.replace(mv[::2], b'z')succeeded where both raise.simple_buffer_bytes/full_ro_buffer_bytesnow share onebuffer_bytes(obj, require_contiguous), andrequire_contiguous_bufferis the gate for the operand side.Argument handling
bytes.startswith/endswithconvert the operand before thestart > len(value)early-out, sob'abc'.startswith('a', 10)raises instead of answeringFalse.Noneis a value, not an omitted argument:bytes.center/ljust/rjustfill char,bytes.decodeencoding and errors,bytearray.popindex,memoryview.castshape.WrappedDefault/if w_shape:apply to the slot, not to app-levelNone.builtin_strspells the utf-8 default out where it previously passedNonethrough, sostr(b'abc', errors='strict')keeps working.str.replace,bytes.center/ljust/rjust,bytearray.remove,memoryview.cast.bytearray.popreads its index before taking the storage borrow — that read can run user code.Messages
__slots__read reports%T, the bare type name, throughraiseattrerror— the same miss taken through the descriptor's own__get__already did. It was printing'__main__.S'.type(1, (), {})names argument 1 instead of reporting an arity error, and argument 1's text saysstringlike its two siblings.SyntaxError.__str__splits its filename withntpathrules on windows (\and the drive prefix), fixingsource_encoding_syntax_error.pythere.Verification
object.__new__on__slots__and multiple-base classes, every accepted arity).cargo test --all101 test binaries, 0 failures.pyre/extra_tests/parity_tests178 scripts: the only red istype_members_python314.pyunder the cpython runner (BaseExceptionGrouptp_basicsize88 vs 96), unrelated and pre-existing.pyre/check.py --backend dynasm --synthetic-only362/362.Known remaining
bytearray's six comparison dunders still accept only bytes/bytearray, sobytearray(b'ab') == array.array('B', [97, 98])isFalsewhere both references sayTrue. Upstream_comparison_helperaccepts anyBUF_SIMPLEexporter.bytesis correct as-is and must not be widened. Left for a follow-up.— authored by Claude
Summary by CodeRabbit
New Features
*args) arguments.Bug Fixes
memoryview,type, exception, string, andstr.replacebehavior.raise ... from ...validation and object construction checks.