impl Default for array::IntoIter

[fee1-dead]

cc #91583

my personal use of this feature comes from https://github.com/fee1-dead/w/blob/092db5df631ea515b688bae99c7f02eef12d7221/src/cont.rs#L154-L170

insta-stable, but I feel like this is small enough to not require an ACP (but a FCP per https://forge.rust-lang.org/libs/maintaining-std.html#when-theres-new-trait-impls)? feel free to correct me if I am wrong.

[rustbot]

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[ChrisDenton]

@rust-lang/libs-api

[jhpratt]

ACP is primarily to avoid unnecessary implementation effort, which is trivial here. I don't see any issue with going straight to FCP for this. Nominating for team discussion.

[BoxyUwU]

It feels somewhat surprising to me that the default value of an array iterator would be empty instead of N default values of T. Though every other collection::IntoIter: Default impl in std just makes an empty iterator so this arguably is the consistent choice.

[dtolnay]

@rfcbot fcp merge

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[BurntSushi]

This "feels" slightly odd to me. From what I can tell, this Default impl for array::IntoIter essentially exposes the currently unstable array::IntoIter::empty constructor. AFAIK, there is no other way to get an empty iterator for arbitrary const N.

I think the thing that makes me feel uneasy about this is that this makes it so array::IntoIter<T, N> might yield fewer than N elements. But maybe there is already a stable way to do that that I'm missing?

I'm not necessarily advocating for returning N elements via T::default() (although, if we add this impl as-is, we will never be able to do that AIUI) as I think that's also somewhat strange too.

[fee1-dead]

makes it so array::IntoIter<T, N> might yield fewer than N elements

I mean... You can already do the following on stable:

fn empty<T: Default + Copy, const N: usize>() -> std::array::IntoIter<T, N> {
    let mut iter = [T::default(); N].into_iter();
    iter.by_ref().for_each(|_| {});
    iter
}

[BurntSushi]

Yeah I guess that renders my concern there moot. That was my poor attempt at articulating my feeling here. It doesn't get rid of my sense of this being a bit weird to me. I guess I'm just echoing @BoxyUwU here, but I'm not convinced that adding the impl at all is worth it.

I don't feel strongly enough to render an objection though.

[fee1-dead]

Well sure, there is a bit of a difference here:

for Vec<T>, Vec::default and vec::IntoIter::default are the same things.

for Option<T>, Option::default and option::IntoIter::default are the same things.

Here <[T; N]>::default and array::IntoIter::default are different.

But array::IntoIter is also different in that it has a dynamic size whereas an array doesn't. When you do a mem::take on [T; N] you zero-fill it (fill it with the default value), but on an iterator it might make slightly more sense to have an empty iterator?

I can see arguments on both sides but I felt that an empty iterator was stronger. Ability to use mem::take instead of having to deal with unsafe shenanigans on an IntoIter was the primary use case for me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[the8472]

In the original ACP adding Default impls for iterators I intentionally skipped this one because the meaning is ambiguous.

[camsteffen]

Interesting. Maybe another way to think of it is that array::IntoIter is semantically more similar to a Vec than an array. In that lens, empty by default makes sense.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[jhpratt]

Reading through this PR, it looks like there aren't any blocking concerns (as @BurntSushi explicitly opted not to block). Merging as FCP accepted.

@bors r+ rollup

[bors]

📌 Commit 62435f9 has been approved by jhpratt

It is now in the queue for this repository.