Skip to content

Force exhaustion in iter::ArrayChunks::into_remainder #123406

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

krtab
Copy link
Contributor

@krtab krtab commented Apr 3, 2024

Closes: #123333

@rustbot
Copy link
Collaborator

rustbot commented Apr 3, 2024

r? @scottmcm

rustbot has assigned @scottmcm.
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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 3, 2024
#[unstable(feature = "iter_array_chunks", reason = "recently added", issue = "100450")]
#[inline]
pub fn into_remainder(self) -> Option<array::IntoIter<I::Item, N>> {
pub fn into_remainder(mut self) -> Option<array::IntoIter<I::Item, N>> {
while let Some(_) = self.next() {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
while let Some(_) = self.next() {}
if self.remainder.is_none() {
while let Some(_) = self.next() {}
}

If I: DoubleEndedIterator + ExactSizeIterator (thus Self: DoubleEndedIterator), and self.next_back() has been called, and there is a remainder, then self.remainder is already set to Some, so we can skip exhausting the iterator. (Optimization , not required for correctness).

Hypothetically, specialization could be used to do the "fast" thing if I: DEI + ESI regardless of if the remainder has already been found, but in general that would be unsound (I think?), since user implementations of DEI/ESI can have lifetime requirements.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch regarding next_back. I'll implement it.
I didn't get your point about specialization.

@krtab krtab force-pushed the fix_remainder_iterchunk branch from ed0f1ce to d9a8903 Compare April 4, 2024 10:20
pub fn into_remainder(self) -> Option<array::IntoIter<I::Item, N>> {
pub fn into_remainder(mut self) -> Option<array::IntoIter<I::Item, N>> {
if self.remainder.is_none() {
while let Some(_) = self.next() {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, normally we say to use for_each(drop) for this

#[must_use = "if you really need to exhaust the iterator, consider `.for_each(drop)` instead"]

but I guess this can't use that because it consumes it :/

Maybe it should be

Suggested change
while let Some(_) = self.next() {}
self.try_for_each(NeverShortCircuit::wrap_mut_1(drop));

?

Meh, we can always change it later if it ends up mattering. This is probably fine.

@scottmcm
Copy link
Member

Thanks for the fix! I wondered for a bit about what would happen with this after exhaustion or if it's called twice, but then I remembered it's self so can't be called twice and ArrayChunks is always fused anyway.

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 18, 2024

📌 Commit d9a8903 has been approved by scottmcm

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 18, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 18, 2024
…cottmcm

Force exhaustion in iter::ArrayChunks::into_remainder

Closes: rust-lang#123333
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Apr 18, 2024
…cottmcm

Force exhaustion in iter::ArrayChunks::into_remainder

Closes: rust-lang#123333
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2024
…kingjubilee

Rollup of 9 pull requests

Successful merges:

 - rust-lang#117919 (Introduce perma-unstable `wasm-c-abi` flag)
 - rust-lang#123406 (Force exhaustion in iter::ArrayChunks::into_remainder)
 - rust-lang#123752 (Properly handle emojis as literal prefix in macros)
 - rust-lang#123935 (Don't inline integer literals when they overflow - new attempt)
 - rust-lang#123980 ( Add an opt-in to store incoming edges in `VecGraph` + misc)
 - rust-lang#124019 (Use raw-dylib for Windows synchronization functions)
 - rust-lang#124110 (Fix negating `f16` and `f128` constants)
 - rust-lang#124112 (Fix ICE when there is a non-Unicode entry in the incremental crate directory)
 - rust-lang#124116 (when suggesting RUST_BACKTRACE=1, add a special note for Miri's env var isolation)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2024
…kingjubilee

Rollup of 7 pull requests

Successful merges:

 - rust-lang#123406 (Force exhaustion in iter::ArrayChunks::into_remainder)
 - rust-lang#123752 (Properly handle emojis as literal prefix in macros)
 - rust-lang#123935 (Don't inline integer literals when they overflow - new attempt)
 - rust-lang#123980 ( Add an opt-in to store incoming edges in `VecGraph` + misc)
 - rust-lang#124019 (Use raw-dylib for Windows synchronization functions)
 - rust-lang#124110 (Fix negating `f16` and `f128` constants)
 - rust-lang#124116 (when suggesting RUST_BACKTRACE=1, add a special note for Miri's env var isolation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3831cbb into rust-lang:master Apr 19, 2024
@rustbot rustbot added this to the 1.79.0 milestone Apr 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2024
Rollup merge of rust-lang#123406 - krtab:fix_remainder_iterchunk, r=scottmcm

Force exhaustion in iter::ArrayChunks::into_remainder

Closes: rust-lang#123333
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

core::iter::ArrayChunks does not return the remainder if None is never yielded
5 participants