Skip to content

Commit a79dfb9

Browse files
committed
Suggest {to,from}_ne_bytes for transmutations between arrays and integers, etc
1 parent 91a0e16 commit a79dfb9

26 files changed

+590
-19
lines changed

compiler/rustc_lint_defs/src/builtin.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ declare_lint_pass! {
8484
PUB_USE_OF_PRIVATE_EXTERN_CRATE,
8585
REDUNDANT_IMPORTS,
8686
REDUNDANT_LIFETIMES,
87+
REDUNDANT_TRANSMUTATION,
8788
REFINING_IMPL_TRAIT_INTERNAL,
8889
REFINING_IMPL_TRAIT_REACHABLE,
8990
RENAMED_AND_REMOVED_LINTS,
@@ -4980,6 +4981,28 @@ declare_lint! {
49804981
"detects pointer to integer transmutes in const functions and associated constants",
49814982
}
49824983

4984+
declare_lint! {
4985+
/// The `redundant_transmutation` lint detects transmutations that have safer alternatives.
4986+
///
4987+
/// ### Example
4988+
///
4989+
/// ```
4990+
/// fn bytes_at_home(x: [u8; 4]) -> u32 {
4991+
/// transmute(x)
4992+
/// }
4993+
/// ```
4994+
///
4995+
/// {{produces}}
4996+
///
4997+
/// ## Explanation
4998+
///
4999+
/// People dont realize that safer methods such as [`u32::to_ne_bytes`] exist,
5000+
/// so this lint exists to lint on cases where people write transmutes that dont need to be there.
5001+
pub REDUNDANT_TRANSMUTATION,
5002+
Warn,
5003+
"detects transmutes that are shadowed by std methods"
5004+
}
5005+
49835006
declare_lint! {
49845007
/// The `tail_expr_drop_order` lint looks for those values generated at the tail expression location,
49855008
/// that runs a custom `Drop` destructor.

compiler/rustc_mir_transform/messages.ftl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ mir_transform_must_not_suspend = {$pre}`{$def_path}`{$post} held across a suspen
4242
.help = consider using a block (`{"{ ... }"}`) to shrink the value's scope, ending before the suspend point
4343
mir_transform_operation_will_panic = this operation will panic at runtime
4444
45+
mir_transform_redundant_transmute = this transmute could be performed safely
46+
4547
mir_transform_tail_expr_drop_order = relative drop order changing in Rust 2024
4648
.temporaries = in Rust 2024, this temporary value will be dropped first
4749
.observers = in Rust 2024, this local variable or temporary value will be dropped second
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
use rustc_middle::mir::visit::Visitor;
2+
use rustc_middle::mir::{Body, Location, Operand, Terminator, TerminatorKind};
3+
use rustc_middle::ty::{TyCtxt, UintTy};
4+
use rustc_session::lint::builtin::REDUNDANT_TRANSMUTATION;
5+
use rustc_span::source_map::Spanned;
6+
use rustc_span::{Span, sym};
7+
use rustc_type_ir::TyKind::*;
8+
9+
use crate::errors::RedundantTransmute as Error;
10+
11+
/// Check for transmutes that overlap with stdlib methods.
12+
/// For example, transmuting `[u8; 4]` to `u32`.
13+
pub(super) struct CheckRedundantTransmutes;
14+
15+
impl<'tcx> crate::MirLint<'tcx> for CheckRedundantTransmutes {
16+
fn run_lint(&self, tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
17+
let mut checker = RedundantTransmutesChecker { body, tcx };
18+
checker.visit_body(body);
19+
}
20+
}
21+
22+
struct RedundantTransmutesChecker<'a, 'tcx> {
23+
body: &'a Body<'tcx>,
24+
tcx: TyCtxt<'tcx>,
25+
}
26+
27+
impl<'a, 'tcx> RedundantTransmutesChecker<'a, 'tcx> {
28+
fn is_redundant_transmute(
29+
&self,
30+
function: &Operand<'tcx>,
31+
arg: String,
32+
span: Span,
33+
) -> Option<Error> {
34+
let fn_sig = function.ty(self.body, self.tcx).fn_sig(self.tcx).skip_binder();
35+
let [input] = fn_sig.inputs() else { return None };
36+
37+
let err = |sugg| Error { span, sugg, help: None };
38+
39+
Some(match (input.kind(), fn_sig.output().kind()) {
40+
// dont check the length; transmute does that for us.
41+
// [u8; _] => primitive
42+
(Array(t, _), Uint(_) | Float(_) | Int(_)) if *t.kind() == Uint(UintTy::U8) => Error {
43+
sugg: format!("{}::from_ne_bytes({arg})", fn_sig.output()),
44+
help: Some(
45+
"there's also `from_le_bytes` and `from_ne_bytes` if you expect a particular byte order",
46+
),
47+
span,
48+
},
49+
// primitive => [u8; _]
50+
(Uint(_) | Float(_) | Int(_), Array(t, _)) if *t.kind() == Uint(UintTy::U8) => Error {
51+
sugg: format!("{input}::to_ne_bytes({arg})"),
52+
help: Some(
53+
"there's also `to_le_bytes` and `to_ne_bytes` if you expect a particular byte order",
54+
),
55+
span,
56+
},
57+
// char → u32
58+
(Char, Uint(UintTy::U32)) => err(format!("({arg}) as u32")),
59+
// u32 → char
60+
(Uint(UintTy::U32), Char) => Error {
61+
sugg: format!("char::from_u32_unchecked({arg})"),
62+
help: Some("consider `char::from_u32(…).unwrap()`"),
63+
span,
64+
},
65+
// uNN → iNN
66+
(Uint(ty), Int(_)) => err(format!("{}::cast_signed({arg})", ty.name_str())),
67+
// iNN → uNN
68+
(Int(ty), Uint(_)) => err(format!("{}::cast_unsigned({arg})", ty.name_str())),
69+
// fNN → uNN
70+
(Float(ty), Uint(..)) => err(format!("{}::to_bits({arg})", ty.name_str())),
71+
// uNN → fNN
72+
(Uint(_), Float(ty)) => err(format!("{}::from_bits({arg})", ty.name_str())),
73+
// bool → { x8 }
74+
(Bool, Int(..) | Uint(..)) => err(format!("({arg}) as {}", fn_sig.output())),
75+
// u8 → bool
76+
(Uint(_), Bool) => err(format!("({arg} == 1)")),
77+
_ => return None,
78+
})
79+
}
80+
}
81+
82+
impl<'tcx> Visitor<'tcx> for RedundantTransmutesChecker<'_, 'tcx> {
83+
// Check each block's terminator for calls to pointer to integer transmutes
84+
// in const functions or associated constants and emit a lint.
85+
fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
86+
if let TerminatorKind::Call { func, args, .. } = &terminator.kind
87+
&& let [Spanned { span: arg, .. }] = **args
88+
&& let Some((func_def_id, _)) = func.const_fn_def()
89+
&& self.tcx.is_intrinsic(func_def_id, sym::transmute)
90+
&& let span = self.body.source_info(location).span
91+
&& let Some(lint) = self.is_redundant_transmute(
92+
func,
93+
self.tcx.sess.source_map().span_to_snippet(arg).expect("ok"),
94+
span,
95+
)
96+
&& let Some(call_id) = self.body.source.def_id().as_local()
97+
{
98+
let hir_id = self.tcx.local_def_id_to_hir_id(call_id);
99+
100+
self.tcx.emit_node_span_lint(REDUNDANT_TRANSMUTATION, hir_id, span, lint);
101+
}
102+
}
103+
}

compiler/rustc_mir_transform/src/errors.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,26 @@ pub(crate) struct MustNotSuspendReason {
158158
pub reason: String,
159159
}
160160

161+
pub(crate) struct RedundantTransmute {
162+
pub span: Span,
163+
pub sugg: String,
164+
pub help: Option<&'static str>,
165+
}
166+
167+
// Needed for def_path_str
168+
impl<'a> LintDiagnostic<'a, ()> for RedundantTransmute {
169+
fn decorate_lint<'b>(self, diag: &'b mut rustc_errors::Diag<'a, ()>) {
170+
diag.primary_message(fluent::mir_transform_redundant_transmute);
171+
diag.span_suggestion(
172+
self.span,
173+
"replace `transmute`",
174+
self.sugg,
175+
lint::Applicability::MachineApplicable,
176+
);
177+
self.help.map(|help| diag.help(help));
178+
}
179+
}
180+
161181
#[derive(LintDiagnostic)]
162182
#[diag(mir_transform_undefined_transmute)]
163183
#[note]

