Skip to content
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
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
* [`mem_replace_option_with_some`](https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_some)
* [`mem_replace_with_default`](https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_default)
* [`missing_const_for_fn`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn)
* [`multiple_unsafe_ops_per_block`](https://rust-lang.github.io/rust-clippy/master/index.html#multiple_unsafe_ops_per_block)
* [`needless_borrow`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow)
* [`non_std_lazy_statics`](https://rust-lang.github.io/rust-clippy/master/index.html#non_std_lazy_statics)
* [`option_as_ref_deref`](https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref)
Expand Down
1 change: 1 addition & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,7 @@ define_Conf! {
mem_replace_option_with_some,
mem_replace_with_default,
missing_const_for_fn,
multiple_unsafe_ops_per_block,
needless_borrow,
non_std_lazy_statics,
option_as_ref_deref,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
store.register_late_pass(move |_| Box::new(semicolon_block::SemicolonBlock::new(conf)));
store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse));
store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef));
store.register_late_pass(|_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock));
store.register_late_pass(move |_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock::new(conf)));
store.register_late_pass(move |_| Box::new(extra_unused_type_parameters::ExtraUnusedTypeParameters::new(conf)));
store.register_late_pass(|_| Box::new(no_mangle_with_rust_abi::NoMangleWithRustAbi));
store.register_late_pass(|_| Box::new(collection_is_never_read::CollectionIsNeverRead));
Expand Down
155 changes: 115 additions & 40 deletions clippy_lints/src/multiple_unsafe_ops_per_block.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
use clippy_utils::desugar_await;
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::visitors::{Descend, Visitable, for_each_expr};
use core::ops::ControlFlow::Continue;
use clippy_utils::msrvs::Msrv;
use clippy_utils::{desugar_await, msrvs};
use hir::def::{DefKind, Res};
use hir::{BlockCheckMode, ExprKind, QPath, UnOp};
use rustc_ast::Mutability;
use rustc_ast::{BorrowKind, Mutability};
use rustc_hir as hir;
use rustc_hir::intravisit::{Visitor, walk_body, walk_expr};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::declare_lint_pass;
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::{self, TypeckResults};
use rustc_session::impl_lint_pass;
use rustc_span::{DesugaringKind, Span};

