Suggest {to,from}_ne_bytes for transmutations between arrays and integers, etc

[bend-n]

implements #136067

Rust has helper methods for many kinds of safe transmutes, for example integer<->bytes. This is a lint against using transmute for these cases.

fn bytes_at_home(x: [u8; 4]) -> u32 {
   transmute(x)
}

// other examples
transmute::<[u8; 2], u16>();
transmute::<[u8; 8], f64>();
transmute::<u32, [u8; 4]>();
transmute::<char, u32>();
transmute::<u32, char>();

It would be handy to suggest u32::from_ne_bytes(x).
This is implemented for [u8; _] -> {float int}

This also implements the cases:
fXX <-> uXX = {from_bits, to_bits}
uXX -> iXX via cast_unsigned and cast_signed
{char -> u32, bool -> n8} via from
u32 -> char via from_u32_unchecked (note: notes from_u32().unwrap()) (contested)
u8 -> bool via == (debatable)

[rustbot]

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[bend-n]

how do i fix stdarch?

[juntyr]

I wouldn't suggest an as conversion here as as is so overloaded that I at least always forget what each conversion does. I remember there was an attempt in the past to create explicit methods for each as behaviour - if they've been merged, perhaps we could recommend them?

[bend-n]

I'm not sure what methods you're referring to... I don't think i should suggest i32::from_ne_bytes(x.to_ne_bytes())? And try_from has different behavior.

[juntyr]

There are the unstable cast_unsigned and cast_signed methods, and there's an ACP for additional methods rust-lang/libs-team#453.

Perhaps for now the code just gets a FIXME so that the recommendation is changed once #125882 is stabilized (feature integer_sign_cast)?

[scottmcm]

The cast_(un)signed methods will be stable on nightly in another 2 days: #125882 (comment)

So maybe we just hold off on merging this PR until that stabilization PR lands? I'm also not a fan of as, so would like to avoid suggesting something that clippy will soon then start suggesting to change again.

[bend-n]

oh wow! i guess ill make it suggest those methods then

[bend-n]

what about bool as integer though?

[joshtriplett]

@bend-n

    let b: bool = true;
    let x = u8::from(b);
    assert_eq!(x, 1);

.into() would be nicer but depends on knowing u8 or i8 from type inference. u8::from and i8::from work in any context, though they don't work in postfix position. For the purposes of this lint, though, the only integer types you can transmute a bool to would be u8 or i8, and transmute already isn't postfix, so I'd suggest that transmutes from bool to u8/i8 should suggest u8::from and i8::from, respectively.

[bend-n]

i can do that with u32::from(char) i think, as well

[Noratrieb]

Is there a reason you made it a MIR lint instead of a HIR lint? HIR lints will make it easier to do the spans.

[bend-n]

@Noratrieb i dont know the difference; i looked for transmute and copied that (ptr_to_integer_transmute_in_consts) lint.

[bend-n]

Also, how should i fix stdarch? Should i make a PR over there to remove the unnecessary trans?

[Noratrieb]

I would first recommend checking out if the lang team wants this lint.
Can you write out a clear PR description with the exact cases you're covering and some examples for the lang team?

[joshtriplett]

We talked about this briefly in today's @rust-lang/lang meeting. This seems likely to always be desirable, and it helps people rewrite unsafe code as safe code. Let's give it a try in nightly and see how it goes! (In particular, that'll let us evaluate possible false positives.)

[scottmcm]

u32 -> char via from_u32_unchecked

I'm slightly torn on this one. It feels more like a clippy lint to me, because to me what we're approving here as a lang team is "suggesting safe alternatives to unsafe transmutes", not "well there's a different unsafe version that would be better" -- it's the removing of unsafe that gets it to the "yes this is in rustc" bar, in my mind. (Other team members may disagree, however.)

[traviscross]

I'm torn on it too. It is marginally safer, because then at least it goes through the asssert_unsafe_precondition:

pub(super) const unsafe fn from_u32_unchecked(i: u32) -> char {
    // SAFETY: the caller must guarantee that `i` is a valid char value.
    unsafe {
        assert_unsafe_precondition!(
            check_language_ub,
            "invalid value for `char`",
            (i: u32 = i) => char_try_from_u32(i).is_ok()
        );
        transmute(i)
    }
}

Playground link

The assertion only fires on debug builds, though.

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[lcnr]

@bors r+ rollup

[bors]

📌 Commit 8e94318 has been approved by lcnr

It is now in the queue for this repository.

[lcnr]

we should likely remove these lints from clippy once this is merged, shouldn't we?

[Zalathar]

Looks like this failed in rollup: #139009 (comment)

@bors r-