Skip to content
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

<core::iter::Filter as Iterator>::next_chunk virtually core::mem::forgets all elements that fail the predicate #126872

Closed
gksato opened this issue Jun 23, 2024 · 0 comments · Fixed by #126879
Assignees
Labels
A-iterators Area: Iterators C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way.

Comments

@gksato
Copy link

gksato commented Jun 23, 2024

The Iterator::next_chunk (see #98326) implementation for core::iter::Filter does not gracefully drop the elements failing the predicate in the original iterator; instead it forgets them.
As an example, you can try this code:

#![feature(iter_next_chunk)]
struct LoudlyDropped(usize);

impl Drop for LoudlyDropped {
    fn drop(&mut self) {
        println!("No. {} getting dropped here!!!", self.0);
    }
}

fn main() {
    let v = (0..10).map(LoudlyDropped).collect::<Vec<_>>();
    let _= v.into_iter().filter(|_| false).next_chunk::<1>();
}

(Playground)

I expected this program to generate the same message as the program without the last line in main, that is:

#![feature(iter_next_chunk)]
struct LoudlyDropped(usize);

impl Drop for LoudlyDropped {
    fn drop(&mut self) {
        println!("No. {} getting dropped here!!!", self.0);
    }
}

fn main() {
    let _ = (0..10).map(LoudlyDropped).collect::<Vec<_>>();
}

(Playground)

However, instead, the first program doesn't print anything.

The origin of this behavior is trivial to find: in the lines 90--98 of the file formod core::iter::adapters::filter of the current master (aabbf84), we have the following code (Github permalink), which is part of the implementation of next_chunk:

        let result = self.iter.try_for_each(|element| {
            let idx = guard.initialized;
            guard.initialized = idx + (self.predicate)(&element) as usize;


            // SAFETY: Loop conditions ensure the index is in bounds.
            unsafe { guard.array.get_unchecked_mut(idx) }.write(element);


            if guard.initialized < N { ControlFlow::Continue(()) } else { ControlFlow::Break(()) }
        });

This core::ptr::writes each element that doesn't comply the element into the place out of the range of guard.initialized.
The elements here, if not overwritten by this process, will be left undropped by the Drop implementation of [T; N]::IntoIter.

Meta

Playground version:

Build using the Nightly version: 1.81.0-nightly
(2024-06-22 3cb521a4344f0b556b81)
@gksato gksato added the C-bug Category: This is a bug. label Jun 23, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 23, 2024
@the8472 the8472 self-assigned this Jun 23, 2024
@the8472 the8472 added A-iterators Area: Iterators requires-nightly This issue requires a nightly compiler in some way. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jun 23, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 26, 2024
…cuviper

fix Drop items getting leaked in Filter::next_chunk

The optimization only makes sense for non-drop elements anyway. Use the default implementation for items that are Drop instead.

It also simplifies the implementation.

fixes rust-lang#126872
tracking issue rust-lang#98326
@bors bors closed this as completed in cf22be1 Jun 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 26, 2024
Rollup merge of rust-lang#126879 - the8472:next-chunk-filter-drop, r=cuviper

fix Drop items getting leaked in Filter::next_chunk

The optimization only makes sense for non-drop elements anyway. Use the default implementation for items that are Drop instead.

It also simplifies the implementation.

fixes rust-lang#126872
tracking issue rust-lang#98326
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-iterators Area: Iterators C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants