user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Count unsafe operations and macro calls once towards the innermost unsafe block (#16117)

Browse source 
changelog: [`multiple_unsafe_ops_per_block`]: count unsafe operations
only towards the innermost unsafe block

Fixes rust-lang/rust-clippy#16116

r? Jarcho
This commit is contained in:
Jason Newcomb 2025-12-15 14:57:23 +00:00 • committed by GitHub
commit 4d4789cdec
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 392 additions and 68 deletions
Download patch file Download diff file Expand all files Collapse all files

70
clippy_lints/src/multiple_unsafe_ops_per_block.rs
View file

@ -3,13 +3,14 @@ use clippy_utils::diagnostics::span_lint_and_then;
use hir::def::{DefKind, Res};
use hir::{BlockCheckMode, ExprKind, QPath, UnOp};
use rustc_ast::{BorrowKind, Mutability};
use rustc_data_structures::fx::FxHashMap;
use rustc_hir as hir;
use rustc_hir::intravisit::{Visitor, walk_body, walk_expr};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::{self, TyCtxt, TypeckResults};
use rustc_session::declare_lint_pass;
use rustc_span::{DesugaringKind, Span};
use rustc_span::Span;
declare_clippy_lint! {
/// ### What it does
@ -56,12 +57,16 @@ declare_clippy_lint! {
/// }
/// ```
///
/// ### Note
/// ### Notes
///
/// Taking a raw pointer to a union field is always safe and will
/// not be considered unsafe by this lint, even when linting code written
/// with a specified Rust version of 1.91 or earlier (which required
/// using an `unsafe` block).
/// - Unsafe operations only count towards the total for the innermost
/// enclosing `unsafe` block.
/// - Each call to a macro expanding to unsafe operations count for one
/// unsafe operation.
/// - Taking a raw pointer to a union field is always safe and will
/// not be considered unsafe by this lint, even when linting code written
/// with a specified Rust version of 1.91 or earlier (which required
/// using an `unsafe` block).
#[clippy::version = "1.69.0"]
pub MULTIPLE_UNSAFE_OPS_PER_BLOCK,
restriction,
@ -71,10 +76,7 @@ declare_lint_pass!(MultipleUnsafeOpsPerBlock => [MULTIPLE_UNSAFE_OPS_PER_BLOCK])
impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
if !matches!(block.rules, BlockCheckMode::UnsafeBlock(_))
|| block.span.in_external_macro(cx.tcx.sess.source_map())
|| block.span.is_desugaring(DesugaringKind::Await)
{
if !matches!(block.rules, BlockCheckMode::UnsafeBlock(_)) || block.span.from_expansion() {
return;
}
let unsafe_ops = UnsafeExprCollector::collect_unsafe_exprs(cx, block);
@ -100,7 +102,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
struct UnsafeExprCollector<'tcx> {
tcx: TyCtxt<'tcx>,
typeck_results: &'tcx TypeckResults<'tcx>,
unsafe_ops: Vec<(&'static str, Span)>,
unsafe_ops: FxHashMap<Span, &'static str>,
}
impl<'tcx> UnsafeExprCollector<'tcx> {
@ -108,10 +110,33 @@ impl<'tcx> UnsafeExprCollector<'tcx> {
let mut collector = Self {
tcx: cx.tcx,
typeck_results: cx.typeck_results(),
unsafe_ops: vec![],
unsafe_ops: FxHashMap::default(),
};
collector.visit_block(block);
collector.unsafe_ops
#[allow(
rustc::potential_query_instability,
reason = "span ordering only needed inside the one expression being walked"
)]
let mut unsafe_ops = collector
.unsafe_ops
.into_iter()
.map(|(span, msg)| (msg, span))
.collect::<Vec<_>>();
unsafe_ops.sort_unstable();
unsafe_ops
}
}
impl UnsafeExprCollector<'_> {
fn insert_span(&mut self, span: Span, message: &'static str) {
if span.from_expansion() {
self.unsafe_ops.insert(
span.source_callsite(),
"this macro call expands into one or more unsafe operations",
);
} else {
self.unsafe_ops.insert(span, message);
}
}
}
@ -126,7 +151,10 @@ impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
return self.visit_expr(e);
},
ExprKind::InlineAsm(_) => self.unsafe_ops.push(("inline assembly used here", expr.span)),
// Do not recurse inside an inner `unsafe` block, it will be checked on its own
ExprKind::Block(block, _) if matches!(block.rules, BlockCheckMode::UnsafeBlock(_)) => return,
ExprKind::InlineAsm(_) => self.insert_span(expr.span, "inline assembly used here"),
ExprKind::AddrOf(BorrowKind::Raw, _, mut inner) => {
while let ExprKind::Field(prefix, _) = inner.kind
@ -139,7 +167,7 @@ impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
ExprKind::Field(e, _) => {
if self.typeck_results.expr_ty(e).is_union() {
self.unsafe_ops.push(("union field access occurs here", expr.span));
self.insert_span(expr.span, "union field access occurs here");
}
},
@ -157,12 +185,11 @@ impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
..
},
)) => {
self.unsafe_ops
.push(("access of a mutable static occurs here", expr.span));
self.insert_span(expr.span, "access of a mutable static occurs here");
},
ExprKind::Unary(UnOp::Deref, e) if self.typeck_results.expr_ty(e).is_raw_ptr() => {
self.unsafe_ops.push(("raw pointer dereference occurs here", expr.span));
self.insert_span(expr.span, "raw pointer dereference occurs here");
},
ExprKind::Call(path_expr, _) => {
@ -172,7 +199,7 @@ impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
_ => None,
};
if opt_sig.is_some_and(|sig| sig.safety().is_unsafe()) {
self.unsafe_ops.push(("unsafe function call occurs here", expr.span));
self.insert_span(expr.span, "unsafe function call occurs here");
}
},
@ -182,7 +209,7 @@ impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
.type_dependent_def_id(expr.hir_id)
.map(|def_id| self.tcx.fn_sig(def_id));
if opt_sig.is_some_and(|sig| sig.skip_binder().safety().is_unsafe()) {
self.unsafe_ops.push(("unsafe method call occurs here", expr.span));
self.insert_span(expr.span, "unsafe method call occurs here");
}
},
@ -203,8 +230,7 @@ impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
}
))
) {
self.unsafe_ops
.push(("modification of a mutable static occurs here", expr.span));
self.insert_span(expr.span, "modification of a mutable static occurs here");
return self.visit_expr(rhs);
}
},

