fix: take_while_inclusive takes iterator by value

[abhikjain360]

this helps in case TakeWhileInclusive is returned from a function, as otherwise we would be returning a struct which takes a reference from a locally created iterator.

example code which breaks, but shouldn't:

use itertools::Itertools;

struct Entry {
    constraint: bool,
}

struct List {
    entries: Vec<Entry>,
}

impl List {
    fn special(&self) -> impl Iterator<Item = &Entry> {
        self
            .entries
            .iter()
            .take_while_inclusive(|entry| entry.constraint)
    }
}

Rust Playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f602ba3c6e7c2ab2f5132a0b40b80ceb

[Philippe-Cholet]

I thought you had a point so I looked at the methods in trait Iterator to find the way the "std" considers this.

There are 4 cases:

• no change (size_hint): immutable borrow ;
• transfer ownership to another iterator type (such as take_while...): take by value ;
• consume the iterator completely (such as count/last/max...): take by value ;
• consume only part of the iterator (such as find/all/try_collect...): mutably borrow.

take_while_inclusive is like take_while, it returns another iterator type. So I think it should take by value. People that does not want that can still do some_it.by_ref().take_while_inclusive(...) then do something else with what remains in some_it.

tldr I fully agree with you.

PS: It's a breaking change but take_while_inclusive is really new (appeared in "0.11.0" a few days ago) so it won't be a big issue for people if it's fixed soon.

[phimuemue]

I think @Philippe-Cholet's assessment is correct.

Thus, I'd like to merge this. @jswrenn Do you have objections?

[abhikjain360]

Hi @phimuemue @jswrenn ,
any updates on this or #710?
I think by now it is late enough that you probably need to release this fix in a separate release 😅 .
I can add fixes for other methods mentioned in #710 if that would help get this merged.

Thanks!

[phimuemue]

Thanks a lot.

bors r+

[bors]

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• bors build finished

[jswrenn]

Thank you for this! Apologizes for not releasing this fast enough to avoid a breaking release; I was on an extended bicycle tour. I'll get out a release soon.

[meithecatte]

@jswrenn I just hit this issue and found this PR. Just wanna make sure that you haven't forgot about the release. Thanks for your work, itertools is a very useful crate ;3