-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add weakproxy richcompare #3909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
vm/src/builtins/weakproxy.rs
Outdated
op: PyComparisonOp, | ||
vm: &VirtualMachine, | ||
) -> PyResult<PyComparisonValue> { | ||
println!("richcompare"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
println!("richcompare"); |
vm/src/builtins/weakproxy.rs
Outdated
vm: &VirtualMachine, | ||
) -> PyResult<PyComparisonValue> { | ||
println!("richcompare"); | ||
op.eq_only(|| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does weakproxy only support eq operation?
What happens if the proxy object is int
and used for >
operation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked cpython source code and it simply calls PyObject_RichCompare
, which is PyObjectRef::rich_compare
in RustPython.
vm/src/builtins/weakproxy.rs
Outdated
vm: &VirtualMachine, | ||
) -> PyResult<PyComparisonValue> { | ||
println!("richcompare"); | ||
op.eq_only(|| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked cpython source code and it simply calls PyObject_RichCompare
, which is PyObjectRef::rich_compare
in RustPython.
721cc74
to
a212d84
Compare
Co-Authored-By: Hyunmin Shin <shm1193@gmail.com>
a212d84
to
5c08546
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, thank you!
No description provided.