Taking a raw pointer on a union field is safe since Rust 1.92

Author: samueltardieu

clippy_lints/src/multiple_unsafe_ops_per_block.rs

@@ -1,13 +1,13 @@
 use clippy_utils::desugar_await;
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::visitors::{Descend, Visitable, for_each_expr};
-use core::ops::ControlFlow::Continue;
 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_middle::hir::nested_filter;
+use rustc_middle::ty::{self, TypeckResults};
 use rustc_session::declare_lint_pass;
 use rustc_span::{DesugaringKind, Span};
 
@@ -70,8 +70,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);
         if unsafe_ops.len() > 1 {
             span_lint_and_then(
                 cx,
@@ -91,25 +90,47 @@ 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| {
+struct UnsafeExprCollector<'cx, 'tcx> {
+    cx: &'cx LateContext<'tcx>,
+    typeck_results: &'tcx TypeckResults<'tcx>,
+    unsafe_ops: Vec<(&'static str, Span)>,
+}
+
+impl<'cx, 'tcx> UnsafeExprCollector<'cx, 'tcx> {
+    fn collect_unsafe_exprs(cx: &'cx LateContext<'tcx>, block: &'tcx hir::Block<'tcx>) -> Vec<(&'static str, Span)> {
+        let mut collector = Self {
+            cx,
+            typeck_results: cx.typeck_results(),
+            unsafe_ops: vec![],
+        };
+        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>) {
         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, _, mut inner) => {
+                while let ExprKind::Field(prefix, _) = inner.kind {
+                    inner = prefix;
+                }
+                return self.visit_expr(inner);
+            },
 
             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() {
+                    self.unsafe_ops.push(("union field access occurs here", expr.span));
                 }
             },
 
@@ -127,32 +148,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));
                 }
             },
 
@@ -173,15 +194,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
+    }
 }

tests/ui/multiple_unsafe_ops_per_block.rs

@@ -209,4 +209,60 @@ 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
+    unsafe {
+        _ = &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() {}

tests/ui/multiple_unsafe_ops_per_block.stderr

@@ -255,5 +255,67 @@ note: unsafe function call occurs here
 LL |         Some(foo_unchecked()).unwrap_unchecked().await;
    |              ^^^^^^^^^^^^^^^
 
-error: aborting due to 11 previous errors
+error: this `unsafe` block contains 2 unsafe operations, expected only one
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:228:5
+   |
+LL | /     unsafe {
+LL | |
+LL | |         _ = &u.i;
+LL | |         _ = &u.i;
+LL | |     }
+   | |_____^
+   |
+note: union field access occurs here
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:230:14
+   |
+LL |         _ = &u.i;
+   |              ^^^
+note: union field access occurs here
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:231:14
+   |
+LL |         _ = &u.i;
+   |              ^^^
+
+error: this `unsafe` block contains 2 unsafe operations, expected only one
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:236:5
+   |
+LL | /     unsafe {
+LL | |
+LL | |         _ = &raw const (*&raw const u).i;
+LL | |         _ = &raw const (*&raw const u).i;
+LL | |     }
+   | |_____^
+   |
+note: raw pointer dereference occurs here
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:238:24
+   |
+LL |         _ = &raw const (*&raw const u).i;
+   |                        ^^^^^^^^^^^^^^^
+note: raw pointer dereference occurs here
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:239:24
+   |
+LL |         _ = &raw const (*&raw const u).i;
+   |                        ^^^^^^^^^^^^^^^
+
+error: this `unsafe` block contains 2 unsafe operations, expected only one
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:262:5
+   |
+LL | /     unsafe {
+LL | |
+LL | |         apply(|| f(0));
+LL | |     }
+   | |_____^
+   |
+note: unsafe function call occurs here
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:264:9
+   |
+LL |         apply(|| f(0));
+   |         ^^^^^^^^^^^^^^
+note: unsafe function call occurs here
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:264:18
+   |
+LL |         apply(|| f(0));
+   |                  ^^^^
+
+error: aborting due to 14 previous errors