-
Notifications
You must be signed in to change notification settings - Fork 647
Add support for the cmp
builtin (#54) (#63)
#80
Conversation
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.
Do the existing LE, LT, Eq, etc. functions need to be rewritten to make use of this? I'm fine doing that in a subsequent PR, but I just want to make sure I understand exactly how this all fits together.
// | ||
// It closely resembles the behavior of CPython's do_cmp in object.c. | ||
func compare(f *Frame, v, w *Object) (*Object, *BaseException) { | ||
cmp := v.typ.slots.Cmp |
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.
Can we just move this logic directly into 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.
Will do.
@@ -101,3 +101,133 @@ def __call__(self, *args, **kwargs): | |||
assert callable(bar) | |||
assert callable(bar()) | |||
|
|||
# cmp(x) |
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.
Thanks for the thorough tests!
Implement the `cmp` as defined here: * https://docs.python.org/2/library/functions.html#cmp Implementing this builtin required implementing a fair amount of supporting logic to do 3-way comparisons. The 3-way comparison implementation in CPython is somewhat complex, thus there may be some cases that still need support. This is a good start, though.
The rich compare implementation will need to be tweak to fall-back on 3-way compare if the appropriate rich comparison operation is not available. I will do that in a separate PR. |
I made the one change you requested and will commit to adding the fallback to 3-way compare for rich comparisons in a follow-on PR. Everything else look okay? |
// halfCompare tries a comparison with the __cmp__ slot, ensures the result | ||
// is an integer, and returns it. It closely resembles the behavior of CPython's | ||
// half_compare in typeobject.c. | ||
func halfCompare(f *Frame, v, w *Object) (*Object, *BaseException) { |
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.
In a subsequent PR, would you mind changing the return value to (*Int, *BaseException)? You pass the result to intNeg() later on in the code and it would be easier to understand if it was an *Int.
Thanks again for all your work on this! |
Fixes `from time import *; print sleep`
Implement the
cmp
as defined here:Implementing this builtin required implementing a fair
amount of supporting logic to do 3-way comparisons. The
3-way comparison implementation in CPython is somewhat
complex, thus there may be some cases that still need
support. This is a good start, though.