Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upReplace IteratorExt::zip with tuple iteration #870
Conversation
and others
added some commits
Feb 16, 2015
This comment has been minimized.
This comment has been minimized.
|
Is this not a bit of a regression since you can theoretically indefinitely Zip, whereas with this you'll run into Tuple impl size limits quickly? Otherwise my only objection to this is the vague "Rust is becoming more magic" problem (that I am incredibly guilty of exacerbating). But maybe this is more intuitive that there being a method called Zip? |
This comment has been minimized.
This comment has been minimized.
|
If that would help my case for this RFC, I could collect a list of cries for help on stackoverflow of people trying to zip in several languages without knowing the keyword zip. |
This comment has been minimized.
This comment has been minimized.
|
Also your comment above:
Doesn't really jive with your statement in the forum:
|
This comment has been minimized.
This comment has been minimized.
|
@Gankro regarding the regression, I'm not sure I follow. Isn't todays zip just equivalent to a 2-tuple here? In particular, can't you nest tuples to achieve arbitrary iteration? for (x, (y, z)) in (xs, (ys, zs)) { ... }Or am I missing something? |
This comment has been minimized.
This comment has been minimized.
|
(I do think that as far as "guessability" goes, zip is probably more guessable than tuples, since it is used in so many languages.) |
This comment has been minimized.
This comment has been minimized.
|
I have mixed feelings. I feel like this approach is beautifully concise, but it may be a learning hazard. On the other hand, that's true for a lot of iterator magic (e.g., collecting into a (That said, for some reason now that |
This comment has been minimized.
This comment has been minimized.
pczarn
commented
Feb 16, 2015
|
Perhaps I would like to have |
This comment has been minimized.
This comment has been minimized.
|
@nikomatsakis. Ah yes, you're right. Didn't occur to me that that would "just work". |
This comment has been minimized.
This comment has been minimized.
|
i could live with |
This comment has been minimized.
This comment has been minimized.
|
What if someone implements IntoIter for arbitrary same-size tuples? I think it would not necessariily be ambiguous (because of the match on (x, y, z)), but it could throw off fellow developers who may be unsure what the code is doing. |
This comment has been minimized.
This comment has been minimized.
|
@llogiq: i guess that's similar to someone implementing IntoIter for [T] or something. The compiler would tell you this exists already. |
This comment has been minimized.
This comment has been minimized.
|
I think, bad searchability is a relatively weak argument against this proposal. |
This comment has been minimized.
This comment has been minimized.
|
Could this be done with polymorphic impls, or does it need to hefty syntactic support? |
This comment has been minimized.
This comment has been minimized.
|
@Ericson2314 well we need some macro magic for tuples with 1 to n elements. n probably being something around 10 |
This comment has been minimized.
This comment has been minimized.
|
I would have assumed this would work: impl<I: IntoIter, J: IntoIter> impl IntoIter for (I, J) {
type IntoIter= (<I as IntoIter>::IntoIter, <J as IntoIter>::IntoIter);
...
}
impl<I: Iter, J: Iter> impl Iter for (I, J) {
type Item = (<I as IntoIter>::Item, <J as IntoIter>::Item);
...
}and so on for larger tuples. |
This comment has been minimized.
This comment has been minimized.
|
yes, that will be generated by macros, as to not repeat ourselves. I have a sample implementation in the rfc. it's rather blunt, but if the rfc is accepted i will actually put in an effort to make it nice |
This comment has been minimized.
This comment has been minimized.
|
Ok, just checking. I thought people were implying the feature itself would require compiler syntactic support, when really it's just that its implementation would benefit from it, but doesn't even need it. I'm more for this then. Sure its a bit spooky but isn't proposing anything that couldn't be done already (baring coherence). |
This comment has been minimized.
This comment has been minimized.
|
I've come around to this. Let's lean into the magic together, everyone |
This comment has been minimized.
This comment has been minimized.
|
Cool consequence when extend is upgraded to use IntoIterator:
|
This comment has been minimized.
This comment has been minimized.
|
I added another alternative: requiring |
This comment has been minimized.
This comment has been minimized.
tikue
commented
Feb 28, 2015
|
Ah...this is gorgeous! It's so obvious that I'm surprised it hasn't been brought up sooner. |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
This is cute. I agree fully with @nikomatsakis though. I feel pretty hesitant about this. |
This comment has been minimized.
This comment has been minimized.
vwim
commented
Feb 28, 2015
|
Looks nice but I think this should be solved in a more structural way instead of applying magic. |
This comment has been minimized.
This comment has been minimized.
|
I like how intuitive this notation is. How do you iterate through a list of elements?
How do you iterate through two lists simultaneously?
, but with some parentheses required. |
This comment has been minimized.
This comment has been minimized.
|
While this is a clever idea with some promise, it is also very much a nice-to-have, and I'm strongly inclined to exercise discretion in adding new features to Rust at this time. I've opened #930 to let this idea continue to bake, but am closing this PR. Thank you. |
oli-obk commentedFeb 16, 2015
Should iterate over
a,bandc(if all of them implementIntoIterator) and return the current element of each of them inx,y, andzuntil any ofa,borcreturn None.internals.rust-lang.org discussion
Rendered