compiler/rustc_mir_transform/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ declare_passes! {
125125
mod check_null : CheckNull;
126126
mod check_packed_ref : CheckPackedRef;
127127
mod check_undefined_transmutes : CheckUndefinedTransmutes;
128+
mod check_redundant_transmutes: CheckRedundantTransmutes;
128129
// This pass is public to allow external drivers to perform MIR cleanup
129130
pub mod cleanup_post_borrowck : CleanupPostBorrowck;
130131

@@ -387,6 +388,7 @@ fn mir_built(tcx: TyCtxt<'_>, def: LocalDefId) -> &Steal<Body<'_>> {
387388
&Lint(check_const_item_mutation::CheckConstItemMutation),
388389
&Lint(function_item_references::FunctionItemReferences),
389390
&Lint(check_undefined_transmutes::CheckUndefinedTransmutes),
391+
&Lint(check_redundant_transmutes::CheckRedundantTransmutes),
390392
// What we need to do constant evaluation.
391393
&simplify::SimplifyCfg::Initial,
392394
&Lint(sanity_check::SanityCheck),

library/core/src/char/convert.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ pub(super) const fn from_u32(i: u32) -> Option<char> {
2121
/// Converts a `u32` to a `char`, ignoring validity. See [`char::from_u32_unchecked`].
2222
#[inline]
2323
#[must_use]
24+
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
2425
pub(super) const unsafe fn from_u32_unchecked(i: u32) -> char {
2526
// SAFETY: the caller must guarantee that `i` is a valid char value.
2627
unsafe {
@@ -229,6 +230,7 @@ impl FromStr for char {
229230
}
230231

231232
#[inline]
233+
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
232234
const fn char_try_from_u32(i: u32) -> Result<char, CharTryFromError> {
233235
// This is an optimized version of the check
234236
// (i > MAX as u32) || (i >= 0xD800 && i <= 0xDFFF),

library/core/src/num/f128.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,7 @@ impl f128 {
901901
#[inline]
902902
#[unstable(feature = "f128", issue = "116909")]
903903
#[must_use = "this returns the result of the operation, without modifying the original"]
904+
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
904905
pub const fn to_bits(self) -> u128 {
905906
// SAFETY: `u128` is a plain old datatype so we can always transmute to it.
906907
unsafe { mem::transmute(self) }
@@ -948,6 +949,7 @@ impl f128 {
948949
#[inline]
949950
#[must_use]
950951
#[unstable(feature = "f128", issue = "116909")]
952+
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
951953
pub const fn from_bits(v: u128) -> Self {
952954
// It turns out the safety issues with sNaN were overblown! Hooray!
953955
// SAFETY: `u128` is a plain old datatype so we can always transmute from it.

library/core/src/num/f16.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,7 @@ impl f16 {
889889
#[inline]
890890
#[unstable(feature = "f16", issue = "116909")]
891891
#[must_use = "this returns the result of the operation, without modifying the original"]
892+
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
892893
pub const fn to_bits(self) -> u16 {
893894
// SAFETY: `u16` is a plain old datatype so we can always transmute to it.
894895
unsafe { mem::transmute(self) }
@@ -935,6 +936,7 @@ impl f16 {
935936
#[inline]
936937
#[must_use]
937938
#[unstable(feature = "f16", issue = "116909")]
939+
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
938940
pub const fn from_bits(v: u16) -> Self {
939941
// It turns out the safety issues with sNaN were overblown! Hooray!
940942
// SAFETY: `u16` is a plain old datatype so we can always transmute from it.

library/core/src/num/f32.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -708,8 +708,7 @@ impl f32 {
708708
pub const fn is_sign_negative(self) -> bool {
709709
// IEEE754 says: isSignMinus(x) is true if and only if x has negative sign. isSignMinus
710710
// applies to zeros and NaNs as well.
711-
// SAFETY: This is just transmuting to get the sign bit, it's fine.
712-
unsafe { mem::transmute::<f32, u32>(self) & 0x8000_0000 != 0 }
711+
self.to_bits() & 0x8000_0000 != 0
713712
}
714713

715714
/// Returns the least number greater than `self`.
@@ -1093,6 +1092,7 @@ impl f32 {
10931092
#[stable(feature = "float_bits_conv", since = "1.20.0")]
10941093
#[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
10951094
#[inline]
1095+
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
10961096
pub const fn to_bits(self) -> u32 {
10971097
// SAFETY: `u32` is a plain old datatype so we can always transmute to it.
10981098
unsafe { mem::transmute(self) }
@@ -1138,6 +1138,7 @@ impl f32 {
11381138
#[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
11391139
#[must_use]
11401140
#[inline]
1141+
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
11411142
pub const fn from_bits(v: u32) -> Self {
11421143
// It turns out the safety issues with sNaN were overblown! Hooray!
11431144
// SAFETY: `u32` is a plain old datatype so we can always transmute from it.

library/core/src/num/f64.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -716,8 +716,7 @@ impl f64 {
716716
pub const fn is_sign_negative(self) -> bool {
717717
// IEEE754 says: isSignMinus(x) is true if and only if x has negative sign. isSignMinus
718718
// applies to zeros and NaNs as well.
719-
// SAFETY: This is just transmuting to get the sign bit, it's fine.
720-
unsafe { mem::transmute::<f64, u64>(self) & Self::SIGN_MASK != 0 }
719+
self.to_bits() & Self::SIGN_MASK != 0
721720
}
722721

723722
#[must_use]
@@ -1092,6 +1091,7 @@ impl f64 {
10921091
without modifying the original"]
10931092
#[stable(feature = "float_bits_conv", since = "1.20.0")]
10941093
#[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
1094+
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
10951095
#[inline]
10961096
pub const fn to_bits(self) -> u64 {
10971097
// SAFETY: `u64` is a plain old datatype so we can always transmute to it.
@@ -1138,6 +1138,7 @@ impl f64 {
11381138
#[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
11391139
#[must_use]
11401140
#[inline]
1141+
#[cfg_attr(not(bootstrap), allow(redundant_transmutation))]
11411142
pub const fn from_bits(v: u64) -> Self {
11421143
// It turns out the safety issues with sNaN were overblown! Hooray!
11431144
// SAFETY: `u64` is a plain old datatype so we can always transmute from it.

0 commit comments

Comments
 (0)