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

Author: bend-n

Diff for: compiler/rustc_codegen_cranelift/example/example.rs

@@ -1,6 +1,6 @@
 #![feature(no_core, unboxed_closures)]
 #![no_core]
-#![allow(dead_code)]
+#![allow(dead_code, unnecessary_transmutes)]
 
 extern crate mini_core;
 

Diff for: compiler/rustc_codegen_gcc/example/example.rs

@@ -1,6 +1,6 @@
 #![feature(no_core, unboxed_closures)]
 #![no_core]
-#![allow(dead_code)]
+#![allow(dead_code, unnecessary_transmutes)]
 
 extern crate mini_core;
 
@@ -11,11 +11,7 @@ fn abc(a: u8) -> u8 {
 }
 
 fn bcd(b: bool, a: u8) -> u8 {
-    if b {
-        a * 2
-    } else {
-        a * 3
-    }
+    if b { a * 2 } else { a * 3 }
 }
 
 fn call() {

Diff for: compiler/rustc_lint_defs/src/builtin.rs

@@ -119,6 +119,7 @@ declare_lint_pass! {
         UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
         UNNAMEABLE_TEST_ITEMS,
         UNNAMEABLE_TYPES,
+        UNNECESSARY_TRANSMUTES,
         UNREACHABLE_CODE,
         UNREACHABLE_PATTERNS,
         UNSAFE_ATTR_OUTSIDE_UNSAFE,
@@ -4944,6 +4945,30 @@ declare_lint! {
     "detects pointer to integer transmutes in const functions and associated constants",
 }
 
+declare_lint! {
+    /// The `unnecessary_transmutes` lint detects transmutations that have safer alternatives.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// fn bytes_at_home(x: [u8; 4]) -> u32 {
+    ///   unsafe { std::mem::transmute(x) }
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Using an explicit method is preferable over calls to
+    /// [`transmute`](https://doc.rust-lang.org/std/mem/fn.transmute.html) as
+    /// they more clearly communicate the intent, are easier to review, and
+    /// are less likely to accidentally result in unsoundness.
+    pub UNNECESSARY_TRANSMUTES,
+    Warn,
+    "detects transmutes that are shadowed by std methods"
+}
+
 declare_lint! {
     /// The `tail_expr_drop_order` lint looks for those values generated at the tail expression location,
     /// that runs a custom `Drop` destructor.

Diff for: compiler/rustc_mir_transform/messages.ftl

@@ -84,3 +84,4 @@ mir_transform_undefined_transmute = pointers cannot be transmuted to integers du
     .help = for more information, see https://doc.rust-lang.org/std/mem/fn.transmute.html
 
 mir_transform_unknown_pass_name = MIR pass `{$name}` is unknown and will be ignored
+mir_transform_unnecessary_transmute = unnecessary transmute

Diff for: compiler/rustc_mir_transform/src/check_unnecessary_transmutes.rs

@@ -0,0 +1,100 @@
+use rustc_middle::mir::visit::Visitor;
+use rustc_middle::mir::{Body, Location, Operand, Terminator, TerminatorKind};
+use rustc_middle::ty::*;
+use rustc_session::lint::builtin::UNNECESSARY_TRANSMUTES;
+use rustc_span::source_map::Spanned;
+use rustc_span::{Span, sym};
+
+use crate::errors::UnnecessaryTransmute as Error;
+
+/// Check for transmutes that overlap with stdlib methods.
+/// For example, transmuting `[u8; 4]` to `u32`.
+pub(super) struct CheckUnnecessaryTransmutes;
+
+impl<'tcx> crate::MirLint<'tcx> for CheckUnnecessaryTransmutes {
+    fn run_lint(&self, tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
+        let mut checker = UnnecessaryTransmuteChecker { body, tcx };
+        checker.visit_body(body);
+    }
+}
+
+struct UnnecessaryTransmuteChecker<'a, 'tcx> {
+    body: &'a Body<'tcx>,
+    tcx: TyCtxt<'tcx>,
+}
+
+impl<'a, 'tcx> UnnecessaryTransmuteChecker<'a, 'tcx> {
+    fn is_unnecessary_transmute(
+        &self,
+        function: &Operand<'tcx>,
+        arg: String,
+        span: Span,
+    ) -> Option<Error> {
+        let fn_sig = function.ty(self.body, self.tcx).fn_sig(self.tcx).skip_binder();
+        let [input] = fn_sig.inputs() else { return None };
+
+        let err = |sugg| Error { span, sugg, help: None };
+
+        Some(match (input.kind(), fn_sig.output().kind()) {
+            // dont check the length; transmute does that for us.
+            // [u8; _] => primitive
+            (Array(t, _), Uint(_) | Float(_) | Int(_)) if *t.kind() == Uint(UintTy::U8) => Error {
+                sugg: format!("{}::from_ne_bytes({arg})", fn_sig.output()),
+                help: Some(
+                    "there's also `from_le_bytes` and `from_be_bytes` if you expect a particular byte order",
+                ),
+                span,
+            },
+            // primitive => [u8; _]
+            (Uint(_) | Float(_) | Int(_), Array(t, _)) if *t.kind() == Uint(UintTy::U8) => Error {
+                sugg: format!("{input}::to_ne_bytes({arg})"),
+                help: Some(
+                    "there's also `to_le_bytes` and `to_be_bytes` if you expect a particular byte order",
+                ),
+                span,
+            },
+            // char → u32
+            (Char, Uint(UintTy::U32)) => err(format!("u32::from({arg})")),
+            // u32 → char
+            (Uint(UintTy::U32), Char) => Error {
+                sugg: format!("char::from_u32_unchecked({arg})"),
+                help: Some("consider `char::from_u32(…).unwrap()`"),
+                span,
+            },
+            // uNN → iNN
+            (Uint(ty), Int(_)) => err(format!("{}::cast_signed({arg})", ty.name_str())),
+            // iNN → uNN
+            (Int(ty), Uint(_)) => err(format!("{}::cast_unsigned({arg})", ty.name_str())),
+            // fNN → uNN
+            (Float(ty), Uint(..)) => err(format!("{}::to_bits({arg})", ty.name_str())),
+            // uNN → fNN
+            (Uint(_), Float(ty)) => err(format!("{}::from_bits({arg})", ty.name_str())),
+            // bool → { x8 }
+            (Bool, Int(..) | Uint(..)) => err(format!("({arg}) as {}", fn_sig.output())),
+            // u8 → bool
+            (Uint(_), Bool) => err(format!("({arg} == 1)")),
+            _ => return None,
+        })
+    }
+}
+
+impl<'tcx> Visitor<'tcx> for UnnecessaryTransmuteChecker<'_, 'tcx> {
+    // Check each block's terminator for calls to pointer to integer transmutes
+    // in const functions or associated constants and emit a lint.
+    fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
+        if let TerminatorKind::Call { func, args, .. } = &terminator.kind
+            && let [Spanned { span: arg, .. }] = **args
+            && let Some((func_def_id, _)) = func.const_fn_def()
+            && self.tcx.is_intrinsic(func_def_id, sym::transmute)
+            && let span = self.body.source_info(location).span
+            && let Some(lint) = self.is_unnecessary_transmute(
+                func,
+                self.tcx.sess.source_map().span_to_snippet(arg).expect("ok"),
+                span,
+            )
+            && let Some(hir_id) = terminator.source_info.scope.lint_root(&self.body.source_scopes)
+        {
+            self.tcx.emit_node_span_lint(UNNECESSARY_TRANSMUTES, hir_id, span, lint);
+        }
+    }
+}

Diff for: compiler/rustc_mir_transform/src/errors.rs

@@ -158,6 +158,26 @@ pub(crate) struct MustNotSuspendReason {
     pub reason: String,
 }
 
+pub(crate) struct UnnecessaryTransmute {
+    pub span: Span,
+    pub sugg: String,
+    pub help: Option<&'static str>,
+}
+
+// Needed for def_path_str
+impl<'a> LintDiagnostic<'a, ()> for UnnecessaryTransmute {
+    fn decorate_lint<'b>(self, diag: &'b mut rustc_errors::Diag<'a, ()>) {
+        diag.primary_message(fluent::mir_transform_unnecessary_transmute);
+        diag.span_suggestion(
+            self.span,
+            "replace this with",
+            self.sugg,
+            lint::Applicability::MachineApplicable,
+        );
+        self.help.map(|help| diag.help(help));
+    }
+}
+
 #[derive(LintDiagnostic)]
 #[diag(mir_transform_undefined_transmute)]
 #[note]

Diff for: compiler/rustc_mir_transform/src/lib.rs

@@ -126,6 +126,7 @@ declare_passes! {
     mod check_null : CheckNull;
     mod check_packed_ref : CheckPackedRef;
     mod check_undefined_transmutes : CheckUndefinedTransmutes;
+    mod check_unnecessary_transmutes: CheckUnnecessaryTransmutes;
     // This pass is public to allow external drivers to perform MIR cleanup
     pub mod cleanup_post_borrowck : CleanupPostBorrowck;
 
@@ -392,6 +393,7 @@ fn mir_built(tcx: TyCtxt<'_>, def: LocalDefId) -> &Steal<Body<'_>> {
             &Lint(check_const_item_mutation::CheckConstItemMutation),
             &Lint(function_item_references::FunctionItemReferences),
             &Lint(check_undefined_transmutes::CheckUndefinedTransmutes),
+            &Lint(check_unnecessary_transmutes::CheckUnnecessaryTransmutes),
             // What we need to do constant evaluation.
             &simplify::SimplifyCfg::Initial,
             &Lint(sanity_check::SanityCheck),

Diff for: library/alloctests/tests/fmt.rs

@@ -1,6 +1,7 @@
 #![deny(warnings)]
 // FIXME(static_mut_refs): Do not allow `static_mut_refs` lint
 #![allow(static_mut_refs)]
+#![cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
 
 use std::cell::RefCell;
 use std::fmt::{self, Write};

Diff for: library/core/src/char/convert.rs

@@ -21,6 +21,7 @@ pub(super) const fn from_u32(i: u32) -> Option<char> {
 /// Converts a `u32` to a `char`, ignoring validity. See [`char::from_u32_unchecked`].
 #[inline]
 #[must_use]
+#[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
 pub(super) const unsafe fn from_u32_unchecked(i: u32) -> char {
     // SAFETY: the caller must guarantee that `i` is a valid char value.
     unsafe {
@@ -221,6 +222,7 @@ impl FromStr for char {
 }
 
 #[inline]
+#[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
 const fn char_try_from_u32(i: u32) -> Result<char, CharTryFromError> {
     // This is an optimized version of the check
     // (i > MAX as u32) || (i >= 0xD800 && i <= 0xDFFF),

Diff for: library/core/src/intrinsics/mod.rs

@@ -1495,6 +1495,7 @@ pub const fn forget<T: ?Sized>(_: T);
 /// Turning raw bytes (`[u8; SZ]`) into `u32`, `f64`, etc.:
 ///
 /// ```
+/// # #![cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
 /// let raw_bytes = [0x78, 0x56, 0x34, 0x12];
 ///
 /// let num = unsafe {

Diff for: library/core/src/num/f128.rs

@@ -898,6 +898,7 @@ impl f128 {
     #[inline]
     #[unstable(feature = "f128", issue = "116909")]
     #[must_use = "this returns the result of the operation, without modifying the original"]
+    #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
     pub const fn to_bits(self) -> u128 {
         // SAFETY: `u128` is a plain old datatype so we can always transmute to it.
         unsafe { mem::transmute(self) }
@@ -945,6 +946,7 @@ impl f128 {
     #[inline]
     #[must_use]
     #[unstable(feature = "f128", issue = "116909")]
+    #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
     pub const fn from_bits(v: u128) -> Self {
         // It turns out the safety issues with sNaN were overblown! Hooray!
         // SAFETY: `u128` is a plain old datatype so we can always transmute from it.

Diff for: library/core/src/num/f16.rs

@@ -886,6 +886,7 @@ impl f16 {
     #[inline]
     #[unstable(feature = "f16", issue = "116909")]
     #[must_use = "this returns the result of the operation, without modifying the original"]
+    #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
     pub const fn to_bits(self) -> u16 {
         // SAFETY: `u16` is a plain old datatype so we can always transmute to it.
         unsafe { mem::transmute(self) }
@@ -932,6 +933,7 @@ impl f16 {
     #[inline]
     #[must_use]
     #[unstable(feature = "f16", issue = "116909")]
+    #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
     pub const fn from_bits(v: u16) -> Self {
         // It turns out the safety issues with sNaN were overblown! Hooray!
         // SAFETY: `u16` is a plain old datatype so we can always transmute from it.

Diff for: library/core/src/num/f32.rs

@@ -705,8 +705,7 @@ impl f32 {
     pub const fn is_sign_negative(self) -> bool {
         // IEEE754 says: isSignMinus(x) is true if and only if x has negative sign. isSignMinus
         // applies to zeros and NaNs as well.
-        // SAFETY: This is just transmuting to get the sign bit, it's fine.
-        unsafe { mem::transmute::<f32, u32>(self) & 0x8000_0000 != 0 }
+        self.to_bits() & 0x8000_0000 != 0
     }
 
     /// Returns the least number greater than `self`.
@@ -1091,6 +1090,7 @@ impl f32 {
     #[stable(feature = "float_bits_conv", since = "1.20.0")]
     #[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
     #[inline]
+    #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
     pub const fn to_bits(self) -> u32 {
         // SAFETY: `u32` is a plain old datatype so we can always transmute to it.
         unsafe { mem::transmute(self) }
@@ -1136,6 +1136,7 @@ impl f32 {
     #[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
     #[must_use]
     #[inline]
+    #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
     pub const fn from_bits(v: u32) -> Self {
         // It turns out the safety issues with sNaN were overblown! Hooray!
         // SAFETY: `u32` is a plain old datatype so we can always transmute from it.

Diff for: library/core/src/num/f64.rs

@@ -713,8 +713,7 @@ impl f64 {
     pub const fn is_sign_negative(self) -> bool {
         // IEEE754 says: isSignMinus(x) is true if and only if x has negative sign. isSignMinus
         // applies to zeros and NaNs as well.
-        // SAFETY: This is just transmuting to get the sign bit, it's fine.
-        unsafe { mem::transmute::<f64, u64>(self) & Self::SIGN_MASK != 0 }
+        self.to_bits() & Self::SIGN_MASK != 0
     }
 
     #[must_use]
@@ -1089,6 +1088,7 @@ impl f64 {
                   without modifying the original"]
     #[stable(feature = "float_bits_conv", since = "1.20.0")]
     #[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
+    #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
     #[inline]
     pub const fn to_bits(self) -> u64 {
         // SAFETY: `u64` is a plain old datatype so we can always transmute to it.
@@ -1135,6 +1135,7 @@ impl f64 {
     #[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
     #[must_use]
     #[inline]
+    #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
     pub const fn from_bits(v: u64) -> Self {
         // It turns out the safety issues with sNaN were overblown! Hooray!
         // SAFETY: `u64` is a plain old datatype so we can always transmute from it.

Diff for: library/core/src/num/int_macros.rs

@@ -3678,6 +3678,7 @@ macro_rules! int_impl {
         /// ```
         #[stable(feature = "int_to_from_bytes", since = "1.32.0")]
         #[rustc_const_stable(feature = "const_int_conversion", since = "1.44.0")]
+        #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
         // SAFETY: const sound because integers are plain old datatypes so we can always
         // transmute them to arrays of bytes
         #[must_use = "this returns the result of the operation, \
@@ -3781,6 +3782,7 @@ macro_rules! int_impl {
         /// ```
         #[stable(feature = "int_to_from_bytes", since = "1.32.0")]
         #[rustc_const_stable(feature = "const_int_conversion", since = "1.44.0")]
+        #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
         #[must_use]
         // SAFETY: const sound because integers are plain old datatypes so we can always
         // transmute to them

Diff for: library/core/src/num/uint_macros.rs

@@ -3523,6 +3523,7 @@ macro_rules! uint_impl {
         #[rustc_const_stable(feature = "const_int_conversion", since = "1.44.0")]
         #[must_use = "this returns the result of the operation, \
                       without modifying the original"]
+        #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
         // SAFETY: const sound because integers are plain old datatypes so we can always
         // transmute them to arrays of bytes
         #[inline]
@@ -3624,6 +3625,7 @@ macro_rules! uint_impl {
         /// ```
         #[stable(feature = "int_to_from_bytes", since = "1.32.0")]
         #[rustc_const_stable(feature = "const_int_conversion", since = "1.44.0")]
+        #[cfg_attr(not(bootstrap), allow(unnecessary_transmutes))]
         #[must_use]
         // SAFETY: const sound because integers are plain old datatypes so we can always
         // transmute to them

Diff for: src/tools/clippy/tests/ui/blocks_in_conditions.fixed

@@ -3,6 +3,7 @@
 #![warn(clippy::blocks_in_conditions)]
 #![allow(
     unused,
+    unnecessary_transmutes,
     clippy::let_and_return,
     clippy::needless_if,
     clippy::missing_transmute_annotations

Diff for: src/tools/clippy/tests/ui/blocks_in_conditions.rs

@@ -3,6 +3,7 @@
 #![warn(clippy::blocks_in_conditions)]
 #![allow(
     unused,
+    unnecessary_transmutes,
     clippy::let_and_return,
     clippy::needless_if,
     clippy::missing_transmute_annotations