-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[mypyc] Optimize list.__add__, list.__iadd__, tuple.__add__ #18845
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.
Thanks for the PR! Left a few minor comments, otherwise looks good.
@@ -21,6 +21,7 @@ Operators | |||
|
|||
* ``tup[n]`` (integer index) | |||
* ``tup[n:m]``, ``tup[n:]``, ``tup[:m]`` (slicing) | |||
* ``tup1 + tup2`` |
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.
Is this only specialized for variable-length tuples? If yes, it's worth adding a note here, since fixed-length tuple concatenation could be quite slow otherwise.
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.
It should work for fixed-length tuple as well. Just the whole box / unbox dance which could possibly be optimized further at a later point.
assert [1, 2] + [3, 4] == res | ||
with assertRaises(TypeError, 'can only concatenate list (not "tuple") to list'): | ||
assert [1, 2] + (3, 4) == res # type: ignore[operator] | ||
assert in_place_add([3, 4]) == res |
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.
Test that the identity of the target object is preserved in +=
?
https://docs.python.org/3/c-api/sequence.html#c.PySequence_Concat
https://docs.python.org/3/c-api/sequence.html#c.PySequence_InPlaceConcat