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

Author: bend-n

compiler/rustc_lint_defs/src/builtin.rs

@@ -82,6 +82,7 @@ declare_lint_pass! {
         PROC_MACRO_DERIVE_RESOLUTION_FALLBACK,
         PTR_CAST_ADD_AUTO_TO_OBJECT,
         PTR_TO_INTEGER_TRANSMUTE_IN_CONSTS,
+        REDUNDANT_TRANSMUTATION,
         PUB_USE_OF_PRIVATE_EXTERN_CRATE,
         REDUNDANT_IMPORTS,
         REDUNDANT_LIFETIMES,
@@ -4950,6 +4951,13 @@ declare_lint! {
     "detects pointer to integer transmutes in const functions and associated constants",
 }
 
+declare_lint! {
+    /// Wow such docs.
+    pub REDUNDANT_TRANSMUTATION,
+    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.

compiler/rustc_mir_transform/messages.ftl

@@ -83,4 +83,6 @@ mir_transform_undefined_transmute = pointers cannot be transmuted to integers du
     .note2 = avoiding this restriction via `union` or raw pointers leads to compile-time undefined behavior
     .help = for more information, see https://doc.rust-lang.org/std/mem/fn.transmute.html
 
+mir_transform_redundant_transmute = this transmute could be performed safely
+
 mir_transform_unknown_pass_name = MIR pass `{$name}` is unknown and will be ignored

compiler/rustc_mir_transform/src/check_redundant_transmutes.rs

@@ -0,0 +1,114 @@
+use rustc_middle::mir::visit::Visitor;
+use rustc_middle::mir::{Body, Location, Operand, Terminator, TerminatorKind};
+use rustc_middle::ty::{Ty, TyCtxt, UintTy};
+use rustc_session::lint::builtin::REDUNDANT_TRANSMUTATION;
+use rustc_span::source_map::Spanned;
+use rustc_span::{Span, sym};
+use rustc_type_ir::TyKind::*;
+
+use crate::errors;
+
+/// Check for transmutes that overlap with stdlib methods.
+/// For example, transmuting `[u8; 4]` to `u32`.
+pub(super) struct CheckRedundantTransmutes;
+
+impl<'tcx> crate::MirLint<'tcx> for CheckRedundantTransmutes {
+    fn run_lint(&self, tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
+        let mut checker = RedundantTransmutesChecker { body, tcx };
+        checker.visit_body(body);
+    }
+}
+
+struct RedundantTransmutesChecker<'a, 'tcx> {
+    body: &'a Body<'tcx>,
+    tcx: TyCtxt<'tcx>,
+}
+
+impl<'a, 'tcx> RedundantTransmutesChecker<'a, 'tcx> {
+    /// This functions checks many things:
+    /// 1. if the source (`transmute::<$x, _>`) is `[u8; _]`, check if the output is a `{uif}xx`
+    /// 2. swap and do the same check.
+    /// 3. in the case of `char` → `u32` suggest `to_u32` and `from_u32_unchecked`
+    /// 4. `uxx` → `ixx` should be `as`
+    /// Returns the replacement.
+    fn is_redundant_transmute(
+        &self,
+        function: &Operand<'tcx>,
+        arg: String,
+        span: Span,
+    ) -> Option<errors::RedundantTransmute> {
+        let fn_sig = function.ty(self.body, self.tcx).fn_sig(self.tcx).skip_binder();
+        let [input] = fn_sig.inputs() else { return None };
+
+        // Checks if `x` is `[u8; _]`
+        let is_u8s = |x: Ty<'tcx>| matches!(x.kind(), Array(t, _) if *t.kind() == Uint(UintTy::U8));
+        // dont check the length; transmute does that for us.
+        if is_u8s(*input) && matches!(fn_sig.output().kind(), Uint(..) | Float(_) | Int(_)) {
+            // FIXME: get the whole expression out?
+            return Some(errors::RedundantTransmute {
+                sugg: format!("{}::from_ne_bytes({arg})", fn_sig.output()),
+                help: Some(
+                    "there's also `from_le_bytes` and `from_ne_bytes` if you expect a particular byte order",
+                ),
+                span,
+            });
+        }
+        if is_u8s(fn_sig.output()) && matches!(input.kind(), Uint(..) | Float(_) | Int(_)) {
+            return Some(errors::RedundantTransmute {
+                sugg: format!("{input}::to_ne_bytes({arg})"),
+                help: Some(
+                    "there's also `to_le_bytes` and `to_ne_bytes` if you expect a particular byte order",
+                ),
+                span,
+            });
+        }
+        return match input.kind() {
+            // char → u32
+            Char => matches!(fn_sig.output().kind(), Uint(UintTy::U32)).then(|| {
+                errors::RedundantTransmute { sugg: format!("({arg}) as u32"), help: None, span }
+            }),
+            // u32 → char
+            Uint(UintTy::U32) if *fn_sig.output().kind() == Char => {
+                Some(errors::RedundantTransmute {
+                    sugg: format!("char::from_u32_unchecked({arg})"),
+                    help: Some("consider `char::from_u32(…).unwrap()`"),
+                    span,
+                })
+            }
+            // bool → (uNN ↔ iNN)
+            Bool | Uint(..) | Int(..) => {
+                matches!(fn_sig.output().kind(), Int(..) | Uint(..)).then(|| {
+                    errors::RedundantTransmute {
+                        sugg: format!("({arg}) as {}", fn_sig.output()),
+                        help: None,
+                        span,
+                    }
+                })
+            }
+            _ => None,
+        };
+    }
+}
+
+impl<'tcx> Visitor<'tcx> for RedundantTransmutesChecker<'_, '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_redundant_transmute(
+                func,
+                self.tcx.sess.source_map().span_to_snippet(arg).expect("ok"),
+                span,
+            )
+            && let Some(call_id) = self.body.source.def_id().as_local()
+        {
+            let hir_id = self.tcx.local_def_id_to_hir_id(call_id);
+
+            self.tcx.emit_node_span_lint(REDUNDANT_TRANSMUTATION, hir_id, span, lint);
+        }
+    }
+}

compiler/rustc_mir_transform/src/errors.rs

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

compiler/rustc_mir_transform/src/lib.rs

@@ -123,6 +123,7 @@ declare_passes! {
     mod check_null : CheckNull;
     mod check_packed_ref : CheckPackedRef;
     mod check_undefined_transmutes : CheckUndefinedTransmutes;
+    mod check_redundant_transmutes: CheckRedundantTransmutes;
     // This pass is public to allow external drivers to perform MIR cleanup
     pub mod cleanup_post_borrowck : CleanupPostBorrowck;
 
@@ -385,6 +386,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_redundant_transmutes::CheckRedundantTransmutes),
             // What we need to do constant evaluation.
             &simplify::SimplifyCfg::Initial,
             &Lint(sanity_check::SanityCheck),

library/core/src/num/int_macros.rs

@@ -3632,6 +3632,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(redundant_transmutation))]
         // 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, \
@@ -3735,6 +3736,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(redundant_transmutation))]
         #[must_use]
         // SAFETY: const sound because integers are plain old datatypes so we can always
         // transmute to them

library/core/src/num/uint_macros.rs

@@ -3473,6 +3473,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(redundant_transmutation))]
         // SAFETY: const sound because integers are plain old datatypes so we can always
         // transmute them to arrays of bytes
         #[inline]
@@ -3574,6 +3575,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(redundant_transmutation))]
         #[must_use]
         // SAFETY: const sound because integers are plain old datatypes so we can always
         // transmute to them