From 722d284e38a23d1afe3311d9ae7444dfb8e26ccf Mon Sep 17 00:00:00 2001 From: bendn Date: Sun, 26 Jan 2025 11:10:35 +0700 Subject: [PATCH] Suggest {to,from}_ne_bytes for transmutations between arrays and integers, etc --- compiler/rustc_lint_defs/src/builtin.rs | 8 ++ compiler/rustc_mir_transform/messages.ftl | 2 + .../src/check_redundant_transmutes.rs | 114 ++++++++++++++++++ compiler/rustc_mir_transform/src/errors.rs | 20 +++ compiler/rustc_mir_transform/src/lib.rs | 2 + library/core/src/num/int_macros.rs | 2 + library/core/src/num/uint_macros.rs | 2 + 7 files changed, 150 insertions(+) create mode 100644 compiler/rustc_mir_transform/src/check_redundant_transmutes.rs diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 5cdd18f5ea903..3a7769e720ffc 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -83,6 +83,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, @@ -5034,6 +5035,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. diff --git a/compiler/rustc_mir_transform/messages.ftl b/compiler/rustc_mir_transform/messages.ftl index 5628f4c9381b3..68f39bd9fca7c 100644 --- a/compiler/rustc_mir_transform/messages.ftl +++ b/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 diff --git a/compiler/rustc_mir_transform/src/check_redundant_transmutes.rs b/compiler/rustc_mir_transform/src/check_redundant_transmutes.rs new file mode 100644 index 0000000000000..942ba0b36a37d --- /dev/null +++ b/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 { + 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); + } + } +} diff --git a/compiler/rustc_mir_transform/src/errors.rs b/compiler/rustc_mir_transform/src/errors.rs index a2fd46043ca0f..c0126ef69daed 100644 --- a/compiler/rustc_mir_transform/src/errors.rs +++ b/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] diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 2dc55e3614e6c..6a58930eae8f4 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -120,6 +120,7 @@ declare_passes! { mod check_const_item_mutation : CheckConstItemMutation; 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; @@ -383,6 +384,7 @@ fn mir_built(tcx: TyCtxt<'_>, def: LocalDefId) -> &Steal> { &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), diff --git a/library/core/src/num/int_macros.rs b/library/core/src/num/int_macros.rs index 96a290ad5a09d..604108adab3f7 100644 --- a/library/core/src/num/int_macros.rs +++ b/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 diff --git a/library/core/src/num/uint_macros.rs b/library/core/src/num/uint_macros.rs index c8433b3bb168a..f5ace5e804bdc 100644 --- a/library/core/src/num/uint_macros.rs +++ b/library/core/src/num/uint_macros.rs @@ -3422,6 +3422,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] @@ -3523,6 +3524,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