`eq_unspanned` is incorrect

[workingjubilee]

The following function is incorrect:

rust/compiler/rustc_ast/src/tokenstream.rs

Lines 698 to 707 in 038d599

	pub fn eq_unspanned(&self, other: &TokenStream) -> bool {
	let mut iter1 = self.iter();
	let mut iter2 = other.iter();
	for (tt1, tt2) in iter::zip(&mut iter1, &mut iter2) {
	if !tt1.eq_unspanned(tt2) {
	return false;
	}
	}
	iter1.next().is_none() && iter2.next().is_none()
	}

Zipped iterators with uneven lengths will still consume an item from both sides of the zip on their final iteration (where one will return None and one will return Some). This allows for both iterators to be exhausted even though their lengths are unequal.

[workingjubilee]

h/t @jameysharp for finding this

[compiler-errors]

This seems to only be used in tests, clippy, and rustdoc. It should probably be removed tbh.

[chenyukang]

I am not sure if I misunderstood, I think this function is correct.

iter::zip(&mut iter1, &mut iter2) will only iterate as long as both iterators yield values, i.e., up to the length of the shorter iterator, see the type in for.

[jameysharp]

This is surprisingly tricky because I've discovered this function would work as intended on some iterators, and I don't know if this is one of them. But here's an example where zip iterates once, where one iterator yields two values but the other only yields one, and yet after zip returns None, both iterators also return None:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=268f9af2bf4b61d9bd13f75cc905dca3

This happens because the default implementation of next() for zip first calls next on the left iterator, and then on the right, so by the time it discovers that there is no next element in the second iterator it has already consumed an element from the first iterator.

But on TrustedRandomAccess iterators, zip is specialized to know exactly how many elements it can return, and doesn't consume from either iterator after that point. So zip's behavior is observably different depending on which specialization you get, which is interesting.

I don't know which category TokenStream::iter() falls into. But at minimum it seems fragile to me.

[chenyukang]

This is surprisingly tricky because I've discovered this function would work as intended on some iterators, and I don't know if this is one of them. But here's an example where zip iterates once, where one iterator yields two values but the other only yields one, and yet after zip returns None, both iterators also return None:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=268f9af2bf4b61d9bd13f75cc905dca3

This happens because the default implementation of next() for zip first calls next on the left iterator, and then on the right, so by the time it discovers that there is no next element in the second iterator it has already consumed an element from the first iterator.

But on TrustedRandomAccess iterators, zip is specialized to know exactly how many elements it can return, and doesn't consume from either iterator after that point. So zip's behavior is observably different depending on which specialization you get, which is interesting.

I don't know which category TokenStream::iter() falls into. But at minimum it seems fragile to me.

You're right, I misunderstood it.

[chenyukang]

The subtle part is only the arguments order:

fn cmp(arr1: &[i32], arr2: &[i32]) -> bool {
    let mut xi = arr1.into_iter();
    let mut yi = arr2.into_iter();

    for (x, y) in std::iter::zip(&mut xi, &mut yi) {
        if x != y {
            return false;
        }
    }
    xi.next().is_none() && yi.next().is_none()
}

fn main() {
    let arr1 = [1];
    let arr2 = [1, 2];

    eprintln!("res1: {:?}", cmp(&arr1, &arr2));   // return `false`
    eprintln!("res2: {:?}", cmp(&arr2, &arr1));   // return `true`
}

[workingjubilee]

@the8472 I feel like I ought to poke you at this point.