Add lint on reference-to-pointer transmutes

[scottmcm]

Inspired by https://github.com/rust-lang/rust/issues/124861, specifically https://github.com/glium/glium/commit/ebdc18e06570d685884b68b8010b01d2aece8e3a

There's no reason to use an unsafe transmute to get a pointer from a reference, and as that code example shows it can easily hide bugs. We should (at least) warn about it.

• For &T to *const T, recommend https://doc.rust-lang.org/std/ptr/fn.from_ref.html (machine-applicable, because it's always right)
• For &mut T to *mut T, recommend https://doc.rust-lang.org/std/ptr/fn.from_mut.html (machine-applicable, because it's always right)
• For &T to *mut T, recommend from_ref+cast_mut, but not auto-applicable because we should comment that it's probably wrong, and they probably want from_mut instead, but that will require larger reworking of the code.
• For &mut T to *const T, recommend https://doc.rust-lang.org/std/ptr/fn.from_mut.html with a note that in a generic context it might also need .cast_const().

[kpreid]

• For &T to *mut T, recommend from_ref+cast_mut, but not auto-applicable because we should comment that it's probably wrong,

I recommend not offering such a suggestion in this case. People are often too quick to accept suggestions that successfully remove warnings, and it would be bad to transform simple wrong code into complex wrong code.

[digama0]

Why are none of the options "use a coercion"? In code review I would generally prefer this over from_ref; there may be some complications around reborrowing but it's not clear to me that the same issues don't also hold for from_ref. There are also type inference issues, so perhaps from_ref should only be suggested when the types would not be inferred correctly without it?

[scottmcm]

@digama0 I was assuming that if they wrote a transmute specifically then it's not a place where a coercion work, as presumably they would have tried that first. Even if it would, I think I'd rather suggest a function here that doesn't need changing the flow of things (or adding a ({ let temp: *const T = expr; temp }) or whatever). They can always change it after they read the docs for from_ref or from_mut if they want, but in general I think we should be recommending very specific "do one thing" functions where possible. Certainly it shouldn't suggest as *const _, at the very least.

Maybe if we had stable type ascription I'd lean that way, but since we don't, I like recommending the methods here for the same reason that Ralf added them in #104977. After all, even coercions can be footguns, as seen with the &mut T-to-*const T coercion.

[workingjubilee]

Yes, we should almost certainly lint on the last case precisely because the coercion is also wrong.

[digama0]

@scottmcm The original bugfix which started this is exactly a case where the right solution was to use a coercion, but mind you a coercion from &mut local instead of &local. If the linter is limited to not ask the question of asking where &local came from and whether it ought to be made mutable (instead proposing things like from_ref and a cast) I think it is considerably less useful, since that's just another way to silence the lint without fixing the actual UB.

Maybe if we had stable type ascription I'd lean that way, but since we don't, I like recommending the methods here for the same reason that Ralf added them in https://github.com/rust-lang/rust/pull/104977. After all, even coercions can be footguns, as seen with the &mut T-to-*const T coercion.

I'm not suggesting type ascriptions or anything like that, I mean when using the coercion alone with no additional annotation does the right thing according to our analysis of the situation in the given context (no changed types, no &mut T to *const T, only cases exactly equivalent to the from_ref we were otherwise going to suggest anyway). Ralf's stated motivation in #104977 was to avoid as-casts, and I'm not talking about using any casts. I mean literally passing x: &T to a function expecting *const T or the mut version of that. As far as I know this usage of coercion is considered as fine and good and there are not really any hazards in it which are not already shared by any other mechanism for converting a &T to a *const T.

[Urgau]

I tried, sometime ago, to add a lint that would do that, but ran into a issue,

• For &mut T to *mut T, recommend https://doc.rust-lang.org/std/ptr/fn.from_mut.html (machine-applicable, because it's always right)

because recommending from_mut is not always right/straight forward, and can lead to unsoundness in code that didn't have it before:

fn main() {
    let _: *mut i32 = &mut 1 as *mut _; // this line is fine
    let _: *mut i32 = std::ptr::from_mut(&mut 1); // but this one isn't, the
                                                  // pointer is dangling
}

the solution to that would be to have temporary like this:

fn main() {
    let tmp0 = &mut 1;
    let _: *mut i32 = std::ptr::from_mut(tmp0);
}

but this begs the question of usability.

[workingjubilee]

That's not a transmute.