bench, parity: widen six floored-denominator ratio gates; scope the platform-specific parity fixtures - #1071
Merged
Conversation
|
Warning Review limit reached
Next review available in: 47 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 (25)
WalkthroughThe PR updates synthetic benchmark baselines, adds platform-aware parity test filtering and metadata, and changes SyntaxError filename formatting to handle Windows paths. ChangesPlatform compatibility and benchmark updates
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant parity_runner as parity_tests.run.py
participant platform as sys.platform
participant parity_scripts as Parity test scripts
parity_runner->>parity_scripts: Read platform markers
parity_runner->>platform: Check current platform
platform-->>parity_runner: Return platform
parity_runner->>parity_scripts: Run matching scripts
parity_runner-->>parity_scripts: Report skipped scripts
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9aefc35793
ℹ️ 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".
| .as_bytes() | ||
| .iter() | ||
| .rposition(|&b| b == b'/') | ||
| .rposition(|&b| b == b'/' || (windows && b == b'\\')) |
There was a problem hiding this comment.
On Windows, when SyntaxError.filename is a UNC share root such as \\server\share or \\?\UNC\server\share, ntpath.splitdrive treats the entire share root as the drive and os.path.basename returns an empty string; this last-separator scan instead returns share. Consequently, str(SyntaxError(...)) still diverges from CPython/PyPy for these valid filenames. Implement Windows splitdrive semantics, including UNC and device roots, rather than only stripping drive letters.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit ea335ed). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptations
|
There was a problem hiding this comment.
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/bench/synth/pickle_ctor_args.py`:
- Around line 1-5: Reconcile the documented benchmark calculations with their
thresholds: in pyre/bench/synth/pickle_ctor_args.py lines 1-5, update 121 or the
explanation so it accurately reflects the stated 2 × 70.3x calculation; in
pyre/bench/synth/type_immutable_reject.py lines 1-4, either document the reason
for using 40 instead of 2 × 18.2x = 36.4x or change the threshold to match the
calculation.
In `@pyre/pyre-interpreter/src/display.rs`:
- Around line 1354-1361: Update the Windows drive-prefix handling in the path
basename logic around fbuf and start so it only recognizes a two-byte ASCII
drive prefix when the colon follows the first WTF-8 character boundary; do not
treat paths beginning with a multi-byte non-ASCII or lone-surrogate character
followed by a colon as drive-prefixed. Add a regression test covering such a
first character and verify it is basenamed according to Windows ntpath
semantics.
🪄 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: 691f79fb-8477-4518-bd57-661e790053c6
📒 Files selected for processing (17)
pyre/bench/synth/closure_per_call.wasm.jitstatspyre/bench/synth/exception_traceback_frame_lineno.wasm.jitstatspyre/bench/synth/gc_iterator_source_drop.wasm.jitstatspyre/bench/synth/getframe_while_subwalk_decline_shapes.pypyre/bench/synth/pickle_ctor_args.pypyre/bench/synth/recursive_call_frame_relocation.wasm.jitstatspyre/bench/synth/type_call_inline_init_branch_deopt.pypyre/bench/synth/type_dict_surrogate.pypyre/bench/synth/type_immutable_reject.pypyre/bench/synth/unary_positive_resume.pypyre/extra_tests/parity_tests/compile_filename_boundary.pypyre/extra_tests/parity_tests/dir_entry_uninstantiable_python314.pypyre/extra_tests/parity_tests/import_unencodable_path_entry.pypyre/extra_tests/parity_tests/repr_surrogate_wtf8.pypyre/extra_tests/parity_tests/run.pypyre/extra_tests/parity_tests/type_members_python314.pypyre/pyre-interpreter/src/display.rs
Owner
Author
Codex parity review — disposition§1 Regressions: None. §2 Other mismatches: None. §3 Pre-existing (1) — fixed now (
Split the drive off first, as §4 Structural adaptations (2) — both won't-fix, documentation confirmed present. Doing the platform-dependent basename in Rust rather than through an app-level Verification at this head
— commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b902570a38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1,5 +1,6 @@ | |||
| # pyre-check: max-pypy-ratio=74 | |||
| # pyre-check: min-pypy-ratio=7.08 | |||
| # pyre-check: max-pypy-ratio=350 | |||
There was a problem hiding this comment.
On any host where this fixture's pyre/PyPy ratio is below 70x, the supposedly removed floor still fails the benchmark: pyre/check.py:1890-1894 derives the minimum as max-pypy-ratio / 5 whenever min-pypy-ratio is absent, so raising the ceiling to 350 silently raises the effective floor from the removed 7.08x to 70x. The same inversion affects the other four clamped fixtures whose explicit minima were deleted; retain an appropriately low explicit minimum or add a way to disable the derived floor.
Useful? React with 👍 / 👎.
| // The root runs through the share, so it ends at the second separator | ||
| // after the two leading ones — or at the end of a path with fewer. | ||
| let mut seen = 0; | ||
| for (i, &b) in path.iter().enumerate().skip(2) { |
There was a problem hiding this comment.
On Windows, a SyntaxError.filename equal to an extended UNC root such as \\?\UNC\server\share still renders share instead of the empty basename returned by ntpath.basename. Fresh evidence in this revision is that the new helper starts every double-separator scan at byte 2, so it stops after \\?\UNC rather than applying the vendored ntpath.splitroot special case that starts share scanning after the eight-byte \\?\UNC\ prefix; preserve that prefix handling when computing the drive length.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ef905cca1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1,3 +1,7 @@ | |||
| # pyre-check: platforms=linux,darwin | |||
There was a problem hiding this comment.
This marker skips the entire fixture on Windows, but only the byte-filename assertions at lines 53–57 depend on filesystem decoding. The core surrogate tests create the value directly in C.__repr__ (lines 18–51), and the recursion/depth checks at lines 59–80 are also platform-independent, so Windows CI now loses coverage for nearly all behavior this fixture guards. Isolate or condition the byte-filename section instead of excluding the whole script.
Useful? React with 👍 / 👎.
`_baseline_exec_time_clamped` marks seven of the ten gate failures these fixtures produce on the CI runners: pypy's startup-subtracted exec time is pinned at `EXEC_TIME_FLOOR_S`, so the printed ratio divides by the floor rather than by a measurement. Raising N does not lift it — the bodies are loop-invariant and pypy hoists them whole. Restore the ceilings the clamped fixtures carried before they were tightened and drop their `min-pypy-ratio` floors, which came from the same non-measurement. `type_immutable_reject` gets twice the slowest ratio observed across hosts (18.2x) instead, because its restored 15 is below that. `pickle_ctor_args` is a real measurement and keeps a gate on both sides: 121 (twice the slowest host) with an explicit floor of 9, since the default ceiling/5 rejects the fastest host at 13.5x. Assisted-by: Claude
…filename `run.py` ran every script everywhere. Six fixtures fail on Windows once the bench step stops failing first and the suite is reached; five of those fail under the reference CPython too, so the comparison measures nothing: * `compile_filename_boundary` / `repr_surrogate_wtf8` compile with a `b"\xff.py"` filename, which only decodes under a UTF-8 filesystem encoding with `surrogateescape`; * `import_unencodable_path_entry` needs `os.fsencode` to reject a surrogate outside U+DC80..U+DCFF, which Windows encodes with `surrogatepass`; * `dir_entry_uninstantiable_python314` asserts the `posix` spelling of names Windows re-exports from `nt`; * `type_members_python314` holds absolute C-struct sizes, and the reference reports `OSError.__basicsize__` 120 on Windows and `BaseExceptionGroup.__basicsize__` 88 on macOS arm64. Give a script a `# pyre-check: platforms=` header naming the `sys.platform` values its subject holds on, and skip it elsewhere; a script without the header still runs everywhere. The sixth, `source_encoding_syntax_error`, is a real divergence: `W_SyntaxError.descr_str` calls `os.path.basename`, which is the platform's — `ntpath` splits on `\` as well as `/` and drops the drive `splitdrive` peeled off first. Splitting on `/` alone left the full temporary path in the rendered message. Assisted-by: Claude
`guard_failures` moves by one or two against the values recorded at `58fcd373e05` and `da5e6fb38c7`; `loops_compiled` and `bridges_compiled` are unchanged on all four. `closure_per_call` returns to the 468 this branch and its CI run both observed before the rebase took main's 470. Assisted-by: Claude
`ntpath.splitdrive` treats a whole `\\server\share` as the drive, so `os.path.basename` of a bare UNC root is the empty string rather than the share. Scanning for the last separator across the whole path returned `share`. Split the drive off first, as `ntpath.split` does, and take what follows the last separator in the remainder. Assisted-by: Claude
…drive `splitroot` counts its two separators from `\\?\UNC\` when the path carries that prefix — matched case-insensitively and through `/` — so `\\?\UNC\server\share` is a drive in full and basenames to the empty string; counting from the two leading separators cut it at `\\?\UNC`. The `X:` form is two CHARACTERS, so a non-ASCII or lone-surrogate first character puts the colon past byte 1 and the drive is longer than two bytes. The unit test carries every case as `len(ntpath.splitdrive(p)[0].encode())` for the same input. Two bench headers stated a calculation their value does not follow: 121 is 1.7x headroom over the slowest host, not twice it, and 40 is twice 18.2x rounded up. Assisted-by: Claude
`int_max_str_digits`, `kwargs_positional_only`, `set_name_filtered_dict` and `type_metatype_data_descr` carry the blanket ceiling `759e6fb5225` gave every synth fixture, never tightened since. pypy's exec time on all four is pinned at `EXEC_TIME_FLOOR_S`, so the gate compares pyre's wall clock against the floor and scales with host speed: this host reads 1.6x to 5.3x where the macOS runner reads 6.4x to 10.7x. Each ceiling becomes twice the slowest ratio the runners observe, and the one `min-pypy-ratio` among them goes with the measurement it was derived from. Assisted-by: Claude
Three more clamped fixtures cross their ceiling on the ubuntu runner's cranelift arm — `attr_instance_shadows_class` 131.0x over 118, `exception_value_op_caught` 25.1x over 24, `unary_negative` 54.5x over 52 — and four of the ceilings already adjusted here sat below twice what the runners read, so they would have crossed next on a slower host. Restoring a pre-tightening value gives an arbitrary margin: 350 is four times the slowest observed while 77 and 80 are barely one and a third. State one rule instead and hold every adjusted fixture to it — the ceiling is twice the slowest ratio any runner has reported, and each header now carries that observation so the next adjustment has the number it was derived from. `type_dict_surrogate` keeps the pre-tightening 350, which already exceeds the rule, and says so. Assisted-by: Claude
`run_bench` derived every floor as `ceiling / PERF_GATE_FLOOR_DIVISOR`, so it can only express a 5x span between the fastest and slowest host. Against pypy, `spectral_norm` spans 0.5x to 4.3x: pypy runs it in 0.13s on the linux and macos runners and 0.39s on the windows one, so pyre reads faster than pypy there and crosses the derived floor of 1x while the ceiling of 5 still fits the slow end. Give `run_bench` the `min_pypy_ratio` override the synthetic path already has, and state spectral_norm's floor at half the fastest ratio observed. Assisted-by: Claude
Deleting a `min-pypy-ratio` header does not disable the floor gate — check.py derives one from `ceiling / PERF_GATE_FLOOR_DIVISOR` instead, so raising a ceiling raises the floor with it. On `type_dict_surrogate` that turned a stated 7.08 into a derived 70, above the 21.5x the windows runner reads. Nothing fails today: `failed_bound` only applies the floor once pypy's exec time reaches `FLOOR_GATE_MIN_BASELINE_S`, and these fixtures sit at the startup-subtraction floor on every host measured. On windows that headroom is two scheduler ticks — the threshold is `3 * WIN_TIMER_QUANTUM_S` and the accounting is quantized — so the floor arms the first time pypy's raw time lands a tick higher. State the floor at half the fastest ratio observed, the mirror of the ceiling rule, and say so in each header. `spectral_norm` already took this shape. Assisted-by: Claude
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea335edb62
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1,3 +1,7 @@ | |||
| # pyre-check: platforms=linux,darwin | |||
There was a problem hiding this comment.
This marker skips the entire fixture on Windows, although only the assertions involving b"\xff.py" require UTF-8 plus surrogateescape. The plain/ASCII and string PathLike filenames, invalid filename types, embedded-NUL handling, invalid mode, and ordinary code-object replacement checks are platform-independent, so Windows CI now loses coverage for most of the compile boundary behavior; condition only the filesystem-decoding cases instead of excluding the script.
Useful? React with 👍 / 👎.
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
`getframe_force_cancel_journal` crossed its ceiling of 29 on the ubuntu runner's cranelift arm in both main CI runs on record — 31.5x and 32.1x in the median-3 fail detail, 30.8x/32.6x in the tables — and #1071, which widened the rest of this family, left it untouched. Held to the same rule as its siblings: the ceiling is twice the slowest ratio a runner has reported (32.6x) and the floor is half the fastest (8.0x, windows), stated rather than derived because ceiling/5 = 13 sits above it. pypy's exec time here is pinned to the startup-subtraction floor, so the printed ratio divides by the floor constant rather than by a measurement. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
The preceding commit lets `check.py`'s `PYPY_GC_NURSERY` / `PYPY_GC_MIN` reach the wasm guest, which pushes the major-collection threshold past every fixture's working set. The collection those readings were counting is gone, so the counters that counted it move. Forty-nine rows across 47 fixtures, and **every one of them falls**: `guard_failures` by 1 to 202 (`exception_escape_hot_callee_tb_node_once` 1016 -> 814, `exception_inline_callee_tb_frames` 1008 -> 807, `closure_per_call` 470 -> 420), plus `bridges_compiled` 5 -> 4 on those same two — the bridge the loop had spent on the eval-breaker poll. Nothing rises anywhere, and no other backend's baselines are touched, because the pins were already reaching those. Four of these are rows the base measures from a threshold that was still free to move, three of them re-recorded at that free value as recently as #1071: `closure_per_call` 470 -> 468 -> 420, `exception_traceback_frame_lineno` 820 -> 819 -> 817, `recursive_call_frame_relocation` 649 -> 648 -> 638, and `gc_iterator_source_drop` 613 -> 614 -> 613. Recorded with `check.py --snapshot --backend wasm`, which then reports `ALL PASSED: wasm 384/384`. Its unrelated side effect of adding two new `field_pos_*_misplaced` fields to every wasm file was stripped, except on the four files whose committed baselines already carry them. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
The preceding commit lets `check.py`'s `PYPY_GC_NURSERY` / `PYPY_GC_MIN` reach the wasm guest, which pushes the major-collection threshold past every fixture's working set. The collection those readings were counting is gone, so the counters that counted it move. Forty-nine rows across 47 fixtures, and **every one of them falls**: `guard_failures` by 1 to 202 (`exception_escape_hot_callee_tb_node_once` 1016 -> 814, `exception_inline_callee_tb_frames` 1008 -> 807, `closure_per_call` 470 -> 420), plus `bridges_compiled` 5 -> 4 on those same two — the bridge the loop had spent on the eval-breaker poll. Nothing rises anywhere, and no other backend's baselines are touched, because the pins were already reaching those. Four of these are rows the base measures from a threshold that was still free to move, three of them re-recorded at that free value as recently as #1071: `closure_per_call` 470 -> 468 -> 420, `exception_traceback_frame_lineno` 820 -> 819 -> 817, `recursive_call_frame_relocation` 649 -> 648 -> 638, and `gc_iterator_source_drop` 613 -> 614 -> 613. Recorded with `check.py --snapshot --backend wasm`, which then reports `ALL PASSED: wasm 384/384`. Its unrelated side effect of adding two new `field_pos_*_misplaced` fields to every wasm file was stripped, except on the four files whose committed baselines already carry them. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
The preceding commit lets `check.py`'s `PYPY_GC_NURSERY` / `PYPY_GC_MIN` reach the wasm guest, which pushes the major-collection threshold past every fixture's working set. The collection those readings were counting is gone, so the counters that counted it move. Forty-nine rows across 47 fixtures, and **every one of them falls**: `guard_failures` by 1 to 202 (`exception_escape_hot_callee_tb_node_once` 1016 -> 814, `exception_inline_callee_tb_frames` 1008 -> 807, `closure_per_call` 470 -> 420), plus `bridges_compiled` 5 -> 4 on those same two — the bridge the loop had spent on the eval-breaker poll. Nothing rises anywhere, and no other backend's baselines are touched, because the pins were already reaching those. Four of these are rows the base measures from a threshold that was still free to move, three of them re-recorded at that free value as recently as #1071: `closure_per_call` 470 -> 468 -> 420, `exception_traceback_frame_lineno` 820 -> 819 -> 817, `recursive_call_frame_relocation` 649 -> 648 -> 638, and `gc_iterator_source_drop` 613 -> 614 -> 613. Recorded with `check.py --snapshot --backend wasm`, which then reports `ALL PASSED: wasm 384/384`. Its unrelated side effect of adding two new `field_pos_*_misplaced` fields to every wasm file was stripped, except on the four files whose committed baselines already carry them. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
…t left behind `min-pypy-ratio=9` and the 13.5x-70.3x host band the ceiling is drawn from were recorded in #1071, before #1079 moved BytesIO and StringIO to interp level. #1079 re-recorded the fixture's three jit-stats baselines but not its ratio gate, so the floor has been rejecting the faster pyre ever since. dynasm reads 3.7x-4.8x and cranelift 3.9x-6.5x against a floor of 9; wasm reads 7.2x and passes. The reading is not this branch's: `--snapshot-diff` over the fixture is clean, so its jit-stats are the ones #1079 recorded and the compiled code is unchanged here. Neither commit on this branch touches _io, pickle or the fixture. The floor moves to 1.75, below the fastest of those readings with margin. The ceiling stays at 145: no CI host has been observed under the port, so there is nothing to re-derive it from, and the header now says so. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
The preceding commit lets `check.py`'s `PYPY_GC_NURSERY` / `PYPY_GC_MIN` reach the wasm guest, which pushes the major-collection threshold past every fixture's working set. The collection those readings were counting is gone, so the counters that counted it move. Forty-nine rows across 47 fixtures, and **every one of them falls**: `guard_failures` by 1 to 202 (`exception_escape_hot_callee_tb_node_once` 1016 -> 814, `exception_inline_callee_tb_frames` 1008 -> 807, `closure_per_call` 470 -> 420), plus `bridges_compiled` 5 -> 4 on those same two — the bridge the loop had spent on the eval-breaker poll. Nothing rises anywhere, and no other backend's baselines are touched, because the pins were already reaching those. Four of these are rows the base measures from a threshold that was still free to move, three of them re-recorded at that free value as recently as #1071: `closure_per_call` 470 -> 468 -> 420, `exception_traceback_frame_lineno` 820 -> 819 -> 817, `recursive_call_frame_relocation` 649 -> 648 -> 638, and `gc_iterator_source_drop` 613 -> 614 -> 613. Recorded with `check.py --snapshot --backend wasm`, which then reports `ALL PASSED: wasm 384/384`. Its unrelated side effect of adding two new `field_pos_*_misplaced` fields to every wasm file was stripped, except on the four files whose committed baselines already carry them. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 6, 2026
…#1073) * test: pin the runtime config a parity script needs `run.py` reads `# parity-env: NAME=VALUE` lines from each script and adds them to the environment of every runner for that script only. `thread_start_walk_abort_no_replay.py` pins `PYRE_FBW_CALLEE_VSTACK=0`. The three kept-stack branch-guard decline hazards are scoped to `!ctx.vstack_valid`, so with the callee operand-stack mirror on by default (7b87e22) an inline sub-walk describes its own stack and the aborting guard is never reached. Measured on a binary built from the commit before the walk-abort fix: the script passes 4/4 at the default setting and fails 4/4 with the mirror off, in both cases at round 97. Verified through `run.py` itself: with the pre-fix binary in `target/release` the run reports this script as its single failure; with both backends built at HEAD the full suite passes. Assisted-by: Claude * interp: call the three class-creation hooks through the frameless dispatcher `build_class`'s non-type metaclass call, its `__prepare__` call and `_init_subclass` each resolved a caller frame with `gettopframe_raw()` only to satisfy `call_with_kwargs(frame, …)`, and each took a fallback arm when that frame was null: the first two dropped the class-definition keywords, and the third raised `TypeError("__init_subclass__() takes no keyword arguments")`. Upstream passes the keywords unconditionally through the frameless `space.call_args` (descroperation.py:189) at all three sites — compiling.py:199 for `__prepare__`, :221 for the metaclass, and typeobject.py:1025-1026 `args = __args__.replace_arguments([])` / `space.call_args(w_func, args)` for `__init_subclass__`. None of the three fallback arms has an upstream counterpart. Call `call_with_kwargs_in_ctx` at each site and delete them. The fallbacks were not reachable from a script: the execution context's frame is non-null throughout normal execution, and a metaclass with `__prepare__` plus class keywords, and `__init_subclass__` with keywords, already matched CPython 3.14 before this change. What this removes is three `force_vref` calls and three arms with no upstream basis, leaving `c_profile_frame` as the only `gettopframe_raw()` in the file. Also correct two nearby line citations: compiling.py:190-196 -> :194-199 and :213-219 -> :214-221. Assisted-by: Claude * wasm: hand the guest the environment the collector sizes itself from `check.py` pins `PYPY_GC_NURSERY` and `PYPY_GC_MIN` so the major-collection threshold is a property of the tree rather than of the machine. Both pins reached the two native backends and neither reached the wasm one: the guest is built for `wasm32-unknown-unknown`, whose `std::env` is permanently empty, and `majit-gc` resolves those names through `std::env::var` (collector.rs:93). The guest therefore kept `min_heap_size = nursery * 8` = 32MB, crossed it mid-run, and counted the back-edge eval-breaker poll's bailouts that the natives no longer see. `warn_inert_guest_env` did not report it either — it matches the `PYRE_` and `MAJIT_` prefixes, and these are `PYPY_`. Measured, `recursive_call_frame_relocation`'s guest-side `guard_failures` against `PYPY_GC_MIN`: unset 648, 8MB 695, 256MB 638. Before this change all three read 648. The native backend moves under the same override — 636 -> 639, and `fib_loop` 189 -> 193 — which is what said the variable was reaching one side and not the other. `majit-gc` takes an embedder-supplied environment, read only where `std::env` misses, and publishes the names it resolves; `pyre-wasm` exports `pyre_set_gc_env` / `pyre_gc_env_names` over it and the runner forwards whatever its own environment carries. That is the pair `pyre_set_launch_env` / `pyre_launch_env_names` already form for the launcher's variables, for the same reason. The wasmi engine path is left as it is: it forwards neither this nor the launch environment, so it already runs without `PYTHONSAFEPATH` and is not a jit-stats engine. Assisted-by: Claude * bench: re-record the wasm jit-stats the guest-side GC pins move The preceding commit lets `check.py`'s `PYPY_GC_NURSERY` / `PYPY_GC_MIN` reach the wasm guest, which pushes the major-collection threshold past every fixture's working set. The collection those readings were counting is gone, so the counters that counted it move. Forty-nine rows across 47 fixtures, and **every one of them falls**: `guard_failures` by 1 to 202 (`exception_escape_hot_callee_tb_node_once` 1016 -> 814, `exception_inline_callee_tb_frames` 1008 -> 807, `closure_per_call` 470 -> 420), plus `bridges_compiled` 5 -> 4 on those same two — the bridge the loop had spent on the eval-breaker poll. Nothing rises anywhere, and no other backend's baselines are touched, because the pins were already reaching those. Four of these are rows the base measures from a threshold that was still free to move, three of them re-recorded at that free value as recently as #1071: `closure_per_call` 470 -> 468 -> 420, `exception_traceback_frame_lineno` 820 -> 819 -> 817, `recursive_call_frame_relocation` 649 -> 648 -> 638, and `gc_iterator_source_drop` 613 -> 614 -> 613. Recorded with `check.py --snapshot --backend wasm`, which then reports `ALL PASSED: wasm 384/384`. Its unrelated side effect of adding two new `field_pos_*_misplaced` fields to every wasm file was stripped, except on the four files whose committed baselines already carry them. Assisted-by: Claude * _io: take the seek whence as a C int on the index protocol `BytesIO.seek` and `StringIO.seek` unwrapped their whence with the machine-int converter, so a value outside a C int's range reached the `0/1/2` range check and came back as `ValueError: invalid whence (4294967296, should be 0, 1 or 2)`. Both `@unwrap_spec(pos=r_longlong, whence=int)` (interp_bytesio.py:162) and `@unwrap_spec(pos=int, mode=int)` (interp_stringio.py:403) take it as a C int, and 3.14 raises `OverflowError: Python int too large to convert to C int` for one that does not fit. Neither existing converter is that parameter. `c_int_w` has the range check but reaches the value through `gateway_int_w` = `int_w` (baseobjspace.py:2043), which converts through `__int__` first; `index_int_w_preserve_negative` is on the index protocol but is a machine int. A whence needs both, so add `index_c_int_w` — `space_index` followed by the 32-bit check — and the `PyIndexCInt` unwrap alias over it, and use it at both sites. Reading the whence through `__int__` is observable, not merely a message difference: an object defining `__index__` as 0 and `__int__` as 2 seeks to the end rather than the start, silently, and one defining only `__int__` is accepted where 3.14 raises TypeError. The parity script asserts the whence by value against a non-empty buffer so neither case can pass, and pins the out-of-range half and the position's wider `Py_ssize_t` range beside it. The position is left alone: upstream's `pos=int` on `StringIO` is a C int too, but 3.14 takes it as a `Py_ssize_t` and accepts `seek(2**32)`. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 7, 2026
… behind (#1078) * posix: take an open file descriptor in the supports_fd entry points that rejected one os.chdir, os.chmod, os.chown, os.pathconf, os.statvfs and os.utime each appeared in os.supports_fd and answered a descriptor with TypeError: expected str, bytes or os.PathLike object, not int They now unwrap through `gateway::fsencode_path_or_fd_w` and branch on `Path.as_fd` where `interp_posix.py` does: chdir to fchdir (:910-918), chmod to fchmod (:1228-1243), chown to fchown (:2475-2500), pathconf to fpathconf (:2420-2433), statvfs to fstatvfs (:704-719) and utime to futimens (:1860-1904). lchown keeps `allow_fd=0` and so keeps the narrower allowed-type list in its own type error. chown and utime reject follow_symlinks=False beside a descriptor, and utime rejects dir_fd, with the ValueErrors 3.14 words; `interp_posix.py` spells the first of them "cannnot" and words utime's dir_fd conflict "can't specify both dir_fd and fd", where 3.14 says "can't specify dir_fd without matching path". The parity suite reads CPython as the oracle. _have_functions is now built from the same constants the entry points branch on, so the advertisement cannot drift from the behaviour: the family drops out under sandbox and on the hosts carrying no host_env::posix, rather than each being spelled twice. HAVE_FUTIMES leaves the list because nothing here calls futimes, and os.py:150-151 reads either bit as the same utime capability. Assisted-by: Claude * posix: implement os.truncate os.truncate was one of the noop placeholders registered so os.py's _exists probes find a name, so it returned None and left the file untouched — on a path and on a descriptor alike — while HAVE_FTRUNCATE advertised it in os.supports_fd. os.ftruncate did the work all along: with open(p, "wb") as f: f.write(b"0123456789") os.truncate(p, 4); os.stat(p).st_size # 10 os.ftruncate(fd, 1); os.stat(fd).st_size # 1 interp_posix.py:414-431 takes a descriptor as it stands and opens a name write-only, truncates whichever it ended up with, and closes only the one it opened. The EINTR retry loop and the length conversion move out of the os.ftruncate closure so both callers share them; the path form reports the name it opened, which the descriptor form has none of. Assisted-by: Claude * posix: decode getcwd with the filesystem handler, not lossily `os.getcwd()` built its str with `to_string_lossy`, so a directory whose name carries a byte with no UTF-8 reading came back with U+FFFD in place of it and no longer named the directory it came from. interp_posix.py:906 is `space.fsdecode(getcwdb(space))`, whose surrogateescape round-trips. `gateway::fsdecode_os_str` and `fsdecode_filename_bytes` are the same decoders sys.argv goes through, so the two boundaries agree. Not exercised end to end here: APFS refuses to create a directory whose name is not valid UTF-8 (OSError errno 92), so the case needs a Linux host. Reported by the Codex parity review on #1066 (section 3). Assisted-by: Claude * extra_tests: check every os.supports_fd member against a descriptor The set is what callers read to choose an fd-relative implementation over a path-based one, so a member that rejects an integer — or accepts one and does nothing — sends the caller down a route that cannot work. The script asserts the eight-name floor before the per-name blocks, so a capability that is dropped rather than fixed fails here instead of quietly shrinking the coverage of the guarded blocks below it. Each name is then called with a descriptor and the result observed rather than the return value trusted: chdir moves the process, chmod and utime are read back through stat, truncate shrinks the file. It also pins the two type errors path_or_fd emits — the caller-named list widens with the descriptor form, so lchown answers "string, bytes or os.PathLike" where chown answers "string, bytes, os.PathLike or integer" — the two ValueErrors a descriptor beside dir_fd or follow_symlinks raises, and that utime's times/ns conflict outranks both. listdir, scandir and execve are skipped: the first two take a nullable path and word their type error differently, and execve takes an argv. Assisted-by: Claude * posix: resolve chown's dir_fd, and advertise HAVE_FCHOWNAT / HAVE_UTIMENSAT chown already reached `fchownat`, but with `AT_FDCWD` hard-coded and the dir_fd keyword rejected with NotImplementedError one step earlier; the descriptor the caller names is now what the name resolves against. `_DirFD_Unavailable.unwrap` (interp_posix.py:285-292) converts before it reports the platform, so the value is unwrapped first and the availability bit consulted second. A descriptor path plus dir_fd is the ValueError interp_posix.py:2481-2483 raises, checked ahead of the follow_symlinks one it already had. Neither macro was in `_have_functions`, so os.py put chown and utime in neither supports_dir_fd (os.py:119,133) nor supports_follow_symlinks (os.py:180,191) while both entry points implemented the modifiers — utime through one `utimensat`, chown through one `fchownat`. HAVE_LCHOWN and HAVE_LUTIMES stay unlisted for the reason HAVE_FUTIMES already carried: os.py reads them as the same capability and neither `lchown` nor `lutimes` is called here. Two lib-python tests move from failing to passing against the same corpus otherwise unchanged (dynasm, 599 tests across the two modules, no other row moves): test_os.UtimeTests.test_utime_invalid_arguments — asserted NotImplementedError for `follow_symlinks=False` because utime was absent from supports_follow_symlinks, and pyre did not raise test_shutil.TestMisc.test_chown — shutil.chown(dir_fd=...) hit the NotImplementedError Assisted-by: Claude * extra_tests: exercise dir_fd and follow_symlinks on every name that claims them The companion to os_supports_fd.py for the other two capability sets. It asserts the chown/stat/utime floor in both supports_dir_fd and supports_follow_symlinks, then calls each with the modifier and reads the result back rather than trusting the return value. Every dir_fd call is repeated against a name that does not exist under the descriptor, which is what separates "resolved the name against dir_fd" from "ignored it and reached the same file through the cwd" — a fixture whose relative name also resolves from the process cwd cannot tell the two apart. follow_symlinks=False is read back through lstat with the target asserted unmoved, the two modifiers are then used together, and the three ValueErrors a descriptor path produces are pinned by message. The last block is what the second set buys a caller: shutil.copystat with follow_symlinks=False substitutes `_nop` (shutil.py:435-439) for any name the set does not carry, so before this the call silently copied nothing. Assisted-by: Claude * majit: fold a virtual's never-stored field read to the zero constant optimize_getfield_gc answered GETFIELD_GC on a virtual only when the trace had already stored that field; an unset field fell through to PassOn and the load was emitted. virtualize.py:184-193 substitutes optimizer.new_const(fielddescr) when opinfo.getfield returns None. Port that fallback. typeptr, w_class and the GETFIELD_RAW_* opcodes stay out of it: the first two are header fields the same function already resolves from class identity, and upstream defines this handler for GETFIELD_GC_{I,R,F} only. pytraceback.rs:462 reads an exception's traceback slot before writing it, so every raise emitted that load and the arg-forcing pass materialized the exception behind it. type_immutable_reject's compiled loop body goes from 62 ops with 8 allocations to 19 ops with none. wasm's exception_value_op_caught baseline drops to guard_failures=1, the value dynasm and cranelift already record. Assisted-by: Claude * posix: derive HAVE_LSTAT from HAVE_FSTATAT and serve the three claims MS_WINDOWS makes `("HAVE_LSTAT", true)` was the one hardcoded entry in a table whose every other bit is the condition its entry point branches on. os.py reads that bit twice: :118 `_add("HAVE_LSTAT", "lstat")` builds supports_dir_fd, and :189 `_add("HAVE_LSTAT", "stat")` builds supports_follow_symlinks. On Windows HAVE_FSTATAT is false, so `os.lstat(name, dir_fd=fd)` raises while the set still listed it: os.supports_dir_fd # {<built-in function lstat>} on windows HAVE_LSTAT now follows HAVE_FSTATAT, which is the call both spellings resolve a dir_fd with. That drops os.stat from supports_follow_symlinks wherever the bit goes false, and os.py offers MS_WINDOWS (:192) as the other vehicle for that claim — which also claims chmod takes a descriptor (:143) and honours follow_symlinks (:184). So os.chmod on Windows grows the two forms the bit advertises. The descriptor form is `host_nt::fchmod` on the handle the CRT descriptor wraps, and dispatches with no follow_symlinks test, as interp_posix.py:1233-1241 does; follow_symlinks=False is `host_nt::win32_lchmod`, the name's own attributes rather than the file the link resolves to. dir_fd stays refused, and refuses the way `_DirFD_Unavailable.unwrap` (interp_posix.py:285-292) does — converting the value first, so a wrongly typed one is a TypeError. MS_WINDOWS is appended after the HAVE_* rows, the position interp_posix.py:2854-2855 gives it. os_supports_fd.py's win32 branch was asserting os.stat alone and exiting; it now exercises every name the platform advertises — the round-trip through a descriptor for chmod and truncate, follow_symlinks read back through the attribute bit, the dir_fd rejection, and the widened allowed-type message — and fails on any advertised name it has no probe for. Assisted-by: Claude * posix: listdir and scandir accept a directory descriptor fdlistdir reads the names a descriptor holds: fdopendir on an F_DUPFD_CLOEXEC duplicate, a readdir loop that clears errno per call, then rewinddir before closedir so the caller's descriptor keeps its offset. listdir returns those names as str. scandir builds its entries from the same call when it is given a descriptor. Each entry's path is the bare name and carries the descriptor, and stat, inode, is_dir, is_file and is_symlink resolve that name with fstatat against it; a failed stat names the entry. The three is_* predicates now share one dir_entry_kind that reads S_IFMT. gateway gains fsencode_path_or_fd_nullable_w, the nullable half of _unwrap_path: None resolves to "." in the unwrapper rather than at each boundary, and the allowed-type list becomes the four-way matrix, which the DeprecationWarning now words the same way as the TypeError. _have_functions lists HAVE_FDOPENDIR, in the position the upstream name list gives it. The constant is HOST_POSIX, the condition fdlistdir compiles under. os_supports_fd.py covers both descriptor forms, the None forms, and the nullable allowed-type message. Assisted-by: Claude * posix: a buffer is not a path path_or_fd_w takes bytes and no longer any readable buffer. _unwrap_path's buffer arm (interp_posix.py:188-198) accepts one with a DeprecationWarning; 3.14 completed that deprecation, so a bytearray now gets the same TypeError every other rejected type gets. posix.fspath carried a second copy of that arm, which is what let os.fsencode and os.fsdecode hand a bytearray back unconverted. It also never checked what __fspath__ answered with; both are now the gateway's rules. The type in these messages is named by _PyType_Name — its own name without the module that qualifies it elsewhere, so array.array is reported as array. os_path_argument_types.py covers the rejection at every path boundary, the two shapes of the message, and that bytes, a bytes subclass and os.PathLike still work. Assisted-by: Claude * posix: chmod takes dir_fd and follow_symlinks, and lchmod is a real call The unix chmod was registered with a fixed arity of 2, so it took no keyword at all — spelling either modifier's own default was a TypeError. It now binds its arguments the way chown_entry does and dispatches like _chmod_path (interp_posix.py:1254-1258): fchmodat where a name has to be resolved against a directory descriptor or the final symlink must not be followed, plain chmod otherwise. A descriptor still answers before either modifier is consulted (interp_posix.py:1233-1242). chown turns both away in that case and chmod does not, so the two entry points differ here on purpose. os.lchmod stops being one of the no-op stubs at the top of the module and becomes chmod's follow_symlinks=False arm, registered only on the hosts that carry a working lchmod. _have_functions gains HAVE_FCHMODAT, which os.py:118 reads as chmod honouring dir_fd, and HAVE_LCHMOD, which os.py:183 reads as chmod honouring follow_symlinks. The second is the narrower bit for the reason os.py:159-177 gives: fchmodat can be present and still not honour AT_SYMLINK_NOFOLLOW. os_chmod_modifiers.py exercises each claim rather than asserting it — the dir_fd arm resolves a name that does not exist in the working directory, and the follow_symlinks arm reads the link's mode and the target's separately. Assisted-by: Claude * posix: a stat that fails for a reason other than ENOENT is the caller's to see DirEntry's is_dir/is_file/is_symlink answered False for every stat failure. check_mode (interp_scandir.py:319-330) answers "not this type" for ENOENT alone — a vanished entry is better reported as not being of the asked-for kind than as an error — and propagates the rest, named by the entry. Both arms of dir_entry_kind now do that. chmod reads ENOTSUP and EOPNOTSUPP on the follow_symlinks=False path as the modifier being unavailable rather than as an OS error (interp_posix.py:1247-1251): a host can accept AT_SYMLINK_NOFOLLOW and not implement it, which is the same fact that makes HAVE_LCHMOD a narrower bit than HAVE_FCHMODAT. listdir and scandir given a descriptor report it as the failure's filename, since it is what named the directory. Found by the Codex parity review of this branch. Assisted-by: Claude * posix: open, mkdir, mkfifo, rmdir and unlink take dir_fd None of the five resolved a name against a directory descriptor, and none accepted the keyword at all: `open`, `mkdir` and `mkfifo` read their arguments positionally, so a call carrying any keyword reached them as a trailing dict and failed converting it to an integer; `rmdir`, `unlink` and `remove` were registered with a declared arity, which turns every keyword away. Each now binds its positional-or-keyword prefix through `bind_path_args` and reads `*, dir_fd=None` through `dir_fd_kwarg`, which spells `DirFD(available)` (`interp_posix.py:274-292`): `None` and an absent argument are the same default, and the value is converted before the platform is reported. The name form dispatches to `openat`, `mkdirat`, `mkfifoat` and `unlinkat` — the last with `AT_REMOVEDIR` for `rmdir` (`rposix.py:2717-2720`). `_have_functions` gains HAVE_MKDIRAT, HAVE_MKFIFOAT, HAVE_OPENAT and HAVE_UNLINKAT, which `os.py:124-132` reads into `supports_dir_fd`. HAVE_MKNODAT is not among them: `mknod` is still a placeholder that creates nothing. `chmod` and `chown` carried the same `dir_fd` block written out twice; both now call `dir_fd_kwarg`. The five path boundaries also name themselves when the argument is not a path — `unlink: path should be string, bytes or os.PathLike, not int`. extra_tests/parity_tests/os_dir_fd_modifiers.py exercises the modifier resolving against a descriptor rather than the working directory, the advertisement matching the behaviour, and the argument-list messages. Assisted-by: Claude * posix: chflags, lchflags and mknod are real calls, or absent The three sat in the block of noop stubs that expects a real implementation further down and had none, so each took any argument, made no syscall and reported success. Their callers probe for presence and believe the answer: `shutil.copystat` (`shutil.py:467`) reaches chflags through `lookup("chflags")`, `tempfile._resetperms` through a `try: _os.chflags`, and `tarfile.makedev` through `hasattr(os, "mknod")` — so the flags were never copied and no fallback ran. The stub list is also not `#[cfg]`-gated, so `hasattr(os, 'chflags')` was true on Linux, where the interface does not exist. `mknod(path, mode=0o600, device=0, *, dir_fd=None)` is now `mknod`/`mknodat`, registered beside `mkfifo` in the POSIX `host_env` block. `chflags(path, flags, follow_symlinks=True)` and `lchflags(path, flags)` are one call whose `follow_symlinks=False` arm is the second name, registered only on the BSD-flavoured hosts that carry the pair. `<sys/stat.h>` declares `lchflags` on the Apple targets, where `libc` carries only `chflags` and `fchflags`, so it is named in an `extern` block. The three names are dropped from the noop list, so a host without the call no longer answers `hasattr`. `os.py:112-114` guards `_add` with `fn in _globals`, and all three consumers above probe before calling. `_have_functions` gains HAVE_LCHFLAGS (`os.py:182` → chflags in supports_follow_symlinks) and HAVE_MKNODAT (`os.py:126` → mknod in supports_dir_fd). `bind_path_args` takes the keyword-only names rather than assuming `dir_fd`, because a signature without a keyword-only tail counts a surplus argument differently: every argument counts against the one limit and it is always "at most", which is why `os.lchflags(p, 0, follow_symlinks=False)` is a count error and not an unknown keyword. HAVE_LCHMOD was spelled without the `host_env` half of the condition its registration carries. Both it and the new HAVE_LCHFLAGS now read `HOST_POSIX && BSD_FLAVOURED`, which is the invariant the table's own comment states: each bit is the condition the entry point itself is compiled under. `mknod` reports the failing name in `filename`, following `interp_posix.py:1360-1372` `wrap_oserror2(space, e, w_path)`; CPython's `os_mknod_impl` uses the pathless `posix_error()` there, as it does for `mkfifo`, which pyre already named. Assisted-by: Claude * posix: major, minor and makedev compute a device number The three sat in the noop-stub block and answered `None`. `tarfile` reads a node's pair out of `st_rdev` to write a header (`tarfile.py:2275-2276`) and puts one back together to recreate the node (`:2735`), so the header field it wrote was not a number. They are `libc::major`/`minor`/`makedev` (`interp_posix.py:2551-2563`), which is the host's own encoding rather than arithmetic that can be spelled portably — macOS splits a `dev_t` 8/24 and Linux 12/20 with the minor bits in two pieces. The device argument is narrowed to `dev_t` rather than to a C int, because that type is wider than an int where the pair is, and a value that does not fit reports the overflow instead of wrapping. `libc` is a shim under `feature = "sandbox"` and carries no `dev_t`, so the names are absent there rather than answering with another host's arithmetic. The remaining 40 lying names in that stub list are measured and filed separately. Assisted-by: Claude * posix: stop binding spawnv, so os.py can define the spawn family `os.py:881` writes the spawn family in Python over fork+exec+waitpid, guarded by `if _exists("fork") and not _exists("spawnv") and _exists("execv")`. A `spawnv` bound here is therefore not a placeholder waiting to be overwritten further down — it is what stops that definition from ever running, and the noop stub won. `fork`, `execv`, `_exit`, `waitpid`, `WIFSTOPPED` and `waitstatus_to_exitcode` are all real, so dropping the four names is the whole implementation. The same block defines P_WAIT and P_NOWAIT, and the constant list above bound both to 0 — so the two modes were equal and `os.spawnv(os.P_NOWAIT, …)` waited instead of returning a pid. POSIX has no spawn call and no such constants; `<process.h>` does, so they are now bound on Windows only, with the values `_spawnv` reads. The sandbox build drops the four names too: the spawn family is app-level code in os.py rather than an external, and binding a name there would take P_WAIT and P_NOWAIT with it. `fork` is a raising stub in that build, so the definition os.py provides refuses at the fork. extra_tests/parity_tests/os_spawn_family.py spawns a child both ways and checks that P_WAIT hands back the exit code where P_NOWAIT hands back a pid — which is the assertion the two modes being equal fails. Assisted-by: Claude * posix: EX_*, ST_*, SCHED_* and RTLD_* carry the header's values The four families sat in a list that bound every name to 0, under a comment saying zero stubs were fine for os.py init. os.py names none of them, so the zero was serving nothing; the readers are the callers. `os._exit` takes an EX_* straight to the exit status, `statvfs(...).f_flag` is masked with the ST_* bits, and the RTLD_* set is handed back to `dlopen`, where a zero asks for `RTLD_LOCAL | RTLD_LAZY` whatever the caller named. Within each family every member also compared equal to every other. EX_* are `<sysexits.h>` literals: `libc` binds none of them, and the header is a verbatim descendant of the 4.3BSD one wherever it is carried, so the values are the same on every host that has it. The rest come from `libc`, except the Apple scheduling policies — `<pthread/pthread_impl.h>` declares those and the crate does not mirror them. They are bound on the POSIX builds, which is where the headers are; SCHED_BATCH and SCHED_IDLE narrow to Linux and RTLD_DEEPBIND to glibc, as `rposix.py:296-300` and `rdynload.py:50-82` read them. Before this they were bound unconditionally, so the Windows and wasm builds carried names whose header their host does not have. `host_seam::sys` names each one, because the sandbox build reaches `libc` through that facade rather than directly. WNOHANG, WCONTINUED, WUNTRACED and the PRIO_* trio stay in the zero list: the POSIX blocks further down overwrite those with the real values, and the comment above them now says so. extra_tests/parity_tests/os_constants.py checks each family for the shape it has to have — the members distinct, the flag members single bits — which is what binding a whole family to one value destroys. Assisted-by: Claude * posix: stop binding the C entry points and the names os.py writes itself Seventeen names in the noop-stub list answered `None` and were never given a body. They fall into four kinds, and none of the four is a name the module should carry. `fstatat`, `faccessat`, `futimens`, `futimes` and `fdopendir` are the C entry points the calls above them are served with — `openat` and its family are how `dir_fd` and a descriptor path are honoured, not calls of their own — and `setenv` is the C spelling of `putenv`. `moduledef.py` publishes none of them. A name bound for one is a capability a caller probes for and believes. `pipe2`, `dup3` and the four scheduling-policy calls are Linux's own additions, and were bound on every host. Nothing serves them on any host here, so they are dropped rather than kept as a stub that reports success; the Linux bodies — `sched_getparam` and `sched_setparam` need a `sched_param` type that does not exist here yet — are filed as their own task. `WEXITED`, `WNOWAIT` and `WSTOPPED` are `waitid`'s option flags, which are numbers rather than calls. They are bound with the other wait options, from libc. The fourth kind is the one `spawnv` was: names os.py writes in Python and lists in its own `__all__`. `popen` (os.py:1020-1067) and `get_exec_path` (os.py:649) were both in the stub list; `getenv` (os.py:818-825) had a real body here, and the SEEK_SET/SEEK_CUR/SEEK_END trio came off the constants table that os.py fixes at 0/1/2 itself (os.py:203-206). os.py's definitions win — unlike the spawn family, none of these four is guarded on the name being free — so what the bindings changed was `os.__all__`, where each arrived twice: once through `_get_exports_list` and once through os.py's own list. `os.__all__` held six duplicates against CPython's none; it now holds none. `popen` leaves the sandbox build's raising-stub list for the same reason the spawn family did: os.py builds it over `subprocess`, whose fork the stubs beside it already refuse. extra_tests/parity_tests/os_module_surface.py checks `os.__all__` for a name listed twice, which is what catches this whole class at once, and checks that each C entry point is absent and each option flag is a number. Assisted-by: Claude * posix: getpgrp, getpgid and ctermid call the host The three answered `None` from the stub list, which is a number a caller cannot tell from a group id and a name it cannot tell from a terminal. `getpgrp` (`interp_posix.py:2167-2172`) cannot fail and so is not checked; `getpgid` (`:2201-2210`) can be asked about a process that is not there and reports it. `ctermid` (`:2603-2608`) is read the way `rposix.py:1724-1728` reads it — the call is handed a null pointer and answers the static buffer it keeps — and the result is a filename, so it is decoded through `fsdecode_filename_bytes` rather than assumed to be text. `<stdio.h>` declares `ctermid` on every POSIX host, and the `libc` crate carries it for a handful, so the declaration is spelled out where the crate has none — the same shape `lchflags` already uses here. The sandbox build refuses all three instead: a process group and the controlling terminal's name are host facts, and the neighbouring reads (`getpid`, `getppid`, `ttyname`, `tcgetpgrp`) are refused there for the same reason. `confstr` and `confstr_names` stay stubs. They need the `_CS_*` table, which the `libc` crate carries one entry of per host while the module publishes 17 on Darwin and about fifty on glibc; spelling the rest out is what `pathconf_names` already does here, and it is filed as its own task rather than written from values this host cannot check. extra_tests/parity_tests/os_process_group.py checks the two group calls against each other and the terminal name for being a path, which is what a `None` fails. Assisted-by: Claude * check.py, extra_tests: keyword-only performance arguments and temp-directory cleanup `run_bench`'s `wasm_float_tol` and `min_pypy_ratio` are now keyword-only. Ruff reports FBT002 for the first; no call site passed either positionally. Ten parity scripts called `tempfile.mkdtemp` and never removed the result, so every run of the suite left a directory behind — three runners over 196 scripts. `atexit.register(shutil.rmtree, ...)` covers the early `raise SystemExit` on Windows and every assertion failure without wrapping each script in a `try`/`finally`. The #1078 review named the two in that diff; the other eight are the same line. os_supports_dir_fd.py's composed dir_fd + follow_symlinks call wrote `LINK_MTIME + 1`, one nanosecond past a whole second, against a file that states two lines above that every timestamp is a whole second so a coarse filesystem still reads back what was written. It also could not tell "the call did nothing" from "the value rounded down", because the step before it had already written `LINK_MTIME`. It writes a second whole-second pair instead. Assisted-by: Claude * posix: truncate's open and close, and utime's signed timestamps Four answers from the #1078 review that were the wrong value rather than the wrong call. `pathconf`'s fifth rides with the conf* tables, which it shares a helper with. `truncate` opened the name it was given with a bare `libc::open`. The name can be a FIFO with no reader, and the call waited there holding the interpreter, so no other thread could reach the other end; it also reported an interrupted open as `InterruptedError` where the `ftruncate` beside it retried. It now goes through `call_external_function` under the same retry loop, which is what `interp_posix.py:418` reaches by opening through the module's own `open`. The close is no longer discarded: `interp_posix.py:429-431` closes in a `finally`, through a `close` that raises, so a writeback error the close is first to see is the caller's. The truncation's own failure still wins when both fail. `truncate_length_w` narrowed to `off_t` with an `as` cast. A length wider than `off_t` became a different length rather than an error, and the file was truncated to that; `off_t::try_from` reports it with the message the helper already had for a too-wide value. `utime` carried both timestamps as `std::time::Duration`, which has no second below the epoch, so every pre-epoch time was refused with "timestamp out of range". `rposix.futimens` and `rposix.utimensat` (`rposix.py:2634-2671`) keep the seconds and the nanoseconds apart and signed; this does the same, with the floor-division `_PyTime_ObjectToTimespec` applies, so `ns=(-1, -1)` is `(-1, 999999999)` and reads back as `-1`. The name form now calls `utimensat` directly rather than `rustpython_host_env::posix::set_file_times_at`, whose signature cannot carry a negative second — which is also what `interp_posix.rs:3136` already said it did. The Windows host call still counts upwards from the epoch, and turns a pre-epoch time away rather than writing a different one. `times` is accepted as a keyword. `interp_posix.py:1862` puts `__kwonly__` after `w_times`, so it is the one argument here a caller may spell either way, and it was positional-only. extra_tests/parity_tests/os_utime_pathconf_truncate.py pins these against CPython 3.14, together with pathconf's answer. Assisted-by: Claude * posix: a descriptor of -1 does not become a BorrowedFd `BorrowedFd::borrow_raw` documents one value it may not be given: `-1`, which the standard library reserves as the niche that makes `Option<BorrowedFd>` cost nothing. Ten call sites built one straight out of an `fd` argument the caller supplied, and `os.fchmod(-1, 0o644)`, `os.chown(p, -1, -1, dir_fd=-1)` and `os.sendfile(-1, ...)` are all reachable from Python. `fd_borrow` answers those with the `EBADF` the syscall would have answered with, so the observable behaviour is unchanged and the one integer that may not become a handle no longer does. The sites reading a descriptor the module itself just produced — `dup2`'s result, and `path.as_fd` past its own `!= -1` guard — are left alone; those are not caller values. Reported in the #1078 review. Assisted-by: Claude * posix: confstr reads the host's string table, and pathconf answers -1 Both were noop stubs: `confstr_names` answered `None` rather than a dict, and `confstr` answered `None` whatever it was asked. `posixmodule.c posix_constants_confstr` and `rposix.py:2248-2300` name the same candidate set, every entry `#ifdef`-guarded, so a host publishes exactly the names its own `<unistd.h>` defines. `libc` carries `_CS_PATH` and nothing else, and the two numberings disagree from that first entry on — 1 on the Apple targets, 0 in glibc's `bits/confname.h`, whose enum also restarts twice, at 1000 and at 1100. Both tables are written out for that reason: 17 names on Darwin, 27 on glibc. The ten the candidate set carries for the System V hosts are defined by neither header, so neither table has them. The tables were derived by crossing `posix_constants_confstr` with each host's header rather than written from memory. The same derivation over the macOS SDK reproduces CPython 3.14's own `os.confstr_names` on this host entry for entry, which is what says the method is right. The glibc half comes from that project's `bits/confname.h`, and every value in it is confirmed a second time by `libc`: it carries `_CS_PATH` 0, `_CS_GNU_LIBC_VERSION` 2 and `_CS_GNU_LIBPTHREAD_VERSION` 3 outright, and it puts `_CS_POSIX_V6_ILP32_OFF32_CFLAGS` at 1116 — one past the end of the XBS5 run, which is what fixes that run at 1100..1115. No Linux compile was reached; this branch has none available. The call is `rposix.confstr` (`rposix.py:2129-2143`): ask for the length, fill a buffer of exactly that size. A zero length is either a name the host has no string for, which is `None`, or one it does not know, which is the errno it set — so errno is cleared before the question is put. The length counts the terminator and the string does not, so `len - 1` bytes are decoded, the way `os_confstr_impl` does; `rffi.charp2strn(buf, n)` keeps it. The value can be a search path, so it is decoded through the filesystem handler. `confname_arg` now takes the table to resolve against, and the dict-building it shares with `pathconf_names` is one function. `pathconf` and `fpathconf` come with it, because the fix lands in the lines that refactor touched. Both answered `None` where the host has no determinate limit; `interp_posix.py:2433` hands whatever `pathconf` returned to `space.newint`, so the answer is the number `-1`. `PC_ASYNC_IO` and `PC_SYMLINK_MAX` are the names that reach it on hosts that do not implement them, and `None` is neither that value nor a type a caller can compare against a limit. Reported in the #1078 review; pinned by extra_tests/parity_tests/os_utime_pathconf_truncate.py. The sandbox build refuses `confstr`: the answer is the host's own search path among other strings, and `pathconf`, `fpathconf` and `sysconf` are refused there beside it. extra_tests/parity_tests/os_confstr.py checks what holds on any host that has the call — the names resolve, the values are distinct, CS_PATH is a real search path, and an unknown name is refused. Assisted-by: Claude * posix: lockf, waitid and the sparse-file whence values None of the three existed. All are POSIX rather than Linux-only — the Apple targets carry every one of them — so they were absent on a host that has them. `lockf` is `interp_posix.py:3006-3012`: one call under the `eintr_retry` loop, put through the call gate because `F_LOCK` waits. Its four commands are published beside it. It answers `None`, which is what `os_lockf_impl` does; `interp_posix.py:3012` answers the `0` the call returns on success, and 3.14 — the oracle the parity suite reads — does not carry it. `waitid` is one `interp_posix.py:1722` names and does not have, so the shape is CPython 3.14's: a five-field `waitid_result` structseq, the three `P_*` id types and the six `CLD_*` codes. A zero `si_pid` is the "nothing to report" answer and is `None` rather than a result of zeroes. `WEXITED`, `WSTOPPED` and `WNOWAIT` were already published. `SEEK_HOLE` and `SEEK_DATA` are the two `whence` values beyond the three os.py fixes itself. The hosts whose headers define them are named rather than excluded, so a host left out is one short of a name rather than one carrying a wrong value. The sandbox build refuses `lockf` and `waitid`; their constants are numbers and stay. extra_tests/parity_tests/os_lockf_waitid_seek.py locks a region, walks a file with both new whence values, and reports a child with WNOWAIT before reaping it — which is the difference between waitid and waitpid. Assisted-by: Claude
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.
Three follow-ups to #1060, all found by reading its CI run.
bench: sixmax-pypy-ratioceilings derived from a floored denominatorTen bench-gate failures on the ubuntu runner, on six fixtures. Seven of the ten carry check.py's own
[pypy exec clamped to floor; ratio not a measurement]marker: pypy's startup-subtracted exec time is pinned atEXEC_TIME_FLOOR_S, so the ratio divides by the floor rather than by a measurement. RaisingNdoes not lift it — the bodies are loop-invariant and pypy hoists them whole.None of the six was touched by this branch; all were last modified by
d1fef848351, which tightened them (e.g.pickle_ctor_args336 → 36).The clamped fixtures get their pre-tightening ceilings back and lose their
min-pypy-ratiofloors, which came from the same non-measurement. Two exceptions:type_immutable_reject— the restored 15 is below the 18.2x observed on the runner, so it gets twice that instead.pickle_ctor_args— a real measurement (pypy 0.04s, never clamped), but spanning 18.6x to 70.3x across hosts. Ceiling 121 (twice the slowest) with an explicit floor of 9, because the default ceiling/5 = 24.2 rejects the fastest host at 13.5x.parity: platform-scoped fixtures, and one real Windows divergenceThe windows job's benches now pass 387/387, which is what let it reach
parity_testsfor the first time — main's own windows run died at the bench step. Six fixtures fail there.Five fail under the reference CPython too, so the comparison measures nothing: a
b"\xff.py"filename only decodes under a UTF-8 filesystem encoding withsurrogateescape;os.fsencodeonly rejects a surrogate outside U+DC80..U+DCFF under that same handler;os.DirEntry's qualified names are theposixmodule's; andtype_members_python314's absolute C-struct sizes are one ABI's — the reference itself reportsOSError.__basicsize__120 on Windows andBaseExceptionGroup.__basicsize__88 on macOS arm64.run.pygains a# pyre-check: platforms=header naming thesys.platformvalues a script's subject holds on; an unmarked script still runs everywhere.The sixth is a real divergence.
W_SyntaxError.descr_strcallsos.path.basename, which is the platform's —ntpathsplits on\as well as/and drops the drivesplitdrivepeeled off first. Splitting on/alone left the full temporary path in the rendered message.bench: four wasm jitstats baselinesguard_failuresmoves by one or two against the values recorded at58fcd373e05andda5e6fb38c7, withloops_compiledandbridges_compiledunchanged on all four.closure_per_callreturns to the 468 that this branch and its own CI run both observed before the rebase took main's 470.— opened by Claude
Summary by CodeRabbit
Bug Fixes
SyntaxErrorfilename display across Windows and Unix-like platforms, including drive-letter paths and mixed separators.Tests