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

iter::Scan should not unconditionally implement FusedIterator #41964

Closed
shepmaster opened this issue May 12, 2017 · 0 comments
Closed

iter::Scan should not unconditionally implement FusedIterator #41964

shepmaster opened this issue May 12, 2017 · 0 comments

Comments

@shepmaster
Copy link
Member

shepmaster commented May 12, 2017

Scan unconditionally implements FusedIterator if the wrapped iterator does:

impl<B, I, St, F> FusedIterator for Scan<I, St, F>
    where I: FusedIterator, F: FnMut(&mut St, I::Item) -> Option<B> {}

Since the closure to scan can choose to return None, this implementation is overly broad. It would need to take the closure into account, which seems... not possible.

Here's a reproduction showing the unexpected behavior when the closure returns None:

#![feature(fused)]

fn dump<I: Iterator<Item = i32>>(label: &str, mut iter: I) {
    println!("= Running: {}", label);
    for _ in 0..10 {
        println!("{:?}", iter.next());
    }
    println!("");
}

fn boxed_internal_fuse() -> Box<Iterator<Item = i32>> {
    Box::new((1..3)
        .scan(1, |_, x| if x < 2 { None } else { Some(x) })
        .fuse())
}

fn boxed_no_fuse() -> Box<Iterator<Item = i32>> {
    Box::new((1..3)
        .scan(1, |_, x| if x < 2 { None } else { Some(x) }))
}

use std::iter::FusedIterator;
fn boxed_no_fuse_but_fused() -> Box<FusedIterator<Item = i32>> {
    Box::new((1..3)
        .scan(1, |_, x| if x < 2 { None } else { Some(x) }))
}

fn main() {
    let i1 = (1..3)
        .scan(1, |_, x| if x < 2 { None } else { Some(x) });
    dump("Scan", i1);
    
    let i2 = (1..3)
        .scan(1, |_, x| if x < 2 { None } else { Some(x) })
        .fuse();
    dump("Fuse<Scan>", i2);
    
    dump("Box<Fuse<Scan>>", boxed_internal_fuse());
    dump("Fuse<Box<Iterator>>", boxed_no_fuse().fuse());
    dump("Fuse<Box<FusedIterator>>", boxed_no_fuse_but_fused().fuse());
}
= Running: Scan
None
Some(2)
None
None
None
None
None
None
None
None

= Running: Fuse<Scan>
None
Some(2)
None
None
None
None
None
None
None
None

= Running: Box<Fuse<Scan>>
None
Some(2)
None
None
None
None
None
None
None
None

= Running: Fuse<Box<Iterator>>
None
None
None
None
None
None
None
None
None
None

= Running: Fuse<Box<FusedIterator>>
None
Some(2)
None
None
None
None
None
None
None
None

This example requires nightly for the feature to demonstrate, but the effect can be felt in stable Rust.

Originally reported via Stack Overflow

@shepmaster shepmaster changed the title iter::Scan should not implement FusedIterator but it currently does iter::Scan should not unconditionally implement FusedIterator May 12, 2017
tbu- added a commit to tbu-/rust that referenced this issue May 15, 2017
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 26, 2017
Remove `FusedIterator` implementation of `iter::Scan`

Fixes rust-lang#41964.

This is a breaking change.
bors added a commit that referenced this issue May 26, 2017
Remove `FusedIterator` implementation of `iter::Scan`

Fixes #41964.

This is a breaking change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant