Builtin-leaf subclass / dict / deque / operator interpreter machinery + simplify_graph (#127) and canonical-flatten (#73) JIT passes

[youknowone]

follow-up #112

Summary

This PR delivers two large areas: CPython-parity machinery in the interpreter (builtin-leaf subclass dispatch, dict-subclass/deque/operator support, class-creation and cellvar fixes that together enable the Enum/IntEnum/Flag/IntFlag and namedtuple stdlib paths) and canonical-graph JIT work (the #127 simplify_graph pass port, the #73 canonical flatten_graph lowering, and vectorizer Codex-parity). The Rust changes are in pyre-interpreter/pyre-jit/majit; the diff carries no enum/namedtuple test fixtures (the only test-ish file touched is check.py, for fib_loop headroom).

Interpreter — builtin-leaf subclass dispatch (custom int·float·str·tuple, enabling enum / IntFlag)

• int/long/float/bool/str keep ob_type at the canonical storage type and carry the Python class in w_class; added builtin_subclass_dunder (display.rs) so py_repr/py_str consult a __repr__/__str__ override resolved above object in the w_class MRO before the storage-keyed formatting (e.g. repr(IntEnum.X)).
• Replaced tuple.__new__'s descr_new_wrapper! with a hand-written tuple_descr_new that, for a subclass, copies into a fresh tuple and sets w_class = cls (mirroring int_descr_new/float_descr_new), so collections.namedtuple field access, repr, _replace/_make, and defaults work.
• descroperation now gates the binary operators, rich comparisons, and unary operators (pos/neg/invert) on a w_class-resolved override (binop_dispatch_first/try_compare_override/try_unary_override) before their is_int_like/is_float_pair/is_str storage fast paths; operand_overrides keys on the resolved method kind (mutable-code FUNCTION_TYPE, excluding fixed-code gateway builtins) so a long is not mistaken for an override and does not recurse — covers Python __add__/__or__, __lt__/__eq__, and enum.IntFlag.__invert__.
• load_method (eval.rs) no longer binds self for non-method descriptors (type/property/member/getset such as __class__) or MRO-absent attributes on a builtin-storage instance, fixing self.__class__(value) (compiled as LOAD_METHOD/CALL_METHOD) prepending an extra argument.
• baseobjspace resolves __getitem__ on type(cls)'s MRO before the PEP 560 __class_getitem__ fallback, consults the metaclass __len__ for a type receiver, and resolves __contains__ on the receiver's dynamic type before the getitem scan — routing Color['RED'], len(Color), and x in Color/x in flag through EnumMeta/IntFlag instance methods.

Interpreter — class creation, metaclass, cellvars

• MAKE_CELL now wraps a slot only when it does not already hold a cell, since initialize_frame_scopes pre-installs an empty cell for every pure cellvar; this stops never-reassigned cellvars like __class__ from becoming a cell-of-cell, fixing self.__class__ and zero-arg super() reads (previously infinite recursion in a dict-subclass __repr__ calling super().__repr__()).
• build_class (call.rs) strips the compiler-internal class scaffolding (__class__, __classdict__, __classcell__, __classdictcell__) from the class namespace before building the type, captures the __classcell__ cell, and binds it to the new class (type_new_classcell); a follow-up keeps __classcell__/__classdictcell__ visible in the namespace a custom metaclass receives while still excluding them from the class __dict__, consuming them in type.__new__ instead.
• build_class executes the class body directly against a custom __prepare__ mapping (a dict subclass such as enum._EnumDict) via setdictscope_object, so its __setitem__/__getitem__ fire mid-body, then mirrors the mapping's contents into class_ns and skips the metaclass-path replay; fixes Flag members like WHITE = RED | GREEN | BLUE reading unresolved auto() sentinels.
• type.__new__ resolves the dict backing of the namespace before its copy and __set_name__ loops, so a dict-subclass namespace (PyDict_Check, not PyDict_CheckExact) is walked instead of producing an empty __dict__.
• Adds the type.mro() method (mro_external), returning the MRO as a fresh list distinct from the __mro__ tuple getset; its absence had blocked import enum.
• Adds regression tests pinning both single-cell MAKE_CELL shapes: a parameter captured by an inner function, and the implicit __class__ cellvar resolving zero-arg super().

Interpreter — dict / dict-subclass / mapdict

• dict.__getitem__ on a dict-subclass instance now looks up the key in the __dict_data__ backing directly and, on a miss, dispatches __missing__ against the subclass instance's type (upstream dictmultiobject.py:166-170) instead of the plain-dict backing — so e.g. defaultdict.__missing__ fires; dict_missing_or_key_error is now pub(crate).
• Added a dict.__repr__ method (extracted into the shared display::dict_repr helper, upstream dictmultiobject.py:130-150 descr_repr) so dict-subclass instances and super().__repr__() format their backing rather than falling back to the object repr; unbound dict.__repr__(x) on a non-dict receiver now raises TypeError (the receiver-rejection lives in typedef.rs, not the .py ref).
• dict.__delitem__ resolves the __dict_data__ backing for subclass instances in typedef.rs (mirroring __setitem__), fixing infinite recursion where the instance branch re-looked-up and re-entered the inherited dict.__delitem__.
• Implemented DevolvedDictTerminator read/write in mapdict (mapdict.py:383-395): read via getdict + finditem_str, write via getdict + setitem_str, gated on attrkind == DICT; added _mapdict_self_ref to the MapdictObject trait to reach _obj_getdict. switch_to_text_strategy past LIMIT_MAP_ATTRIBUTES remains a documented deferral.

(Rust changes for the dict items above live in typedef.rs/baseobjspace.rs/display.rs/mapdict.rs; the dictmultiobject.py line citations are upstream PyPy parity attributions, not Rust paths.)

Interpreter — deque, operator, builtin-kwargs ABI, misc / bench

• _collections deque: bounded the list-backed W_Deque to maxlen, trimming from the opposite end on append/appendleft/extend/extendleft, with the bound in the private __maxlen__ slot and a read-only maxlen property; added extendleft, rotate, count, remove, __contains__, reverse, index, copy, __setitem__, __delitem__, __repr__, routing pop/popleft and the append family through shared snapshot/store helpers. maxlen is validated at construction via gateway_nonnegint_w (TypeError/ValueError), __init__ propagates iterable errors, __repr__ is ReprGuard-protected ([...]), and __getitem__/__setitem__/__delitem__ go through a deque_index helper mirroring space.decode_index4. Added rich comparison (__eq__…__ge__, element-wise over both backings, NotImplemented for non-deque) and repetition (__mul__/__rmul__/__imul__, re-bounded by maxlen). The port covers these methods specifically; it is not a complete deque.
• operator: ported itemgetter/attrgetter/methodcaller as app-level callable classes plus the _resolve_attr_chain helper (verbatim app_operator.py), installed via the appleveldefs arm, replacing interp-level stubs that returned args[0] unchanged (which broke the stdlib namedtuple's itemgetter(n) accessors); also added app-level countOf. length_hint stays interp-level.
• #[pyre_function] keyword binding (pyre-macros): when a call carries the trailing __pyre_kw__ dict, the wrapper rebinds positional+keyword args into a resolved scope via bind_builtin_kwargs (mirroring the gateway Arguments._match_signature) using parameter-name/required tables collected at expansion time; positionals fill left-to-right, keywords match by name, an absent optional becomes PY_NULL, and unknown/duplicate/missing-required raises TypeError. The positional fast path is unchanged when no kwargs dict is present; varargs fns keep the positional path. Fixes deque(maxlen=3) previously binding the dict as iterable.
• pyframe: peekvalues(n) now asserts only the lower base bound; the upper-bound assert is skipped for the empty peek (n == 0), which spuriously failed at peak stack depth and crashed collections/reprlib imports in debug builds. (Its commit also carries the call.rs build_class scaffolding-strip rustfmt rewrap and a stray formatting-only edit to a simplify.rs test — both non-behavioral.)
• Parity-comment corrections: document why the Forwarded enum has no VectorInfo variant (scratch is not clone-stable, lives in the pos-keyed vecinfo_cache), and fix the defaultdict doc-comment to state __getitem__ invokes/stores default_factory (raising KeyError without one) rather than short-circuiting to w_none() — the same comment records that W_DefaultDict remains a stub subclassing object not dict (so isinstance(d, dict) is False) with __missing__/__repr__/copy/__reduce__ still absent.
• bench/list_reverse: raised REPS from 15 to 401 (odd, keeping the reversed result) so the build loop and JIT trace warmup are amortised and the measurement reflects reverse().
• check.py: gave cranelift fib_loop 3x-vs-cpython headroom (dynasm stays 2x) to absorb slower windows-runner variance on the bignum-add-bound benchmark.

JIT — simplify_graph pass port (#127)

• Added a faithful operations-based eliminate_empty_blocks to simplify.rs (port of simplify.py:52-69, not link.target.operations), retargeting each predecessor link through an empty forwarding block, and wired it into all_passes(); the walker pipeline keeps the block.dead-predicate codewriter::eliminate_empty_blocks. Adds a graph-shape test for collapsing a non-dead arg-carrying forwarder.
• Fixed remove_trivial_links' merge bridge to strip the source block's trailing boundary goto TLabel(target) + Unreachable before absorbing the merged target's per_block_ssarepr, so the target's own terminator is the first terminator and the emit_link_renamings_into_block splice lands after the target opcodes.
• Reordered rewrite_dead_forwarder_gotos to run before remove_trivial_links/rewrite_trivial_link_merges so inline byte-stream gotos already name the surviving target when the merge bridge's strip_trailing_boundary_goto reads source terminators (fixes the source -> dead_forwarder -> target strip miss).
• Registered the None, NotImplemented, Ellipsis, True, and False prebuilt-singleton addresses in jit_static_ref_addrs under their module::NAME catalogue keys, so the front-end same-file Expr::Path fold emits ConstRefAddr instead of a rejected cross-block body-Input for w_none/w_not_implemented/w_ellipsis/w_bool_from.
• Documented the issue Port RPython simplify_graph parity and track remaining graph/codewriter parity gaps #112 scope Optimize JIT for list operations and enhance method dispatch #3 unmarked-label conclusion (per-pass coverage of the walker-safe subset, assembler.rs::patch_labels fail-loud note) and added a regression test asserting no reachable link targets a dead/dropped block after the subset runs on a graph with a dead switch arm and a trivial forwarder.

JIT — canonical flatten_graph lowering (#73)

• Lower a returned graph Constant ref (Operand::ConstRef) in the canonical flatten_graph path via a new JitCodeBuilder::ref_return_const, mirroring load_const_r by encoding it as a constants-window register index (num_regs_r + pool_idx) patched in finish(); route ref_return ConstRef to it instead of expect_reg. Also names the dispatch opname in the expect_reg panic via a CURRENT_DISPATCH_OP thread-local.
• Add setattr to is_pyre_canonical_elidable_hlop alongside getattr and type: all three are paired with an inline abort_permanent by the walker (StoreAttr arm), so the canonical SSARepr elides the undispatchable HLOp (upstream rclass.py rtype_setattr rewrites to setfield_gc).
• Emit a trailing -live- after canonical residual_call_* / inline_call_* Insns under lowering_ctx, per jtransform.py:467-482 handle_residual_call / handle_regular_call, supplying the post-call guard_no_exception / inline-boundary resume marker. Adds a unit test pinning the marker after a residual_call_ir_r. (A follow-up rustfmt-only commit rewraps the trailing_live binding/closure; non-behavioral.)
• Refined the residual-call gate from an unconditional superset to calldescr_canraise (effect_info.check_can_raise(false)), reading the CallDescrStub EffectInfo off the Insn, so the EF_CANNOT_RAISE get_current_exception call drops its marker while inline_call_* stays unconditional; adds a unit test covering both the can-raise and cannot-raise cases.

JIT — vectorizer Codex-parity

• Threaded jitcell_token: Option<&Arc<JitCellToken>> through optimize_vector, VectorizingOptimizer::run_optimization, and try_vectorize, passing it to finaloplist in place of a hardcoded None; the standalone caller and the Optimization-trait propagate_forward path pass None while the compile path is disconnected, matching the upstream jitcell_token=None default (vector.py:123,143,271).
• Documented why the forwarded_vecinfo scheduling scratch (schedule.py:20-28) uses a pos-keyed vecinfo_cache instead of op._forwarded: Op::clone resets forwarded but preserves pos (resoperation.rs:1344,1352), the scheduler reads vecinfo off cloned ops (dependency.rs:221, unroll/schedule clones), and INT_SIGNEXT bytesize is the dynamic arg1 value (cast_to_bytesize_static returns None) recoverable only via int_signext_vecinfo's setup-time resolver that vectorization_info_for_op(&Op) cannot reach.

Self-review

Prompt & Model

Model:

Prompt:

Answer

Summary by CodeRabbit

Release Notes

• New Features
  • Complete deque implementation with extend, rotate, count, remove, copy, and rich comparisons
  • operator module now provides countOf, attrgetter, itemgetter, and methodcaller
  • Improved metaclass dispatch for type __getitem__, __len__, and __contains__ operations
  • Better support for operation overrides in builtin-leaf subclasses
  • Enhanced tuple and dict subclass handling
  • Added mro() method for type objects

[coderabbitai]

Walkthrough

This PR significantly extends Pyre's interpreter semantics, macro system, and JIT pipeline. It refactors class construction to support dict-subclass prepared namespaces with proper cell-variable routing, adds override-aware dispatch for arithmetic and comparison operators on builtin-leaf subclasses, implements builtin keyword-argument binding, expands the deque and operator modules, improves JIT graph simplification and bridging, and enhances diagnostics with better opcode context and singleton address registration.

Changes

Pyre Interpreter: Type System, Collections, and Operations

Layer / File(s)	Summary
Class Namespace and __classcell__ Handling pyre/pyre-interpreter/src/call.rs, pyre/pyre-interpreter/src/eval.rs, pyre/pyre-interpreter/src/builtins.rs	Class body execution routes through dict-subclass prepared namespaces via setdictscope_object; __classcell__ is captured, validated as a cell, and bound to the new type after construction; MAKE_CELL now wraps only when the slot is NULL or not already a cell, and tests verify no double-wrapping in closures or super().
Type and Dict Protocol Semantics pyre/pyre-interpreter/src/baseobjspace.rs, pyre/pyre-interpreter/src/typedef.rs	Type subscripting tries metaclass __getitem__ first; len() consults metaclass __len__ for type objects; dict.__getitem__ and dict.__delitem__ route dict subclasses through backed dict; dict.__repr__ formats via descriptor-aware helper; tuple.__new__ preserves subtype identity; type.mro returns the MRO as a list.
Display and String Override Routing pyre/pyre-interpreter/src/display.rs	Exposes ReprGuard crate-wide, adds dict_repr helper for recursion guarding, centralizes builtin-leaf repr/str formatting, and routes leaf-subclass __repr__/__str__ overrides through override-aware dispatch before returning raw storage.
Frame Runtime Binding and Stack Operations pyre/pyre-interpreter/src/pyframe.rs	Load_method suppresses binding for non-method descriptor kinds on builtin-storage subclasses; empty-peek assertions use debug_assert! to avoid stricter upper-bound checks.
Mapdict Devolved DICT Routing pyre/pyre-interpreter/src/objspace/std/mapdict.rs	Implements devolved DICT terminator read/write through per-instance dict obtained via _mapdict_self_ref(); adds trait requirement for stable identity; includes routing and cycle tests.
Deque Bounded Container with Full API pyre/pyre-interpreter/src/module/_collections/mod.rs	Expands deque from stub to list-backed bounded container with snapshot storage, validated maxlen, bounds-trimming on all growth paths, centralized index normalization, element-wise comparisons, mutation, repetition, and non-recursive repr.
Operator App-Level Callable Implementations pyre/pyre-interpreter/src/module/operator/app_operator.py, pyre/pyre-interpreter/src/module/operator/mod.rs	Adds app-level countOf, attrgetter, itemgetter, and methodcaller with __reduce__/__repr__ support; wires through appleveldefs while removing interpreter stubs.
Override-Aware Binary, Unary, and Comparison Dispatch pyre/pyre-interpreter/src/objspace/descroperation.rs	Adds operand_overrides and binop_dispatch_first to detect genuine user overrides and gate dunder dispatch before storage fast paths; applies to 11 binary operators, 3 unary operators, and rich comparisons.

Pyre Builtins and Macros: Keyword Argument Resolution

Layer / File(s)	Summary
Builtin Kwargs Helpers and Macro Support pyre/pyre-interpreter/src/builtins.rs, pyre/pyre-macros/src/lib.rs	Introduces has_builtin_kwargs and bind_builtin_kwargs for CPython-like argument validation; #[pyre_function] macro emits conditional kwargs preamble for non-varargs functions and treats PY_NULL as omitted in default/Option unwrapping.

Pyre JIT: Graph Simplification and Code Generation

Layer / File(s)	Summary
Empty Block Elimination and Pass Wiring pyre/pyre-jit/src/jit/simplify.rs	Adds local eliminate_empty_blocks ported from RPython with forwarding substitution and loop prevention; wires into all_passes in correct relative order; includes regression test for unmarked-block cleanup.
Merge Bridge Boundary Goto Stripping pyre/pyre-jit/src/jit/codewriter.rs	Adds strip_trailing_boundary_goto to remove obsolete boundary terminators before block absorption in merge bridging; reorders dead-forwarder rewrite before trivial-link merging to prevent stale gotos.
Lowering Elision and Trailing Live Markers pyre/pyre-jit/src/jit/flatten.rs	Extends canonical elidable hlops to include setattr; adds insn_needs_trailing_live predicate to emit -live- markers for inline/residual calls based on effect info; includes can-raise behavior tests.
Assembler Diagnostics and Static References pyre/pyre-jit/src/jit/assembler.rs, pyre/pyre-interpreter/src/jit_fnaddr.rs	Tracks current dispatch opcode in thread-local context to enrich panic messages; publishes singleton object addresses (None, NotImplemented, Ellipsis, True, False) in static reference table.

Documentation and Benchmarks

Layer / File(s)	Summary
Metainterp Documentation and List Reverse Benchmark majit/majit-metainterp/src/optimizeopt/schedule.rs, majit/majit-metainterp/src/jitcode/assembler.rs, pyre/bench/list_reverse.py	Clarifies schedule vecinfo_cache keying for clone-stable pos-based lookup with INT_SIGNEXT reasoning; expands unmarked-label panic scope with named simplify passes; increases list reverse repetition count from 15 to 401 for better measurement amortization.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• youknowone/pyre#89: Implements JIT walker trampolines and multi-predecessor terminator machinery directly referenced by this PR's codewriter.rs boundary-goto stripping and merge-bridge reordering in the same file.

Poem

🐰 From classes born with cells and bounds,
To dicts that speak in type's own tongue,
Deques count their maxlen rounds,
While ops dispatch true overrides sung.
The JIT graph flows ever lean—
All optimized, all clean! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the main changes: builtin-leaf subclass machinery, dict/deque/operator enhancements, and JIT simplify_graph and canonical-flatten passes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch simplify-graph

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cf727dd064

ℹ️ 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".

[chatgpt-codex-connector]

Strip collapsed-forwarder gotos before absorbing target

In the walker pipeline, eliminate_empty_blocks(&graph) has already retargeted graph links through dead forwarders, but rewrite_dead_forwarder_gotos does not retarget the inline TLabels until after this merge bridge. For a shape source -> dead_forwarder -> target that then records a trivial merge (source, target), the source block still ends in goto TLabel(dead_forwarder), so stripping only block_label_name(target) leaves the old terminator in front of the absorbed target ops. Then walker_post_walk_insert_renamings still sees that stale first terminator and can splice the target-exit renamings before the target's opcodes, reintroducing the stale-slot issue this strip is meant to prevent.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ea03288e5f

ℹ️ 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".

[chatgpt-codex-connector]

Preserve classcell for custom metaclasses

When a class body references __class__ or uses zero-argument super(), Python passes __classcell__ through the namespace to the metaclass; custom metaclasses can inspect it or forward it to type.__new__. This removes the key before the metaclass path builds w_namespace_dict, so class C(metaclass=M): ... where M.__new__ expects __classcell__ now observes a different namespace than CPython/PyPy even though the cell is set later.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject negative deque maxlen instead of clamping

For deque(..., maxlen=-1) Python raises ValueError during construction, but this clamps the stored negative value to 0 whenever the bound is read. That lets invalid deques be created and makes later append/appendleft silently discard every element, so callers that rely on constructor validation continue with incorrect container semantics.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc697aaa2f

ℹ️ 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".

[chatgpt-codex-connector]

Add a recursion guard before rendering deque items

When a deque contains itself (for example d = deque(); d.append(d); repr(d)), this new __repr__ maps each element through py_repr, which dispatches back into this same deque.__repr__ with no ReprGuard or placeholder path. That turns a valid recursive container into unbounded recursion/stack overflow instead of the guarded representation used by the built-in container reprs.

Useful? React with 👍 / 👎.

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pyre/pyre-interpreter/src/eval.rs`:
- Around line 2005-2023: Add regression tests to cover both MAKE_CELL shapes so
the double-wrap fix is pinned: write one test that creates a class with a
__class__ closure cell (ensuring initialize_frame_scopes installs an empty cell)
and asserts that calling MAKE_CELL via make_cell does not wrap an existing cell
again; and a second test that defines a closure over a parameter (argument slot
promoted to a cellvar) and asserts make_cell wraps the raw slot exactly once
producing a W_CellObject containing the original value. Reference the
interpreter behavior around make_cell, initialize_frame_scopes and the MAKE_CELL
semantics when adding these focused tests.

In `@pyre/pyre-interpreter/src/module/_collections/mod.rs`:
- Around line 245-255: The deque __repr__ implementation calls crate::py_repr on
each element without guarding against self-referential structures, so add a
recursion guard around the element formatting in the __repr__ function: obtain
or use the existing interpreter/representation guard mechanism (e.g., a repr
guard/context used elsewhere) inside __repr, push self_obj before iterating
snapshot(self_obj), and for each element call a guarded representation helper
(or check if the element is the guarded object and return a placeholder like
"..." instead of recursing) before invoking crate::py_repr; ensure you still
format maxlen via maxlen_bound as before.
- Around line 81-89: The __init__ implementation currently swallows errors by
calling crate::builtins::collect_iterable(it).unwrap_or_default(), which hides
iterable/type errors; change __init__ to propagate errors instead: call
crate::builtins::collect_iterable(it) and match its Result (or use ? to
propagate) so that on Err you return the error rather than iterating an empty
list, then iterate the Ok(iterator) and call do_append for each item; update the
function signature to return a PyResult if required and ensure any error
returned uses the existing exception types/propagation conventions used
elsewhere in this module (e.g., propagate the Result from collect_iterable
rather than using unwrap_or_default).

In `@pyre/pyre-interpreter/src/objspace/std/mapdict.rs`:
- Around line 715-728: Add a regression test that constructs a Mapdict object
whose terminator kind is TerminatorKind::Devolved and verifies the read/write
DICT branch in terminator_read (and node_read) is exercised: create an object
with a Devolved terminator, use the write path to set a DICT attribute (so it
ends up in the object's dict via _obj_getdict), then call
node_read/terminator_read (via MapRef operations used in your tests) and assert
the returned PyObjectRef comes from _obj_getdict (i.e. matches the dict-stored
value) rather than the map storage; reference the terminator_read function,
MapdictObject/_obj_getdict, TerminatorKind::Devolved, DICT, and node_read when
adding the test.

In `@pyre/pyre-interpreter/src/typedef.rs`:
- Around line 2277-2283: The descriptor currently treats a null result from
resolve_dict_backing(args[0]) as an empty dict string; instead, detect when args
is non-empty and dict.is_null() and return a TypeError indicating an
invalid/unbound receiver rather than Ok(w_str_new("{}")). Update the call site
handling in the function containing resolve_dict_backing so that when
args.is_empty() you still return "{}" but when args.len() > 0 and dict.is_null()
you raise/return a TypeError (using the project's existing error
construction/return pattern) mentioning the descriptor and the bad receiver.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 503c3d1e-1f3d-46a0-8265-5394cf78589b

📥 Commits

Reviewing files that changed from the base of the PR and between d58be8c and fc697aa.

📒 Files selected for processing (20)

• majit/majit-ir/src/box_ref.rs
• majit/majit-metainterp/src/jitcode/assembler.rs
• majit/majit-metainterp/src/optimizeopt/schedule.rs
• majit/majit-metainterp/src/optimizeopt/vector.rs
• majit/majit-translate/src/front/ast.rs
• pyre/pyre-interpreter/src/baseobjspace.rs
• pyre/pyre-interpreter/src/call.rs
• pyre/pyre-interpreter/src/display.rs
• pyre/pyre-interpreter/src/eval.rs
• pyre/pyre-interpreter/src/jit_fnaddr.rs
• pyre/pyre-interpreter/src/module/_collections/mod.rs
• pyre/pyre-interpreter/src/module/operator/app_operator.py
• pyre/pyre-interpreter/src/module/operator/mod.rs
• pyre/pyre-interpreter/src/objspace/std/mapdict.rs
• pyre/pyre-interpreter/src/pyframe.rs
• pyre/pyre-interpreter/src/typedef.rs
• pyre/pyre-jit/src/jit/assembler.rs
• pyre/pyre-jit/src/jit/codewriter.rs
• pyre/pyre-jit/src/jit/flatten.rs
• pyre/pyre-jit/src/jit/simplify.rs

[chatgpt-codex-connector]

💡 Codex Review

https://github.com/youknowone/pyre/blob/76290e3ef58858a895e3832cabc1963120900cd2/pyre-interpreter/src/module/_collections/mod.rs#L236-L238
Validate rotate's count before reading it as an int

When rotate() is called with a non-int argument (for example d.rotate(None) or an object that should be handled via __index__), this unconditionally reads the object layout as a W_IntObject instead of raising/converting through the index protocol. Because w_int_get_value is unsafe and assumes an actual int object, these inputs can produce bogus rotation counts rather than the required TypeError/__index__ behavior.

---

https://github.com/youknowone/pyre/blob/76290e3ef58858a895e3832cabc1963120900cd2/pyre-interpreter/src/display.rs#L581-L584
Route str() through the new str-subclass override

This only makes direct py_str() callers honor a str subclass's __str__; the public str() path still bypasses it because builtin_str returns immediately for any STR_TYPE object and py_str_wtf8 also returns raw string contents before delegating. In a case like class S(str): def __str__(self): return 'override', str(S('x')) will still ignore the override, so the new dispatch needs to be shared with the WTF-8/builtin path as well.

ℹ️ 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".

[coderabbitai]

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pyre/pyre-interpreter/src/builtins.rs (1)

1638-1645: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Propagate __set_name__ failures from the resolved-namespace path.

Line 1645 still uses unchecked call_function. As this module's call_and_check helper shows at Lines 2381-2393, that suppresses Python exceptions behind PY_NULL, so a descriptor that raises in __set_name__ can leave class creation succeeding with partially initialized state on the new dict-subclass namespace path.

Suggested fix

-                        let _ = crate::call_function(set_name, &[w_type, k]);
+                        call_and_check(set_name, &[w_type, k])?;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/builtins.rs` around lines 1638 - 1645, The code
currently calls crate::call_function unchecked for descriptors' __set_name__ in
the resolved-namespace path (symbols: w_ns_backing, pyre_object::w_dict_items,
is_str, crate::baseobjspace::getattr, crate::call_function), which will swallow
Python exceptions; change this to use the existing call_and_check helper (the
pattern used at call_and_check) instead of crate::call_function, check its
return for PY_NULL, and if PY_NULL return/propagate the error immediately so
exceptions raised by __set_name__ are not suppressed and class creation fails
consistently with the other path.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pyre/pyre-interpreter/src/baseobjspace.rs`:
- Around line 6855-6859: The current dynamic-type path calls
crate::call_function on a found __contains__ (in the block using
crate::typedef::r#type(haystack) and lookup_in_type_where) and then uses
is_true(result), which treats PY_NULL (an exception) as true; change this to use
the error-aware caller (call_and_check or call_function_impl_result) so
exceptions from __contains__ propagate. Update the dynamic-type branch to call
the error-checking helper and mirror the earlier instance-__contains__ branch’s
semantics (use the same call_and_check/call_function_impl_result flow and
is_true on the successful result) so both paths have identical error handling.

In `@pyre/pyre-interpreter/src/call.rs`:
- Around line 2257-2273: The prepared mapping path currently bypasses the
earlier cleanup of scaffolding keys: after computing classcell from class_ns
(using class_ns_ptr) ensure the same "__class__" and "__classdict__" keys are
removed from the prepared/backing mapping before passing it to the metaclass;
specifically, when the code uses w_prepared_dict or the mapping_namespace branch
to materialize the metaclass namespace, either delete "__class__" and
"__classdict__" from that mapping or construct the metaclass namespace from the
cleaned class_ns (the mutable reference used to set classcell) so the metaclass
never sees those keys.
- Around line 2237-2255: The code currently overlays entries from the backing
mapping onto the existing class_ns (class_ns_ptr) which leaves stale keys that
were deleted in the prepared mapping; change the logic in the mapping_namespace
branch (where resolve_dict_backing, pyre_object::w_dict_items,
dict_storage_store, and fix_ptr are used) to rebuild class_ns from the backing
dict instead of only inserting items — e.g., clear or reinitialize the storage
for *class_ns_ptr (use the existing dict storage API or a
dict_storage_clear/recreate equivalent) and then iterate
pyre_object::w_dict_items(backing) to repopulate via dict_storage_store, finally
call (*class_ns_ptr).fix_ptr().

In `@pyre/pyre-interpreter/src/display.rs`:
- Around line 228-260: builtin_subclass_dunder currently swallows both
call_function failures and wrong-type returns by returning None, hiding Python
exceptions and TypeErrors; change the helper to return a fallible result (e.g.
Result<Option<String>, PyError> or a dedicated error type) instead of
Option<String>, propagate call_function errors (do not treat non-str returns as
silent None; return an appropriate TypeError), and update the Python-facing
entry points (py_repr / py_str and any other callers that used
builtin_subclass_dunder) to propagate the error back into the Python VM instead
of falling back to builtin formatting; locate uses by the function name
builtin_subclass_dunder and the call to crate::call_function (and similar helper
calls at other sites that route int/long/float/bool/str subclasses) and adjust
their signatures and error handling accordingly so exceptions raised by
overrides are surfaced to the caller.

In `@pyre/pyre-interpreter/src/module/_collections/mod.rs`:
- Around line 43-53: maxlen_bound, rotate, index and other deque
integer-handling paths must stop unconditionally unboxing user objects with
w_int_get_value and instead coerce via crate::builtins::getindex_w; update
maxlen_bound to call getindex_w on the "__maxlen__" attribute, check the
returned isize/usize for non-negative and propagate a TypeError on failure, and
replace unsafe w_int_get_value usages in rotate and index with getindex_w
(making rotate return Result<_, PyError> to propagate errors similar to
deque_index); likewise change __init__, deque_repeat and __imul__ to accept
objects implementing __index__ by using getindex_w rather than is_int, and
enforce the non-negative bound where applicable.

In `@pyre/pyre-interpreter/src/module/operator/app_operator.py`:
- Around line 82-124: Add __reduce__ implementations to itemgetter and
methodcaller so they can be pickled when pickle support is enabled: for
itemgetter implement __reduce__ to return (self.__class__, (self._idx,)) when
self._single is True or (self.__class__, tuple(self._idx)) when multi-index, and
for methodcaller implement __reduce__ to return (self.__class__,
(self._method_name,)+tuple(self._args), self._kwargs) (or equivalent form
returning the constructor and its args/state) so the stored attributes (_idx for
itemgetter; _method_name, _args, _kwargs for methodcaller) are preserved; keep
in mind copyreg currently disables pickle but add these reducers to prepare for
future enabling.

---

Outside diff comments:
In `@pyre/pyre-interpreter/src/builtins.rs`:
- Around line 1638-1645: The code currently calls crate::call_function unchecked
for descriptors' __set_name__ in the resolved-namespace path (symbols:
w_ns_backing, pyre_object::w_dict_items, is_str, crate::baseobjspace::getattr,
crate::call_function), which will swallow Python exceptions; change this to use
the existing call_and_check helper (the pattern used at call_and_check) instead
of crate::call_function, check its return for PY_NULL, and if PY_NULL
return/propagate the error immediately so exceptions raised by __set_name__ are
not suppressed and class creation fails consistently with the other path.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: de455dac-a8d5-48eb-845b-60e04d43eace

📥 Commits

Reviewing files that changed from the base of the PR and between fc697aa and 76290e3.

📒 Files selected for processing (23)

• majit/majit-metainterp/src/jitcode/assembler.rs
• majit/majit-metainterp/src/optimizeopt/schedule.rs
• majit/majit-metainterp/src/optimizeopt/vector.rs
• pyre/bench/list_reverse.py
• pyre/check.py
• pyre/pyre-interpreter/src/baseobjspace.rs
• pyre/pyre-interpreter/src/builtins.rs
• pyre/pyre-interpreter/src/call.rs
• pyre/pyre-interpreter/src/display.rs
• pyre/pyre-interpreter/src/eval.rs
• pyre/pyre-interpreter/src/jit_fnaddr.rs
• pyre/pyre-interpreter/src/module/_collections/mod.rs
• pyre/pyre-interpreter/src/module/operator/app_operator.py
• pyre/pyre-interpreter/src/module/operator/mod.rs
• pyre/pyre-interpreter/src/objspace/descroperation.rs
• pyre/pyre-interpreter/src/objspace/std/mapdict.rs
• pyre/pyre-interpreter/src/pyframe.rs
• pyre/pyre-interpreter/src/typedef.rs
• pyre/pyre-jit/src/jit/assembler.rs
• pyre/pyre-jit/src/jit/codewriter.rs
• pyre/pyre-jit/src/jit/flatten.rs
• pyre/pyre-jit/src/jit/simplify.rs
• pyre/pyre-macros/src/lib.rs

💤 Files with no reviewable changes (9)

• pyre/pyre-interpreter/src/pyframe.rs
• pyre/pyre-jit/src/jit/assembler.rs
• pyre/pyre-interpreter/src/objspace/std/mapdict.rs
• pyre/pyre-jit/src/jit/simplify.rs
• pyre/pyre-interpreter/src/typedef.rs
• pyre/pyre-jit/src/jit/flatten.rs
• pyre/pyre-macros/src/lib.rs
• pyre/pyre-jit/src/jit/codewriter.rs
• pyre/pyre-interpreter/src/objspace/descroperation.rs

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the target file
FILE="pyre/pyre-interpreter/src/module/_collections/mod.rs"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }

# Show the relevant functions around the cited lines
sed -n '1,220p' "$FILE" | nl -ba | sed -n '1,220p'
echo "----"
sed -n '220,520p' "$FILE" | nl -ba | sed -n '1,300p'
echo "----"
sed -n '520,900p' "$FILE" | nl -ba | sed -n '1,380p'

# Extract specific function bodies if present (quick n^2 search)
for sym in maxlen_bound deque_repeat rotate index "__imul__" "deque_init" "__init__"; do
  echo "==== searching for $sym"
  rg -n "$sym" "$FILE" || true
done

echo "---- helper searches in this file"
rg -n "w_int_get_value|__index__|as_int|index.*coerc|coerc|int_get|TypeError|is_int" "$FILE" || true

# Find integer coercion helpers used elsewhere in repo (names may differ)
echo "---- searching repo for likely helpers"
rg -n "fn (.*index.*|.*as_int.*|.*int.*coerc.*|.*get.*int.*)|__index__" pyre/pyre-interpreter/src/module/_collections/mod.rs pyre/pyre-interpreter/src/module -S || true
rg -n "w_int_get_value" pyre/pyre-interpreter/src/module/_collections/mod.rs pyre/pyre-interpreter/src/module -S || true

# Also find how "__maxlen__" is set and read, if there are helpers around it
rg -n "__maxlen__" pyre/pyre-interpreter/src/module/_collections/mod.rs -S || true

Repository: youknowone/pyre

Length of output: 102

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="pyre/pyre-interpreter/src/module/_collections/mod.rs"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }

# Line-numbered print using awk (since nl isn't available)
print_range() {
  local start="$1" end="$2"
  awk -v s="$start" -v e="$end" '{if(NR>=s && NR<=e) printf "%6d  %s\n", NR, $0}' "$FILE"
}

echo "== header: first 220 lines"
print_range 1 220
echo "----"
echo "== middle: 221-520"
print_range 221 520
echo "----"
echo "== tail: 521-900"
print_range 521 900 || true

echo "== rg symbols in file"
for sym in maxlen_bound deque_repeat rotate index "__imul__" "__init__"; do
  echo "---- $sym"
  rg -n "$sym" "$FILE" || true
done

echo "== rg for likely coercion helpers / integer conversion"
rg -n "__index__|TypeError|w_int_get_value|as_int|is_int|coerc" "$FILE" || true

echo "== rg for __maxlen__ usage"
rg -n "__maxlen__" "$FILE" || true

echo "== repo-wide helpers (scoped to interpreter module area)"
rg -n "__index__|TypeError|as_int|is_int|coerc|int.*get.*value|w_int_get_value" pyre/pyre-interpreter/src/module -S || true

Repository: youknowone/pyre

Length of output: 50371

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Targeted helper lookups to avoid huge output
rg -n "fn\s+w_int_get_value" pyre -S --max-count 20
rg -n "getindex_w" pyre/pyre-interpreter/src -S --max-count 50
rg -n "fn\s+getindex_w" pyre/pyre-interpreter/src -S --max-count 20
rg -n "gateway_nonnegint_w" pyre/pyre-interpreter/src -S --max-count 50
rg -n "fn\s+gateway_nonnegint_w" pyre/pyre-interpreter/src -S --max-count 20

Repository: youknowone/pyre

Length of output: 1930

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show w_int_get_value implementation
sed -n '90,170p' pyre/pyre-object/src/intobject.rs | nl -ba | sed -n '1,200p' || true

echo "---- getindex_w definitions"
# Show getindex_w and nearby helpers
sed -n '2520,2585p' pyre/pyre-interpreter/src/builtins.rs | nl -ba | sed -n '1,120p' || true

echo "---- baseobjspace getindex_w docs/notes (for error behavior)"
sed -n '820,920p' pyre/pyre-interpreter/src/baseobjspace.rs | nl -ba | sed -n '1,160p' || true

echo "---- gateway_nonnegint_w (search for exact helper name in repo)"
rg -n "gateway_nonnegint_w" pyre/pyre-interpreter/src -S

# Print the implementation if located
GW_FILE="$(rg -n "gateway_nonnegint_w" pyre/pyre-interpreter/src -S | head -n 1 | cut -d: -f1 || true)"
if [ -n "${GW_FILE}" ] && [ -f "$GW_FILE" ]; then
  echo "---- showing around gateway_nonnegint_w in $GW_FILE"
  # crude: print 200 lines around first occurrence
  LINE="$(rg -n "gateway_nonnegint_w" "$GW_FILE" -S | head -n 1 | cut -d: -f2 || true)"
  start=$((LINE-60)); end=$((LINE+140))
  sed -n "${start},${end}p" "$GW_FILE" | nl -ba | sed -n '1,220p' || true
fi

echo "---- int coercion helper candidates for __index__"
rg -n "getindex_w|call_function_or_identity\(.*__index__|__index__" pyre/pyre-interpreter/src -S --max-count 80

Repository: youknowone/pyre

Length of output: 8158

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect w_int_get_value to assess safety on non-int objects
sed -n '110,170p' pyre/pyre-object/src/intobject.rs

echo "---- builtins::getindex_w"
sed -n '2510,2585p' pyre/pyre-interpreter/src/builtins.rs

echo "---- baseobjspace getindex_w note (error behavior)"
sed -n '820,900p' pyre/pyre-interpreter/src/baseobjspace.rs

echo "---- rotate/index relevant uses already in _collections/mod.rs"
sed -n '35,120p' pyre/pyre-interpreter/src/module/_collections/mod.rs
sed -n '228,280p' pyre/pyre-interpreter/src/module/_collections/mod.rs
sed -n '300,370p' pyre/pyre-interpreter/src/module/_collections/mod.rs

Repository: youknowone/pyre

Length of output: 17472

---

Fix deque integer handling: stop unconditionally unboxing user objects with w_int_get_value

• maxlen_bound (43-53) reads __maxlen__ and then does unsafe { w_int_get_value(w) } after only None checks; if __maxlen__ is mutated to a non-int (or negative int), this can hit UB / wrong usize casting—validate/coerce the attribute via crate::builtins::getindex_w (and enforce >= 0).
• rotate (236-249) and index (250-265) call unsafe { w_int_get_value(...) } on user-provided n/start/stop with no is_int / __index__ coercion; route through crate::builtins::getindex_w (matching deque_index), and propagate TypeError (likely requires rotate to return Result<_, PyError>).
• __init__ maxlen / deque_repeat / __imul__ (105-122, 139-159, 330-356) currently require concrete int via is_int, rejecting valid __index__ inputs; use getindex_w for parity.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/module/_collections/mod.rs` around lines 43 - 53,
maxlen_bound, rotate, index and other deque integer-handling paths must stop
unconditionally unboxing user objects with w_int_get_value and instead coerce
via crate::builtins::getindex_w; update maxlen_bound to call getindex_w on the
"__maxlen__" attribute, check the returned isize/usize for non-negative and
propagate a TypeError on failure, and replace unsafe w_int_get_value usages in
rotate and index with getindex_w (making rotate return Result<_, PyError> to
propagate errors similar to deque_index); likewise change __init__, deque_repeat
and __imul__ to accept objects implementing __index__ by using getindex_w rather
than is_int, and enforce the non-negative bound where applicable.

[chatgpt-codex-connector]

💡 Codex Review

https://github.com/youknowone/pyre/blob/c28931b141369c853b04f27eed1abadf4481bd8d/pyre-interpreter/src/typedef.rs#L4546-L4549
Validate type.mro receiver before reading its MRO

When type.mro is called unbound with bad arguments, e.g. type.mro() or type.mro(1), this varargs builtin has no arity or type guard: the empty call indexes args[0], and the non-type call passes an arbitrary object to w_type_get_mro, which casts it to W_TypeObject. That turns a normal Python TypeError path into a panic or unsafe layout read; register this with fixed arity and reject non-type receivers before calling the unsafe accessor.

---

https://github.com/youknowone/pyre/blob/c28931b141369c853b04f27eed1abadf4481bd8d/pyre-interpreter/src/typedef.rs#L1212-L1215
Reject non-tuple classes in tuple.new

For direct calls like tuple.__new__(list, [1]), this new subclass path treats any type object other than the exact tuple type as a tuple subclass and stamps it into w_class. That produces a tuple-storage object whose reported class can be an unrelated type (instead of raising TypeError), so later attribute dispatch can see an impossible layout; please run the same tuple-subclass validation used by the other __new__ wrappers before setting w_class.

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 25f9a70cc3

ℹ️ 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".

[chatgpt-codex-connector]

Accept index objects for deque repetition

When the repeat count is an object that implements __index__ but is not an int (for example class N: __index__ = lambda self: 2), deque([1]) * N() should repeat successfully, but this gate returns NotImplemented and the operation falls through to a TypeError. Other deque methods in this file already use getindex_w for index-like arguments, so the repeat count should be decoded the same way rather than requiring is_int; the same issue also affects the in-place repeat path.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Avoid reflected dispatch for identical operand types

For two operands of the same builtin-leaf subclass, this predicate now sends the operation through try_dispatch_binary_special; if the forward method returns NotImplemented, that helper tries the right operand's reflected method even though Python/PyPy do not try __radd__/__rmul__ for identical operand types. A subclass such as class I(int): def __add__(...): return NotImplemented; def __radd__(...): return 5 will make I(1) + I(2) return 5 here instead of raising the unsupported-operand TypeError.

Useful? React with 👍 / 👎.

[coderabbitai]

Actionable comments posted: 3

♻️ Duplicate comments (11)

pyre/pyre-interpreter/src/module/_collections/mod.rs (3)

105-109: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

deque_repeat rejects __index__-able objects.

CPython's deque accepts objects implementing __index__ for the repeat count. The current is_int check is overly restrictive. Consider:

 fn deque_repeat(self_obj: PyObjectRef, n: PyObjectRef) -> Result<PyObjectRef, crate::PyError> {
-    if !unsafe { is_int(n) } {
-        return Ok(pyre_object::w_not_implemented());
-    }
-    let num = unsafe { w_int_get_value(n) }.max(0);
+    let num = match crate::builtins::getindex_w(n) {
+        Ok(v) => v.max(0),
+        Err(_) => return Ok(pyre_object::w_not_implemented()),
+    };

This preserves NotImplemented for non-integer-like operands while accepting __index__-able types.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/module/_collections/mod.rs` around lines 105 - 109,
The deque_repeat implementation currently uses is_int(n) and thus rejects
objects implementing __index__; change it to accept indexable objects by first
attempting to obtain an integer via the Python number protocol (e.g. call the
equivalent of PyNumber_Index on n), returning NotImplemented if that attempt
yields NotImplemented or an error, then extract the integer value from the
resulting int object using w_int_get_value (as currently done), clamp to >=0
(num.max(0)), and proceed; update references in deque_repeat to use that
index-conversion path instead of the is_int check so __index__-able types are
accepted while non-integer-like operands still produce NotImplemented.

---

43-53: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

maxlen_bound assumes __maxlen__ was validated but the attribute could be mutated.

If user code reassigns self.__maxlen__ to a non-int (e.g., d.__maxlen__ = "bad"), the direct w_int_get_value call on line 51 can produce undefined behavior or garbage. While __init__ validates the initial value, Python semantics allow attribute mutation afterward.

Consider either:

1. Re-validating is_int(w) before unboxing, or
2. Using getindex_w to coerce through __index__ (matching CPython's behavior where maxlen accepts __index__-able objects).

🛡️ Proposed defensive fix

 fn maxlen_bound(self_obj: PyObjectRef) -> Option<usize> {
     let w = crate::baseobjspace::getattr(self_obj, "__maxlen__").ok()?;
     if w.is_null() || unsafe { is_none(w) } {
         None
+    } else if !unsafe { is_int(w) } {
+        // Corrupted / mutated attribute — treat as unbounded rather than UB.
+        None
     } else {
         Some(unsafe { w_int_get_value(w) } as usize)
     }
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/module/_collections/mod.rs` around lines 43 - 53,
maxlen_bound currently unboxes __maxlen__ with w_int_get_value trusting
init-time validation; instead defend against attribute mutation by first
checking is_null/is_none, then if is_int use w_int_get_value as before,
otherwise call the existing getindex_w helper to coerce via __index__ (matching
CPython) and convert that result to usize; ensure any conversion or getindex_w
failures are handled the same way the function does today (returning None) and
reference the symbols maxlen_bound, __maxlen__, is_none, is_int,
w_int_get_value, and getindex_w when making the change.

---

340-346: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

__imul__ returns NotImplemented for non-int, but should raise TypeError.

In-place multiplication (d *= n) should raise TypeError if n is not an integer-like object, rather than returning NotImplemented. Returning NotImplemented from __imul__ falls back to __mul__, which can produce unexpected behavior (creating a new deque instead of mutating in place).

Additionally, CPython's deque accepts __index__-able objects for the repeat count.

🔧 Proposed fix

 fn __imul__(self_obj: PyObjectRef, n: PyObjectRef) -> Result<PyObjectRef, crate::PyError> {
-    if !unsafe { is_int(n) } {
-        return Ok(pyre_object::w_not_implemented());
-    }
-    let num = unsafe { w_int_get_value(n) };
+    let num = crate::builtins::getindex_w(n)?;
     let base = snapshot(self_obj);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/module/_collections/mod.rs` around lines 340 - 346,
The __imul__ implementation for W_Deque should not return NotImplemented for
non-int operands; instead accept objects supporting __index__ and raise
TypeError on failure. In the W_Deque::__imul__ function, replace the unsafe
is_int/w_int_get_value checks with a call to the interpreter’s index conversion
(use the existing PyNumber_Index/try_index helper or equivalent) to obtain an
integer index; if that conversion errors or returns non-integer, propagate/raise
a TypeError with an explanatory message; then use the resulting integer
(handling overflow/negative semantics as before) to perform the in-place repeat
logic. Ensure you reference the W_Deque::__imul__ (W_Deque.imul) function and
use the existing index conversion helper rather than returning
pyre_object::w_not_implemented().

pyre/pyre-jit/src/jit/flatten.rs (1)

1371-1374: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't gate post-call -live- emission on lowering_ctx.

flatten_space_operation() still passes through already-lowered residual_call_* / inline_call_* ops when lowering_ctx is unset, so this branch still drops the upstream-required post-call marker on those canonical streams. That loses the fallthrough resume point for can-raise residual calls and inline calls.

Suggested fix

-        let trailing_live = self.lowering_ctx.is_some() && insn_needs_trailing_live(&insn);
+        let trailing_live = insn_needs_trailing_live(&insn);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-jit/src/jit/flatten.rs` around lines 1371 - 1374, The code
currently only emits the post-call Insn::live when self.lowering_ctx.is_some();
change this so post-call live markers are emitted whenever
insn_needs_trailing_live(&insn) is true regardless of lowering_ctx. Concretely,
update the trailing_live computation in the block that defines trailing_live
(remove the is_some() check) and keep the subsequent
self.emitline(Insn::live(Vec::new())) branch so residual_call_* and
inline_call_* streams handled by flatten_space_operation() still get their
trailing live markers; keep using insn_needs_trailing_live, emitline, and
Insn::live as the referenced symbols.

pyre/pyre-interpreter/src/builtins.rs (2)

1646-1653: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Skip hidden class-cell scaffolding during __set_name__ dispatch.

This still walks the original namespace backing after __classcell__ / __classdictcell__ were intentionally stripped from the type dict, so a descriptor stored under either hidden key gets __set_name__ for an attribute that never exists on the class.

Suggested fix

         if !w_ns_backing.is_null() {
             let entries = unsafe { pyre_object::w_dict_items(w_ns_backing) };
             for (k, v) in entries {
                 if unsafe { is_str(k) } {
+                    let key = unsafe { pyre_object::w_str_get_value(k) };
+                    if key == "__classcell__" || key == "__classdictcell__" {
+                        continue;
+                    }
                     if let Ok(set_name) = crate::baseobjspace::getattr(v, "__set_name__") {
                         // getattr returns a bound method, so self is already bound.
                         // Call: bound_set_name(owner, name); propagate a raise.
                         call_and_check(set_name, &[w_type, k])?;
                     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/builtins.rs` around lines 1646 - 1653, The loop
over entries from w_ns_backing calls __set_name__ on any descriptor whose key is
a string, but it must skip the hidden class-cell keys so descriptors under
"__classcell__" or "__classdictcell__" (if present) do not get __set_name__ for
attributes that were intentionally removed; update the loop in the block that
iterates entries from pyre_object::w_dict_items(w_ns_backing) to check the key
string (k) after confirming is_str(k) and continue/skip when k equals
"__classcell__" or "__classdictcell__" before attempting
crate::baseobjspace::getattr(v, "__set_name__") and call_and_check.

---

2341-2348: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Return an exact str from the default str-subclass path.

With the leaf-subclass storage model, is_str(obj) also matches str subclasses. Falling through to Ok(obj) lets str(MyStr("x")) return the subclass instance instead of a base str whenever there is no user __str__ override.

Suggested fix

         if is_str(obj) {
             // A `str` subclass keeps `ob_type` at STR_TYPE but carries the
             // Python class in `w_class`; honor its `__str__` override before
             // returning the raw value.
             let tp = (*obj).ob_type;
             if let Some(s) = crate::display::builtin_subclass_dunder(obj, tp, "__str__") {
                 return Ok(w_str_new(&s));
             }
-            return Ok(obj);
+            let w_builtin_str = crate::typedef::gettypeobject(&STR_TYPE);
+            if crate::typedef::r#type(obj) != Some(w_builtin_str) {
+                return Ok(w_str_new(w_str_get_value(obj)));
+            }
+            return Ok(obj);
         }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/builtins.rs` around lines 2341 - 2348, The current
path returns the subclass instance (obj) when a str-subclass has no __str__
override; instead extract the raw base string value from obj and return a fresh
base str object (e.g. call w_str_new with the extracted string) rather than
Ok(obj). In other words, in the branch after checking
crate::display::builtin_subclass_dunder(obj, tp, "__str__"), convert the
underlying str contents of obj into a Rust string and return
Ok(w_str_new(&value)) so callers get an exact base str; keep the checks
involving is_str(obj), tp and the "__str__" lookup unchanged.

pyre/pyre-interpreter/src/eval.rs (1)

2859-2867: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Mirror the builtin-function exclusion in this binding path.

Line 2866 still binds any remaining descriptor as a method. For class MyInt(int): f = len, lookup_in_type finds len, load_method pushes (len, obj), and CALL turns c.f([1, 2, 3]) into len(obj, [1, 2, 3]). This branch needs the same builtin-function classification the is_instance path already uses before falling through to Some(_) => obj.

Suggested fix

                 match crate::baseobjspace::lookup_in_type(w_type, name) {
                     Some(d) if pyre_object::is_staticmethod(d) => PY_NULL,
                     Some(d) if pyre_object::is_classmethod(d) => w_type,
                     Some(d) if pyre_object::is_type(d) => PY_NULL,
                     Some(d) if pyre_object::is_property(d) => PY_NULL,
                     Some(d) if pyre_object::is_member(d) => PY_NULL,
                     Some(d) if pyre_object::getsetproperty::is_getset_property(d) => PY_NULL,
+                    Some(d) if crate::is_function(d) => {
+                        let ob_type = (*d).ob_type;
+                        if std::ptr::eq(ob_type, &crate::BUILTIN_FUNCTION_TYPE as *const _) {
+                            PY_NULL
+                        } else {
+                            obj
+                        }
+                    }
                     Some(_) => obj,
                     None => PY_NULL,
                 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/eval.rs` around lines 2859 - 2867, The match arm in
eval.rs that handles lookup_in_type currently falls through to Some(_) => obj
and treats remaining descriptors as methods; mirror the instance-path
builtin-function exclusion by checking for builtin functions here (e.g., call
pyre_object::is_builtin_function(d)) before the Some(_) => obj branch so builtin
functions are not bound as methods; update the match in the same block used by
load_method/CALL to return PY_NULL (or obj as appropriate) when the descriptor
is a builtin-function, using the same classification function the is_instance
path uses.

pyre/pyre-interpreter/src/display.rs (1)

228-260: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Propagate failing builtin-leaf __repr__ / __str__ overrides.

Line 256 still maps both call_function failure and a non-str return to None, so the new leaf-subclass path at Lines 274-277 / 582-585 / 672-675 silently falls back to builtin formatting instead of surfacing the Python exception or the usual TypeError.

Also applies to: 274-277, 582-585, 672-675

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/display.rs` around lines 228 - 260, The
builtin_subclass_dunder path currently swallows two failure modes (call_function
returning null and call_function returning a non-str) and returns None, causing
builtin formatting to run instead of surfacing the Python exception or raising
the usual TypeError; update the logic around crate::call_function(found, &[obj])
in builtin_subclass_dunder (and the analogous sites at the other occurrences) so
that if call_function returns null you propagate/return the Python exception (do
not convert to None), and if the call returns a non-str you raise/propagate a
TypeError explaining that __repr__/__str__ must return str rather than silently
falling back to builtin formatting. Ensure you use the existing
pyre_object/exception helpers to fetch and return or re-raise the exception
rather than returning None.

pyre/pyre-interpreter/src/objspace/descroperation.rs (1)

1015-1019: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't re-enter storage fast paths after user overrides declined the op.

When binop_dispatch_first(...) is true, a None from try_dispatch_binary_special(...) means an override existed and returned NotImplemented. Falling through to the int/long/float/sequence fast paths here manufactures builtin results, and on non-fast-path pairs the later fallback re-invokes the same special methods a second time.

Also applies to: 1085-1089, 1138-1142, 1243-1249, 1277-1281, 1321-1326, 1362-1366, 1729-1733, 1760-1764, 1791-1795, 1879-1883, 1952-1956

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/objspace/descroperation.rs` around lines 1015 -
1019, If binop_dispatch_first(...) is true and try_dispatch_binary_special(...)
returns None (meaning a user override existed and returned NotImplemented),
short-circuit and do not fall through into builtin fast paths—propagate/return
the NotImplemented result immediately instead of continuing to
int/long/float/sequence fast-path logic; update the blocks around
binop_dispatch_first + try_dispatch_binary_special (e.g., the __add__/__radd__
case and the other listed locations) to treat None as the override declining the
operation and return that NotImplemented outcome to the caller.

pyre/pyre-interpreter/src/typedef.rs (2)

1192-1215: ⚠️ Potential issue | 🟠 Major

Validate cls before tagging tuple subclasses.

This still skips check_user_subclass, so tuple.__new__(dict, ...) can manufacture a tuple-layout object whose w_class claims to be dict (or any other type) instead of raising TypeError.

🛠️ Minimal fix

 fn tuple_descr_new(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError> {
     let cls = if args.is_empty() {
         pyre_object::PY_NULL
     } else {
         args[0]
     };
+    if !cls.is_null() && unsafe { pyre_object::is_type(cls) } {
+        if let Some(w_tuple) = gettypefor(&pyre_object::pyobject::TUPLE_TYPE) {
+            check_user_subclass(w_tuple, cls)?;
+        }
+    }
     let value = crate::builtins::builtin_tuple(&args[1..])?;
     if cls.is_null() || !unsafe { pyre_object::is_type(cls) } {
         return Ok(value);
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/typedef.rs` around lines 1192 - 1215, In
tuple_descr_new, validate that cls is actually a user subclass of tuple before
copying and tagging the new tuple: after determining tuple_typeobj and before
the "Subclass path" block, call the existing check_user_subclass (or equivalent
routine used elsewhere) with cls and the tuple type object and if it returns
false return a TypeError instead of proceeding; this prevents arbitrary types
(e.g. dict) from being assigned to w_class when creating the new tuple in
tuple_descr_new.

---

4543-4556: ⚠️ Potential issue | 🟠 Major

Guard type.mro before the unsafe MRO read.

args[0] is indexed without an arity check, and w_type_get_mro(cls) is called for any receiver. type.mro() can panic, and type.mro(non_type) can walk non-type memory instead of raising TypeError.

🛠️ Minimal fix

-    let mro_method = make_builtin_function("mro", |args| {
+    let mro_method = make_builtin_function_with_arity("mro", |args| {
         let cls = args[0];
+        if cls.is_null() || !unsafe { pyre_object::is_type(cls) } {
+            let tp_name = if cls.is_null() {
+                "NoneType".to_string()
+            } else {
+                unsafe { (*(*cls).ob_type).name.to_string() }
+            };
+            return Err(crate::PyError::type_error(format!(
+                "descriptor 'mro' for 'type' objects doesn't apply to a '{tp_name}' object"
+            )));
+        }
         unsafe {
             let mro_ptr = pyre_object::w_type_get_mro(cls);
             if mro_ptr.is_null() {
                 return Ok(pyre_object::w_list_new(vec![]));
             }
             Ok(pyre_object::w_list_new((*mro_ptr).clone()))
         }
-    });
+    }, 1);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/typedef.rs` around lines 4543 - 4556, The mro
builtin reads args[0] unsafely and calls pyre_object::w_type_get_mro without
verifying arity or that the receiver is actually a type, which can panic or read
invalid memory; update the closure passed to make_builtin_function("mro", ...)
to first check args.len() > 0 and validate the receiver is a type using the
existing type-check helper (e.g., the project's is_type/w_type_check helper or
equivalent) before calling pyre_object::w_type_get_mro, and if the check fails
return a TypeError (using the project's error/exception construction path)
instead of doing the unsafe read; keep the subsequent logic that builds the list
with pyre_object::w_list_new and stores it with dict_storage_store unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pyre/pyre-interpreter/src/objspace/descroperation.rs`:
- Around line 2016-2049: The compare override flow currently treats a method
that returns the NotImplemented singleton as "no override" and falls through to
numeric fast paths; change the logic so an actual method call that returns
NotImplemented is preserved and propagated as an explicit result. Concretely, in
try_compare_override (and the analogous blocks around lookup_type_special /
try_call_special), ensure that try_call_special does not map a NotImplemented
return into None — either update try_call_special to return Some(NotImplemented)
when the callee returned NotImplemented, or modify try_compare_override to treat
a returned PyObjectRef equal to the NotImplemented singleton as Some(result) and
immediately return Ok(Some(result)) (using operand_overrides,
lookup_type_special, should_try_reverse_first to locate the same call sites).
This preserves the intended fallback semantics where an explicit NotImplemented
from an override prevents falling into numeric fast paths.

In `@pyre/pyre-interpreter/src/typedef.rs`:
- Around line 2356-2369: The current deletion code silently no-ops when args[0]
is neither a dict nor a dict-subclass with a valid "__dict_data__" backing,
which masks invalid receiver errors; update the branch handling in the block
that uses pyre_object::is_dict and pyre_object::is_instance so that if args[0]
is not a plain dict and either is not an instance or getattr(args[0],
"__dict_data__") is missing/non-dict, you return a TypeError instead of silently
returning None; use the same error-construction helper/pattern used elsewhere in
this module to raise a TypeError with a clear message (e.g. "descriptor delete
for non-dict or missing __dict_data__") and keep the existing behavior of
calling crate::baseobjspace::delitem when a dict or valid backing dict is
present.

In `@pyre/pyre-jit/src/jit/assembler.rs`:
- Around line 1646-1655: The panic in expect_reg already includes
current_dispatch_op(), but other helper functions (e.g., expect_tlabel,
expect_small_u16, expect_descr_vable_array_field, expect_descr_vable_array,
expect_descr_vable_static_field, etc.) do not; update each expect_* helper to
include the current opcode context by appending current_dispatch_op() (or a
similar descriptive call used in expect_reg) to their panic messages so all
operand-shape mismatch errors report the dispatch opcode for consistent
diagnostics.

---

Duplicate comments:
In `@pyre/pyre-interpreter/src/builtins.rs`:
- Around line 1646-1653: The loop over entries from w_ns_backing calls
__set_name__ on any descriptor whose key is a string, but it must skip the
hidden class-cell keys so descriptors under "__classcell__" or
"__classdictcell__" (if present) do not get __set_name__ for attributes that
were intentionally removed; update the loop in the block that iterates entries
from pyre_object::w_dict_items(w_ns_backing) to check the key string (k) after
confirming is_str(k) and continue/skip when k equals "__classcell__" or
"__classdictcell__" before attempting crate::baseobjspace::getattr(v,
"__set_name__") and call_and_check.
- Around line 2341-2348: The current path returns the subclass instance (obj)
when a str-subclass has no __str__ override; instead extract the raw base string
value from obj and return a fresh base str object (e.g. call w_str_new with the
extracted string) rather than Ok(obj). In other words, in the branch after
checking crate::display::builtin_subclass_dunder(obj, tp, "__str__"), convert
the underlying str contents of obj into a Rust string and return
Ok(w_str_new(&value)) so callers get an exact base str; keep the checks
involving is_str(obj), tp and the "__str__" lookup unchanged.

In `@pyre/pyre-interpreter/src/display.rs`:
- Around line 228-260: The builtin_subclass_dunder path currently swallows two
failure modes (call_function returning null and call_function returning a
non-str) and returns None, causing builtin formatting to run instead of
surfacing the Python exception or raising the usual TypeError; update the logic
around crate::call_function(found, &[obj]) in builtin_subclass_dunder (and the
analogous sites at the other occurrences) so that if call_function returns null
you propagate/return the Python exception (do not convert to None), and if the
call returns a non-str you raise/propagate a TypeError explaining that
__repr__/__str__ must return str rather than silently falling back to builtin
formatting. Ensure you use the existing pyre_object/exception helpers to fetch
and return or re-raise the exception rather than returning None.

In `@pyre/pyre-interpreter/src/eval.rs`:
- Around line 2859-2867: The match arm in eval.rs that handles lookup_in_type
currently falls through to Some(_) => obj and treats remaining descriptors as
methods; mirror the instance-path builtin-function exclusion by checking for
builtin functions here (e.g., call pyre_object::is_builtin_function(d)) before
the Some(_) => obj branch so builtin functions are not bound as methods; update
the match in the same block used by load_method/CALL to return PY_NULL (or obj
as appropriate) when the descriptor is a builtin-function, using the same
classification function the is_instance path uses.

In `@pyre/pyre-interpreter/src/module/_collections/mod.rs`:
- Around line 105-109: The deque_repeat implementation currently uses is_int(n)
and thus rejects objects implementing __index__; change it to accept indexable
objects by first attempting to obtain an integer via the Python number protocol
(e.g. call the equivalent of PyNumber_Index on n), returning NotImplemented if
that attempt yields NotImplemented or an error, then extract the integer value
from the resulting int object using w_int_get_value (as currently done), clamp
to >=0 (num.max(0)), and proceed; update references in deque_repeat to use that
index-conversion path instead of the is_int check so __index__-able types are
accepted while non-integer-like operands still produce NotImplemented.
- Around line 43-53: maxlen_bound currently unboxes __maxlen__ with
w_int_get_value trusting init-time validation; instead defend against attribute
mutation by first checking is_null/is_none, then if is_int use w_int_get_value
as before, otherwise call the existing getindex_w helper to coerce via __index__
(matching CPython) and convert that result to usize; ensure any conversion or
getindex_w failures are handled the same way the function does today (returning
None) and reference the symbols maxlen_bound, __maxlen__, is_none, is_int,
w_int_get_value, and getindex_w when making the change.
- Around line 340-346: The __imul__ implementation for W_Deque should not return
NotImplemented for non-int operands; instead accept objects supporting __index__
and raise TypeError on failure. In the W_Deque::__imul__ function, replace the
unsafe is_int/w_int_get_value checks with a call to the interpreter’s index
conversion (use the existing PyNumber_Index/try_index helper or equivalent) to
obtain an integer index; if that conversion errors or returns non-integer,
propagate/raise a TypeError with an explanatory message; then use the resulting
integer (handling overflow/negative semantics as before) to perform the in-place
repeat logic. Ensure you reference the W_Deque::__imul__ (W_Deque.imul) function
and use the existing index conversion helper rather than returning
pyre_object::w_not_implemented().

In `@pyre/pyre-interpreter/src/objspace/descroperation.rs`:
- Around line 1015-1019: If binop_dispatch_first(...) is true and
try_dispatch_binary_special(...) returns None (meaning a user override existed
and returned NotImplemented), short-circuit and do not fall through into builtin
fast paths—propagate/return the NotImplemented result immediately instead of
continuing to int/long/float/sequence fast-path logic; update the blocks around
binop_dispatch_first + try_dispatch_binary_special (e.g., the __add__/__radd__
case and the other listed locations) to treat None as the override declining the
operation and return that NotImplemented outcome to the caller.

In `@pyre/pyre-interpreter/src/typedef.rs`:
- Around line 1192-1215: In tuple_descr_new, validate that cls is actually a
user subclass of tuple before copying and tagging the new tuple: after
determining tuple_typeobj and before the "Subclass path" block, call the
existing check_user_subclass (or equivalent routine used elsewhere) with cls and
the tuple type object and if it returns false return a TypeError instead of
proceeding; this prevents arbitrary types (e.g. dict) from being assigned to
w_class when creating the new tuple in tuple_descr_new.
- Around line 4543-4556: The mro builtin reads args[0] unsafely and calls
pyre_object::w_type_get_mro without verifying arity or that the receiver is
actually a type, which can panic or read invalid memory; update the closure
passed to make_builtin_function("mro", ...) to first check args.len() > 0 and
validate the receiver is a type using the existing type-check helper (e.g., the
project's is_type/w_type_check helper or equivalent) before calling
pyre_object::w_type_get_mro, and if the check fails return a TypeError (using
the project's error/exception construction path) instead of doing the unsafe
read; keep the subsequent logic that builds the list with
pyre_object::w_list_new and stores it with dict_storage_store unchanged.

In `@pyre/pyre-jit/src/jit/flatten.rs`:
- Around line 1371-1374: The code currently only emits the post-call Insn::live
when self.lowering_ctx.is_some(); change this so post-call live markers are
emitted whenever insn_needs_trailing_live(&insn) is true regardless of
lowering_ctx. Concretely, update the trailing_live computation in the block that
defines trailing_live (remove the is_some() check) and keep the subsequent
self.emitline(Insn::live(Vec::new())) branch so residual_call_* and
inline_call_* streams handled by flatten_space_operation() still get their
trailing live markers; keep using insn_needs_trailing_live, emitline, and
Insn::live as the referenced symbols.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 655ea195-2a16-4220-846b-5c7ad31b63d9

📥 Commits

Reviewing files that changed from the base of the PR and between c28931b and 25f9a70.

📒 Files selected for processing (21)

• majit/majit-metainterp/src/jitcode/assembler.rs
• majit/majit-metainterp/src/optimizeopt/schedule.rs
• pyre/bench/list_reverse.py
• pyre/pyre-interpreter/src/baseobjspace.rs
• pyre/pyre-interpreter/src/builtins.rs
• pyre/pyre-interpreter/src/call.rs
• pyre/pyre-interpreter/src/display.rs
• pyre/pyre-interpreter/src/eval.rs
• pyre/pyre-interpreter/src/jit_fnaddr.rs
• pyre/pyre-interpreter/src/module/_collections/mod.rs
• pyre/pyre-interpreter/src/module/operator/app_operator.py
• pyre/pyre-interpreter/src/module/operator/mod.rs
• pyre/pyre-interpreter/src/objspace/descroperation.rs
• pyre/pyre-interpreter/src/objspace/std/mapdict.rs
• pyre/pyre-interpreter/src/pyframe.rs
• pyre/pyre-interpreter/src/typedef.rs
• pyre/pyre-jit/src/jit/assembler.rs
• pyre/pyre-jit/src/jit/codewriter.rs
• pyre/pyre-jit/src/jit/flatten.rs
• pyre/pyre-jit/src/jit/simplify.rs
• pyre/pyre-macros/src/lib.rs

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve rich-compare fallback semantics after overridden methods return NotImplemented.

try_compare_override() already proved that at least one user-level comparison override exists. If that dispatch returns None, continuing into the numeric fast paths turns an explicit decline into plain value comparison; a builtin-leaf subclass whose __lt__ / __gt__ both return NotImplemented will currently compare like a bare int instead of falling through to the generic comparison path.

Also applies to: 2058-2069, 2070-2100

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/objspace/descroperation.rs` around lines 2016 -
2049, The compare override flow currently treats a method that returns the
NotImplemented singleton as "no override" and falls through to numeric fast
paths; change the logic so an actual method call that returns NotImplemented is
preserved and propagated as an explicit result. Concretely, in
try_compare_override (and the analogous blocks around lookup_type_special /
try_call_special), ensure that try_call_special does not map a NotImplemented
return into None — either update try_call_special to return Some(NotImplemented)
when the callee returned NotImplemented, or modify try_compare_override to treat
a returned PyObjectRef equal to the NotImplemented singleton as Some(result) and
immediately return Ok(Some(result)) (using operand_overrides,
lookup_type_special, should_try_reverse_first to locate the same call sites).
This preserves the intended fallback semantics where an explicit NotImplemented
from an override prevents falling into numeric fast paths.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't turn invalid dict deletions into no-ops.

If args[0] is a non-dict instance, or a dict subclass whose __dict_data__ is missing/non-dict, this now returns None without deleting anything or raising. That masks bad receivers and silently drops real deletions.

🛠️ Minimal fix

             |args| {
                 if args.len() < 2 {
                     return Err(crate::PyError::type_error("__delitem__ requires 2 args"));
                 }
-                // For plain dict: direct delete. For dict subclass instance: use backing dict.
-                unsafe {
-                    if pyre_object::is_dict(args[0]) {
-                        crate::baseobjspace::delitem(args[0], args[1])?;
-                    } else if pyre_object::is_instance(args[0]) {
-                        // dict subclass — delete from __dict_data__ backing dict
-                        if let Ok(backing) = crate::baseobjspace::getattr(args[0], "__dict_data__")
-                        {
-                            if pyre_object::is_dict(backing) {
-                                crate::baseobjspace::delitem(backing, args[1])?;
-                            }
-                        }
-                    }
+                let recv = args[0];
+                let dict = crate::type_methods::resolve_dict_backing(recv);
+                if dict.is_null() {
+                    let tp_name = unsafe { (*(*recv).ob_type).name };
+                    return Err(crate::PyError::type_error(format!(
+                        "descriptor '__delitem__' for 'dict' objects doesn't apply to a '{tp_name}' object"
+                    )));
                 }
+                crate::baseobjspace::delitem(dict, args[1])?;
                 Ok(pyre_object::w_none())
             },

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-interpreter/src/typedef.rs` around lines 2356 - 2369, The current
deletion code silently no-ops when args[0] is neither a dict nor a dict-subclass
with a valid "__dict_data__" backing, which masks invalid receiver errors;
update the branch handling in the block that uses pyre_object::is_dict and
pyre_object::is_instance so that if args[0] is not a plain dict and either is
not an instance or getattr(args[0], "__dict_data__") is missing/non-dict, you
return a TypeError instead of silently returning None; use the same
error-construction helper/pattern used elsewhere in this module to raise a
TypeError with a clear message (e.g. "descriptor delete for non-dict or missing
__dict_data__") and keep the existing behavior of calling
crate::baseobjspace::delitem when a dict or valid backing dict is present.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Extend opcode context to other expect_* helpers for uniform diagnostics.

Only expect_reg includes the current opcode in its panic message. Other operand-shape helpers (expect_tlabel, expect_small_u16, expect_descr_vable_array_field, etc.) would benefit equally from this context when diagnosing SSARepr operand mismatches.

Suggested enhancement for consistency

Apply the same pattern to other expect_* helpers:

 fn expect_tlabel(op: &Operand) -> &TLabel {
     match op {
         Operand::TLabel(label) => label,
-        _ => panic!("expected TLabel, got {:?}", op),
+        _ => panic!(
+            "expected TLabel, got {:?} (op={})",
+            op,
+            current_dispatch_op()
+        ),
     }
 }

 fn expect_small_u16(op: &Operand) -> u16 {
     match op {
         Operand::ConstInt(value) => u16::try_from(*value).expect("expected u16-sized ConstInt"),
-        _ => panic!("expected ConstInt(u16), got {:?}", op),
+        _ => panic!(
+            "expected ConstInt(u16), got {:?} (op={})",
+            op,
+            current_dispatch_op()
+        ),
     }
 }

And similarly for expect_descr_vable_array_field, expect_descr_vable_array, expect_descr_vable_static_field, etc.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-jit/src/jit/assembler.rs` around lines 1646 - 1655, The panic in
expect_reg already includes current_dispatch_op(), but other helper functions
(e.g., expect_tlabel, expect_small_u16, expect_descr_vable_array_field,
expect_descr_vable_array, expect_descr_vable_static_field, etc.) do not; update
each expect_* helper to include the current opcode context by appending
current_dispatch_op() (or a similar descriptive call used in expect_reg) to
their panic messages so all operand-shape mismatch errors report the dispatch
opcode for consistent diagnostics.