-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Unstably constify impl<I: Iterator> IntoIterator for I
#90602
Conversation
r? @scottmcm (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
Do I need to do a test to make sure it is unstable? const fn check() {
let a = (0..5).into_iter();
} |
impl<I: Iterator> IntoIterator for I
impl<I: Iterator> IntoIterator for I
We need a decision for If we do, then this impl would have stricter bounds on const impls. This should be decided before stabilizing |
Assuming you are meaning on the trait definition: EDIT: It turns out that I did comment about this in the unresolved questions in the tracking issue:
|
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 |
Triage: still looks like this is waiting on someone from libs-api to comment. |
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 |
EDIT: This is only partly related to this PR in that it is about the trait definition, not the identity implementation. 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 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 convert_and_iterate<I: ~const IntoIterator<Item = i32>>(iter: I) -> i32 {
iter.into_iter().next()
} SummaryThe first approach means that I would prefer the second approach, because of the usability aspect of it. |
☔ The latest upstream changes (presumably #93691) made this pull request unmergeable. Please resolve the merge conflicts. |
I might wait for this until #93412 is done. |
7c39dee
to
e0f804e
Compare
This comment has been minimized.
This comment has been minimized.
e0f804e
to
2cff30b
Compare
Do we currently plan on stabilizing ~const without removing the workaround from 92433? |
If we stabilize |
cc @oli-obk |
Status update:
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. |
📌 Commit b8ef1db has been approved by |
Has @bors forgotten this? |
Closing and reopening may help bors update this PR's state. |
@bors r- |
@bors r+ |
📌 Commit b8ef1db has been approved by |
☀️ Test successful - checks-actions |
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. |
This constifies the default
IntoIterator
implementation under theconst_intoiterator_identity
feature.Tracking Issue: #90603