Skip to content
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

Implement IntoIterator for tuples with values which are IntoIterator themselves #513

Closed
shahn opened this issue Dec 26, 2024 · 10 comments
Closed
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@shahn
Copy link

shahn commented Dec 26, 2024

Proposal

Problem statement

It is awkward to use Iterator::zip for tuples of larger arities than 2, as you need to chain calls to zip and iter() in the right places, as well as wrap your head around the correct nesting of the tuples. Implementing IntoIterator for tuples would make it very natural to call.

Motivating examples or use cases

Here's a small example of two ways to use the IntoIterator impl.

    let a_s = vec![1, 3, 5];
    let b_s = vec![2, 4, 6];
    for (a, b) in (&a_s, &b_s) {
        assert!(*a + 1 == *b);
    }
    let v = (a_s, b_s).into_iter().collect::<Vec<_>>();
    assert_eq!(v, vec![(1, 2), (3, 4), (5, 6)]);

itertools has also identified that nested tuples can be confusing, and added a convenience helper method (https://docs.rs/itertools/latest/itertools/fn.cons_tuples.html).

In addition, it would play nice with the upcoming stabilizations of Extend and FromIterator for tuples.

Solution sketch

The implementations are pretty straight forward. Specialization similar to what is done in Zip could be added, as well, but isn't required just to get the API out there.

Alternatives

Do nothing and keep using chained zip calls and nested tuples.

Links and related work

This was already discussed many years ago with the intent to replace zip: rust-lang/rfcs#870. An old RFC discussion was begun in rust-lang/rfcs#930.

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@shahn shahn added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Dec 26, 2024
@kennytm
Copy link
Member

kennytm commented Dec 27, 2024

The previous attempt in 2020 rust-lang/rust#78204 (comment) was closed because

We discussed this in the libs team meeting, and agreed that this trait implementation might lead to confusion situations where it's not directly clear whether this is zip, chain, iterating over tuple elements, or the cartesian product. A free zip function might be a good alternative.

I don't think the stance is changed here (the free function std::iter::zip also stabilized since 1.59).

@shahn
Copy link
Author

shahn commented Dec 27, 2024

Thanks for the context. std::iter::zip suffers from the same issue of requiring destructuring of tuples and nesting. Might an alternative be a zip function on tuples directly, e.g. let mut iter = (a_s, b_s, c_s).iter_zip() or something like that?

In addition, this would allow for iter_chain, iter_cons, each with the correct constraints

@joshtriplett
Copy link
Member

We discussed this in today's @rust-lang/libs-api meeting.

We had a long and vigorous discussion. :)

We were very sympathetic to this approach. We also had concerns about possible ambiguity. Note, in particular, that Python has different behavior here:

>>> for a, b in ([1, 3], [5, 7]): print(a)
... 
1
5
>>> for a, b in zip([1, 3], [5, 7]): print(a)
... 
1
3

Our current thinking is that we're inclined to not add these IntoIterator impls, and instead to eagerly await variadic generics and make zip use them to support an arbitrary number of arguments. And in the meantime, we'd encourage people to use itertools.

However, we're going to sleep on it and consider again in a future meeting.

@joshtriplett
Copy link
Member

joshtriplett commented Jan 7, 2025

Also, some of us in the meeting were amenable to adding std::iter::zip to the prelude.

@traviscross
Copy link

let xs = vec![1, 3, 5];
let ys = vec![2, 4, 6];
let zs = vec![3, 7, 11];
for (x, y, z) in (&xs, &ys, &zs) {
    assert!(*x + *y == *z);
}

For me, at least, this just seems, aesthetically, a really nice language outcome for Rust. It feels very natural in this case, and this case seems like it would be by far the predominant one.

My feeling is that there's no other plausible meaning for IntoIterator on tuples in Rust. We don't allow heterogeneous collections (unlike Python), and so there's world in which we would want this to mean chain rather than zip (@joshtriplett also brought this up in the meeting).

And so, since we'd never use this space for anything else, I do feel like we should seriously consider using the space for this, given the pleasingness of the outcome.

This has to be weighed, of course, against the fact that people would need to learn about this meaning of IntoIterator on tuples. Given the naturalness of the for case, though, it seems possible that the onramp to this would be smooth.

@joshtriplett
Copy link
Member

FWIW, I personally agree, but we didn't have consensus for that path.

@the8472
Copy link
Member

the8472 commented Jan 7, 2025

I agree that the for (x, y, z) in (&xs, &ys, &zs) case has no ambiguity and would be nice to have. But it's not the only way that iterators are used and the ambiguity in those counterweighs its usefulness.

In let v = (a_s, b_s).into_iter().collect::<Vec<_>>(); it's not clear what _ gets inferred to and one has to read the context to understand this.

Similarly for t in (&xs, &ys, &zs) makes it non-obvious what a loop is doing and one has to look at how t gets used (destructured/fed to a function) at some point.

Having an explicit zip improves readability at no loss of terseness for the collect case. I do agree that it is a tradeoff in the for in case.

@shahn
Copy link
Author

shahn commented Jan 7, 2025

Thanks a lot for the consideration and discussion!

One question with regard to variadic generics is, what would that look like? std::iter::zip() takes two arguments, not a tuple - does that mean for let tuple = (xs, ys, zs) I would need to do something like zip(tuple.0, tuple.1, tuple.2)? And for std::iterator::zip() would it similarly be a little bit awkward? Entirely possible I'm overlooking something, but that was the reason for thinking about methods on the tuple itself.

@programmerjake
Copy link
Member

well, with rust-lang/rfcs#3723, you can just write zip(...tuple) which seems good enough for me

@Amanieu
Copy link
Member

Amanieu commented Jan 14, 2025

Since we couldn't get a team consensus on this, we're closing this ACP. We may re-open this in the future if variadic generics end up not happening for some reason.

@Amanieu Amanieu closed this as completed Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

7 participants