Skip to content

_io: port BytesIO and StringIO to interp level; objspace: two __getattribute__ deviations - #1079

Merged
youknowone merged 6 commits into
mainfrom
single-walker
Aug 6, 2026
Merged

_io: port BytesIO and StringIO to interp level; objspace: two __getattribute__ deviations#1079
youknowone merged 6 commits into
mainfrom
single-walker

Conversation

@youknowone

Copy link
Copy Markdown
Owner

Ports _io.BytesIO / _io.StringIO down to interp level, and corrects two
__getattribute__ deviations found while measuring that path.

_io port

_io_app.py loses 391 lines; bytesio.rs and stringio.rs carry
W_BytesIO / W_StringIO as #[pyre_class] types, with the three GC
censuses (build_gc, all_subclass_range_aliases,
SUBCLASS_RANGE_HIERARCHY) appended at the tail so no established type id
renumbers.

pickle_ctor_args was the CI red row this addresses, and it is the only red
row whose denominator was a real measurement rather than the exec-time floor:

before after
dynasm 0.80s 0.28s
cranelift 0.84s 0.28s

test_memoryio is 183/183 OK. The dedicated export-semantics probe matches
cpython3.14 on all ten rows, including close() while a getbuffer() result
is live — interp_bytesio.py:194 close_w is the one that diverges there, so
W_BytesIO::close keeps the export check and now cites that line.

__getattribute__

Two deviations, both found by a 54-case conformance probe that otherwise
matched cpython3.14 exactly:

  1. object.__getattribute__(SomeClass, name) walked the class's own MRO and
    returned inherited attributes. descroperation.py:88-112
    Object.descr__getattribute__ looks up through space.lookup(w_obj, name)
    — the metatype for a type object — and reads only w_obj.getdictvalue,
    never the receiver type's MRO. The instance arm already had exactly that
    body, so the two arms now share it with the lookup type and namespace
    extracted.
  2. A surrogate-name AttributeError on a type receiver used the generic
    'type' object has no attribute … template instead of the type-specific
    type object 'Sub' has no attribute …. attr_error_wtf8 now takes the same
    receiver-kind split the &str path uses, via a shared
    missing_attribute_subject.

Separately, getattr_str_impl's call_getattr && is_type(obj) arm no longer
wraps and unwraps an already-validated name to re-enter callable dispatch when
the metatype's __getattribute__ is the canonical type one — that body is
what object_getattr_miss already inlines. A metaclass override still takes
the descriptor-call path.

jitstats

Verification

check.py dynasm ALL PASSED 388/388
check.py cranelift ALL PASSED 388/388
check.py wasm ALL PASSED 384/384
cpython_tests 46 PASS / 0 FAIL / 0 CRASH, no regressions
cargo test --all --no-default-features --features dynasm 101 suites, 7477 tests
cargo fmt --all -- --check clean
conformance probe (54 cases) identical to cpython3.14 on both backends

Not addressed here

The remaining red perf rows on ubuntu are unchanged by design. Their pypy
denominator is pinned at EXEC_TIME_FLOOR_S, so the printed "ratio" is an
absolute wall-clock gate that scales 1:1 with machine speed — 240/374 rows are
clamped locally, 203/374 on ubuntu. check.py already disables the FLOOR arm for
those rows via FLOOR_GATE_MIN_BASELINE_S; the CEILING arm has no such guard.
Un-gating the clamped ceilings would remove the gate from 240/374 rows, and
scaling fixture N does not help — type_dict_surrogate's pypy exec is still
0.006s at 50x N.

🤖 Generated with Claude Code

`class BytesIO` and `class StringIO` in `_io_app.py` are replaced by
`W_BytesIO` (bytesio.rs) and `W_StringIO` (stringio.rs), following
`pypy/module/_io/interp_bytesio.py` and `interp_stringio.py`.
`_io_app.py` keeps only `IncrementalNewlineDecoder`.

Both types hold their storage in a GC object field: `W_BytesIO` a
`bytearray`, `W_StringIO` an `array('w')` of code points, standing in
for the `RStringIO`/`UnicodeIO` split that exists because RPython
strings are immutable.

