Undocumented soundness fix between 1.34 and 1.35

[scottmcm]

I compiled a rather old project of mine for the first time in quite a while and was surprised that it no longer compiled on stable, which it did way back in 1.34.

Non-minimized regression demo: https://rust.godbolt.org/z/f9n6h8

The core of the problem is this bit of code:

        if x == y {
            return false;
        }
        let [x, y] = unsafe {
            let x: *mut _ = &mut self.0.get_unchecked_mut(x);
            let y: *mut _ = &mut self.0.get_unchecked_mut(y);
            debug_assert_ne!(x, y);
            [&mut *x, &mut *y]
        };

It turns out that it was always wrong -- I meant to get a *mut Entry, not a *mut &mut Entry -- so I think this is probably an allowed breaking change?

But it's not documented in the release notes for 1.35, which surprised me.

It's not obvious to me exactly what got fixed here. Is a temporary lifetime different, or is borrowck just detecting something it didn't used to? (Both examples are on edition 2018, so I think MIR borrowck?)

[RalfJung]

Going over the git log doesn't show anything like an associated type soundness fix. We should at the very least make sure this is covered by a test.^^

@rustbot ping cleanup-crew
Would be good to get this nailed down via a bisect. The regression range is 1.34..1.35.

[rustbot]

Error: Parsing ping command in comment failed: ...'eanup-crew' | error: expected end of command at >| ' Would be'...

Please let @rust-lang/release know if you're having trouble with this bot.

[rustbot]

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @camelid @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @senden9 @shekohex @sinato @spastorino @turboladen @woshilapin @yerke

[lcnr]

minimized

pub fn union(v: &mut Vec<String>, x: usize, y: usize) {
    unsafe {
        let x: *mut &mut String = &mut v.get_unchecked_mut(x);
        let y: *mut &mut String = &mut v.get_unchecked_mut(y);
        debug_assert_ne!(x, y);
    }
}

[RalfJung]

@lcnr that minimized version also fails to compile with Rust 1.34, though? So it doesn't represent the regression.

EDIT: never mind, I forgot to pass --edition=2018. This will also be important for the bisect!

[lcnr]

you need --edition=2018

pub fn union<'a>(v: &'a mut Vec<String>, x: usize, y: usize) {
    unsafe {
        let x: *mut &'a mut String = &mut v.get_unchecked_mut(x);
        let y: &'a mut String = v.get_unchecked_mut(y);
        debug_assert_ne!(*x, y);
    }
}

[lcnr]

This does compile though, even in on the current nightly, so I am not quite sure what's the intended behavior here:

pub fn union<'a>(mut v: &'a mut String) {
    unsafe {
        let x: *mut &'a mut String = &mut v;
        let y: &'a mut String = v;
        debug_assert_ne!(*x, y);
    }
}

[lcnr]

As a completely self contained example:

fn ok<'a>(v: &'a mut ()) -> &'a mut () {
    v
}

pub fn union<'a>(v: &'a mut ()) {
    let x: *mut &'a mut () = &mut ok(v);
    let _ = (x, v);
}

https://godbolt.org/z/9415nM

[RalfJung]

This does compile though, even in on the current nightly, so I am not quite sure what's the intended behavior here:

That one should compile. The &mut v can have an arbitrarily short lifetime because it is turned into a raw pointer immediately.

[lcnr]

The &mut v can have an arbitrarily short lifetime because it is turned into a raw pointer immediately.

I don't yet see the difference between &mut v and &mut ok(v) here. Why does the same not apply to &mut ok(v)?
Do we implicitly reborrow in the &mut v case?

[matthewjasper]

I think this is #58673

[RalfJung]

I don't yet see the difference between &mut v and &mut ok(v) here. Why does the same not apply to &mut ok(v)?

v is a place expression, so &mut v borrows an existing place.
In contrast, ok(v) is a value expression, so this value has to be stored somewhere before we can create a reference to it.

That said, I thought that let x = &mut foo; was equivalent to let _tmp = foo; let x = &mut _tmp; (the "enclosing scope" rule). Is that not the case here?