rust-itertools / itertools

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

izip! is just wrong #970

Closed DeltaEpsilon7787 closed 2 months ago

DeltaEpsilon7787 commented 2 months ago

itertools = "0.13.0"

izip!(alpha, beta, gamma, delta, epsilon)

expands into


$crate::__std_iter::IntoIterator::into_iter(alpha)
    .zip(beta)
    .zip(gamma)
    .zip(delta)
    .zip(epsilon)
    .map(|((((a, b), b), b), b)| (a, b, b, b, b))

which is obviously wrong.

jswrenn commented 2 months ago

Yikes, thanks for the report! Disappointed that this slipped through our CI.

Philippe-Cholet commented 2 months ago

@DeltaEpsilon7787 Macro expansion is weird. My guess is that the compiler distinguish the four different versions of "b".

It seems to be working just fine: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=435b75b4411195ae0a4fcce78aac4ca9

Do you have an example of something going wrong?

DeltaEpsilon7787 commented 2 months ago

Looks like VSCode (and thus, rust LSP) are incorrectly determining the types of the vars in that situation. image Or rather, there is a disconnect in the LSP and the compiler.

To be fair, I can't blame it. What damn magic makes this work?


image


Thus, not a bug with itertools itself.

Philippe-Cholet commented 2 months ago

Copying your example in Sublime Text, I get the same (i32, f64, f64) weirdness. rust-analyzer does not distinguish the bs as good as the compiler does.

phimuemue commented 2 months ago

Yikes, thanks for the report! Disappointed that this slipped through our CI.

Just out of curiosity: Does anybody know if our CI would have cought this? (Would dig into this myself, but lack time.)

Philippe-Cholet commented 2 months ago

Well there is nothing to catch. But the detected weirdness appears when there are at least 3 arguments to izip (the only interesting case since a.zip(b) is good enough for 2 arguments). We have tests with izip!(3 arguments) so yeah, we should totally know if something breaks. And one test for izip!(4 args) apparently.

DeltaEpsilon7787 commented 2 months ago

We figured out the reason for the weirdness, so to speak. It's due to hygienic macros. The macro expanded code cannot represent the fact that those different b have different context assigned to them, so they are not, in fact, 4 variables named b, they are 4 variables that are essentially named b+[ID1], b+[ID2], b+[ID3], b+[ID4] wherein ID is an anonymous identifier constructed on every macro expansion. Kind of like variable owner aspect, which is also invisible.