The two classes are registered at the tail of the three GC censuses
(`build_gc`, `all_subclass_range_aliases`, `SUBCLASS_RANGE_HIERARCHY`)
as ids 160 and 161. `tag_io_instance_with_finalizer` is split so
`W_BytesIO` can pass `add_to_autoflusher=False` (interp_bytesio.py:70).

Methods that can run Python (`buffer_w`, `__index__`, `dict.update`)
re-derive the receiver from a pinned root afterwards, because a
collection inside such a callback moves the stream and leaves the
entered `&mut self` behind the forwarding pointer.

lib-python `test_memoryio` goes from IMPORTERROR to 183 tests, 0
errors, 0 failures. `synth/pickle_ctor_args` runs 0.80s -> 0.28s
(dynasm) and 0.84s -> 0.28s (cranelift); its jitstats and those of
`synth/pickle_terminal_raise_resume` are re-recorded, both losing the
function-entry loops that traced the removed app-level methods.

Assisted-by: Claude
`getattr_str_impl` reaches the metatype `__getattribute__` slot for every
type receiver. `type` defines `__getattribute__`, so
`getattribute_if_not_from_object` returns it and the slot was invoked
through `get_and_call_function` — wrapping the name into a `w_str`,
entering callable dispatch, and re-validating the name through
`core::str::from_utf8` — only to reach `typeobject.py:811-828`
`W_TypeObject.descr_getattribute`, whose body `object_getattr_miss`
already inlines below.

`is_type_getattribute_descr` recognises that descriptor by identity
against `type`'s own slot (typeobject.py:1322), the same shape
`is_object_getattribute_descr` uses for `object`. A metaclass that
overrides `__getattribute__` keeps the descriptor-call path.

800k `getattr(SubClass, name)`, medians of 7 interleaved runs:
ascii names 0.344s -> 0.238s (-31%), lone-surrogate names 0.451s ->
0.443s (the surrogate path never entered this dispatch).

A 54-case type-attribute conformance probe — metaclass `__getattr__`
hooks, `__getattribute__` overrides, metatype data descriptors,
descriptor `__get__` raising AttributeError, abc/enum, attribute
mutation, and installing `__getattribute__` on the metaclass after the
fact — produces byte-identical output before and after, and matches
cpython3.14 on 52 of those 54 lines.

`synth/type_metatype_method_call` loses one wasm guard failure with the
residual call.

Assisted-by: Claude
… type's MRO

`object_getattribute`'s non-instance tail delegated to `getattr_str_impl`,
so a type receiver ran `typeobject.py:811-828`
`W_TypeObject.descr_getattribute` — the class-MRO walk.
`object.__getattribute__(Sub, "b")` therefore returned the value
inherited from `Base`; cpython3.14 and pypy3 both raise AttributeError.

descroperation.py:88-112 `Object.descr__getattribute__` looks the name up
with `space.lookup(w_obj, name)` — the metatype for a type object — and
reads only `w_obj.getdictvalue`, never the receiver type's own MRO. The
type receiver now shares the instance arm with the metatype as lookup
type and the type's own namespace as the receiver dict.

`type.__getattribute__` keeps the MRO walk: typedef.rs routes its slot to
a named `type_getattribute` instead of the object default.

`attr_error_wtf8` reported `'type' object has no attribute` for a type
receiver where the `&str` path already reported
`type object 'Sub' has no attribute`. Both now share
`missing_attribute_subject`, and the message is built as WTF-8 so a lone
surrogate survives into `AttributeError.name` and `.obj`.

The 54-case type-attribute conformance probe now matches cpython3.14 on
every line, on dynasm and cranelift alike; it matched on 52 before.
Vendored test_descr (162), test_funcattrs (35), test_descrtut, test_super
(40), test_enum (1081), test_abc (72) and test_property (31) report
identical counts to a build without this change.

Assisted-by: Claude
`pickle_ctor_args` and `pickle_terminal_raise_resume` lose the
function-entry loops that traced the app-level `_io.BytesIO` methods:
loops_compiled 4 -> 2 and 36 -> 31 (wasm 73 -> 68), with
`pickle_ctor_args` cranelift also dropping its one bridge and its
guard failures 201 -> 1. `loops_aborted` is unchanged on every backend.

