while_let_loop fails to account for lifetimes

[cuviper]

I have a loop with a match which is suggested to convert to while let. But it's important in this case that the borrows from the match don't extend through the whole loop.

Here's my function:

/// Merges two sorted vectors into one.
pub fn merge_sorted<T>(xs: Vec<T>, ys: Vec<T>) -> Vec<T>
where T: PartialOrd {
    let total_len = xs.len() + ys.len();
    let mut res = Vec::with_capacity(total_len);
    let mut ix = xs.into_iter().peekable();
    let mut iy = ys.into_iter().peekable();
    loop {
        let lt = match (ix.peek(), iy.peek()) {
            (Some(x), Some(y)) => x < y,
            _ => break
        };
        res.push(if lt { &mut ix } else { &mut iy }.next().unwrap());
    }
    res.extend(ix);
    res.extend(iy);
    res
}

(Nevermind whether this is particularly good code -- I'm reconsidering that. But the lint is flawed regardless.)

Clippy's while_let_loop suggests replacing the loop with:

while let (Some(x), Some(y)) = (ix.peek(), iy.peek()) {
    res.push(if lt { &mut ix } else { &mut iy }.next().unwrap());
}

First of all, it missed the lt binding. I can add let lt = x < y; or use x < y directly for that. But now the x/y borrows on ix/iy live for the entire loop body, so they can't be taken mutably for next().

error: cannot borrow `ix` as mutable more than once at a time [E0499]
         res.push(if lt { &mut ix } else { &mut iy }.next().unwrap());
                               ^~
[...]
error: cannot borrow `iy` as mutable more than once at a time [E0499]
         res.push(if lt { &mut ix } else { &mut iy }.next().unwrap());
                                                ^~

Maybe non-lexical lifetimes could someday let this work, but for now it's a bad suggestion.

PS- while_let_loop is missing from the wiki.

[cuviper]

This might be related to the extension done for #262. That was easier to justify for a match like Some(bi) => bi, but my case is not so trivial and too greedily captured by the lint, I think.

[wafflespeanut]

looking into it, thanks for reporting :)

/cc @Manishearth

[basil-cow]

@flip1995 with the introduction of NLL, this issue is issue no more. Goes to show you that if you wait long enough all problems solve themselves :)

[ghost]

The suggestion still misses the lt binding though.

[basil-cow]

It does hide it behind an ellipsis now, do you think can we make it machine applicable?

[basil-cow]

I am just afraid that for the big loops the suggestion will be long and ugly and as a consequence, a bit confusing

[ghost]

The current suggestion is

while let (Some(x), Some(y)) = (ix.peek(), iy.peek()) { .. }

I thought it could be:

while let (Some(x), Some(y)) = (ix.peek(), iy.peek()) { 
    let lt = x < y;
    ..
}

Or something like that.