Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/example/example.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![feature(no_core, unboxed_closures)]
#![no_core]
#![allow(dead_code)]
#![allow(dead_code, redundant_transmutation)]

extern crate mini_core;

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/example/example.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![feature(no_core, unboxed_closures)]
#![no_core]
#![allow(dead_code)]
#![allow(dead_code, redundant_transmutation)]

extern crate mini_core;

Expand Down
24 changes: 24 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,29 @@ 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
///
/// ```rust
/// fn bytes_at_home(x: [u8; 4]) -> u32 {
/// unsafe { std::mem::transmute(x) }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// People dont realize that safer methods such as
/// [`u32::to_ne_bytes`](https://doc.rust-lang.org/std/primitive.u32.html#method.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!("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 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
1 change: 1 addition & 0 deletions library/alloctests/tests/fmt.rs
Original file line number Diff line number Diff line change
@@ -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(redundant_transmutation))]

use std::cell::RefCell;
use std::fmt::{self, Write};
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 @@ -221,6 +222,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
1 change: 1 addition & 0 deletions library/core/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,7 @@ pub const fn forget<T: ?Sized>(_: T);
/// Turning raw bytes (`[u8; SZ]`) into `u32`, `f64`, etc.:
///
/// ```
/// # #![cfg_attr(not(bootstrap), allow(redundant_transmutation))]
/// let raw_bytes = [0x78, 0x56, 0x34, 0x12];
///
/// let num = unsafe {
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
1 change: 1 addition & 0 deletions src/tools/clippy/tests/ui/blocks_in_conditions.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![warn(clippy::blocks_in_conditions)]
#![allow(
unused,
redundant_transmutation,
clippy::let_and_return,
clippy::needless_if,
clippy::missing_transmute_annotations
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/tests/ui/blocks_in_conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![warn(clippy::blocks_in_conditions)]
#![allow(
unused,
redundant_transmutation,
clippy::let_and_return,
clippy::needless_if,
clippy::missing_transmute_annotations
Expand Down
Loading
Loading