133
tests/ui/multiple_unsafe_ops_per_block.rs
View file

@ -312,4 +312,137 @@ fn check_closures() {
}
}
fn issue16116() {
unsafe fn foo() -> u32 {
0
}
// Do not lint even though `format!` expansion
// contains unsafe calls.
unsafe {
let _ = format!("{}", foo());
}
unsafe {
//~^ multiple_unsafe_ops_per_block
let _ = format!("{}", foo());
let _ = format!("{}", foo());
}
// Do not lint: only one `assert!()` argument is unsafe
unsafe {
assert_eq!(foo(), 0, "{}", 1 + 2);
}
// Each argument of a macro call may count as an unsafe operation.
unsafe {
//~^ multiple_unsafe_ops_per_block
assert_eq!(foo(), 0, "{}", foo()); // One unsafe operation
}
macro_rules! twice {
($e:expr) => {{
$e;
$e;
}};
}
// Do not lint, a repeated argument used twice by a macro counts
// as at most one unsafe operation.
unsafe {
twice!(foo());
}
unsafe {
//~^ multiple_unsafe_ops_per_block
twice!(foo());
twice!(foo());
}
unsafe {
//~^ multiple_unsafe_ops_per_block
assert_eq!(foo(), 0, "{}", 1 + 2);
assert_eq!(foo(), 0, "{}", 1 + 2);
}
macro_rules! unsafe_twice {
($e:expr) => {
unsafe {
$e;
$e;
}
};
};
// A macro whose expansion contains unsafe blocks will not
// check inside the blocks.
unsafe {
unsafe_twice!(foo());
}
macro_rules! double_non_arg_unsafe {
() => {{
_ = str::from_utf8_unchecked(&[]);
_ = str::from_utf8_unchecked(&[]);
}};
}
// Do not lint: each unsafe expression contained in the
// macro expansion will count towards the macro call.
unsafe {
double_non_arg_unsafe!();
}
unsafe {
//~^ multiple_unsafe_ops_per_block
double_non_arg_unsafe!();
double_non_arg_unsafe!();
}
// Do not lint: the inner macro call counts as one unsafe op.
unsafe {
assert_eq!(double_non_arg_unsafe!(), ());
}
unsafe {
//~^ multiple_unsafe_ops_per_block
assert_eq!(double_non_arg_unsafe!(), ());
assert_eq!(double_non_arg_unsafe!(), ());
}
unsafe {
//~^ multiple_unsafe_ops_per_block
assert_eq!((double_non_arg_unsafe!(), double_non_arg_unsafe!()), ((), ()));
}
macro_rules! unsafe_with_arg {
($e:expr) => {{
_ = str::from_utf8_unchecked(&[]);
$e;
}};
}
// A confusing situation: the macro call counts towards two unsafe calls,
// one coming from inside the macro itself, and one coming from its argument.
// The error message may seem a bit strange as both the macro call and its
// argument will be marked as counting as unsafe ops, but a short investigation
// in those rare situations should sort it out easily.
unsafe {
//~^ multiple_unsafe_ops_per_block
unsafe_with_arg!(foo());
}
macro_rules! ignore {
($e: expr) => {};
}
// Another surprising case is when the macro argument is not
// used in the expansion, but in this case we won't see the
// unsafe operation at all.
unsafe {
ignore!(foo());
ignore!(foo());
}
}
fn main() {}

