Tracking issue for by-value array iterator (feature array_value_iter)

[LukasKalbertodt]

This is a tracking issue for core::array::IntoIter. Implemented in #62959.

Blocked by:

• Const generics (Tracking issue for const generics (RFC 2000) #44580)

Unresolved questions:

• Can/should we add an IntoIterator impl for arrays? (see Add IntoIterator impl for arrays by value (for [T; N]) #65819 to discuss)
• Do we want to add a method like owned_iter to arrays (returning array:IntoIter) ?

[CryZe]

For an IntoIterator impl we should probably issue warnings as soon as possible (not just via clippy).

[LukasKalbertodt]

Can we even stabilize this before stabilizing const generics? No, right? Because the public API clearly contains parts of const generics. Or is it possible to stabilize because the only way programmers can interact with the type is not dependent on const generics?

Also: could we add the IntoIterator impl for arrays without stabilizing this (the std::array::IntoIter type)? As the PR showed, people can use the iterator without the #![feature(array_value_iter)]. The unstable feature is just required to name the type. But it also seems strange to have a (necessarily) stable impl that uses an unstable type.

[SimonSapin]

Right, unlike impls like for example impl<T: Eq, const N: usize> Eq for [T; N] where [T; N]: LengthAtMost32 {} which we could revert to 32 separate macro-generated impls, array::IntoIter is a generic type that has a const paramater which couldn’t be represented if we ever came to remove const generics from the language.

[LukasKalbertodt]

@SimonSapin Just to be sure I understand you correctly, what is your answer to this question?

Could we add the IntoIterator impl for arrays without stabilizing std::array::IntoIter?

Using the impl without #![feature(array_value_iter)] works, but it seems strange to me that we can "use" an unstable type as long as we don't name it. On the other hand, since the IntoIterator impl is bound by LengthAtMost32, if const generics were to be removed, we could simply add 32 different iterator types to the standard library. Right? So in theory we could add the impl? Is there some precedence to this?

[SimonSapin]

My previous comment was saying that we cannot stabilize a struct IntoIter<const N: usize, T> {…} type without definitely relying on const generics, because the const parameter is visible in that signature. Adding a where [T; N]: LengthAtMost32 bound is not enough, unlike for trait impls.

---

Additionally, cross-posting from #65819 (comment):

With my Libs team hat on: I’d be uncomfortable stabilizing a method that returns an unstable type. Yes this does mean blocking array by-value iterators on const generics. (Or stabilizing an array trait and a const-generics-free iterator type.)

F-const_generics `#![feature(const_generics)]` shows active work. It’ll come. In the meantime, as @mpdn mentioned the arrayvec crate provides an alternative.

[Plecra]

The DoubleEndedIterator implementation seems counter-intuitive to me. I was under the impression that the trait was typically implemented where it could be done in a zero-cost way, like how slices are required to track their length anyway. Many of the std iterators could implement more of the iterator traits if they just tracked more state.

Is the DoubleEndedIterator implementation particularly urgent? Maybe it doesn't need to be stabilized with the main Iterator implementation...

[cuviper]

I think it would be strange if array::IntoIter didn't implement DoubleEndedIterator, as it would be deficient compared to slice and Vec iterators. There are plenty of standard iterators that already track extra state to support reversing -- e.g. FlatMap would be a lot simpler if it didn't need that.

[KodrAus]

Have we done any crater runs recently on where the lint has landed us? If not, would anybody be interested in opening up an experimental PR to check?

[LukasKalbertodt]

The last crater run AFAIK was this one. The problem is that crater doesn't seem to update the Cargo.lock (also see this comment. This means there are still a ton of non-root regressions that can be fixed by a simple cargo update. Also take a look at my comment here, where I estimate that roughly 4000 of the 4771 regressions crater found would be trivially fixable.

It would be really beneficial if we could update (or remove) the Cargo.lock on the crater machines, but I'm not sure how easy that would be.

If you want to run crater again, I can just update my PR again. Or are you mentioning "experimental PR" to reduce noise on the main PR?

[SimonSapin]

This is unblocked! #79135 has landed. What remains here is to decide how should one obtain an array::IntoIter value:

• Wait until we add impl IntoIterator for [T; N] Add IntoIterator impl for arrays by value (for [T; N]) #65819. The status there is that this PR would still cause some regressions: Add IntoIterator impl for arrays by value (for [T; N]) #65819 (comment)
• Stabilize array::IntoIter::new, with the option to deprecate it after IntoIterator can be implemented.
• Pick a name (that is not into_iter) and replace new with an inherent method on arrays, also with the option to deprecate after IntoIterator can be implemented

[SimonSapin]

@scottmcm weighed in on Zulip: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/array.20by-value.20iterator.20stabilization/near/221116093

I like stabilizing the IntoIter::new method. That's a totally reasonable API to exist (even if eventually deprecated later) and conveniently is over on a type that people will rarely look at once the IntoIterator finally happens. Needing the import is unfortunate, but I think the goal here is to unblock people which it does fine, and it'd be nice to avoid a vestigial method on the array type (at which people will actually commonly look). Especially to avoid any confusion about "Why isn't there iter_owned on a Vec? Can I not get an owning iterator to the items in a vector?"

Sounds good to me. I’ve submitted a stabilization PR and requested FCP: #80470

[SimonSapin]

Stabilized in 1.51.0 by #80470.

The IntoInterator impl is tracked separately at #65819.