Skip to content

Commit

Permalink
Suggest {to,from}_ne_bytes for transmutations between arrays and inte…
Browse files Browse the repository at this point in the history
…gers, etc
  • Loading branch information
bend-n committed Mar 7, 2025
1 parent 91a0e16 commit 9ec94aa
Show file tree
Hide file tree
Showing 27 changed files with 591 additions and 20 deletions.
23 changes: 23 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ declare_lint_pass! {
PUB_USE_OF_PRIVATE_EXTERN_CRATE,
REDUNDANT_IMPORTS,
REDUNDANT_LIFETIMES,
REDUNDANT_TRANSMUTATION,
REFINING_IMPL_TRAIT_INTERNAL,
REFINING_IMPL_TRAIT_REACHABLE,
RENAMED_AND_REMOVED_LINTS,
Expand Down Expand Up @@ -4980,6 +4981,28 @@ declare_lint! {
"detects pointer to integer transmutes in const functions and associated constants",
}

declare_lint! {
/// The `redundant_transmutation` lint detects transmutations that have safer alternatives.
///
/// ### Example
///
/// ```
/// fn bytes_at_home(x: [u8; 4]) -> u32 {
/// transmute(x)
/// }
/// ```
///
/// {{produces}}
///
/// ## Explanation
///
/// People dont realize that safer methods such as [`u32::to_ne_bytes`] exist,
/// so this lint exists to lint on cases where people write transmutes that dont need to be there.
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.
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_mir_transform/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ mir_transform_must_not_suspend = {$pre}`{$def_path}`{$post} held across a suspen
.help = consider using a block (`{"{ ... }"}`) to shrink the value's scope, ending before the suspend point
mir_transform_operation_will_panic = this operation will panic at runtime
mir_transform_redundant_transmute = this transmute could be performed safely
mir_transform_tail_expr_drop_order = relative drop order changing in Rust 2024
.temporaries = in Rust 2024, this temporary value will be dropped first
.observers = in Rust 2024, this local variable or temporary value will be dropped second
Expand Down
103 changes: 103 additions & 0 deletions compiler/rustc_mir_transform/src/check_redundant_transmutes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{Body, Location, Operand, Terminator, TerminatorKind};
use rustc_middle::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::RedundantTransmute as Error;

/// 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> {
fn is_redundant_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_ne_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_ne_bytes` if you expect a particular byte order",
),
span,
},
// char → u32
(Char, Uint(UintTy::U32)) => err(format!("({arg}) as u32")),
// 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 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);
}
}
}
20 changes: 20 additions & 0 deletions compiler/rustc_mir_transform/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,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;

Expand Down Expand Up @@ -387,6 +388,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),
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/char/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(redundant_transmutation))]
pub(super) const unsafe fn from_u32_unchecked(i: u32) -> char {
// SAFETY: the caller must guarantee that `i` is a valid char value.
unsafe {
Expand Down Expand Up @@ -229,6 +230,7 @@ impl FromStr for char {
}

#[inline]
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
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),
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/num/f128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,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(redundant_transmutation))]
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) }
Expand Down Expand Up @@ -948,6 +949,7 @@ impl f128 {
#[inline]
#[must_use]
#[unstable(feature = "f128", issue = "116909")]
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
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.
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/num/f16.rs
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,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(redundant_transmutation))]
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) }
Expand Down Expand Up @@ -935,6 +936,7 @@ impl f16 {
#[inline]
#[must_use]
#[unstable(feature = "f16", issue = "116909")]
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
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.
Expand Down
5 changes: 3 additions & 2 deletions library/core/src/num/f32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,8 +708,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`.
Expand Down Expand Up @@ -1093,6 +1092,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(redundant_transmutation))]
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) }
Expand Down Expand Up @@ -1138,6 +1138,7 @@ impl f32 {
#[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
#[must_use]
#[inline]
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
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.
Expand Down
5 changes: 3 additions & 2 deletions library/core/src/num/f64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,8 +716,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]
Expand Down Expand Up @@ -1092,6 +1091,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(redundant_transmutation))]
#[inline]
pub const fn to_bits(self) -> u64 {
// SAFETY: `u64` is a plain old datatype so we can always transmute to it.
Expand Down Expand Up @@ -1138,6 +1138,7 @@ impl f64 {
#[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
#[must_use]
#[inline]
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
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.
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/num/int_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(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, \
Expand Down Expand Up @@ -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(redundant_transmutation))]
#[must_use]
// SAFETY: const sound because integers are plain old datatypes so we can always
// transmute to them
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/num/uint_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3518,6 +3518,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]
Expand Down Expand Up @@ -3619,6 +3620,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
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/pass/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#![feature(f128)]
#![feature(f16)]
#![allow(arithmetic_overflow)]
#![allow(internal_features)]
#![allow(internal_features, redundant_transmutation)]

use std::any::type_name;
use std::cmp::min;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/const-eval/raw-bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//@ ignore-endian-big
// ignore-tidy-linelength
//@ normalize-stderr: "╾─*ALLOC[0-9]+(\+[a-z0-9]+)?(<imm>)?─*╼" -> "╾ALLOC_ID$1╼"
#![allow(invalid_value)]
#![allow(invalid_value, redundant_transmutation)]
#![feature(never_type, rustc_attrs, ptr_metadata, slice_from_ptr_range, const_slice_from_ptr_range)]

use std::mem;
Expand Down
1 change: 1 addition & 0 deletions tests/ui/consts/const-eval/transmute-const-promotion.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(redundant_transmutation)]
use std::mem;

fn main() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0716]: temporary value dropped while borrowed
--> $DIR/transmute-const-promotion.rs:4:37
--> $DIR/transmute-const-promotion.rs:5:37
|
LL | let x: &'static u32 = unsafe { &mem::transmute(3.0f32) };
| ------------ ^^^^^^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
Expand Down
1 change: 1 addition & 0 deletions tests/ui/consts/const-eval/transmute-const.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(redundant_transmutation)]
use std::mem;

static FOO: bool = unsafe { mem::transmute(3u8) };
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/const-eval/transmute-const.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0080]: it is undefined behavior to use this value
--> $DIR/transmute-const.rs:3:1
--> $DIR/transmute-const.rs:4:1
|
LL | static FOO: bool = unsafe { mem::transmute(3u8) };
| ^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0x03, but expected a boolean
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/const-eval/ub-enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//@ normalize-stderr: "([0-9a-f][0-9a-f] |╾─*ALLOC[0-9]+(\+[a-z0-9]+)?(<imm>)?─*╼ )+ *│.*" -> "HEX_DUMP"
//@ normalize-stderr: "0x0+" -> "0x0"
#![feature(never_type)]
#![allow(invalid_value)]
#![allow(invalid_value, redundant_transmutation)]

use std::mem;

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/const-eval/ub-wide-ptr.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// ignore-tidy-linelength
#![allow(unused)]
#![allow(unused, redundant_transmutation)]
#![feature(ptr_metadata)]

use std::{ptr, mem};
Expand Down
1 change: 1 addition & 0 deletions tests/ui/consts/extra-const-ub/detect-extra-ub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//@ [no_flag] check-pass
//@ [with_flag] compile-flags: -Zextra-const-ub-checks
#![feature(never_type)]
#![allow(redundant_transmutation)]

use std::mem::transmute;
use std::ptr::addr_of;
Expand Down
Loading

0 comments on commit 9ec94aa

Please sign in to comment.