_abc, _typing and update typing,test_types from 3.14.2#6797
Conversation
Sorry, something went wrong.
📝 WalkthroughWalkthroughThis PR standardizes binary operator hooks to return PyResult, reworks PyUnion to track hashable vs unhashable components with dedup/flatten logic, adds a new stdlib Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant make_union as make_union()
participant dedup as dedup_and_flatten_args()
participant PyUnion as PyUnion
Caller->>make_union: call with args (PyTuple)
make_union->>dedup: dedup_and_flatten_args(args)
dedup->>dedup: flatten strings -> ForwardRef, separate hashable/unhashable, dedupe
dedup-->>make_union: UnionComponents {args, hashable_args?, unhashable_args?}
alt single arg
make_union-->>Caller: return underlying arg (unwrapped)
else multiple args
make_union->>PyUnion: from_components(UnionComponents)
PyUnion-->>make_union: PyUnion instance (PyResult)
make_union-->>Caller: PyUnion (PyResult)
end
sequenceDiagram
participant Code
participant ABCMethods as _abc methods
participant Cache as Registry/Cache
participant Counter as InvalidationCounter
Code->>ABCMethods: _abc_subclasscheck(cls, subclass)
ABCMethods->>Cache: check positive cache
alt cached hit
Cache-->>ABCMethods: result
else cache miss
ABCMethods->>Cache: check negative_cache version
alt negative cache stale
ABCMethods->>Counter: increment token
ABCMethods->>Cache: clear negative cache
end
ABCMethods->>ABCMethods: run subclass hooks / direct checks / recursive registry traversal
ABCMethods->>Cache: update positive or negative cache
end
ABCMethods-->>Code: boolean result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Sorry, something went wrong.
|
Code has been automatically formatted The code in this PR has been formatted using:
git pull origin abc |
Sorry, something went wrong.
youknowone
force-pushed
the
abc
branch
3 times, most recently
from
6df2b89 to
e8077f2
Compare
January 19, 2026 15:39
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/vm/src/builtins/union.rs`:
- Around line 248-332: The dedup_and_flatten_args function currently treats any
error from arg.hash(vm) as "unhashable", masking real exceptions; change the
Err(_) arm to capture the exception (Err(e)) and propagate it unless it's a
TypeError, in which case proceed with unhashable handling. Concretely, in
dedup_and_flatten_args replace the match on arg.hash(vm) so that if Err(e) you
check the exception type (compare against vm.ctx.exceptions.type_error or use
vm.is_instance) and return Err(e) for non-TypeError, otherwise handle as now by
doing equality comparisons and adding to unhashable_list.
🧹 Nitpick comments (2)
crates/vm/src/stdlib/_abc.rs (1)
341-350: Avoid calling into the VM while holdingAbcDatalocks.Drop the lock guard before invoking
set.clear()to reduce reentrancy/deadlock risk; apply the same pattern in_reset_registryand_reset_caches.♻️ Suggested refactor pattern (apply similarly in reset helpers)
- let negative_cache = impl_data.negative_cache.read(); - if let Some(ref set) = *negative_cache { - vm.call_method(set.as_ref(), "clear", ())?; - } - drop(negative_cache); + let negative_cache = impl_data.negative_cache.read(); + let set = negative_cache.as_ref().cloned(); + drop(negative_cache); + if let Some(set) = set { + vm.call_method(set.as_ref(), "clear", ())?; + } impl_data.set_cache_version(invalidation_counter);Also applies to: 453-477
crates/vm/src/builtins/union.rs (1)
389-441: Optional: fast equality path when all args are hashable.If both unions have
unhashable_args == None, consider comparinghashable_args(frozensets) directly to avoid the O(n²) nested loops.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/vm/src/builtins/namespace.rs`:
- Around line 46-69: The __replace__ implementation currently calls cls.call((),
vm) then mutates the new instance's __dict__; instead, build a mapping from the
current namespace (src_dict) and call the constructor with that mapping as the
single positional argument and the replacement kwargs (args.kwargs) as **kwargs
so subclasses' __init__ runs normally. Concretely, stop creating result then
copying src_dict and calling set_attr; instead prepare a positional tuple
containing the namespace mapping and pass args.kwargs into cls.call(...) so
cls.call receives (mapping,) and the kwargs, preserving existing checks for
positional args and using the same vm/context objects (referencing __replace__,
cls.call, zelf, src_dict, and args.kwargs).
♻️ Duplicate comments (1)
crates/vm/src/builtins/union.rs (1)
290-307: Hash error handling catches all exceptions.The
Err(_)pattern treats any hash exception as "unhashable," including non-TypeErrorexceptions likeValueError. While this could mask real errors from faulty__hash__implementations, I note that a similar concern was raised previously and the maintainer indicated the current behavior is intentional.
🧹 Nitpick comments (1)
crates/vm/src/builtins/union.rs (1)
458-492: Minor: Potentially misleading error message when previously-unhashable args become hashable.Lines 470-474 raise
"union contains N unhashable element(s)"even when all previously-unhashable args have since become hashable (passedarg.hash(vm)?at line 466). This maintains consistent hashing semantics (objects that were unhashable at creation time remain treated as such), but the error message could be confusing since the elements are no longer actually unhashable.Consider updating the message to something like:
"union was created with unhashable element(s)"to clarify the semantic.Suggested message improvement
- return Err(vm.new_type_error(format!( - "union contains {} unhashable element{}", - n, - if n > 1 { "s" } else { "" } - ))); + return Err(vm.new_type_error(format!( + "union was created with {} unhashable element{}", + n, + if n > 1 { "s" } else { "" } + )));
Sorry, something went wrong.
youknowone
merged commit 6f41a94
into
RustPython:main
Jan 20, 2026
The last commit is generated by #6796
python3 scripts/update_lib quick typingSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.