declare_clippy_lint! {
Expand Down Expand Up @@ -60,7 +62,18 @@ declare_clippy_lint! {
restriction,
"more than one unsafe operation per `unsafe` block"
}
declare_lint_pass!(MultipleUnsafeOpsPerBlock => [MULTIPLE_UNSAFE_OPS_PER_BLOCK]);

pub struct MultipleUnsafeOpsPerBlock {
msrv: Msrv,
}

impl_lint_pass!(MultipleUnsafeOpsPerBlock => [MULTIPLE_UNSAFE_OPS_PER_BLOCK]);

impl MultipleUnsafeOpsPerBlock {
pub fn new(conf: &Conf) -> Self {
Self { msrv: conf.msrv }
}
}

impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
Expand All @@ -70,8 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
{
return;
}
let mut unsafe_ops = vec![];
collect_unsafe_exprs(cx, block, &mut unsafe_ops);
let unsafe_ops = UnsafeExprCollector::collect_unsafe_exprs(cx, block, self.msrv);
if unsafe_ops.len() > 1 {
span_lint_and_then(
cx,
Expand All @@ -91,25 +103,77 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
}
}

fn collect_unsafe_exprs<'tcx>(
cx: &LateContext<'tcx>,
node: impl Visitable<'tcx>,
unsafe_ops: &mut Vec<(&'static str, Span)>,
) {
for_each_expr(cx, node, |expr| {
#[derive(Clone, Copy)]
enum UnderRawPtr {
/// The expression is not located under a raw pointer
No,
/// The expression is located under a raw pointer, MSRV yet unknown
Yes,
/// The expression is located under a raw pointer and MSRV has been determined.
/// `true` means that taking a raw pointer to a union field is a safe operation.
WithSafeMsrv(bool),
}

struct UnsafeExprCollector<'cx, 'tcx> {
cx: &'cx LateContext<'tcx>,
typeck_results: &'tcx TypeckResults<'tcx>,
msrv: Msrv,
unsafe_ops: Vec<(&'static str, Span)>,
under_raw_ptr: UnderRawPtr,
}

impl<'cx, 'tcx> UnsafeExprCollector<'cx, 'tcx> {
fn collect_unsafe_exprs(
cx: &'cx LateContext<'tcx>,
block: &'tcx hir::Block<'tcx>,
msrv: Msrv,
) -> Vec<(&'static str, Span)> {
let mut collector = Self {
cx,
typeck_results: cx.typeck_results(),
msrv,
unsafe_ops: vec![],
under_raw_ptr: UnderRawPtr::No,
};
collector.visit_block(block);
collector.unsafe_ops
}
}

impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'_, 'tcx> {
type NestedFilter = nested_filter::OnlyBodies;

fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
// `self.under_raw_ptr` is preventively reset, while the current value is
// preserved in `under_raw_ptr`.
let under_raw_ptr = self.under_raw_ptr;
self.under_raw_ptr = UnderRawPtr::No;

match expr.kind {
// The `await` itself will desugar to two unsafe calls, but we should ignore those.
// Instead, check the expression that is `await`ed
_ if let Some(e) = desugar_await(expr) => {
collect_unsafe_exprs(cx, e, unsafe_ops);
return Continue(Descend::No);
return self.visit_expr(e);
},

ExprKind::InlineAsm(_) => unsafe_ops.push(("inline assembly used here", expr.span)),
ExprKind::InlineAsm(_) => self.unsafe_ops.push(("inline assembly used here", expr.span)),

ExprKind::AddrOf(BorrowKind::Raw, _, _) => {
self.under_raw_ptr = UnderRawPtr::Yes;
},
Comment on lines +161 to +163
Copy link
Contributor

@Jarcho Jarcho Nov 13, 2025

Choose a reason for hiding this comment

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

Without the MSRV handling this could be:

ExprKind::AddrOf(BorrowKind::Raw, _, mut e) => {
  while let ExprKind::Field(base, _) = e.kind
    && self.typeck_results.expr_adjustments(base).is_empty() // No deref
  { e = base; }
  return self.visit_expr(e); 
}

This removes all the complexity of Self::under_raw_ptr.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, but with Rust < 1.92 we would not flag multiple unsafe operations in the block as the lint indicates, while not letting the user get the ref to union field outside of the unsafe block because Rust won't allow it. That would be a regression in itself, as it could break some #[expect] that would have been placed on the block/function.

The MSRV logic is about 5 lines long + declarations in order to implement some caching, so it is neither complex nor particularly inefficient.

But I don't have a strong opinion about it, I'll let @y21 weigh in.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't guarantee expect will continue to trigger when updating so that's not, on it's own, a regression. Not taking the MSRV doesn't inconvenience anyone who already split the blocks, and anyone with an expect thought it was better to keep it as a single block. Anybody who separates this into a new block will also get rustc's unused_unsafe warning (they probably shouldn't). Who is actually in the set of people who actually want this MSRV dependent behaviour.

Copy link
Contributor

@Jarcho Jarcho Nov 13, 2025

Choose a reason for hiding this comment

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

I wasn't trying to say the logic is particularly complex in this case. It isn't, but it is additional logic that seems to benefit nobody.


ExprKind::Field(e, _) => {
if cx.typeck_results().expr_ty(e).is_union() {
unsafe_ops.push(("union field access occurs here", expr.span));
if self.typeck_results.expr_ty(e).is_union() {
// Restore `self.under_raw_pointer` and determine safety of taking a raw pointer to
// a union field if this is not known already.
self.under_raw_ptr = if matches!(under_raw_ptr, UnderRawPtr::Yes) {
UnderRawPtr::WithSafeMsrv(self.msrv.meets(self.cx, msrvs::SAFE_RAW_PTR_TO_UNION_FIELD))
} else {
under_raw_ptr
};
if matches!(self.under_raw_ptr, UnderRawPtr::No | UnderRawPtr::WithSafeMsrv(false)) {
self.unsafe_ops.push(("union field access occurs here", expr.span));
}
}
},

Expand All @@ -127,32 +191,32 @@ fn collect_unsafe_exprs<'tcx>(
..
},
)) => {
unsafe_ops.push(("access of a mutable static occurs here", expr.span));
self.unsafe_ops
.push(("access of a mutable static occurs here", expr.span));
},

ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty_adjusted(e).is_raw_ptr() => {
unsafe_ops.push(("raw pointer dereference occurs here", expr.span));
ExprKind::Unary(UnOp::Deref, e) if self.typeck_results.expr_ty_adjusted(e).is_raw_ptr() => {
self.unsafe_ops.push(("raw pointer dereference occurs here", expr.span));
},

ExprKind::Call(path_expr, _) => {
let sig = match *cx.typeck_results().expr_ty(path_expr).kind() {
ty::FnDef(id, _) => cx.tcx.fn_sig(id).skip_binder(),
ty::FnPtr(sig_tys, hdr) => sig_tys.with(hdr),
_ => return Continue(Descend::Yes),
let opt_sig = match *self.typeck_results.expr_ty(path_expr).kind() {
ty::FnDef(id, _) => Some(self.cx.tcx.fn_sig(id).skip_binder()),
ty::FnPtr(sig_tys, hdr) => Some(sig_tys.with(hdr)),
_ => None,
};
if sig.safety().is_unsafe() {
unsafe_ops.push(("unsafe function call occurs here", expr.span));
if opt_sig.is_some_and(|sig| sig.safety().is_unsafe()) {
self.unsafe_ops.push(("unsafe function call occurs here", expr.span));
}
},

ExprKind::MethodCall(..) => {
if let Some(sig) = cx
.typeck_results()
let opt_sig = self
.typeck_results
.type_dependent_def_id(expr.hir_id)
.map(|def_id| cx.tcx.fn_sig(def_id))
&& sig.skip_binder().safety().is_unsafe()
{
unsafe_ops.push(("unsafe method call occurs here", expr.span));
.map(|def_id| self.cx.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));
}
},

Expand All @@ -173,15 +237,26 @@ fn collect_unsafe_exprs<'tcx>(
}
))
) {
unsafe_ops.push(("modification of a mutable static occurs here", expr.span));
collect_unsafe_exprs(cx, rhs, unsafe_ops);
return Continue(Descend::No);
self.unsafe_ops
.push(("modification of a mutable static occurs here", expr.span));
return self.visit_expr(rhs);
}
},

_ => {},
}