Assisted-by: Claude
…oves

`closure_per_call` 470 -> 468, `exception_traceback_frame_lineno`
820 -> 819, `recursive_call_frame_relocation` 649 -> 648 and
`gc_iterator_source_drop` 613 -> 614 on wasm.

These are not this branch's: check.py ran wasm 383/383 on the previous
base with both objspace commits already applied, and the four moved only
after rebasing onto 1de95e0, which carries #1060, #1072 and #1047 —
all three change guard emission. Each count reproduces exactly across
repeated runs, so it is a transition and not the back-edge poll
oscillation. dynasm and cranelift are 388/388 either way.

Assisted-by: Claude
`interp_bytesio.py:194` `close_w` delegates straight to `RStringIO.close`
with no export check, so it releases the storage under a live
`getbuffer()` result. `_io.BytesIO.close` raises `BufferError: Existing
exports of data: object cannot be re-sized` in that state, which the
`check_exports()` call here already reproduced; only the comment naming
the upstream line was missing.

Comment-only change.

Assisted-by: Claude
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@youknowone, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 51 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b7fc8720-e425-4939-a455-3b618a7d9002

📥 Commits

Reviewing files that changed from the base of the PR and between 1de95e0 and 31ac337.

📒 Files selected for processing (22)
  • pyre/bench/synth/closure_per_call.wasm.jitstats
  • pyre/bench/synth/exception_traceback_frame_lineno.wasm.jitstats
  • pyre/bench/synth/gc_iterator_source_drop.wasm.jitstats
  • pyre/bench/synth/pickle_ctor_args.cranelift.jitstats
  • pyre/bench/synth/pickle_ctor_args.dynasm.jitstats
  • pyre/bench/synth/pickle_ctor_args.wasm.jitstats
  • pyre/bench/synth/pickle_terminal_raise_resume.cranelift.jitstats
  • pyre/bench/synth/pickle_terminal_raise_resume.dynasm.jitstats
  • pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats
  • pyre/bench/synth/recursive_call_frame_relocation.wasm.jitstats
  • pyre/bench/synth/type_metatype_method_call.cranelift.jitstats
  • pyre/bench/synth/type_metatype_method_call.dynasm.jitstats
  • pyre/bench/synth/type_metatype_method_call.wasm.jitstats
  • pyre/pyre-interpreter/src/baseobjspace.rs
  • pyre/pyre-interpreter/src/lib.rs
  • pyre/pyre-interpreter/src/module/_io/_io_app.py
  • pyre/pyre-interpreter/src/module/_io/bytesio.rs
  • pyre/pyre-interpreter/src/module/_io/mod.rs
  • pyre/pyre-interpreter/src/module/_io/stringio.rs
  • pyre/pyre-interpreter/src/typedef.rs
  • pyre/pyre-jit/src/eval.rs
  • pyre/pyre-object/src/pyobject.rs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit 31ac337).
Updated: 2026-08-06T07:05:58.388Z

Files in the reviewed diff
pyre/pyre-interpreter/src/baseobjspace.rs
pyre/pyre-interpreter/src/lib.rs
pyre/pyre-interpreter/src/module/_io/_io_app.py
pyre/pyre-interpreter/src/module/_io/bytesio.rs
pyre/pyre-interpreter/src/module/_io/mod.rs
pyre/pyre-interpreter/src/module/_io/stringio.rs
pyre/pyre-interpreter/src/typedef.rs
pyre/pyre-jit/src/eval.rs
pyre/pyre-object/src/pyobject.rs

1. Regressions to PyPy parity introduced by this patch

  • pyre/pyre-interpreter/src/baseobjspace.rs:6295 ↔ pypy/objspace/descroperation.py:58: lone-surrogate attribute names are interpolated literally (message.push_wtf8(name)) rather than rendered with PyPy’s %R representation. A name such as "\udcff" therefore appears as a raw surrogate instead of the escaped repr '\udcff'. Main used format_wtf8_repr, which matched PyPy.

