Skip to content

Commit 95c62ff

Browse files
borsbors
committed
Auto merge of #12239 - GuillaumeGomez:missing_transmute_annotation, r=y21
Add `missing_transmute_annotations` lint Fixes #715. r? `@blyxyas` changelog: Add `missing_transmute_annotations` lint
2 parents 12f7c17 + ee25582 commit 95c62ff

Some content is hidden

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

53 files changed

+603
-189
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5481,6 +5481,7 @@ Released 2018-09-13
54815481
[`missing_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc
54825482
[`missing_spin_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_spin_loop
54835483
[`missing_trait_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_trait_methods
5484+
[`missing_transmute_annotations`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_transmute_annotations
54845485
[`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
54855486
[`mixed_attributes_style`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_attributes_style
54865487
[`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
678678
crate::trait_bounds::TYPE_REPETITION_IN_BOUNDS_INFO,
679679
crate::transmute::CROSSPOINTER_TRANSMUTE_INFO,
680680
crate::transmute::EAGER_TRANSMUTE_INFO,
681+
crate::transmute::MISSING_TRANSMUTE_ANNOTATIONS_INFO,
681682
crate::transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS_INFO,
682683
crate::transmute::TRANSMUTE_BYTES_TO_STR_INFO,
683684
crate::transmute::TRANSMUTE_FLOAT_TO_INT_INFO,

‎clippy_lints/src/transmute/missing_transmute_annotations.rs

+87
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use rustc_errors::Applicability;
3+
use rustc_hir::{GenericArg, HirId, Local, Node, Path, TyKind};
4+
use rustc_lint::LateContext;
5+
use rustc_middle::lint::in_external_macro;
6+
use rustc_middle::ty::Ty;
7+
8+
use crate::transmute::MISSING_TRANSMUTE_ANNOTATIONS;
9+
10+
fn get_parent_local_binding_ty<'tcx>(cx: &LateContext<'tcx>, expr_hir_id: HirId) -> Option<Local<'tcx>> {
11+
let mut parent_iter = cx.tcx.hir().parent_iter(expr_hir_id);
12+
if let Some((_, node)) = parent_iter.next() {
13+
match node {
14+
Node::Local(local) => Some(*local),
15+
Node::Block(_) => {
16+
if let Some((parent_hir_id, Node::Expr(expr))) = parent_iter.next()
17+
&& matches!(expr.kind, rustc_hir::ExprKind::Block(_, _))
18+
{
19+
get_parent_local_binding_ty(cx, parent_hir_id)
20+
} else {
21+
None
22+
}
23+
},
24+
_ => None,
25+
}
26+
} else {
27+
None
28+
}
29+
}
30+
31+
fn is_function_block(cx: &LateContext<'_>, expr_hir_id: HirId) -> bool {
32+
let def_id = cx.tcx.hir().enclosing_body_owner(expr_hir_id);
33+
if let Some(body_id) = cx.tcx.hir().maybe_body_owned_by(def_id) {
34+
let body = cx.tcx.hir().body(body_id);
35+
return body.value.peel_blocks().hir_id == expr_hir_id;
36+
}
37+
false
38+
}
39+
40+
pub(super) fn check<'tcx>(
41+
cx: &LateContext<'tcx>,
42+
path: &Path<'tcx>,
43+
from_ty: Ty<'tcx>,
44+
to_ty: Ty<'tcx>,
45+
expr_hir_id: HirId,
46+
) -> bool {
47+
let last = path.segments.last().unwrap();
48+
if in_external_macro(cx.tcx.sess, last.ident.span) {
49+
// If it comes from a non-local macro, we ignore it.
50+
return false;
51+
}
52+
let args = last.args;
53+
let missing_generic = match args {
54+
Some(args) if !args.args.is_empty() => args.args.iter().any(|arg| match arg {
55+
GenericArg::Infer(_) => true,
56+
GenericArg::Type(ty) => matches!(ty.kind, TyKind::Infer),
57+
_ => false,
58+
}),
59+
_ => true,
60+
};
61+
if !missing_generic {
62+
return false;
63+
}
64+
// If it's being set as a local variable value...
65+
if let Some(local) = get_parent_local_binding_ty(cx, expr_hir_id) {
66+
// ... which does have type annotations.
67+
if let Some(ty) = local.ty
68+
// If this is a `let x: _ =`, we should lint.
69+
&& !matches!(ty.kind, TyKind::Infer)
70+
{
71+
return false;
72+
}
73+
// We check if this transmute is not the only element in the function
74+
} else if is_function_block(cx, expr_hir_id) {
75+
return false;
76+
}
77+
span_lint_and_sugg(
78+
cx,
79+
MISSING_TRANSMUTE_ANNOTATIONS,
80+
last.ident.span.with_hi(path.span.hi()),
81+
"transmute used without annotations",
82+
"consider adding missing annotations",
83+
format!("{}::<{from_ty}, {to_ty}>", last.ident.as_str()),
84+
Applicability::MaybeIncorrect,
85+
);
86+
true
87+
}

‎clippy_lints/src/transmute/mod.rs

+34
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
mod crosspointer_transmute;
22
mod eager_transmute;
3+
mod missing_transmute_annotations;
34
mod transmute_float_to_int;
45
mod transmute_int_to_bool;
56
mod transmute_int_to_char;
@@ -520,6 +521,37 @@ declare_clippy_lint! {
520521
"eager evaluation of `transmute`"
521522
}
522523

524+
declare_clippy_lint! {
525+
/// ### What it does
526+
/// Checks if transmute calls have all generics specified.
527+
///
528+
/// ### Why is this bad?
529+
/// If not set, some unexpected output type could be retrieved instead of the expected one,
530+
/// potentially leading to invalid code.
531+
///
532+
/// This is particularly dangerous in case a seemingly innocent/unrelated change can cause type
533+
/// inference to start inferring a different type. E.g. the transmute is the tail expression of
534+
/// an `if` branch, and a different branches type changes, causing the transmute to silently
535+
/// have a different type, instead of a proper error.
536+
///
537+
/// ### Example
538+
/// ```no_run
539+
/// # unsafe {
540+
/// let x: i32 = std::mem::transmute([1u16, 2u16]);
541+
/// # }
542+
/// ```
543+
/// Use instead:
544+
/// ```no_run
545+
/// # unsafe {
546+
/// let x = std::mem::transmute::<[u16; 2], i32>([1u16, 2u16]);
547+
/// # }
548+
/// ```
549+
#[clippy::version = "1.77.0"]
550+
pub MISSING_TRANSMUTE_ANNOTATIONS,
551+
suspicious,
552+
"warns if a transmute call doesn't have all generics specified"
553+
}
554+
523555
pub struct Transmute {
524556
msrv: Msrv,
525557
}
@@ -542,6 +574,7 @@ impl_lint_pass!(Transmute => [
542574
TRANSMUTING_NULL,
543575
TRANSMUTE_NULL_TO_FN,
544576
EAGER_TRANSMUTE,
577+
MISSING_TRANSMUTE_ANNOTATIONS,
545578
]);
546579
impl Transmute {
547580
#[must_use]
@@ -579,6 +612,7 @@ impl<'tcx> LateLintPass<'tcx> for Transmute {
579612
| transmuting_null::check(cx, e, arg, to_ty)
580613
| transmute_null_to_fn::check(cx, e, arg, to_ty)
581614
| transmute_ptr_to_ref::check(cx, e, from_ty, to_ty, arg, path, &self.msrv)
615+
| missing_transmute_annotations::check(cx, path, from_ty, to_ty, e.hir_id)
582616
| transmute_int_to_char::check(cx, e, from_ty, to_ty, arg, const_context)
583617
| transmute_ref_to_ref::check(cx, e, from_ty, to_ty, arg, const_context)
584618
| transmute_ptr_to_ptr::check(cx, e, from_ty, to_ty, arg)

‎tests/ui/author/issue_3849.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#![allow(dead_code)]
22
#![allow(clippy::zero_ptr)]
33
#![allow(clippy::transmute_ptr_to_ref)]
4-
#![allow(clippy::transmuting_null)]
4+
#![allow(clippy::transmuting_null, clippy::missing_transmute_annotations)]
55

66
pub const ZPTR: *const usize = 0 as *const _;
77

‎tests/ui/auxiliary/macro_rules.rs

+7
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,10 @@ macro_rules! macro_with_panic {
5050
panic!()
5151
};
5252
}
53+
54+
#[macro_export]
55+
macro_rules! bad_transmute {
56+
($e:expr) => {
57+
std::mem::transmute($e)
58+
};
59+
}

‎tests/ui/blocks_in_conditions.fixed

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
//@aux-build:proc_macro_attr.rs
22

33
#![warn(clippy::blocks_in_conditions)]
4-
#![allow(unused, clippy::let_and_return, clippy::needless_if)]
4+
#![allow(
5+
unused,
6+
clippy::let_and_return,
7+
clippy::needless_if,
8+
clippy::missing_transmute_annotations
9+
)]
510
#![warn(clippy::nonminimal_bool)]
611

712
macro_rules! blocky {

‎tests/ui/blocks_in_conditions.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
//@aux-build:proc_macro_attr.rs
22

33
#![warn(clippy::blocks_in_conditions)]
4-
#![allow(unused, clippy::let_and_return, clippy::needless_if)]
4+
#![allow(
5+
unused,
6+
clippy::let_and_return,
7+
clippy::needless_if,
8+
clippy::missing_transmute_annotations
9+
)]
510
#![warn(clippy::nonminimal_bool)]
611

712
macro_rules! blocky {

‎tests/ui/blocks_in_conditions.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: in an `if` condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let`
2-
--> tests/ui/blocks_in_conditions.rs:25:5
2+
--> tests/ui/blocks_in_conditions.rs:30:5
33
|
44
LL | / if {
55
LL | |
@@ -20,13 +20,13 @@ LL ~ }; if res {
2020
|
2121

2222
error: omit braces around single expression condition
23-
--> tests/ui/blocks_in_conditions.rs:37:8
23+
--> tests/ui/blocks_in_conditions.rs:42:8
2424
|
2525
LL | if { true } { 6 } else { 10 }
2626
| ^^^^^^^^ help: try: `true`
2727

2828
error: this boolean expression can be simplified
29-
--> tests/ui/blocks_in_conditions.rs:43:8
29+
--> tests/ui/blocks_in_conditions.rs:48:8
3030
|
3131
LL | if true && x == 3 { 6 } else { 10 }
3232
| ^^^^^^^^^^^^^^ help: try: `x == 3`
@@ -35,7 +35,7 @@ LL | if true && x == 3 { 6 } else { 10 }
3535
= help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]`
3636

3737
error: in a `match` scrutinee, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let`
38-
--> tests/ui/blocks_in_conditions.rs:70:5
38+
--> tests/ui/blocks_in_conditions.rs:75:5
3939
|
4040
LL | / match {
4141
LL | |

‎tests/ui/crashes/ice-1782.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![allow(dead_code, unused_variables)]
2-
#![allow(clippy::unnecessary_cast)]
2+
#![allow(clippy::unnecessary_cast, clippy::missing_transmute_annotations)]
33

44
/// Should not trigger an ICE in `SpanlessEq` / `consts::constant`
55
///

‎tests/ui/eager_transmute.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![feature(rustc_attrs)]
22
#![warn(clippy::eager_transmute)]
3-
#![allow(clippy::transmute_int_to_non_zero)]
3+
#![allow(clippy::transmute_int_to_non_zero, clippy::missing_transmute_annotations)]
44

55
use std::num::NonZeroU8;
66

‎tests/ui/eager_transmute.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![feature(rustc_attrs)]
22
#![warn(clippy::eager_transmute)]
3-
#![allow(clippy::transmute_int_to_non_zero)]
3+
#![allow(clippy::transmute_int_to_non_zero, clippy::missing_transmute_annotations)]
44

55
use std::num::NonZeroU8;
66

‎tests/ui/missing_const_for_fn/could_be_const.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::missing_const_for_fn)]
2-
#![allow(incomplete_features, clippy::let_and_return)]
2+
#![allow(incomplete_features, clippy::let_and_return, clippy::missing_transmute_annotations)]
33
#![feature(const_mut_refs)]
44
#![feature(const_trait_impl)]
55

‎tests/ui/missing_transmute_annotations.fixed

+78
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
//@aux-build:macro_rules.rs
2+
3+
#![warn(clippy::missing_transmute_annotations)]
4+
#![allow(clippy::let_with_type_underscore)]
5+
6+
#[macro_use]
7+
extern crate macro_rules;
8+
9+
macro_rules! local_bad_transmute {
10+
($e:expr) => {
11+
std::mem::transmute::<[u16; 2], i32>($e)
12+
//~^ ERROR: transmute used without annotations
13+
};
14+
}
15+
16+
fn bar(x: i32) -> i32 {
17+
x
18+
}
19+
20+
unsafe fn foo1() -> i32 {
21+
// Should not warn!
22+
std::mem::transmute([1u16, 2u16])
23+
}
24+
25+
// Should not warn!
26+
const _: i32 = unsafe { std::mem::transmute([1u16, 2u16]) };
27+
28+
#[repr(i32)]
29+
enum Foo {
30+
A = 0,
31+
}
32+
33+
unsafe fn foo2() -> i32 {
34+
let mut i: i32 = 0;
35+
i = std::mem::transmute::<[u16; 2], i32>([1u16, 2u16]);
36+
//~^ ERROR: transmute used without annotations
37+
i = std::mem::transmute::<[u16; 2], i32>([1u16, 2u16]);
38+
//~^ ERROR: transmute used without annotations
39+
i = std::mem::transmute::<[u16; 2], i32>([1u16, 2u16]);
40+
//~^ ERROR: transmute used without annotations
41+
i = std::mem::transmute::<[u16; 2], i32>([1u16, 2u16]);
42+
//~^ ERROR: transmute used without annotations
43+
44+
let x: i32 = bar(std::mem::transmute::<[u16; 2], i32>([1u16, 2u16]));
45+
//~^ ERROR: transmute used without annotations
46+
bar(std::mem::transmute::<[u16; 2], i32>([1u16, 2u16]));
47+
//~^ ERROR: transmute used without annotations
48+
49+
i = local_bad_transmute!([1u16, 2u16]);
50+
51+
// Should not warn.
52+
i = bad_transmute!([1u16, 2u16]);
53+
54+
i = std::mem::transmute::<[i16; 2], i32>([0i16, 0i16]);
55+
//~^ ERROR: transmute used without annotations
56+
57+
i = std::mem::transmute::<Foo, i32>(Foo::A);
58+
//~^ ERROR: transmute used without annotations
59+
60+
i
61+
}
62+
63+
fn main() {
64+
let x: _ = unsafe { std::mem::transmute::<[u16; 2], i32>([1u16, 2u16]) };
65+
//~^ ERROR: transmute used without annotations
66+
unsafe {
67+
let x: _ = std::mem::transmute::<[u16; 2], i32>([1u16, 2u16]);
68+
//~^ ERROR: transmute used without annotations
69+
70+
// Should not warn.
71+
std::mem::transmute::<[u16; 2], i32>([1u16, 2u16]);
72+
let x = std::mem::transmute::<[u16; 2], i32>([1u16, 2u16]);
73+
let x: i32 = std::mem::transmute::<[u16; 2], _>([1u16, 2u16]);
74+
let x: i32 = std::mem::transmute::<_, i32>([1u16, 2u16]);
75+
let x: i32 = std::mem::transmute([1u16, 2u16]);
76+
}
77+
let x: i32 = unsafe { std::mem::transmute([1u16, 2u16]) };
78+
}

0 commit comments

Comments
 (0)