Skip to content

use uX::from instead of _ as uX in non - const contexts #140435

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

Open
wants to merge 1 commit 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
25 changes: 22 additions & 3 deletions compiler/rustc_mir_transform/src/check_unnecessary_transmutes.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use rustc_hir::def::DefKind::*;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{Body, Location, Operand, Terminator, TerminatorKind};
use rustc_middle::ty::*;
Expand Down Expand Up @@ -29,6 +30,7 @@ impl<'a, 'tcx> UnnecessaryTransmuteChecker<'a, 'tcx> {
function: &Operand<'tcx>,
arg: String,
span: Span,
is_in_const: bool,
) -> 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 };
Expand Down Expand Up @@ -69,9 +71,15 @@ impl<'a, 'tcx> UnnecessaryTransmuteChecker<'a, 'tcx> {
(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())),
// bool → { x8 } in const context
// is it possible to know when the parentheses arent necessary?
(Bool, Int(..) | Uint(..)) if is_in_const => {
err(format!("({arg}) as {}", fn_sig.output()))
}
// " using `x8::from`
(Bool, Int(..) | Uint(..)) => err(format!("{}::from({arg})", fn_sig.output())),
// u8 → bool
// also want to fix parentheses, maybe
(Uint(_), Bool) => err(format!("({arg} == 1)")),
_ => return None,
})
Expand All @@ -88,7 +96,18 @@ impl<'tcx> Visitor<'tcx> for UnnecessaryTransmuteChecker<'_, 'tcx> {
&& self.tcx.is_intrinsic(func_def_id, sym::transmute)
&& let span = self.body.source_info(location).span
&& let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(arg)
&& let Some(lint) = self.is_unnecessary_transmute(func, snippet, span)
&& let def_id = self.body.source.def_id()
&& let Some(lint) = self.is_unnecessary_transmute(
func,
snippet,
span,
self.tcx.is_const_fn(def_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this heuristic is complete. This leaves out const items and statics, for example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add some tests for statics, consts, and in const blocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im not sure the heuristic is complete, but i added those tests and changed it a little.

|| self.tcx.is_conditionally_const(def_id)
|| matches!(
self.tcx.def_kind(def_id),
AnonConst | Const | Static { .. } | AssocConst | InlineConst
),
)
&& let Some(hir_id) = terminator.source_info.scope.lint_root(&self.body.source_scopes)
{
self.tcx.emit_node_span_lint(UNNECESSARY_TRANSMUTES, hir_id, span, lint);
Expand Down
24 changes: 22 additions & 2 deletions tests/ui/transmute/unnecessary-transmutation.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,27 @@ pub fn bytes_at_home(x: u32) -> [u8; 4] {
//~^ ERROR
}

pub const fn intinator_const(from: bool) -> u8 {
unsafe { (from) as u8 }
//~^ ERROR
}

pub static X: u8 = unsafe { (true) as u8 };
//~^ ERROR
pub const Y: u8 = unsafe { (true) as u8 };
//~^ ERROR

pub struct Z {}
impl Z {
pub const fn intinator_assoc(x: bool) -> u8 {
unsafe { (x) as u8 }
//~^ ERROR
}
}

fn main() {
const { unsafe { (true) as u8 } };
//~^ ERROR
unsafe {
let x: u16 = u16::from_ne_bytes(*b"01");
//~^ ERROR
Expand Down Expand Up @@ -74,12 +94,12 @@ fn main() {

let z: bool = (1u8 == 1);
//~^ ERROR
let z: u8 = (z) as u8;
let z: u8 = u8::from(z);
//~^ ERROR

let z: bool = transmute(1i8);
// no error!
let z: i8 = (z) as i8;
let z: i8 = i8::from(z);
//~^ ERROR
}
}
20 changes: 20 additions & 0 deletions tests/ui/transmute/unnecessary-transmutation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,27 @@ pub fn bytes_at_home(x: u32) -> [u8; 4] {
//~^ ERROR
}

pub const fn intinator_const(from: bool) -> u8 {
unsafe { transmute(from) }
//~^ ERROR
}

pub static X: u8 = unsafe { transmute(true) };
//~^ ERROR
pub const Y: u8 = unsafe { transmute(true) };
//~^ ERROR

pub struct Z {}
impl Z {
pub const fn intinator_assoc(x: bool) -> u8 {
unsafe { transmute(x) }
//~^ ERROR
}
}

fn main() {
const { unsafe { transmute::<_, u8>(true) } };
//~^ ERROR
unsafe {
let x: u16 = transmute(*b"01");
//~^ ERROR
Expand Down
Loading
Loading