Skip to content

Commit 0abd167

Browse files
committed
Suggest {to,from}_ne_bytes for transmutations between arrays and integers, etc
1 parent c4b38a5 commit 0abd167

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+669
-86
lines changed

compiler/rustc_codegen_cranelift/example/example.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![feature(no_core, unboxed_closures)]
22
#![no_core]
3-
#![allow(dead_code)]
3+
#![allow(dead_code, unnecessary_transmutate)]
44

55
extern crate mini_core;
66

compiler/rustc_codegen_gcc/example/example.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![feature(no_core, unboxed_closures)]
22
#![no_core]
3-
#![allow(dead_code)]
3+
#![allow(dead_code, unnecessary_transmutate)]
44

55
extern crate mini_core;
66

compiler/rustc_lint_defs/src/builtin.rs

+25
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ declare_lint_pass! {
119119
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
120120
UNNAMEABLE_TEST_ITEMS,
121121
UNNAMEABLE_TYPES,
122+
UNNECESSARY_TRANSMUTATE,
122123
UNREACHABLE_CODE,
123124
UNREACHABLE_PATTERNS,
124125
UNSAFE_ATTR_OUTSIDE_UNSAFE,
@@ -4943,6 +4944,30 @@ declare_lint! {
49434944
"detects pointer to integer transmutes in const functions and associated constants",
49444945
}
49454946

4947+
declare_lint! {
4948+
/// The `unnecessary_transmutate` lint detects transmutations that have safer alternatives.
4949+
///
4950+
/// ### Example
4951+
///
4952+
/// ```rust
4953+
/// fn bytes_at_home(x: [u8; 4]) -> u32 {
4954+
/// unsafe { std::mem::transmute(x) }
4955+
/// }
4956+
/// ```
4957+
///
4958+
/// {{produces}}
4959+
///
4960+
/// ### Explanation
4961+
///
4962+
/// Using an explicit method is preferable over calls to
4963+
/// [`transmute`](https://doc.rust-lang.org/std/mem/fn.transmute.html) as
4964+
/// they more clearly communicate the intent, are easier to review, and
4965+
/// are less likely to accidentally result in unsoundness.
4966+
pub UNNECESSARY_TRANSMUTATE,
4967+
Warn,
4968+
"detects transmutes that are shadowed by std methods"
4969+
}
4970+
49464971
declare_lint! {
49474972
/// The `tail_expr_drop_order` lint looks for those values generated at the tail expression location,
49484973
/// that runs a custom `Drop` destructor.

compiler/rustc_mir_transform/messages.ftl

+1
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,4 @@ mir_transform_undefined_transmute = pointers cannot be transmuted to integers du
8484
.help = for more information, see https://doc.rust-lang.org/std/mem/fn.transmute.html
8585
8686
mir_transform_unknown_pass_name = MIR pass `{$name}` is unknown and will be ignored
87+
mir_transform_unnecessary_transmute = unnecessary transmute
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
use rustc_middle::mir::visit::Visitor;
2+
use rustc_middle::mir::{Body, Location, Operand, Terminator, TerminatorKind};
3+
use rustc_middle::ty::*;
4+
use rustc_session::lint::builtin::UNNECESSARY_TRANSMUTATE;
5+
use rustc_span::source_map::Spanned;
6+
use rustc_span::{Span, sym};
7+
8+
use crate::errors::UnnecessaryTransmute as Error;
9+
10+
/// Check for transmutes that overlap with stdlib methods.
11+
/// For example, transmuting `[u8; 4]` to `u32`.
12+
pub(super) struct CheckUnnecessaryTransmutes;
13+
14+
impl<'tcx> crate::MirLint<'tcx> for CheckUnnecessaryTransmutes {
15+
fn run_lint(&self, tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
16+
let mut checker = UnnecessaryTransmuteChecker { body, tcx };
17+
checker.visit_body(body);
18+
}
19+
}
20+
21+
struct UnnecessaryTransmuteChecker<'a, 'tcx> {
22+
body: &'a Body<'tcx>,
23+
tcx: TyCtxt<'tcx>,
24+
}
25+
26+
impl<'a, 'tcx> UnnecessaryTransmuteChecker<'a, 'tcx> {
27+
fn is_redundant_transmute(
28+
&self,
29+
function: &Operand<'tcx>,
30+
arg: String,
31+
span: Span,
32+
) -> Option<Error> {
33+
let fn_sig = function.ty(self.body, self.tcx).fn_sig(self.tcx).skip_binder();
34+
let [input] = fn_sig.inputs() else { return None };
35+
36+
let err = |sugg| Error { span, sugg, help: None };
37+
38+
Some(match (input.kind(), fn_sig.output().kind()) {
39+
// dont check the length; transmute does that for us.
40+
// [u8; _] => primitive
41+
(Array(t, _), Uint(_) | Float(_) | Int(_)) if *t.kind() == Uint(UintTy::U8) => Error {
42+
sugg: format!("{}::from_ne_bytes({arg})", fn_sig.output()),
43+
help: Some(
44+
"there's also `from_le_bytes` and `from_ne_bytes` if you expect a particular byte order",
45+
),
46+
span,
47+
},
48+
// primitive => [u8; _]
49+
(Uint(_) | Float(_) | Int(_), Array(t, _)) if *t.kind() == Uint(UintTy::U8) => Error {
50+
sugg: format!("{input}::to_ne_bytes({arg})"),
51+
help: Some(
52+
"there's also `to_le_bytes` and `to_ne_bytes` if you expect a particular byte order",
53+
),
54+
span,
55+
},
56+
// char → u32
57+
(Char, Uint(UintTy::U32)) => err(format!("u32::from({arg})")),
58+
// u32 → char
59+
(Uint(UintTy::U32), Char) => Error {
60+
sugg: format!("char::from_u32_unchecked({arg})"),
61+
help: Some("consider `char::from_u32(…).unwrap()`"),
62+
span,
63+
},
64+
// uNN → iNN
65+
(Uint(ty), Int(_)) => err(format!("{}::cast_signed({arg})", ty.name_str())),
66+
// iNN → uNN
67+
(Int(ty), Uint(_)) => err(format!("{}::cast_unsigned({arg})", ty.name_str())),
68+
// fNN → uNN
69+
(Float(ty), Uint(..)) => err(format!("{}::to_bits({arg})", ty.name_str())),
70+
// uNN → fNN
71+
(Uint(_), Float(ty)) => err(format!("{}::from_bits({arg})", ty.name_str())),
72+
// bool → { x8 }
73+
(Bool, Int(..) | Uint(..)) => err(format!("({arg}) as {}", fn_sig.output())),
74+
// u8 → bool
75+
(Uint(_), Bool) => err(format!("({arg} == 1)")),
76+
_ => return None,
77+
})
78+
}
79+
}
80+
81+
impl<'tcx> Visitor<'tcx> for UnnecessaryTransmuteChecker<'_, 'tcx> {
82+
// Check each block's terminator for calls to pointer to integer transmutes
83+
// in const functions or associated constants and emit a lint.
84+
fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
85+
if let TerminatorKind::Call { func, args, .. } = &terminator.kind
86+
&& let [Spanned { span: arg, .. }] = **args
87+
&& let Some((func_def_id, _)) = func.const_fn_def()
88+
&& self.tcx.is_intrinsic(func_def_id, sym::transmute)
89+
&& let span = self.body.source_info(location).span
90+
&& let Some(lint) = self.is_redundant_transmute(
91+
func,
92+
self.tcx.sess.source_map().span_to_snippet(arg).expect("ok"),
93+
span,
94+
)
95+
&& let Some(call_id) = self.body.source.def_id().as_local()
96+
{
97+
let hir_id = self.tcx.local_def_id_to_hir_id(call_id);
98+
99+
self.tcx.emit_node_span_lint(UNNECESSARY_TRANSMUTATE, hir_id, span, lint);
100+
}
101+
}
102+
}

compiler/rustc_mir_transform/src/errors.rs

+20
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 UnnecessaryTransmute {
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 UnnecessaryTransmute {
169+
fn decorate_lint<'b>(self, diag: &'b mut rustc_errors::Diag<'a, ()>) {
170+
diag.primary_message(fluent::mir_transform_unnecessary_transmute);
171+
diag.span_suggestion(
172+
self.span,
173+
"replace this with",
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

+2
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_unnecessary_transmutes: CheckUnnecessaryTransmutes;
128129
// This pass is public to allow external drivers to perform MIR cleanup
129130
pub mod cleanup_post_borrowck : CleanupPostBorrowck;
130131

@@ -391,6 +392,7 @@ fn mir_built(tcx: TyCtxt<'_>, def: LocalDefId) -> &Steal<Body<'_>> {
391392
&Lint(check_const_item_mutation::CheckConstItemMutation),
392393
&Lint(function_item_references::FunctionItemReferences),
393394
&Lint(check_undefined_transmutes::CheckUndefinedTransmutes),
395+
&Lint(check_unnecessary_transmutes::CheckUnnecessaryTransmutes),
394396
// What we need to do constant evaluation.
395397
&simplify::SimplifyCfg::Initial,
396398
&Lint(sanity_check::SanityCheck),

library/alloctests/tests/fmt.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#![deny(warnings)]
22
// FIXME(static_mut_refs): Do not allow `static_mut_refs` lint
33
#![allow(static_mut_refs)]
4+
#![cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
45

56
use std::cell::RefCell;
67
use std::fmt::{self, Write};

library/core/src/char/convert.rs

+2
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(unnecessary_transmutate))]
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 {
@@ -221,6 +222,7 @@ impl FromStr for char {
221222
}
222223

223224
#[inline]
225+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
224226
const fn char_try_from_u32(i: u32) -> Result<char, CharTryFromError> {
225227
// This is an optimized version of the check
226228
// (i > MAX as u32) || (i >= 0xD800 && i <= 0xDFFF),

library/core/src/intrinsics/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1498,6 +1498,7 @@ pub const fn forget<T: ?Sized>(_: T);
14981498
/// Turning raw bytes (`[u8; SZ]`) into `u32`, `f64`, etc.:
14991499
///
15001500
/// ```
1501+
/// # #![cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
15011502
/// let raw_bytes = [0x78, 0x56, 0x34, 0x12];
15021503
///
15031504
/// let num = unsafe {

library/core/src/num/f128.rs

+2
Original file line numberDiff line numberDiff line change
@@ -898,6 +898,7 @@ impl f128 {
898898
#[inline]
899899
#[unstable(feature = "f128", issue = "116909")]
900900
#[must_use = "this returns the result of the operation, without modifying the original"]
901+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
901902
pub const fn to_bits(self) -> u128 {
902903
// SAFETY: `u128` is a plain old datatype so we can always transmute to it.
903904
unsafe { mem::transmute(self) }
@@ -945,6 +946,7 @@ impl f128 {
945946
#[inline]
946947
#[must_use]
947948
#[unstable(feature = "f128", issue = "116909")]
949+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
948950
pub const fn from_bits(v: u128) -> Self {
949951
// It turns out the safety issues with sNaN were overblown! Hooray!
950952
// SAFETY: `u128` is a plain old datatype so we can always transmute from it.

library/core/src/num/f16.rs

+2
Original file line numberDiff line numberDiff line change
@@ -886,6 +886,7 @@ impl f16 {
886886
#[inline]
887887
#[unstable(feature = "f16", issue = "116909")]
888888
#[must_use = "this returns the result of the operation, without modifying the original"]
889+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
889890
pub const fn to_bits(self) -> u16 {
890891
// SAFETY: `u16` is a plain old datatype so we can always transmute to it.
891892
unsafe { mem::transmute(self) }
@@ -932,6 +933,7 @@ impl f16 {
932933
#[inline]
933934
#[must_use]
934935
#[unstable(feature = "f16", issue = "116909")]
936+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
935937
pub const fn from_bits(v: u16) -> Self {
936938
// It turns out the safety issues with sNaN were overblown! Hooray!
937939
// SAFETY: `u16` is a plain old datatype so we can always transmute from it.

library/core/src/num/f32.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -705,8 +705,7 @@ impl f32 {
705705
pub const fn is_sign_negative(self) -> bool {
706706
// IEEE754 says: isSignMinus(x) is true if and only if x has negative sign. isSignMinus
707707
// applies to zeros and NaNs as well.
708-
// SAFETY: This is just transmuting to get the sign bit, it's fine.
709-
unsafe { mem::transmute::<f32, u32>(self) & 0x8000_0000 != 0 }
708+
self.to_bits() & 0x8000_0000 != 0
710709
}
711710

712711
/// Returns the least number greater than `self`.
@@ -1090,6 +1089,7 @@ impl f32 {
10901089
#[stable(feature = "float_bits_conv", since = "1.20.0")]
10911090
#[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
10921091
#[inline]
1092+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
10931093
pub const fn to_bits(self) -> u32 {
10941094
// SAFETY: `u32` is a plain old datatype so we can always transmute to it.
10951095
unsafe { mem::transmute(self) }
@@ -1135,6 +1135,7 @@ impl f32 {
11351135
#[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
11361136
#[must_use]
11371137
#[inline]
1138+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
11381139
pub const fn from_bits(v: u32) -> Self {
11391140
// It turns out the safety issues with sNaN were overblown! Hooray!
11401141
// SAFETY: `u32` is a plain old datatype so we can always transmute from it.

library/core/src/num/f64.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -713,8 +713,7 @@ impl f64 {
713713
pub const fn is_sign_negative(self) -> bool {
714714
// IEEE754 says: isSignMinus(x) is true if and only if x has negative sign. isSignMinus
715715
// applies to zeros and NaNs as well.
716-
// SAFETY: This is just transmuting to get the sign bit, it's fine.
717-
unsafe { mem::transmute::<f64, u64>(self) & Self::SIGN_MASK != 0 }
716+
self.to_bits() & Self::SIGN_MASK != 0
718717
}
719718

720719
#[must_use]
@@ -1089,6 +1088,7 @@ impl f64 {
10891088
without modifying the original"]
10901089
#[stable(feature = "float_bits_conv", since = "1.20.0")]
10911090
#[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
1091+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
10921092
#[inline]
10931093
pub const fn to_bits(self) -> u64 {
10941094
// SAFETY: `u64` is a plain old datatype so we can always transmute to it.
@@ -1135,6 +1135,7 @@ impl f64 {
11351135
#[rustc_const_stable(feature = "const_float_bits_conv", since = "1.83.0")]
11361136
#[must_use]
11371137
#[inline]
1138+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
11381139
pub const fn from_bits(v: u64) -> Self {
11391140
// It turns out the safety issues with sNaN were overblown! Hooray!
11401141
// SAFETY: `u64` is a plain old datatype so we can always transmute from it.

library/core/src/num/int_macros.rs

+2
Original file line numberDiff line numberDiff line change
@@ -3678,6 +3678,7 @@ macro_rules! int_impl {
36783678
/// ```
36793679
#[stable(feature = "int_to_from_bytes", since = "1.32.0")]
36803680
#[rustc_const_stable(feature = "const_int_conversion", since = "1.44.0")]
3681+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
36813682
// SAFETY: const sound because integers are plain old datatypes so we can always
36823683
// transmute them to arrays of bytes
36833684
#[must_use = "this returns the result of the operation, \
@@ -3781,6 +3782,7 @@ macro_rules! int_impl {
37813782
/// ```
37823783
#[stable(feature = "int_to_from_bytes", since = "1.32.0")]
37833784
#[rustc_const_stable(feature = "const_int_conversion", since = "1.44.0")]
3785+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
37843786
#[must_use]
37853787
// SAFETY: const sound because integers are plain old datatypes so we can always
37863788
// transmute to them

library/core/src/num/uint_macros.rs

+2
Original file line numberDiff line numberDiff line change
@@ -3523,6 +3523,7 @@ macro_rules! uint_impl {
35233523
#[rustc_const_stable(feature = "const_int_conversion", since = "1.44.0")]
35243524
#[must_use = "this returns the result of the operation, \
35253525
without modifying the original"]
3526+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
35263527
// SAFETY: const sound because integers are plain old datatypes so we can always
35273528
// transmute them to arrays of bytes
35283529
#[inline]
@@ -3624,6 +3625,7 @@ macro_rules! uint_impl {
36243625
/// ```
36253626
#[stable(feature = "int_to_from_bytes", since = "1.32.0")]
36263627
#[rustc_const_stable(feature = "const_int_conversion", since = "1.44.0")]
3628+
#[cfg_attr(not(bootstrap), allow(unnecessary_transmutate))]
36273629
#[must_use]
36283630
// SAFETY: const sound because integers are plain old datatypes so we can always
36293631
// transmute to them

src/tools/clippy/tests/ui/blocks_in_conditions.fixed

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#![warn(clippy::blocks_in_conditions)]
44
#![allow(
55
unused,
6+
unnecessary_transmutate,
67
clippy::let_and_return,
78
clippy::needless_if,
89
clippy::missing_transmute_annotations

src/tools/clippy/tests/ui/blocks_in_conditions.rs

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#![warn(clippy::blocks_in_conditions)]
44
#![allow(
55
unused,
6+
unnecessary_transmutate,
67
clippy::let_and_return,
78
clippy::needless_if,
89
clippy::missing_transmute_annotations

0 commit comments

Comments
 (0)