descr: catch the field-position invariant where it is produced, not where it is repaired - #1053
Merged
Conversation
|
Warning Review limit reached
Next review available in: 16 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
WalkthroughThe PR adds producer-side field-position diagnostics, resolves descriptor indices by names or unique offsets, exports four counters through runtime and wasm interfaces, and adds allocation-rooting documentation and tests. ChangesField position diagnostics
Allocation rooting validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DescriptorFactories
participant GcCache
participant JitcodeRuntime
participant WasmExports
participant WasmRunner
DescriptorFactories->>GcCache: Record field-position checks
GcCache->>JitcodeRuntime: Provide checked and misplaced counts
JitcodeRuntime->>WasmExports: Expose field_position_counts
WasmExports->>WasmRunner: Return four counters
WasmRunner->>WasmRunner: Emit MAJIT_STATS
Possibly related PRs
Suggested reviewers: 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 |
![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: 1ba7db5471
ℹ️ 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 Some(pos) = parent_spec | ||
| .all_fielddescrs | ||
| .iter() | ||
| .position(|spec| spec.offset == offset) |
There was a problem hiding this comment.
When this fallback handles a field that was deliberately omitted from all_fielddescrs (for example an inline aggregate/nested struct, or a zero-sized layout-only field) but shares its byte offset with a real leaf, this offset-only search stamps the leaf's slot onto the missing field. That is exactly the found_parent_field == false case; downstream virtual materialization looks up fields by all_fielddescrs()[index_in_parent], so the aggregate/layout-only field can be emitted through the sibling leaf descriptor instead of staying unresolved. Only re-rank here when the parent entry also matches the field key/name; otherwise keep the unresolved fallback.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit c98b31d). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptations
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Owner
Author
Codex parity review — §1 and §2 confirmed and fixedBoth findings were about the same thing and both were real, so I measured Offset is not an identity. A flattened layout puts an inline aggregate and its What changed
Applied at all four sites the review named:
The §3 — one of the two pre-existing items is now closed
Correct, and it was feeding a guessed name into the resolution above, which The nested-struct flattening item (§3, second) is untouched — that is the same Tests
Verificationcheck.py dynasm 379/379, cranelift 379/379, wasm 375/375; The macos-latest
|
Owner
Author
§1 and §2 are now
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8feea75759
ℹ️ 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".
Comment on lines
+3730
to
+3734
| if !found_parent_field | ||
| && let Some(parent_spec) = parent.as_ref() | ||
| && let Some(pos) = unique_slot_at_offset(&parent_spec.all_fielddescrs, offset) | ||
| { | ||
| index_in_parent = pos; |
There was a problem hiding this comment.
Fresh evidence in this revision is that the new unique_slot_at_offset re-rank still runs after the parent-name lookup has failed, so a field intentionally omitted from all_fielddescrs but sharing an offset with a single leaf (for example a flattened inline aggregate or a zero-sized layout-only field) gets stamped with that leaf's slot. The resulting descr keeps the omitted field's key/offset but points index_in_parent at a sibling, and later virtual materialization indexes all_fielddescrs()[index_in_parent], emitting through the sibling descriptor instead of leaving the field unresolved; upstream derives this by field name and does not invent an offset match for nested structs/void fields. Please only re-rank when the parent entry also matches the field key/name.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
`execute_new`'s result is already reachable: the dispatch arm stamps it onto the recorded op's `value` cell (`set_opref_concrete` → `history.py:803-807 *FrontendOp(pos, value)`) and `MetaInterp::walk_active_trace_refs` forwards every recorder `Op`/`InputArg` `value` holding a `Value::Ref`. Nothing between `bh_new` and that stamp allocates from the GC heap, so no collection can observe the object first. Record that contract on `execute_new_allocation` and on the two dispatch arms, and pin the stamp with `new_stamps_its_allocation_onto_the_recorded_op` / `new_with_vtable_...` — the walker already had a test, but nothing held the arms to stamping at all. A stub `Backend` overriding `bh_new` / `bh_new_with_vtable` exercises them; it needs real backing storage because `new_with_vtable` writes `w_class` into the block it returns. Also note on `FBW_FINISH_CONCRETE` why it is a genuine exception: the compile path does `self.tracing.take()`, so that stash outlives the op graph and is the one window no op-graph slot covers. Assisted-by: Claude
…e parent `add_struct_field_descr` derives `index_in_parent` and `name` by locating `offset` in the `struct_size_specs` entry as it stands at the mint site, and `register_struct_layout` re-indexes every entry on each merge, so a field minted before a lower-offset sibling is registered carries a rank the merge invalidates. `patch_field_descr_parents` already replaces that snapshot with the final merged spec, which leaves the descr's two halves describing different fields: the `descr.py:228` index points at a slot the `descr.py:238` parent fills with another offset. Re-resolve both against the spec being installed, using the mint's own `position(|fd| fd.offset == offset)` lookup over the complete list rather than a prefix of it. A field absent from the final spec keeps what the mint left it — those are the inline aggregates the flattened layout covers only through their leaves. `field_descr_index_follows_the_final_layout_not_the_mint_time_prefix` emits high-offset-first and asserts `all_fielddescrs[index_in_parent].offset` is the field itself; without the re-resolution it reports index 0 against a parent whose slot 0 is the other field. Measured on a hot two-field-instance loop: `field_pos_rederived` and `positional_misplaced` are 0 on both sides, so the runtime's by-name `derive_index_in_parent` was masking the stale index wherever the name resolved; `field_pos_unresolved` is 152 on both sides. `field_pos_parent_absent` 1210 -> 1201, `key_compared` 1210 -> 1201, `positional_slots` 1279 -> 1270, deterministic across runs. Assisted-by: Claude
`field_pos_rederived` counted disagreements found by `derive_index_in_parent`,
which replaces the caller's `index_in_parent` in the same expression and only
runs on the mint path with a published, non-empty parent whose field name
resolves. `make_simple_descr_group_keyed_with_headerless` mints every field
before `register_keyed_size` publishes the parent, so most mints take the
`FIELD_PARENT_ABSENT` early return and are never compared.
Add two censuses taken before that:
* `field_pos_spec_{checked,misplaced}` — `field_specs[i].index_in_parent == i`
on the list as handed to either group factory, covering the `cache_key == 0`
groups that never enter `_cache_size`.
* `field_pos_attached_{checked,misplaced}` — a standalone field descr's
`index_in_parent` against the slot its own attached parent puts the offset
in, counted at both `BhDescr::Field` readers.
Both read 0 over the corpus (1713 and 248 checked), as do `positional_misplaced`
and `field_pos_rederived`, with and without `patch_field_descr_parents`'
index/name re-resolution — the ordering the pass fixes does not occur there.
So add `field_descr_position_disagreement`, a `debug_assert!` postcondition over
every emitted `Field` descr at `try_finish`, covering the resolved case, the
unresolved `(0, "")` fallback and the descrs whose `type_id` the pass skips.
Removing the two re-resolution lines trips it in
`field_descr_index_follows_the_final_layout_not_the_mint_time_prefix`.
Gate the two `misplaced` counters in `JITSTATS_BADNESS_FIELDS`; absent from a
baseline they read 0, so no re-record.
check.py dynasm 378/378, cargo test --workspace 101 suites, no failures.
Assisted-by: Claude
…lback arms `fielddescrof`'s `struct_layout_for` and `heuristic_field_layout` arms overwrite `offset` from a second source and leave `index_in_parent` at its `0` initialiser, while `parent` still carries the whole flattened list — the descr names slot 0 of a list whose slot 0 is another field, and `all_fielddescrs()[index_in_parent]` is what `optimizeopt/info.rs force_box` indexes by. Resolve the slot against the attached parent by that offset. The `found_parent_field` arm is left alone: it picked its spec by name, so re-resolving by offset would return the wrong one for a struct with two fields at one offset. Census the same two halves at this producer, which stays meaningful after the fix — the `found_parent_field` arm takes the matched spec's stored `index_in_parent`, not its position in the list. `field_pos_attached_misplaced` reads 0 before and after over the corpus, so `fielddescrof_resolves_the_slot_when_the_offset_comes_from_the_layout_registry` constructs the state directly; removing the re-resolution fails it 0 vs 1. Replace the `debug_assert!` added in 6a5e644dd6d with a `cfg!(debug_assertions)` branch: the message needs the same walk as the predicate and `debug_assert!` ran it twice. check.py dynasm 378/378 and cranelift 378/378, cargo test --workspace 101 suites, no failures. Assisted-by: Claude
…wasm `JITSTATS_BADNESS_FIELDS` gained `field_pos_spec_misplaced` and `field_pos_attached_misplaced`, but the wasm guest has no stderr and never printed the `field_pos_*` line, so `_jit_stats_change` read both as absent and therefore zero — the healthy value. Export them the way the `descr_set_*` counters already are, with `pyre-wasm-runner` printing the line and failing the run when an export is missing. Not redundant with the native readings: the invariant is that `index_in_parent` names the slot the field's byte offset occupies, and wasm32 lays structs out on a 4-byte word (`symbolic.py:12 WORD = sizeof(lltype.Signed)`). The wasm run reports its own denominators — `field_pos_spec_checked=1665`, `field_pos_attached_checked=245` against 1712/247 on this host — so the exports measure the wasm-side descr universe rather than echoing the native one. `*_checked` are exported on the same footing as `pyre_jit_descr_set_resolved`: host-dependent, printed for diagnosis, absent from `JITSTATS_SNAPSHOT_FIELDS`. check.py wasm 374/374, dynasm 378/378, cargo test --workspace 101 suites, no failures. Assisted-by: Claude
…ous offset Codex parity review found the field-slot resolution keyed on byte offset where `heaptracker.py:60-72 get_fielddescr_index_in(STRUCT, fieldname)` keys on the name. Measured: 7 of 1714 submitted field specs sit at an offset another field of the same parent also occupies — a flattened inline aggregate and its first leaf share an address (`heaptracker.py:68-69`) — so offset is not an identity and `position(|fd| fd.offset == offset)` can name a sibling. `field_slot_in` resolves by name when the descr carries one and falls back to the offset only when it does not, and then only when exactly one field sits there. `add_struct_field_descr` is such a site: `getfield_gc_*` / `setfield_gc_*` reach it with `(offset, type, type_id)` and no `fieldname`. Where the offset is ambiguous it now takes the `(0, String::new())` fallback instead of the first match — a guessed name is the damaging half, since it becomes the descr's `_cache_field` key downstream. Same rule at the other three sites the review named: the codewriter's `fielddescrof` fallback arm (`unique_slot_at_offset`), and the two `census_attached_index` callers, which would otherwise report a correctly-named descr as misplaced and put a false reading behind `JITSTATS_BADNESS_FIELDS`. The `pyre-jit-trace` census already keyed on `(offset, field_key)`. Two tests: an ambiguous offset leaves the mint's number alone (control: arbitrating fails it `(0, "agg")` vs `(0, "")`), and a name resolves through an ambiguous offset (control: `Some(1)` vs `None`). check.py dynasm 379/379, cranelift 379/379, wasm 375/375; cargo test --workspace 101 suites, no failures. Assisted-by: Claude
`descr.py:218-239 get_field_descr(gccache, STRUCT, fieldname)` keys on the field name, and `heaptracker.py:60-72 get_fielddescr_index_in(STRUCT, fieldname)` derives the slot from it. `add_struct_field_descr` had no such argument and stood the byte offset in for it, which cannot select between two fields a flattened layout puts at one address. The name was already at the emit sites: `jitcode_lower` hands `register_struct_layout` a `(offset, is_ref, stringify!(#member))` triple and then called `setfield_gc_*` / `getfield_gc_*` with the offset alone, in the same `quote!` expansion. Thread it through the five emitters so read and write agree by construction. `field_slot_in` now resolves those descrs by name; the unique-offset fallback stays for a caller with no name to give. check.py dynasm 379/379, cranelift 379/379, wasm 375/375; cargo test --workspace 101 suites, no failures. `field_pos_*` and `positional_*` unchanged. Assisted-by: Claude
`field_slot_in` and `patch_field_descr_parents` both said `add_struct_field_descr` is handed `(offset, type, type_id)` and never sees a `fieldname`. It takes `field_name: &str`; every emit site supplies it — `jitcode_lower` passes the same `stringify!(#member)` it hands `register_struct_layout`, and `newlist_clear` passes the `"length"` / `"items"` it registered. The offset fallback is unchanged. What reaches it is a recorded name the final spec does not list, not a field that arrived anonymous. Assisted-by: Claude
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c98b31d811
ℹ️ 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".
| /// find_index_in_parent` documents for the runtime lookup, for the same reason. | ||
| fn field_slot_in(fields: &[BhFieldSpec], name: &str, offset: usize) -> Option<usize> { | ||
| if !name.is_empty() | ||
| && let Some(idx) = fields.iter().position(|fd| fd.name == name) |
There was a problem hiding this comment.
When register_struct_layout has merged multiple layouts under one type_id, the list can contain the same field name at different offsets (the surrounding diagnostics track this as key_conflicting, e.g. the documented last_instr collision). This name-only lookup returns the first matching name even when the current descr’s offset is the later slot, and the new debug postcondition reuses the same helper without checking slot.offset, so it will bless an index_in_parent that points at a different field than the load/store offset. Please require the named slot to also match the descr offset, or treat duplicate names as unresolved/ambiguous.
Useful? React with 👍 / 👎.
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 7, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 7, 2026
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
youknowone
added a commit
that referenced
this pull request
Aug 7, 2026
…x argument-handling sites (#1063) * builtins: layout-checked object.__new__, buffer request kinds, and six 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 * bytearray: compare against any BUF_SIMPLE exporter, not only bytes-like `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 * builtins: metaclass one-argument guard, declaring-type arity names, and nine argument-rejection sites `type_descr_new` accepted `Metaclass(x)` as the one-argument `type(x)` form and returned `x`'s type; `descr__new__` (typeobject.py:901-908) takes that path only when the metatype is `type` itself. Restore the guard and report the count through the two wordings upstream splits it into. `type.mro` carried no declared arity, so `int.mro(1)` computed the MRO and dropped the surplus argument. A builtin's arity and keyword errors named the receiver's own class. An instance receiver now names the class that declares the method (`MyList([1]).append()` reports `list.append()`); a type receiver keeps naming itself, which is what a builtin bound to a class reports. Argument rejections reworded at nine sites: - `issubclass()` arg 2 names the accepted kinds - `raise X from Y` distinguishes its cause check from `e.__cause__ = x` - `UnicodeTranslateError` / `UnicodeDecodeError` / `UnicodeEncodeError` `__init__` report the count they received - `BaseExceptionGroup.__new__` names itself - `int.to_bytes` / `int.from_bytes` name the total parameter count once the call passes every parameter, and the positional limit otherwise - `bytes.decode` renders a `None` argument as `None` - `str.center` / `ljust` / `rjust` name the type of a non-str fill char rather than reporting its length - `float.fromhex` rejects a non-str operand Assisted-by: Claude * typedef: restore the fully-qualified name in the empty-slot AttributeError `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 * jit: virtualize BUILD_TUPLE at arity 2 and seed a `*args` callee's vararg 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 * bench: re-record the jitstats baselines the rebase left disagreeing 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 * bench: mark the thirteen fixtures cpython cannot usefully run 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 * parity: cover the arity-2 specialised tuple consumers `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 * objspace: compare same-class specialised tuple pairs on their raw slots `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 * bench: re-record the synthetic jitstats baselines from a fresh LLBC 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 * builtins: check the metatype is a type before naming it in type.__new__ `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 * baseobjspace: hand the buffer request kind's flags to a `__buffer__` 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 * jit: decline the object-slot arm of the specialised-pair subscript fold `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 * bench: restore the sre_wasm_min1 jit-stats baselines to what this tree 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 * bench: re-record getattribute_override_no_bind's wasm jit-stats baseline 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 * jit: seed the vararg tuple into the inlined callee's concrete frame 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 * type_methods: word the fill-character refusal per padding method `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 * builtins: prebuild the default encoding `str` hands to bytes.decode `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 * type_methods: report a non-bytes fill operand the way decode_object does `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: precheck the metatype on the four-argument type.__new__ path `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 * jit: decline the arity-2 BUILD_TUPLE virtualization `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
This was referenced Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A field descr's
index_in_parent(descr.py:228) must name the slot its parentputs the field in — upstream gets this for free because
heaptracker.py:60-72 get_fielddescr_index_inand:96-112 all_fielddescrsare one walker sharing oneskip set. pyre has several independent producers, and
all_fielddescrs()[index_in_parent]is a load-bearing lookup(
optimizeopt/info.rs force_box), so a stale rank either runs off the end oremits the store against a different field.
Two producers were leaving that rank stale, and the counter that looks like it
guards this could not see either of them.
The counter could not have caught this
field_pos_rederivedwas documented as "the one to watch". It cannot be:GcCache::derive_index_in_parentis the judge and the repairman — itreplaces the caller's number with the parent's in the same expression, so a
nonzero reading is a log of repairs already applied, never a defect still
present.
make_simple_descr_group_keyed_with_headerlessmintsevery field before
register_keyed_sizepublishes the parent those fieldswill be indexed against, so the dominant path takes the
FIELD_PARENT_ABSENTearly return. Measured on this tree:
parent_absent1202 againstrederived0.
General shape, worth keeping: a counter placed inside a normalising reader
measures repairs, not defects, over whatever subset the reader chose to examine.
What replaces it
Two censuses taken before anything can normalise, each with a real denominator:
field_pos_spec_{checked,misplaced}cache_key == 0groups that never enter_cache_sizefield_pos_attached_{checked,misplaced}Both
*_misplacedjoinJITSTATS_BADNESS_FIELDS. A field absent from a baselinereads as 0, so they gate from the first run with no re-record.
…and why a census was still not enough
Negative control, fix reverted, full corpus: all four of
field_pos_rederived,field_pos_spec_misplaced,field_pos_attached_misplacedandpositional_misplacedstill read 0, withattached_checkedunchanged at 248.The defect is input-order dependent — it needs a field minted before a
lower-offset sibling registers — so a corpus that registers in offset order
exercises the producer without exercising the bug. Building a better census did
not help; that is the finding.
So the load-bearing check is at the construction site.
field_descr_position_disagreementruns attry_finishundercfg!(debug_assertions)over every emittedFielddescr, covering all threestates the pass can leave behind: the offset resolves (index and name are that
slot's), the offset is absent (the
(0, "")fallback is intact — those are theinline aggregates
heaptracker.py:68-69mints no descr for), and the parent'stype_idis absent fromstruct_size_specs(thecontinuethat patchednothing). Removing the re-resolution panics with
field descr at offset 16 … claims slot 0 named "hi", but that offset is slot 1.The two producers
jitcode/assembler.rs—patch_field_descr_parentsalready swapped thesnapshotted
parentfor the final merged spec but leftindex_in_parent/name, which came from that same snapshot, so one descr's two halvesdescribed different fields.
register_struct_layoutre-indexes on everymerge, which is what invalidates the snapshot.
codewriter/assembler.rs—fielddescrof'sstruct_layout_forandheuristic_field_layoutarms overwriteoffsetfrom a second source andleave
index_in_parentat its0initialiser whileparentcarries thewhole flattened list. The
found_parent_fieldarm is deliberately left alone:it picked its spec by name, and re-resolving by offset would return the wrong
one for a struct with two fields at one offset.
Both fixes are pinned by a test that constructs the state directly, each with the
revert-and-fail control shown.
wasm
The two new badness fields would have read absent-and-therefore-zero on wasm, the
exact hole the
descr_set_*exports were added to close. Exported the same way.Not redundant with the native readings — the invariant is stated in byte offsets
and wasm32 lays structs out on a 4-byte word (
symbolic.py:12); the wasm runreports its own denominators (
spec_checked=1665,attached_checked=245against1712/247 on the dev host), so it is measuring the wasm-side universe.
Note on the first three commits
738a524/39bd8a9add an executed-allocation GC root list and then patch acrash that root itself introduced;
e8204d9removes both. They net to zero andare kept as the record: the finding they answered ("the
execute_newresultlives only in a borrowed slice that is not a GC root") is false — the result
is stamped onto the recorded op's
valuecell (history.py:803-807 *FrontendOp(pos, value)) andMetaInterp::walk_active_trace_refshas forwardedthose cells since
d3bfd17. Say the word and I will fold them out.Verification
check.pydynasm 378/378, cranelift 378/378, wasm 374/374;cargo test --workspace --no-default-features --features dynasm101 suites, nofailures;
cargo fmt --all --checkclean. No jitstats baseline was re-recorded.— authored by Claude
Summary by CodeRabbit
New Features
Bug Fixes
Tests