objspace: pypy-parity for operator errors, __index__ coercion, 3-arg pow - #552
Conversation
Operator error paths derived the operand type name from (*ll_type(x)).name, which is 'object' for user-defined classes, and used cpython-style unary/abs wording that pyre does not target. - Binary (+, -, *, /, //, %, <<, >>, &, |, ^) and comparison error messages take the operand name from object_functionstr_type_name (the Python __class__ name), so a user class reads 'Foo' not 'object'. - The ** operator (and 2-arg pow) message reads 'unsupported operand type(s) for ** or pow():'. - Unary +/-/~ and abs on an unsupported operand read 'unsupported operand type for unary pos|neg|~|abs:' (_make_unaryop_impl) instead of cpython's 'bad operand type'. Add synth/operator_error_typename regression test (binop/comparison/** are cpython==pypy; unary/abs match pypy but diverge from cpython, so are verified out of band). Assisted-by: Claude
set.remove(x) on a missing element raised KeyError with the fixed string "set.remove(x): x not in set"; it now raises key_error_with_key(x) so the KeyError carries x itself (str and args match dict[missing]). ord() on a non-string argument reported "but other type found"; it now names the argument's type via object_functionstr_type_name (e.g. "but int found"). Add synth/set_remove_ord_errors regression test. Assisted-by: Claude
getitem_list/setitem_list/delitem_slot and list.insert/list.pop run a non-int, non-slice index through __index__ (getindex_w) instead of rejecting or ignoring it: - getitem_list and setitem_list inline the coercion (is_int fast path plus the __index__ slow path) so the hot integer subscript keeps a concrete Int repr in the rtyper; delitem and insert/pop route through the subscript_index_w / getindex_w_index helpers on their cold paths. - delitem no longer lets a non-int key fall through to the generic __delitem__ slot (bound to delitem_slot), which recursed into itself and raised RecursionError; it coerces or raises here. - an index too large for a machine word raises "cannot fit '<type>' into an index-sized integer" (IndexError for subscript, OverflowError for insert/pop); a non-index, non-slice key raises "<descr> indices must be integers or slices, not '<type>'" naming the key's real class. - setitem/delitem out-of-range raise IndexError "list index out of range". subscript_index_w generalizes the former bytearray_index over the descr. Assisted-by: Claude
getitem_tuple, getitem_str, and getitem_bytes_like coerce a non-int,
non-slice key through __index__ (getindex_w), mirroring getitem_list: an
inline is_int fast path keeps the hot integer subscript's Int repr concrete
in the rtyper, an overflowing index raises IndexError "cannot fit '<type>'
into an index-sized integer", and a non-index key raises "<descr> indices
must be integers or slices, not '<type>'".
getitem_str routes its non-index TypeError through index_type_error, adding
the "or slices" clause it was missing ("string indices must be integers or
slices, not '<type>'"). A bytes index-out-of-range now reads "byte index out
of range".
Assisted-by: Claude
Three-argument pow(base, exp, mod) now consults only the forward __pow__ on the base (descroperation.py:459) and raises the three-operand "unsupported operand type(s) for pow(): T, T, T" TypeError when it returns NotImplemented; the reflected __rpow__ is no longer threaded through the modulus, so pow3 mirrors PyPy rather than CPython here. The integer modular-power fast path moves out of pow3 into int.__pow__, so a base whose type overrides __pow__ is honoured for three-arg power (pow(MyInt(2), 3, 5) calls the override) instead of being shadowed by the fast path. int.__pow__ returns NotImplemented for a non-integer modulus and computes the modular power directly rather than re-entering pow3, fixing an infinite recursion that crashed pow(2, 10, 100.0) and pow(2, 10, 100j) with RecursionError. A float base (or float __rpow__) rejects a non-None modulus with TypeError "pow() 3rd argument not allowed unless all arguments are integers" (floatobject.py:588), and a complex base with ValueError "complex modulo" (complexobject.py:525), instead of silently ignoring the modulus. The now-unused try_dispatch_ternary_special and its should_try_reverse_first helper are removed, and binary/ternary operand-type-error formatting shares an operand_type_name helper. The proxy 3-arg test is updated to expect the forward-only behavior. synth pow3_arg_types covers the cpython==pypy cases. Assisted-by: Claude
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (12)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/00eccce001dcb54b7603b49f2e4be2b7e7143192/pyre-interpreter/src/baseobjspace.rs#L1291
When an object defines __index__ but that method returns a non-int, this propagates space_index's TypeError (for example __index__ returned non-int...) instead of applying the getindex_w(index, IndexError, "list") behavior cited above. In PyPy, pypy/interpreter/baseobjspace.py:1567-1579 catches TypeError from space.index() whenever objdescr is set and re-raises the sequence-specific list indices must be integers or slices... error; the same inlined pattern is copied to tuple/str/bytes and list assignment, so bad custom index objects now diverge on all of those subscript paths.
ℹ️ 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".
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 198f196). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patch
3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
1 participant
Five interpreter object-space slices bringing error/coercion behavior in line
with PyPy (
rpython//pypy/upstream). Each is independently verified againstpypy3and, where cpython==pypy, guarded by a synthetic regression test.Slices
Operator TypeError messages: name real class, pypy text (
b15f509af26)Binary (
+ - * / // % << >> & | ^) and comparison error messages take theoperand name from
object_functionstr_type_name(the__class__name) insteadof
(*ll_type(x)).name— a user class now reads'Foo', not'object'. The**operator / 2-argpowmessage readsfor ** or pow():, and unary+ - ~/abson an unsupported operand readunsupported operand type for unary pos|neg|~|abs:(_make_unaryop_impl). Synthoperator_error_typenameguards the binop/comparison/
**cases (cpython==pypy); unary/abs match pypy butdiverge from cpython, so are verified out of band.
set.remove KeyError carries key; ord() names arg type (
0d8e2973b0d)set.remove(x)on a missing element now raiseskey_error_with_key(x)(theKeyError carries
x, matchingdict[missing]) instead of a fixed string.ord()on a non-string names the argument's type viaobject_functionstr_type_name(e.g.but int found). Synthset_remove_ord_errors.list: coerce subscript/insert/pop index through
__index__(c2e91f3e481)getitem_list/setitem_list/delitem_slotandlist.insert/list.poprun anon-int, non-slice index through
__index__(getindex_w). Hot get/set inlinethe coercion (is_int fast path +
__index__slow path) to keep a concrete Intrepr in the rtyper; cold del/insert/pop route through
subscript_index_w/getindex_w_index. Fixes a RecursionError where a non-intdel list[k]fell through to the generic__delitem__slot and recursed.Overflow raises
cannot fit '<type>' into an index-sized integer; a non-indexkey raises
<descr> indices must be integers or slices, not '<type>'; out-of-rangeraises
list index out of range. Synthlist_subscript_index,list_insert_pop_index.seq: coerce tuple/str/bytes subscript index through
__index__(3b08028f82f)getitem_tuple/getitem_str/getitem_bytes_likecoerce a non-int, non-slicekey through
__index__, mirroringgetitem_list.getitem_strroutes itsnon-index TypeError through
index_type_error, adding the missingor slicesclause; a bytes index-out-of-range reads
byte index out of range. Synthtuple_str_bytes_subscript_index.pow: 3-arg power tries only forward
__pow__, not__rpow__(00eccce001d)3-arg
pow(base, exp, mod)consults only the forward__pow__on the base(
descroperation.py:459) and raises the three-operandunsupported operand type(s) for pow(): T, T, Ton NotImplemented — matching PyPy, not cpython. Theinteger modular-power fast path moves from
pow3intoint.__pow__, so a basewhose type overrides
__pow__is honoured (pow(MyInt(2), 3, 5)calls theoverride).
int.__pow__returns NotImplemented for a non-integer modulus andcomputes the modular power directly rather than re-entering
pow3, fixing aninfinite recursion that crashed
pow(2, 10, 100.0)/pow(2, 10, 100j)withRecursionError. A float base rejects a non-None modulus with TypeError
pow() 3rd argument not allowed unless all arguments are integers, a complexbase with ValueError
complex modulo. Removes the now-unusedtry_dispatch_ternary_special/should_try_reverse_first. Synthpow3_arg_types.Verification
pyre==pypymatrices for each slice; pow 11/11 + subscript value/oob/big cases.python ./pyre/check.py: cranelift 187/187, wasm 186/186 green; the soledynasm miss is the pre-existing
nested_loopperf-boundary flake (outputbyte-identical to pypy, passes in isolation, uses none of this code).
🤖 Generated with Claude Code