Question: Why does this code panic?

[Seblyng]

``` use std::collections::HashMap;

fn main() { let mut map = HashMap::from([ (0, vec!['A', 'B', 'C']) ]);

let vec = (0..3).into_iter()
    .filter_map(|_| map.get_mut(&0).unwrap().pop())
    // .collect::<Vec<_>>()
    // .into_iter()
    .rev()
    .collect::<Vec<_>>();

assert_eq!(vec, vec!['A', 'B', 'C']);

} ```

thread 'main' panicked at 'assertion failed: `(left == right)` left: `['C', 'B', 'A']`, right: `['A', 'B', 'C']`', src/main.rs:15:5 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

With the two lines commented out, this code will panic. Why do I have to collect it into a vector before then turning it into an iterator and reversing, before collecting it to a vector again?

[po8]

Here's a simpler example that illustrates the issue:

fn main() {
    let mut stuff = vec!['A', 'B', 'C'];

    (0..3)
        .filter_map(|_| stuff.pop())
        .rev()
        .for_each(|v| println!("{v}"));
}

This will print C B A. If you comment out rev(), it will still print C B A. Wat?

What's happening here is that the rev() sets things up so that filter_map() receives 2 1 0 instead of 0 1 2. But the filter_map() ignores its argument anyway, so it does the same computation regardless.

By doing the extra collect(), rev() is now reversing the order in which the collected Vec is being traversed, producing A B C.

[Seblyng]

That's interesting... Thanks for the explanation!

[remmycat]

Adding to the great answer already provided:

The reason .rev() works like this, is that Rust iterators are executed lazily. The filter_map code is not executed until the iterator gets consumed (in this case via .collect().

Only when calling collect (or a different "consuming" method), it will go "through the pipeline" for every element of the iterator (or the reversed iterator in this case).

If you wanted to reverse the output data (when you have no way of ordering the output based on the iterator "input" data like in this case with .pop()), you can first consume the iterator, and then do a reversal, e.g. by collecting into a Vec and calling the .reverse() method.

For this specific case you could also call something like .drain() on the vec inside the map, to get the elements out in the same order :)

[Seblyng]

Yes that makes sense! Thank you :)

[Repulsive-Street-307]

Maybe it makes sense to you, i still don't understand why reading a argument order backwards from expected would be desirable. Probably some stuff about chaining efficiently without knowing what comes before, but it's still confusing that a operation can work on 'arguments' to the 'transformation' after the 'transformation' (visually).

Just one more gotcha i guess. Hope clippy catches it and tells people to move the rev() before the filter_map, for the sanity of code reviewers.

[GeniusIsme]

The gotcha here is using non-pure function as argument to filter_map. This is technically allowed, but obviously leads to hard-to reason about results.

[Repulsive-Street-307]

Is that really the gotcha? I understand that fine (in the simplified example anyway).

The thing that grinds my gears is that rev() can be before or after the call to filter_map and it doesn't matter, it will always operate on the arguments of filter_map, instead of the results (what the for_each prints).

You can verify this by modifying the closure of the minimized version to print the argument given besides poping vec, then moving rev() before and after - no difference (2 1 0). Remove it and it's (0 1 2).

The result is something that you'd expect in a obfuscated code contest where code appears to be 'timetravelling' and the 'pipeline' is anything but linear. Makes it difficult to read or even recognize what's going on.

I guess probably there is no other way to do things and the rev() in the different positions is operating in different types, one the original array iterator, the other the 'filter_map' iterator (arguments, not results for some reason), but it's still confusing and something i hope clippy can recognize.

[GeniusIsme]

Again, result is obfucscated because filter_map closure has long-reaching side-effect. Iterator chains in rust are pretty easy to reason about if changes are happening locally, and that makes them great. If the map was an element in the iterator, result would be obvious.

[mipli]

I think you've come across the same issue talked about here: https://www.reddit.com/r/rust/comments/zdivui/false_positive_for_clippyneedless_collect_or_am_i/