-
Notifications
You must be signed in to change notification settings - Fork 770
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
Add IntoIterator for &Bound types #3923
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.
This looks useful to me, maybe it would make sense to also do this for the other iterables (PyTuple, Py(Frozen)Set and PyDict come to mind)?
Thanks, I think these do make sense, I since remembered that I hit this a fair bit in Some history on why these didn't yet exist is in #3695 (comment) and #3694 (comment) - basically I think @adamreichold and I had held back on adding these previously because I'd forgotten the use case at the time :) I think the only question I have left remaining is what to do about |
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 think let's merge this as-is. I think the only possible design question is whether we would want to have extra iterator types that contain &Bound
instead of Bound
.
The only advantage of the extra types would be to be able to avoid one reference count operation, and iteration necessarily is N reference counting ops anyway. Due to this I think it's not worth bothering with the extra complexity of additional types.
This allows using common types like
&Bound<'py, PyList>
in afor
loop without explicitly callingiter()
.