Continue::<(), _>(Descend::Yes)
});
walk_expr(self, expr);
}

fn visit_body(&mut self, body: &hir::Body<'tcx>) {
let saved_typeck_results = self.typeck_results;
self.typeck_results = self.cx.tcx.typeck_body(body.id());
walk_body(self, body);
self.typeck_results = saved_typeck_results;
}

fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
self.cx.tcx
}
}
1 change: 1 addition & 0 deletions clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ macro_rules! msrv_aliases {

// names may refer to stabilized feature flags or library items
msrv_aliases! {
1,92,0 { SAFE_RAW_PTR_TO_UNION_FIELD }
1,88,0 { LET_CHAINS }
1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, CONST_CHAR_IS_DIGIT, UNSIGNED_IS_MULTIPLE_OF, INTEGER_SIGN_CAST }
1,85,0 { UINT_FLOAT_MIDPOINT, CONST_SIZE_OF_VAL }
Expand Down
66 changes: 66 additions & 0 deletions tests/ui/multiple_unsafe_ops_per_block.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@needs-asm-support
//@aux-build:proc_macros.rs
#![feature(stmt_expr_attributes)]
#![expect(
dropping_copy_types,
clippy::unnecessary_operation,
Expand Down Expand Up @@ -209,4 +210,69 @@ async fn issue13879() {
}
}

fn issue16076() {
#[derive(Clone, Copy)]
union U {
i: u32,
f: f32,
}

let u = U { i: 0 };

// Taking a raw pointer to a place is safe since Rust 1.92
#[clippy::msrv = "1.92"]
unsafe {
_ = &raw const u.i;
_ = &raw const u.i;
}

// However it was not the case before Rust 1.92
#[clippy::msrv = "1.91"]
unsafe {
//~^ multiple_unsafe_ops_per_block
_ = &raw const u.i;
_ = &raw const u.i;
}

// Taking a reference to a union field is not safe
unsafe {
//~^ multiple_unsafe_ops_per_block
_ = &u.i;
_ = &u.i;
}

// Check that we still check and lint the prefix of the raw pointer to a field access
#[expect(clippy::deref_addrof)]
unsafe {
//~^ multiple_unsafe_ops_per_block
_ = &raw const (*&raw const u).i;
_ = &raw const (*&raw const u).i;
}

union V {
u: U,
}

// Taking a raw pointer to a union field of an union field (etc.) is safe
let v = V { u };
unsafe {
_ = &raw const v.u.i;
_ = &raw const v.u.i;
}
}

fn check_closures() {
unsafe fn apply(f: impl Fn()) {
todo!()
}
unsafe fn f(_x: i32) {
todo!()
}

unsafe {
//~^ multiple_unsafe_ops_per_block
apply(|| f(0));
}
}

fn main() {}
Loading