2. Other mismatches introduced by this patch

  • pyre/pyre-interpreter/src/module/_io/bytesio.rs:443 ↔ pypy/module/_io/interp_bytesio.py:194: BytesIO.close() now calls check_exports() and raises BufferError for a live getbuffer() view; PyPy’s close_w unconditionally calls self.close() and has no export check.

3. Pre-existing mismatches (already present before this patch)

  • pyre/pyre-interpreter/src/module/_io/bytesio.rs:374 ↔ pypy/module/_io/interp_bytesio.py:149: getbuffer() exposes a direct memoryview of the backing bytearray, so its .obj is that bytearray. PyPy constructs BytesIOView(BytesIOBuffer(self), w_bytesio=self), preserving the BytesIO-owned exporter/view shape. This behavior already existed in upstream/main’s _io_app.py fallback.

4. Structural adaptations

  • pyre/pyre-interpreter/src/module/_io/bytesio.rs:7 ↔ pypy/module/_io/interp_bytesio.py:65: Rust stores the stream data in a managed bytearray field instead of mixing in RPython’s RStringIO; this is a storage/GC adaptation, while preserving its position and stream operations.

  • pyre/pyre-interpreter/src/module/_io/stringio.rs:7 ↔ pypy/module/_io/interp_stringio.py:123: Rust replaces PyPy’s READING/ACCUMULATING/RWBUFFER representation with one managed array('w')-style codepoint buffer. This is a Rust ownership and GC-layout adaptation.

  • pyre/pyre-jit/src/eval.rs:3464 ↔ pypy/module/_io/interp_bytesio.py:65: explicit native GC type registration is required for the Rust classes’ inline managed references; RPython derives tracing from its translated object layout.

  • pyre/pyre-object/src/pyobject.rs:637 ↔ pypy/module/_io/interp_bytesio.py:65: append-only subclass-range IDs are a Pyre JIT/runtime bookkeeping requirement with no corresponding source-level PyPy class declaration.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 31ac337492

