Still lint raw pointer to union field if MSRV < 1.92

Author: samueltardieu

book/src/lint_configuration.md

@@ -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)

clippy_config/src/conf.rs

@@ -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,

clippy_lints/src/lib.rs

@@ -765,7 +765,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(move |_| Box::new(semicolon_block::SemicolonBlock::new(conf))),
         Box::new(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse)),
         Box::new(|_| Box::new(size_of_ref::SizeOfRef)),
-        Box::new(|_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock)),
+        Box::new(move |_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock::new(conf))),
         Box::new(move |_| Box::new(extra_unused_type_parameters::ExtraUnusedTypeParameters::new(conf))),
         Box::new(|_| Box::new(no_mangle_with_rust_abi::NoMangleWithRustAbi)),
         Box::new(|_| Box::new(collection_is_never_read::CollectionIsNeverRead)),

clippy_lints/src/multiple_unsafe_ops_per_block.rs

@@ -1,14 +1,16 @@
-use clippy_utils::desugar_await;
+use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_then;
+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::{BorrowKind, Mutability};
 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_middle::ty::{self, TypeckResults};
+use rustc_session::impl_lint_pass;
 use rustc_span::{DesugaringKind, Span};
 
 declare_clippy_lint! {
@@ -55,19 +57,23 @@ declare_clippy_lint! {
     ///     unsafe { char::from_u32_unchecked(int_value) }
     /// }
     /// ```
-    ///
-    /// ### Note
-    ///
-    /// 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,
     "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<'_>) {
@@ -77,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
         {
             return;
         }
-        let unsafe_ops = UnsafeExprCollector::collect_unsafe_exprs(cx, block);
+        let unsafe_ops = UnsafeExprCollector::collect_unsafe_exprs(cx, block, self.msrv);
         if unsafe_ops.len() > 1 {
             span_lint_and_then(
                 cx,
@@ -97,28 +103,52 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
     }
 }
 
-struct UnsafeExprCollector<'tcx> {
-    tcx: TyCtxt<'tcx>,
+#[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<'tcx> UnsafeExprCollector<'tcx> {
-    fn collect_unsafe_exprs(cx: &LateContext<'tcx>, block: &'tcx hir::Block<'tcx>) -> Vec<(&'static str, Span)> {
+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 {
-            tcx: cx.tcx,
+            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> {
+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
@@ -128,17 +158,21 @@ impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
 
             ExprKind::InlineAsm(_) => self.unsafe_ops.push(("inline assembly used here", expr.span)),
 
-            ExprKind::AddrOf(BorrowKind::Raw, _, mut inner) => {
-                while let ExprKind::Field(prefix, _) = inner.kind
-                    && self.typeck_results.expr_adjustments(prefix).is_empty()
-                {
-                    inner = prefix;
-                }
-                return self.visit_expr(inner);
+            ExprKind::AddrOf(BorrowKind::Raw, _, _) => {
+                self.under_raw_ptr = UnderRawPtr::Yes;
             },
 
-            ExprKind::Field(e, _) => {
-                if self.typeck_results.expr_ty(e).is_union() {
+            ExprKind::Field(e, _) if self.typeck_results.expr_adjustments(e).is_empty() => {
+                // 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 self.typeck_results.expr_ty(e).is_union()
+                    && matches!(self.under_raw_ptr, UnderRawPtr::No | UnderRawPtr::WithSafeMsrv(false))
+                {
                     self.unsafe_ops.push(("union field access occurs here", expr.span));
                 }
             },
@@ -167,7 +201,7 @@ impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
 
             ExprKind::Call(path_expr, _) => {
                 let opt_sig = match *self.typeck_results.expr_ty_adjusted(path_expr).kind() {
-                    ty::FnDef(id, _) => Some(self.tcx.fn_sig(id).skip_binder()),
+                    ty::FnDef(id, _) => Some(self.cx.tcx.fn_sig(id).skip_binder()),
                     ty::FnPtr(sig_tys, hdr) => Some(sig_tys.with(hdr)),
                     _ => None,
                 };
@@ -180,7 +214,7 @@ impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
                 let opt_sig = self
                     .typeck_results
                     .type_dependent_def_id(expr.hir_id)
-                    .map(|def_id| self.tcx.fn_sig(def_id));
+                    .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));
                 }
@@ -217,12 +251,12 @@ impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
 
     fn visit_body(&mut self, body: &hir::Body<'tcx>) {
         let saved_typeck_results = self.typeck_results;
-        self.typeck_results = self.tcx.typeck_body(body.id());
+        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.tcx
+        self.cx.tcx
     }
 }

clippy_utils/src/msrvs.rs

@@ -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 }

tests/ui/multiple_unsafe_ops_per_block.rs

@@ -1,5 +1,6 @@
 //@needs-asm-support
 //@aux-build:proc_macros.rs
+#![feature(stmt_expr_attributes)]
 #![expect(
     dropping_copy_types,
     clippy::unnecessary_operation,
@@ -227,11 +228,20 @@ fn issue16076() {
     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