257
tests/ui/multiple_unsafe_ops_per_block.stderr
View file

@ -31,16 +31,16 @@ LL | | *raw_ptr();
LL | | }
| |_____^
|
note: union field access occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:50:14
|
LL | drop(u.u);
| ^^^
note: raw pointer dereference occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:51:9
|
LL | *raw_ptr();
| ^^^^^^^^^^
note: union field access occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:50:14
|
LL | drop(u.u);
| ^^^
error: this `unsafe` block contains 3 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:56:5
@ -58,16 +58,16 @@ note: inline assembly used here
|
LL | asm!("nop");
| ^^^^^^^^^^^
note: unsafe method call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:59:9
|
LL | sample.not_very_safe();
| ^^^^^^^^^^^^^^^^^^^^^^
note: modification of a mutable static occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:60:9
|
LL | STATIC = 0;
| ^^^^^^^^^^
note: unsafe method call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:59:9
|
LL | sample.not_very_safe();
| ^^^^^^^^^^^^^^^^^^^^^^
error: this `unsafe` block contains 6 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:66:5
@ -81,36 +81,36 @@ LL | | asm!("nop");
LL | | }
| |_____^
|
note: union field access occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:68:14
|
LL | drop(u.u);
| ^^^
note: access of a mutable static occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:69:14
|
LL | drop(STATIC);
| ^^^^^^
note: unsafe method call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:70:9
|
LL | sample.not_very_safe();
| ^^^^^^^^^^^^^^^^^^^^^^
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:71:9
|
LL | not_very_safe();
| ^^^^^^^^^^^^^^^
note: raw pointer dereference occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:72:9
|
LL | *raw_ptr();
| ^^^^^^^^^^
note: inline assembly used here
--> tests/ui/multiple_unsafe_ops_per_block.rs:73:9
|
LL | asm!("nop");
| ^^^^^^^^^^^
note: raw pointer dereference occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:72:9
|
LL | *raw_ptr();
| ^^^^^^^^^^
note: union field access occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:68:14
|
LL | drop(u.u);
| ^^^
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:71:9
|
LL | not_very_safe();
| ^^^^^^^^^^^^^^^
note: unsafe method call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:70:9
|
LL | sample.not_very_safe();
| ^^^^^^^^^^^^^^^^^^^^^^
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:109:5
@ -139,16 +139,16 @@ error: this `unsafe` block contains 2 unsafe operations, expected only one
LL | unsafe { char::from_u32_unchecked(*ptr.cast::<u32>()) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:118:18
|
LL | unsafe { char::from_u32_unchecked(*ptr.cast::<u32>()) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: raw pointer dereference occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:118:43
|
LL | unsafe { char::from_u32_unchecked(*ptr.cast::<u32>()) }
| ^^^^^^^^^^^^^^^^^^
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:118:18
|
LL | unsafe { char::from_u32_unchecked(*ptr.cast::<u32>()) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:139:9
@ -224,16 +224,16 @@ LL | | foo().await;
LL | | }
| |_____^
|
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:194:9
|
LL | not_very_safe();
| ^^^^^^^^^^^^^^^
note: modification of a mutable static occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:195:9
|
LL | STATIC += 1;
| ^^^^^^^^^^^
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:194:9
|
LL | not_very_safe();
| ^^^^^^^^^^^^^^^
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:207:5
@ -265,16 +265,16 @@ LL | | Some(foo_unchecked()).unwrap_unchecked().await;
LL | | }
| |_____^
|
note: unsafe method call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:216:9
|
LL | Some(foo_unchecked()).unwrap_unchecked().await;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:216:14
|
LL | Some(foo_unchecked()).unwrap_unchecked().await;
| ^^^^^^^^^^^^^^^
note: unsafe method call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:216:9
|
LL | Some(foo_unchecked()).unwrap_unchecked().await;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:236:5
@ -359,5 +359,170 @@ note: unsafe function call occurs here
LL | apply(|| f(0));
| ^^^^
error: aborting due to 16 previous errors
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:326:5
|
LL | / unsafe {
LL | |
LL | | let _ = format!("{}", foo());
LL | | let _ = format!("{}", foo());
LL | | }
| |_____^
|
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:328:31
|
LL | let _ = format!("{}", foo());
| ^^^^^
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:329:31
|
LL | let _ = format!("{}", foo());
| ^^^^^
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:338:5
|
LL | / unsafe {
LL | |
LL | | assert_eq!(foo(), 0, "{}", foo()); // One unsafe operation
LL | | }
| |_____^
|
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:340:20
|
LL | assert_eq!(foo(), 0, "{}", foo()); // One unsafe operation
| ^^^^^
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:340:36
|
LL | assert_eq!(foo(), 0, "{}", foo()); // One unsafe operation
| ^^^^^
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:356:5
|
LL | / unsafe {
LL | |
LL | | twice!(foo());
LL | | twice!(foo());
LL | | }
| |_____^
|
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:358:16
|
LL | twice!(foo());
| ^^^^^
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:359:16
|
LL | twice!(foo());
| ^^^^^
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:362:5
|
LL | / unsafe {
LL | |
LL | | assert_eq!(foo(), 0, "{}", 1 + 2);
LL | | assert_eq!(foo(), 0, "{}", 1 + 2);
LL | | }
| |_____^
|
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:364:20
|
LL | assert_eq!(foo(), 0, "{}", 1 + 2);
| ^^^^^
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:365:20
|
LL | assert_eq!(foo(), 0, "{}", 1 + 2);
| ^^^^^
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:396:5
|
LL | / unsafe {
LL | |
LL | | double_non_arg_unsafe!();
LL | | double_non_arg_unsafe!();
LL | | }
| |_____^
|
note: this macro call expands into one or more unsafe operations
--> tests/ui/multiple_unsafe_ops_per_block.rs:398:9
|
LL | double_non_arg_unsafe!();
| ^^^^^^^^^^^^^^^^^^^^^^^^
note: this macro call expands into one or more unsafe operations
--> tests/ui/multiple_unsafe_ops_per_block.rs:399:9
|
LL | double_non_arg_unsafe!();
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:407:5
|
LL | / unsafe {
LL | |
LL | | assert_eq!(double_non_arg_unsafe!(), ());
LL | | assert_eq!(double_non_arg_unsafe!(), ());
LL | | }
| |_____^
|
note: this macro call expands into one or more unsafe operations
--> tests/ui/multiple_unsafe_ops_per_block.rs:409:20
|
LL | assert_eq!(double_non_arg_unsafe!(), ());
| ^^^^^^^^^^^^^^^^^^^^^^^^
note: this macro call expands into one or more unsafe operations
--> tests/ui/multiple_unsafe_ops_per_block.rs:410:20
|
LL | assert_eq!(double_non_arg_unsafe!(), ());
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:413:5
|
LL | / unsafe {
LL | |
LL | | assert_eq!((double_non_arg_unsafe!(), double_non_arg_unsafe!()), ((), ()));
LL | | }
| |_____^
|
note: this macro call expands into one or more unsafe operations
--> tests/ui/multiple_unsafe_ops_per_block.rs:415:21
|
LL | assert_eq!((double_non_arg_unsafe!(), double_non_arg_unsafe!()), ((), ()));
| ^^^^^^^^^^^^^^^^^^^^^^^^
note: this macro call expands into one or more unsafe operations
--> tests/ui/multiple_unsafe_ops_per_block.rs:415:47
|
LL | assert_eq!((double_non_arg_unsafe!(), double_non_arg_unsafe!()), ((), ()));
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: this `unsafe` block contains 2 unsafe operations, expected only one
--> tests/ui/multiple_unsafe_ops_per_block.rs:430:5
|
LL | / unsafe {
LL | |
LL | | unsafe_with_arg!(foo());
LL | | }
| |_____^
|
note: this macro call expands into one or more unsafe operations
--> tests/ui/multiple_unsafe_ops_per_block.rs:432:9
|
LL | unsafe_with_arg!(foo());
| ^^^^^^^^^^^^^^^^^^^^^^^
note: unsafe function call occurs here
--> tests/ui/multiple_unsafe_ops_per_block.rs:432:26
|
LL | unsafe_with_arg!(foo());
| ^^^^^
error: aborting due to 24 previous errors