Make vec::IntoIter covariant again

[apasel422]

Closes #35721

r? @alexcrichton

[apasel422]

This uses integer indices instead of pointers to avoid aliasing RawVec's underlying Unique, but could be rewritten, albeit with some duplication from RawVec, to use a struct layout like

struct IntoIter<T> {
  buf: Shared<T>,
  cap: usize,
  head: Shared<T>,
  tail: Shared<T>,
}

[frewsxcv]

This could also be: if self.is_empty(), I think?

[alexcrichton]

In the past this has been pretty performance-sensitive, could you benchmark just a forward iteration of a IntoIter to ensure that it didn't regress? Other than that looks great to me, thanks!

Also nominating for backport as beta was cut today and we don't want to miss this.

[apasel422]

@alexcrichton Ah, if someone else could help out with benchmarks, that'd be great. If they indicate that this approach is slower, I'd be happy to rewrite this to use the other approach I mentioned.

[alexcrichton]

@apasel422 could you gist the IR for this:

extern {
    fn bar(a: u8);
}
pub fn sum(v: Vec<u8>) {
    for e in v.into_iter() {
        unsafe { bar(e); }
    }
}

For me, rustc bar.rs -O --crate-type lib --emit llvm-ir looks like this. I think we basically just need to be sure that this still optimizes to a tight loop.

[apasel422]

IR Gist: https://gist.github.com/apasel422/c16aac732979a21c5a69c5180ca434a2

[alexcrichton]

Hm so that's different because LLVM still has an add instruction, I'm not sure how much that'll end up translating to missed optimizations though. Maybe it's worth to stick to a two-pointers-approaching-each-other implementation though?

[apasel422]

@alexcrichton Done.

[alexcrichton]

@bors: r+ 7e148cd

Thanks! (also wow that was fast)

[bors]

⌛ Testing commit 7e148cd with merge 76fa587...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-cross-freebsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-no-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-no-mir, auto-win-msvc-64-opt-rustbuild
Approved by: alexcrichton
Pushing 76fa587 to master...

[retep998]

Might be worth backporting this to beta, as the regression lived just long enough to jump into beta.

[apasel422]

@alexcrichton Can we get this backported to beta?

[alexcrichton]

Yes this is tagged with beta-nominated, and we'll discuss at the next libs triage meeting. It'll likely be an obvious "yes let's backport"

[alexcrichton]

Discussion at @rust-lang/libs today decided to accept for backport