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

IteratorExt: by_ref + take_while consumes one extra element #22802

Closed
oli-obk opened this Issue Feb 25, 2015 · 5 comments

Comments

Projects
None yet
4 participants
@oli-obk
Copy link
Contributor

oli-obk commented Feb 25, 2015

take_while also consumes the first element that does NOT fullfill the condition.

fn main() {
    let a = [1, 2, 3, 4, 5];
    let mut it = a.iter();
    for _ in it.by_ref().take_while(|a| **a < 3) {}
    for i in it {
        println!("{}", i);
    }

    let a = [1, 2, 3, 4, 5];
    let mut it = a.iter();
    for _ in it.by_ref().take(3) {}
    for i in it {
        println!("{}", i);
    }
}

prints

4
5
4
5
@Thiez

This comment has been minimized.

Copy link
Contributor

Thiez commented Feb 25, 2015

So how would you expect take_while to know when to stop unless it also evaluates the first element that does not fulfill the condition? I suppose it could demand an iterator that is std::iter::Peekable, but even that would not be sufficient, because the iterator trait says:

The Iterator protocol does not define behavior after None is returned. A concrete Iterator implementation may choose to behave however it wishes, either by returning None infinitely, or by doing something else.

So when all items match the take_while condition (e.g. by changing the condition to |a| **a < 100) the it in your example does not guarantee any behavior.

I suppose take_while could mention its behavior in the documentation, but apart from that I don't think there's anything that can be done here.

@oli-obk

This comment has been minimized.

Copy link
Contributor Author

oli-obk commented Feb 25, 2015

I think this is a somewhat general issue with &mut iterators. Peakable iterators also suffer from this.

fn main() {
    let a = [1, 2, 3, 4, 5];
    let mut it = a.iter();
    {
        let mut it2 = it.by_ref().peekable();
        let _ = it2.peek();
    }
    for i in it {
        println!("{}", i);
    }
}
@oli-obk

This comment has been minimized.

Copy link
Contributor Author

oli-obk commented Feb 25, 2015

Note: in the Peakable example None is never returned.

@steveklabnik steveklabnik added the A-libs label Feb 25, 2015

@bluss

This comment has been minimized.

Copy link
Contributor

bluss commented Feb 26, 2015

It's kind of a gotcha, but you're explicitly giving a mutable ref of your iterator to another object, so it's completely natural.

What has been proposed before is a peeking version of take_while that can only be used on .peekable, that way you could use both take_while and still access the boundary iterator element afterwards.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented May 24, 2016

This is documented behavior of a stable method, so I'm going to give it a close. It is a bit of a gotcha, but it also makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.