ℹ️ 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 +241 to +244
let obj = W_StringIO::allocate_stable(W_StringIO {
buffer: pyre_object::gc_roots::shadow_stack_get(slot),
..W_StringIO::default()
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject operations on an uninitialized StringIO

Initialize the newline state here or track an explicit uninitialized state. Calling StringIO.__new__(StringIO) returns an apparently open object with readnl == PY_NULL; it accepts write("x"), but after seeking to zero, readline() passes that null pointer to w_str_get_wtf8, causing an unsafe null dereference instead of the expected ValueError for an uninitialized stream.

Useful? React with 👍 / 👎.

Comment on lines +374 to +378
fn getbuffer(&mut self) -> Result<PyObjectRef, crate::PyError> {
// interp_bytesio.py:149-152. The bytearray exporter owns the release
// accounting for the writable view returned here.
self.check_closed()?;
crate::builtins::w_memoryview_new_with_flags(self.buffer, 0x0001)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve per-stream BytesIO buffer ownership

Return a stream-owned view rather than wrapping the backing bytearray directly. PyPy's BytesIOView stores w_bytesio and releases the stream's export_count, whereas this memoryview roots only self.buffer; consequently a live view no longer keeps a BytesIO subclass alive, and view.obj exposes the bytearray so callers can retain and resize the stream's storage after releasing the view. This also results from replacing the upstream RStringIO/per-stream export shape with a convenience bytearray instead of the required literal port.

AGENTS.md reference: AGENTS.md:L231-L233

Useful? React with 👍 / 👎.

Comment on lines +241 to +245
#[crate::pyre_methods(
base = super::buffered_iobase_type(),
weakrefable,
doc = "read-write"
)]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Register flush directly on BytesIO

Restore a flush method in this BytesIO method table. The removed app-level class defined it directly, and pyre/extra_tests/snippets/stdlib_io_bytesio.py explicitly requires "flush" in BytesIO.__dict__; the new type only inherits _IOBase.flush, so that existing conformance test fails even though calling f.flush() otherwise has similar behavior.

Useful? React with 👍 / 👎.

Comment on lines +6295 to +6300
let mut message = Wtf8Buf::from_string(format!(
"{} has no attribute '",
missing_attribute_subject(obj)
));
message.push_wtf8(name);
message.push_wtf8(Wtf8::new("'"));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve repr escaping in surrogate AttributeErrors

Format the name with its WTF-8-aware repr instead of inserting it verbatim. This path also accepts names containing a lone surrogate plus quotes, backslashes, or control characters; for those inputs the new message is malformed (for example, an embedded quote is left unescaped), while PyPy's %R path and the removed format_wtf8_repr call produce a valid escaped representation.

AGENTS.md reference: AGENTS.md:L231-L233

Useful? React with 👍 / 👎.

@youknowone
youknowone merged commit fd709f2 into main Aug 6, 2026
15 of 17 checks passed
@youknowone
youknowone deleted the single-walker branch August 6, 2026 08:28
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 13, 2026
… jit: pop-fold guard order and recorded-raise roots; typedef: __init_subclass__ keywords (#1204)

* jit: read the recorded-raise roots back out of their shadow slots

`walker_emit_recorded_builtin_raise` pinned `exc` and each argument and then
kept using the locals it had handed to `pin_root`. `RootScope::pin_root`
normalizes the address it publishes once a second mutator has existed
(`gc_roots.rs`), so past that point the slot and the caller's copy can name
different objects, and these values are baked into the trace as `ConstPtr`s
that outlive the walk.

`args_storage` carried no root at all: it was read off `exc` before the
normalization could apply and then indexed once per argument.

Take every value back from `shadow_stack_get` after pinning it, and pin
`args_storage`, which is the shape the concrete-shadow build for
zip/tuple in this file already uses.

Assisted-by: Claude

* typedef: refuse __init_subclass__ keywords the way parse_obj does

The default `__init_subclass__` reported a leftover class-definition keyword
as `ArgErr::UnknownKwds`, so `class C(Base, flag=1)` raised "got an
unexpected keyword argument 'flag'". `parse_obj` does not report the keyword
when the signature has neither `**kwargs` nor a keyword-only argument
(`argument.py:377-380`); it collapses every such refusal to "takes no
keyword arguments". This signature is `cls` alone, so that branch always
applies. CPython 3.14 raises the same sentence.

The qualname ahead of the `()` is left as it was: pyre spells it
`object.__init_subclass__` and CPython spells it with the subclass, and that
choice is not what this changes.

The snippet asserted only that the message mentioned `__init_subclass__`,
which held for both shapes; it now pins the sentence and that the keyword is
not named.

Assisted-by: Claude

* parity-tests: pin isinstance over a class that overrides __class__

`isinstance` reads `obj.__class__` on an MRO miss, so a `@property
__class__` decides the answer and runs on every call: `isinstance(Masked(),
int)` is True here, not the False a miss looks like. Nothing in the CPython
suite runs that shape hot enough to trace.

Counts are printed rather than asserted, so a fold that elided the call,
cached its result, or answered False shows up as a diff against CPython.

Scope is recorded in the header and was measured, not assumed: this does not
reach `observed_replay_safe_isinstance`, whose only consumer is the nested
residual abort inside an inline sub-walk. At this call depth the residual is
a plain `call_may_force` and that gate no-ops; substituting the weaker
predicate it used to carry leaves every line byte-identical. The gate's
witness is `bench/synth/foriter_isinstance_class_property_replay.py`.

Assisted-by: Claude

* parity-tests: end isinstance_class_property with the harness's OK line

run.py:172 accepts a case only when the last non-empty stdout line is
"OK"; without it all three runners, cpython included, report the file as
a failure.

Assisted-by: Claude

* intobject: correct the claim that gc_interp is off on the native backends

`w_int_gc_alloc`'s doc justified keeping its `dont_look_inside` boundary on
the grounds that it "costs nothing where the arm is unreachable:
`gc_interp::enabled()` is false on the native backends". `enabled_from_env`
(`gc_interp.rs`) answers true for every `PYRE_GC_INTERP` value except exactly
`0`, so it is on by default on every target, and this arm — not the
`malloc_typed` one `fuse_boxing_alloc` rewrites — is the arm `w_int_new`
takes.

Assisted-by: Claude

* jit: separate the pop fold's lock-guard-free label from its trace-guard order

`w_list_pop_end`'s doc defines "the descended body must hold no guard" as the
absence of a `w_list_lock` acquire/release pair, which is what declines the
fold's sub-walk. `try_walker_orthodox_list_pop` and `list_pop_end_jitcode`
repeated the word without that qualifier, where it reads as a claim about
trace guards.

The sub-walk gets no callee frame, so a guard recorded inside it resumes at
the caller's CALL boundary and re-executes the whole `pop()`. Every guard the
Integer arm can record lands ahead of its `ll_list_int_set_len`: the sole op
after that store is the `w_int_new` call, which `dispatch_inline_call_dir_kind`
short-circuits into `walker_box_int` (`NewWithVtable` + `SetfieldGc`, no guard
recorded) and returns before `run_sub_jitcode_walk`. Nothing asserts the
ordering.

Assisted-by: Claude

* jitcode_dispatch: check the pop fold's guard/store order instead of assuming it

`orthodox_list_pop_commit` descends `w_list_pop_end_inner` with no callee
frame, so a guard recorded inside resumes at the caller's CALL boundary and
re-executes the whole `pop()`. That is sound only while every guard lands
before the body's first committed store. Today it does — the Integer arm's
`ll_list_int_set_len` is followed only by the `w_int_new` call, which
`dispatch_inline_call_dir_kind` short-circuits into `NewWithVtable` +
`SetfieldGc` and records no guard — but nothing read the order.

`subwalk_guard_follows_store` scans the ops recorded since a `TracePosition`
and reports whether a guard follows a `setfield` / `setarrayitem` /
`setinteriorfield`. A `start` past the end of the ops vector reports true: the
window is gone, so an empty read is not an answer.

The commit captures the position before `run_sub_jitcode_walk` and declines
with `OrthodoxSubWalkTraceUnsupported` on a positive read, which cuts the
tentative IR back to the generic residual. The decline takes the same
`w_list_len == len_before` re-read the apply below does: where the arm's store
keeps a runtime binding the sub-walk executed it for real, and cutting back to
a residual that pops again is the double-apply the append side already had to
fix.

Unit test covers guard-then-store, store-then-guard, a store recorded before
the captured position, `SetarrayitemGc`, and a position past the end.

`pyre/bench/synth/list_pop_append.py` still reads 2.2x against its
`max-pypy-ratio=22`; it read 73.5x before the fold existed.

Assisted-by: Claude

* docs: record the spec-vs-implementation ruling the parity review keeps re-deriving

Six review findings across PRs #1001, #1079, #1081, #1085 and #1113 are one
policy question, not six bugs: pyre follows CPython for what a Python program
observes while the review measures every line against PyPy. Nothing in the repo
stated the split, so each cycle re-filed them under sections 1/2.

The ruling is that pyre's implementation is a port of PyPy and pyre's spec is
CPython 3.14. Six of the seven adjudicated cases carry no version delta at all
(`sched_setscheduler` has returned None since 3.3, `PyUnicode_FSConverter` has
accepted bytes since 3.3, PEP 529 surrogatepass is 3.6, `DirEntry` has cached
its `stat_result` since PEP 471), so "3.14" names which CPython to read rather
than a lag PyPy is expected to close.

- AGENTS.md gains the normative section and the six tests in short form.
- The `/parity` skill gains a fourth deviation class, SPEC-DEVIATION, exempt
  from Principle 6's auto-fix (reverting one re-introduces a known bug), plus
  the full procedure with its evidence rules and worked examples.
- `.github/codex-review-prompt.md` replaces the "Python 3.11 vs 3.14" exception
  with the four conditions a section-4 entry must carry.

Structure — names, module paths, control-flow order, data structures, storage
owner, JIT hints — is outside the ruling and follows PyPy unconditionally. A
finding where PyPy's shape serves a mechanism pyre also has stops at PyPy: the
`DirEntry.stat()` object cache is one, since `interp_posix.py:537-542` states
the per-call rebuild is what keeps the allocation virtual.

Assisted-by: Claude

* _sre: drive an ASCII str subject as bytes and read the stored length

`Subject::len()` called `code_points().count()` and `char_to_byte` called
`code_point_indices().nth(pos)`, so every match walked the subject before
the engine started; `Request::new` and `create_cursor` then walked it
again.

Add `Subject::AsciiStr` for a `str` whose code points are one byte each,
selected by `w_str_is_ascii` where `make_ctx` selects `is_ascii()`
(interp_sre.py:246).  It drives the WTF-8 payload as bytes, so a character
position is already a byte offset -- `UnicodeAsciiMatchContext`
(interp_sre.py:52).  `StrDrive` is `count` and cursor arithmetic only and
every unicode decision keys on the compiled pattern's opcode, which is the
property that lets upstream spell that context as a bare `StrMatchContext`
subclass.

`Subject::Str` now carries the object (`ctx.w_unicode_obj`,
interp_sre.py:250) and reads the stored `_len()` and `_index_to_byte`
rather than re-deriving them.  Its positions remain code point indices
that the `Wtf8` driver still resolves by walking; the note on the variant
records what converting the reported spans would take.

`slice_subject`, `empty_subject` and `finish_output` branch on
`is_unicode()`, and `subject_span_bytes` extracts the position mapping and
slices once; `char_len` and `char_slice` are gone.

On an ASCII subject with n=1.6M, `pat.match(s, pos)` measured 130.7us at
pos=0 and 762us at pos=n-10; both are now 0.35us, flat in n and in pos.
A differential run over match/search/fullmatch spans, pos/endpos sweeps,
findall/finditer/split/sub/subn/expand, bytes/bytearray/memoryview, type
mismatches, a str subclass and scanner positions is byte-identical to
CPython 3.14.6 on all 608 lines, as it was before the change.
check.py --backend dynasm: 425/425.

Assisted-by: Claude

* BINARY_SLICE: convert str bounds through the index storage

`binary_slice_values`'s `str` branch collected the byte offset of every
code point in the subject into a `Vec<usize>` to resolve two bounds, so
`s[a:b]` cost the whole string.  A one-character slice of a 200k subject
measured 752us, against 0.42us for the same slice written as a prebuilt
slice object, which reaches `w_str_slice_codepoints` and walks only the
sliced elements.

Read the stored `_length` and convert the two bounds with `_index_to_byte`
(unicodeobject.py:1251), which is what the slice-object path already does.
The clamping and the `.max(s)` on the stop bound are unchanged, and a
bound equal to the count still resolves to the end of the buffer.

`binary_slice_values` is shared with the JIT residual
(`bh_binary_slice_fn`, call_jit.rs:5765), so both consumers get it.

The compiler folds constant bounds to `LOAD_CONST slice` + `BINARY_OP []`
and emits `BINARY_SLICE` only for computed ones, so this is the path
`json/decoder.py` takes with its per-token `s[end:end + 1]`.  Decoding a
flat 208 KB ASCII payload with the pure-Python scanner: 25.85s -> 0.089s,
with the size sweep going from x3.95/x4.13/x9.39 per doubling to
x2.07/x2.62/x1.77.  `s[p:p+1]` on a 200k subject: 752us -> 0.54us.

A differential run over 12 subjects (ASCII, 2/3/4-byte, lone surrogates,
empty, and lengths on the 63/64/65/128 index-storage block boundaries)
against 19x19 bound pairs in both spellings, plus list/tuple/bytes slicing
and slice assignment, is byte-identical to CPython 3.14.6 on all 4430
lines.  check.py --backend dynasm: 425/425.

Assisted-by: Claude

* _sre: resolve non-ASCII str positions through the stored index storage

Subject::Str drove the engine over &Wtf8, whose StrDrive::count counts every
code point and whose create_cursor(n) steps over the first n of them. Both run
once per match, so a scan that restarts at successive positions walked the
subject again on every call.

Add Utf8Drive, which carries the W_UnicodeObject next to the payload and
answers count with w_str_len and create_cursor with w_str_index_to_byte,
minting the cursor at the head of an O(1) suffix reslice. Positions stay code
point indices and stepping delegates to the &Wtf8 impl, so the engine's
position arithmetic is unchanged.

Assisted-by: Claude
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant