KEMBAR78
[mypyc] Optimize list.__add__, list.__iadd__, tuple.__add__ by cdce8p · Pull Request #18845 · python/mypy · GitHub
Skip to content

Conversation

cdce8p
Copy link
Collaborator

@cdce8p cdce8p commented Mar 27, 2025

@cdce8p cdce8p added the topic-mypyc mypyc bugs label Mar 27, 2025
Copy link
Collaborator

@JukkaL JukkaL left a 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.


* ``tup[n]`` (integer index)
* ``tup[n:m]``, ``tup[n:]``, ``tup[:m]`` (slicing)
* ``tup1 + tup2``
Copy link
Collaborator

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.

Copy link
Collaborator Author

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
Copy link
Collaborator

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 +=?

@cdce8p cdce8p requested a review from JukkaL March 28, 2025 15:06
@JukkaL JukkaL merged commit 12aa642 into python:master Mar 31, 2025
13 checks passed
@cdce8p cdce8p deleted the mypyc-sequence-concat branch March 31, 2025 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic-mypyc mypyc bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants