rust-itertools / itertools

Extra iterator adaptors, iterator methods, free functions, and macros.
https://docs.rs/itertools/
Apache License 2.0
2.72k stars 309 forks source link

group_by referencing temporary value issue #665

Open PSeitz opened 1 year ago

PSeitz commented 1 year ago

I really like group_by, but there is a limitation I sometimes run into, when transforming iterators, due to the temporary values. Currently that forces to consume the group_by always locally.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=18e06b09b59c1260ba0574d48c272eff


use itertools::Itertools;
fn return_group_by(iter: impl Iterator<Item=u32> + 'static) -> impl Iterator<Item=u32> + 'static{

    iter.group_by(move |idx| {
        get_block_id(*idx)
    })
    .into_iter()
    .flat_map(move |(block_id, chunk)| {
        // handle all elements in one block at once
        chunk.map(move |idx_in_block| {
            idx_in_block + 10
        })
    })
}

fn get_block_id(val: u32) -> u32{
    val >> 16
}
Compiling playground v0.0.1 (/playground)
error[[E0515]](https://doc.rust-lang.org/stable/error-index.html#E0515): cannot return value referencing temporary value
  [--> src/lib.rs:5:5](https://play.rust-lang.org/#)
BernhardPosselt commented 7 months ago

Running into the same issue, what's the solution to deal with that?

Philippe-Cholet commented 7 months ago

IntoIterator is implemented for &'a GroupBy<K, I, F> (not for GroupBy<K, I, F>) so .into_iter() does not take ownership of GroupBy so the iter.group_by is dropped at the end of the function while .into_iter().flat_map() is supposed to live longer.

We must ensure the iter.group_by() lives at least as long as where it's used. So here I would suggest to drop the return_group_by function and do things where it is called.