Unstably constify impl<I: Iterator> IntoIterator for I

[mbartlett21]

This constifies the default IntoIterator implementation under the const_intoiterator_identity feature.

Tracking Issue: #90603

[rust-highfive]

r? @scottmcm

(rust-highfive has picked a reviewer for you, use r? to override)

[mbartlett21]

Do I need to do a test to make sure it is unstable?
Something like:

const fn check() {
    let a = (0..5).into_iter();
}

[fee1-dead]

We need a decision for IntoIterator: Should we make IntoIter: ~const Iterator?

If we do, then this impl would have stricter bounds on const impls. This should be decided before stabilizing const_trait_impl.

[mbartlett21]

We need a decision for IntoIterator: Should we make IntoIter: ~const Iterator?

Assuming you are meaning on the trait definition:
That seems like an improvement, since it would shorten bounds where impl IntoIterator is taken:
where T: ~const IntoIterator<IntoIter: ~const Iterator, Item = u32>
versus
where T: ~const IntoIterator<Item = u32>

EDIT: It turns out that I did comment about this in the unresolved questions in the tracking issue:

• Should it somehow bound on the resultant Iterator being a const Iterator?

[scottmcm]

It sounds like there are questions about what the bounds should be here, and I'm not really familiar with impl constification (though the change appears fine to me), so I'm going to try sending this to someone from api for comment.

r? rust-lang/libs-api

[JohnCSimon]

Triage: still looks like this is waiting on someone from libs-api to comment.

[yaahc]

We need a decision for IntoIterator: Should we make IntoIter: ~const Iterator?

If we do, then this impl would have stricter bounds on const impls. This should be decided before stabilizing const_trait_impl.

I'm not familiar enough with the effects of the conditional const bounds on trait impls as suggested to understand what you mean. Can you elaborate some more on what change you're considering and how the bounds it introduces would be stricter?

Also, as a side note as of #92433 (review) we're "tapping the brakes a little on filling the standard library with ~const and allow rustdoc to catch up a little" so if the second suggested change to IntoIterator would be visible in rustdoc we'd likely want to wait for the same changes that 92433 is waiting on. Though I'm assuming the changes in the diff currently don't affect the generated rustdoc output, in which case we should be able to merge those separately (assuming that even makes sense here).

[mbartlett21]

@yaahc

EDIT: This is only partly related to this PR in that it is about the trait definition, not the identity implementation.
EDIT2: Correct the impls below

Here are the two different approaches for the actual trait:

Approach 1 - Bounds on the use:

// in core::iter

trait IntoIterator {
    type IntoIter: Iterator;
    ...
}

// the identity implementation can either be:
impl<I: Iterator> const IntoIterator for I { ... }
// or
impl<I: ~const Iterator> const IntoIterator for I { ... }

Use in a const fn:

const fn convert_and_iterate<I: ~const IntoIterator<IntoIter = J, Item = i32>, J: ~const Iterator<Item = i32>>(iter: I) -> i32 {
    iter.into_iter().next()
}

RFC 2289 would shorten it a bit:

#![feature(associated_type_bounds)]

const fn convert_and_iterate<I: ~const IntoIterator<IntoIter: ~const Iterator, Item = i32>>(iter: I) -> i32 {
    iter.into_iter().next()
}

Approach 2 - Bounds on the trait definition:

// in core::iter

trait IntoIterator {
    type IntoIter: ~const Iterator;
    ...
}

// this would also force the identity implementation to be
impl<I: ~const Iterator> const IntoIterator for I { ... }

Use in a const fn:

const fn convert_and_iterate<I: ~const IntoIterator<Item = i32>>(iter: I) -> i32 {
    iter.into_iter().next()
}

Summary

The first approach means that const IntoIterator will apply to more types, but unfortunately also means more verbose type bounds. The second approach shortens the bounds, but means that it doesn't apply to as many types.

I would prefer the second approach, because of the usability aspect of it.

[bors]

☔ The latest upstream changes (presumably #93691) made this pull request unmergeable. Please resolve the merge conflicts.

[mbartlett21]

I might wait for this until #93412 is done.

[yaahc]

If ~const IntoIterator and I: ~const Iterator were to be made orthogonal, it would require some changes in rustdoc to temporarily work around #92433 (review). And once we stabilize ~const, we would have to display it in rustdoc which means the temporary workaround would have to be removed and these elaborated bounds will have to be displayed.

Do we currently plan on stabilizing ~const without removing the workaround from 92433?

[fee1-dead]

If we stabilize ~const, then it will be part of the language. Rustdoc should document those bounds accurately.

[yaahc]

cc @oli-obk

[oli-obk]

Status update:

1. rustdoc hides the ~const part of ~const Trait bounds and only displays them as Trait, so there is no problem anymore with this polluting stable docs
2. the original version of the PR was more verbose, but the current change is very simple. It changes the blanket impl<I: Iterator> IntoIterator for I { to a const impl while also binding I to ~const Iterator.
   • the effects of this are that you can only use into_iter on iterators if the iterator has a const impl.
   • this is overly conservative, so we can relax it in the future, even if we end up deciding to stabilize it
3. The current @rust-lang/wg-const-eval stance is to only add trivial uses of ~const Trait
   • so no new where bounds that weren't there before
   • no ~const Trait bounds on associated type projections
   • this PR satisfies the "trival" criteria

So, imo we should merge this now, and in the future push for the trivial-use-stance more strongly in order to not put more burden on libs-api. We will revisit this stance once the const eval story is better fleshed out.

[oli-obk]

r? @oli-obk

as wg-const-eval has previously merged such changes, I'm going to go ahead and merge this one, too.

@bors r+ rollup

[bors]

📌 Commit b8ef1db has been approved by oli-obk

[mbartlett21]

Has @bors forgotten this?

[faptc]

Closing and reopening may help bors update this PR's state.

[oli-obk]

@bors r-

[oli-obk]

@bors r+

[bors]

📌 Commit b8ef1db has been approved by oli-obk

[bors]

⌛ Testing commit b8ef1db with merge 6b4563b...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 6b4563b to master...

[rust-timer]

Finished benchmarking commit (6b4563b): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.