MIR should never emit references to `std::intrinsics::drop_in_place`

[bstrie]

When the drop_in_place feature was first stabilized, its lang item (and stable interface) was intrinsics::drop_in_place. Later in f2c7917 , intrinsics::drop_in_place was deprecated in favor of ptr::drop_in_place. However the deprecation never actually happened in practice, because #[rustc_deprecated] does nothing when applied to a re-export. This was remedied in #82122 , which also discovered that somehow, somewhere, MIR is still emitting hardcoded references to intrinsics::drop_in_place, ignoring the fact that the lang item is ptr::drop_in_place.

Test case:

fn main() {
    unsafe { std::ptr::drop_in_place(&mut 1); }
}

Output when compiled with rustc --emit=mir:

// WARNING: This output format is intended for human consumers only
// and is subject to change without notice. Knock yourself out.
fn main() -> () {
    let mut _0: ();                      // return place in scope 0 at dip.rs:1:11: 1:11
    let _1: ();                          // in scope 0 at dip.rs:2:18: 2:49
    let mut _2: *mut i32;                // in scope 0 at dip.rs:2:42: 2:48
    let mut _3: &mut i32;                // in scope 0 at dip.rs:2:42: 2:48
    let mut _4: i32;                     // in scope 0 at dip.rs:2:47: 2:48
    scope 1 {
    }

    bb0: {
        StorageLive(_1);                 // scope 1 at dip.rs:2:18: 2:49
        StorageLive(_2);                 // scope 1 at dip.rs:2:42: 2:48
        StorageLive(_3);                 // scope 1 at dip.rs:2:42: 2:48
        StorageLive(_4);                 // scope 1 at dip.rs:2:47: 2:48
        _4 = const 1_i32;                // scope 1 at dip.rs:2:47: 2:48
        _3 = &mut _4;                    // scope 1 at dip.rs:2:42: 2:48
        _2 = &raw mut (*_3);             // scope 1 at dip.rs:2:42: 2:48
        _1 = drop_in_place::<i32>(move _2) -> bb1; // scope 1 at dip.rs:2:18: 2:49
                                         // mir::Constant
                                         // + span: dip.rs:2:18: 2:41
                                         // + literal: Const { ty: unsafe fn(*mut i32) {std::intrinsics::drop_in_place::<i32>}, val: Value(Scalar(<ZST>)) }
    }

    bb1: {
        StorageDead(_2);                 // scope 1 at dip.rs:2:48: 2:49
        StorageDead(_4);                 // scope 1 at dip.rs:2:49: 2:50
        StorageDead(_3);                 // scope 1 at dip.rs:2:49: 2:50
        StorageDead(_1);                 // scope 1 at dip.rs:2:49: 2:50
        _0 = const ();                   // scope 1 at dip.rs:2:9: 2:52
        return;                          // scope 0 at dip.rs:3:2: 3:2
    }
}

Notice the line // + literal: Const { ty: unsafe fn(*mut i32) {std::intrinsics::drop_in_place::<i32>}. This should be referring to ptr::drop_in_place instead.

[bstrie]

Note that the test case here will probably stop working once #82122 lands, but that PR contains nothing that should be having any effect on this. The underlying bug will still remain somewhere. This matters because if #82123 is ever fixed it might cause #82122 to be reverted, which would cause this bug to manifest again.

[RalfJung]

Cc @oli-obk @eddyb any idea what might be causing this?

[eddyb]

MIR is still emitting hardcoded references to intrinsics::drop_in_place

Given that one is a reexport of the other, there is no meaningful difference between the two, MIR isn't making a choice here (nor could it - either way to refer to that function resolves to the same DefId, which is what the MIR sees).

---

So this is just an arbitrary printing choice between std::ptr::drop_in_place and std::intrinsics::drop_in_place - I would've thought that if the original definition is visible, we don't use a reexport, but maybe this hits an edge case.

Also, while std::ptr isn't necessary "less of a reexport" than std::intrinsics (because they're both reexports of core modules), core::ptr::drop_in_place should take precedence over core::intrinsics::drop_in_place, and yet it doesn't in:

#![no_std]
fn foo() {
    unsafe { core::ptr::drop_in_place(&mut 1); }
}

The only reason #82122 changes anything is because it removes the reexport, and causes two functions to exist instead (so the DefId path printing logic can't see any reexports anymore).

[eddyb]

You can also see it in this example (playground):

fn main() {
    std::collections::BTreeSet::<()>::new();
}

        _1 = BTreeSet::<()>::new() -> bb1; // scope 0 at src/main.rs:2:5: 2:44
                                         // mir::Constant
                                         // + span: src/main.rs:2:5: 2:42
                                         // + user_ty: UserType(0)
                                         // + literal: Const { ty: fn() -> std::collections::BTreeSet<()> {std::collections::BTreeSet::<()>::new}, val: Value(Scalar(<ZST>)) }

std::collections::btree_set::BTreeSet is publicly accessible, but the printing picks its reexport std::collections::BTreeSet.

[RalfJung]

Okay so seems like we should close this as not-a-bug? MIR is using the right DefId, printing is just making a somewhat odd choice?

[eddyb]

Yeah, and while you should be able to replicate with let () = function; normally, most of the user-facing diagnostics etc. were changed to use the opportunistic short path printing, so you can't even see it in there. Tho, hmm, give me a moment...

You can see it in the error message on 1.47 and earlier: https://godbolt.org/z/f6sj8d (i.e. before the short path changes).