feat: make izip! temporary friendly

[IceTDrinker]

• some izip! calls with temporary values would not compile with the current izip! implementation due to the use of a block to create the temporary iter chain, if a user called izip! with an iterator from a temporary value the compilation would fail

• this change adapts the macro to stop making use of the block and calls $crate::__std_iter::Iterator::zip directly to avoid the need for temporary iter bindings, this means the recursion produces a tuple of the form (a, (b, (c, d))) instead of (((a, b), c), d), needing a slight adaptation of the closure arm to invert the way arguments are unpacked, it also adds two arms to recursively build the zipped iterators without the final map adapter as we don't process all iterators at once as before

This should also prevent some failure linked to Rust Edition 2024 changes for some temporary lifetime (which actually prompted this patch in the first place) https://doc.rust-lang.org/edition-guide/rust-2024/temporary-tail-expr-scope.html

Example Rust code that this patch fixes:

struct Owned {
    data: Vec<u64>,
}

impl Owned {
    fn new(val: u64) -> Self {
        Self {
            data: vec![val; 10],
        }
    }

    fn as_view(&self) -> View<'_> {
        View {
            data: self.data.as_slice(),
        }
    }
}

struct View<'a> {
    data: &'a [u64],
}

impl View<'_> {
    fn iter(&self) -> impl Iterator<Item = &u64> {
        self.data.iter()
    }
}

fn main() {
    let a = Owned::new(0);
    let b = Owned::new(1);
    let c = Owned::new(2);

    for (x, y, z) in itertools::izip!(a.as_view().iter(), b.as_view().iter(), c.as_view().iter()) {
        println!("{x}, {y}, {z}")
    }
}

Failing Cargo.toml

[package]
name = "itertools_2024"
version = "0.1.0"
edition = "2021"

[dependencies]
# Updated dependency compiles fine and runs fine
itertools = { version = "0.14.0" }

Sample error:

error[E0716]: temporary value dropped while borrowed
  --> src/main.rs:84:39
   |
84 |     for (x, y, z) in itertools::izip!(a.as_view().iter(), b.as_view().iter(), c.as_view().iter()) {
   |                      -----------------^^^^^^^^^^^------------------------------------------------
   |                      |                |
   |                      |                creates a temporary value which is freed while still in use
   |                      temporary value is freed at the end of this statement
   |                      borrow later used here
   |
   = note: consider using a `let` binding to create a longer lived value

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.39%. Comparing base (6814180) to head (5ca995e).
Report is 138 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #1021    +/-   ##
========================================
  Coverage   94.38%   94.39%            
========================================
  Files          48       50     +2     
  Lines        6665     6883   +218     
========================================
+ Hits         6291     6497   +206     
- Misses        374      386    +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[IceTDrinker]

If needed I can add the example in the PR comment as a test to avoid future regressions

[phimuemue]

Hi there, first of all: Huge thanks for a playground-runnable example.

Honestly, I did not fully review the patch yet, because I found we have 3 macros: izip, iproduct and chain - and to me it seems that:

• They all repeatedly apply an iterator method.
• They all should work on IntoIter things.
• All of them should accept the same trailing comma (and other shenanigans I may have forgotten).
• izip and iproduct somehow flatten their tuple. (I think iproduct has an upper bound on tuple components, izip does not.)
• izip and iproduct called with one iterator should return something similar (currently it's Item vs (Item,) (i.e. a 1-element tuple)). We should strive for always-tuple to facilitate usage in generic code.

If @jswrenn is ok with it, I suggest to seize the opportunity and bring them in line instead of only patching izip. Preferably, all should use the same technique to repeatedly apply the iterator method, and tuple flattening should work the same way.

Maybe we can split things up into two parts: One macro to repeatedly call an iterator method usable in Rust 2024, and one macro to do tuple flattening.

OT: We should somehow test Rust 2024 in our CI.

[IceTDrinker]

For now I'll start by adding the non regression test as proposed as I see two thumbs up, if you want a more extensive patch, or would rework it in a later PR to manage the other macros as you said let me know!

[IceTDrinker]

hey @phimuemue I know what it's like (to an extent) to manage PRs on open source projects, just was wondering if you had reached a decision regarding the PR ? I'm in no rush on my end to get this working, just felt that it would be better to get this chugging along before we forget too much about it :)

[jswrenn]

I'm happy to approve the point fix provided by this PR. :-)