Skip to content

Commit 54a5bb4

Browse files
committed
Taking a raw pointer on a union field is safe since Rust 1.92
1 parent d599529 commit 54a5bb4

File tree

7 files changed

+257
-78
lines changed

7 files changed

+257
-78
lines changed

book/src/lint_configuration.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
892892
* [`mem_replace_option_with_some`](https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_some)
893893
* [`mem_replace_with_default`](https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_default)
894894
* [`missing_const_for_fn`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn)
895+
* [`multiple_unsafe_ops_per_block`](https://rust-lang.github.io/rust-clippy/master/index.html#multiple_unsafe_ops_per_block)
895896
* [`needless_borrow`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow)
896897
* [`non_std_lazy_statics`](https://rust-lang.github.io/rust-clippy/master/index.html#non_std_lazy_statics)
897898
* [`option_as_ref_deref`](https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref)

clippy_config/src/conf.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,7 @@ define_Conf! {
781781
mem_replace_option_with_some,
782782
mem_replace_with_default,
783783
missing_const_for_fn,
784+
multiple_unsafe_ops_per_block,
784785
needless_borrow,
785786
non_std_lazy_statics,
786787
option_as_ref_deref,

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
725725
store.register_late_pass(move |_| Box::new(semicolon_block::SemicolonBlock::new(conf)));
726726
store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse));
727727
store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef));
728-
store.register_late_pass(|_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock));
728+
store.register_late_pass(move |_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock::new(conf)));
729729
store.register_late_pass(move |_| Box::new(extra_unused_type_parameters::ExtraUnusedTypeParameters::new(conf)));
730730
store.register_late_pass(|_| Box::new(no_mangle_with_rust_abi::NoMangleWithRustAbi));
731731
store.register_late_pass(|_| Box::new(collection_is_never_read::CollectionIsNeverRead));

clippy_lints/src/multiple_unsafe_ops_per_block.rs

Lines changed: 99 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
1-
use clippy_utils::desugar_await;
1+
use clippy_config::Conf;
22
use clippy_utils::diagnostics::span_lint_and_then;
3-
use clippy_utils::visitors::{Descend, Visitable, for_each_expr};
4-
use core::ops::ControlFlow::Continue;
3+
use clippy_utils::msrvs::Msrv;
4+
use clippy_utils::{desugar_await, msrvs};
55
use hir::def::{DefKind, Res};
66
use hir::{BlockCheckMode, ExprKind, QPath, UnOp};
7-
use rustc_ast::Mutability;
7+
use rustc_ast::{BorrowKind, Mutability};
88
use rustc_hir as hir;
9+
use rustc_hir::intravisit::{Visitor, walk_expr};
910
use rustc_lint::{LateContext, LateLintPass};
1011
use rustc_middle::ty;
11-
use rustc_session::declare_lint_pass;
12+
use rustc_session::impl_lint_pass;
1213
use rustc_span::{DesugaringKind, Span};
1314

1415
declare_clippy_lint! {
@@ -60,7 +61,18 @@ declare_clippy_lint! {
6061
restriction,
6162
"more than one unsafe operation per `unsafe` block"
6263
}
63-
declare_lint_pass!(MultipleUnsafeOpsPerBlock => [MULTIPLE_UNSAFE_OPS_PER_BLOCK]);
64+
65+
pub struct MultipleUnsafeOpsPerBlock {
66+
msrv: Msrv,
67+
}
68+
69+
impl_lint_pass!(MultipleUnsafeOpsPerBlock => [MULTIPLE_UNSAFE_OPS_PER_BLOCK]);
70+
71+
impl MultipleUnsafeOpsPerBlock {
72+
pub fn new(conf: &Conf) -> Self {
73+
Self { msrv: conf.msrv }
74+
}
75+
}
6476

6577
impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
6678
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
@@ -70,8 +82,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
7082
{
7183
return;
7284
}
73-
let mut unsafe_ops = vec![];
74-
collect_unsafe_exprs(cx, block, &mut unsafe_ops);
85+
let unsafe_ops = UnsafeExprCollector::collect_unsafe_exprs(cx, block, self.msrv);
7586
if unsafe_ops.len() > 1 {
7687
span_lint_and_then(
7788
cx,
@@ -91,25 +102,74 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
91102
}
92103
}
93104

94-
fn collect_unsafe_exprs<'tcx>(
95-
cx: &LateContext<'tcx>,
96-
node: impl Visitable<'tcx>,
97-
unsafe_ops: &mut Vec<(&'static str, Span)>,
98-
) {
99-
for_each_expr(cx, node, |expr| {
105+
#[derive(Clone, Copy)]
106+
enum UnderRawPtr {
107+
/// The expression is not located under a raw pointer
108+
No,
109+
/// The expression is located under a raw pointer, MSRV yet unknown
110+
Yes,
111+
/// The expression is located under a raw pointer and MSRV has been determined.
112+
/// `true` means that taking a raw pointer to a union field is a safe operation.
113+
WithSafeMsrv(bool),
114+
}
115+
116+
struct UnsafeExprCollector<'cx, 'tcx> {
117+
cx: &'cx LateContext<'tcx>,
118+
msrv: Msrv,
119+
unsafe_ops: Vec<(&'static str, Span)>,
120+
under_raw_ptr: UnderRawPtr,
121+
}
122+
123+
impl<'cx, 'tcx> UnsafeExprCollector<'cx, 'tcx> {
124+
fn collect_unsafe_exprs(
125+
cx: &'cx LateContext<'tcx>,
126+
block: &'tcx hir::Block<'tcx>,
127+
msrv: Msrv,
128+
) -> Vec<(&'static str, Span)> {
129+
let mut collector = Self {
130+
cx,
131+
msrv,
132+
unsafe_ops: vec![],
133+
under_raw_ptr: UnderRawPtr::No,
134+
};
135+
collector.visit_block(block);
136+
collector.unsafe_ops
137+
}
138+
}
139+
140+
impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'_, 'tcx> {
141+
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
142+
// `self.under_raw_ptr` is preventively reset, while the current value is
143+
// preserved in `under_raw_ptr`.
144+
let under_raw_ptr = self.under_raw_ptr;
145+
self.under_raw_ptr = UnderRawPtr::No;
146+
100147
match expr.kind {
101148
// The `await` itself will desugar to two unsafe calls, but we should ignore those.
102149
// Instead, check the expression that is `await`ed
103150
_ if let Some(e) = desugar_await(expr) => {
104-
collect_unsafe_exprs(cx, e, unsafe_ops);
105-
return Continue(Descend::No);
151+
return self.visit_expr(e);
106152
},
107153

108-
ExprKind::InlineAsm(_) => unsafe_ops.push(("inline assembly used here", expr.span)),
154+
ExprKind::InlineAsm(_) => self.unsafe_ops.push(("inline assembly used here", expr.span)),
155+
156+
ExprKind::AddrOf(BorrowKind::Raw, _, place) => {
157+
self.under_raw_ptr = UnderRawPtr::Yes;
158+
return self.visit_expr(place);
159+
},
109160

110161
ExprKind::Field(e, _) => {
111-
if cx.typeck_results().expr_ty(e).is_union() {
112-
unsafe_ops.push(("union field access occurs here", expr.span));
162+
if self.cx.typeck_results().expr_ty(e).is_union() {
163+
// Restore `self.under_raw_pointer` and determine safety of taking a raw pointer to
164+
// a union field if this is not known already.
165+
self.under_raw_ptr = if matches!(under_raw_ptr, UnderRawPtr::Yes) {
166+
UnderRawPtr::WithSafeMsrv(self.msrv.meets(self.cx, msrvs::SAFE_RAW_PTR_TO_UNION_FIELD))
167+
} else {
168+
under_raw_ptr
169+
};
170+
if matches!(self.under_raw_ptr, UnderRawPtr::No | UnderRawPtr::WithSafeMsrv(false)) {
171+
self.unsafe_ops.push(("union field access occurs here", expr.span));
172+
}
113173
}
114174
},
115175

@@ -127,32 +187,33 @@ fn collect_unsafe_exprs<'tcx>(
127187
..
128188
},
129189
)) => {
130-
unsafe_ops.push(("access of a mutable static occurs here", expr.span));
190+
self.unsafe_ops
191+
.push(("access of a mutable static occurs here", expr.span));
131192
},
132193

133-
ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty_adjusted(e).is_raw_ptr() => {
134-
unsafe_ops.push(("raw pointer dereference occurs here", expr.span));
194+
ExprKind::Unary(UnOp::Deref, e) if self.cx.typeck_results().expr_ty_adjusted(e).is_raw_ptr() => {
195+
self.unsafe_ops.push(("raw pointer dereference occurs here", expr.span));
135196
},
136197

137198
ExprKind::Call(path_expr, _) => {
138-
let sig = match *cx.typeck_results().expr_ty(path_expr).kind() {
139-
ty::FnDef(id, _) => cx.tcx.fn_sig(id).skip_binder(),
140-
ty::FnPtr(sig_tys, hdr) => sig_tys.with(hdr),
141-
_ => return Continue(Descend::Yes),
199+
let opt_sig = match *self.cx.typeck_results().expr_ty(path_expr).kind() {
200+
ty::FnDef(id, _) => Some(self.cx.tcx.fn_sig(id).skip_binder()),
201+
ty::FnPtr(sig_tys, hdr) => Some(sig_tys.with(hdr)),
202+
_ => None,
142203
};
143-
if sig.safety().is_unsafe() {
144-
unsafe_ops.push(("unsafe function call occurs here", expr.span));
204+
if opt_sig.is_some_and(|sig| sig.safety().is_unsafe()) {
205+
self.unsafe_ops.push(("unsafe function call occurs here", expr.span));
145206
}
146207
},
147208

148209
ExprKind::MethodCall(..) => {
149-
if let Some(sig) = cx
210+
let opt_sig = self
211+
.cx
150212
.typeck_results()
151213
.type_dependent_def_id(expr.hir_id)
152-
.map(|def_id| cx.tcx.fn_sig(def_id))
153-
&& sig.skip_binder().safety().is_unsafe()
154-
{
155-
unsafe_ops.push(("unsafe method call occurs here", expr.span));
214+
.map(|def_id| self.cx.tcx.fn_sig(def_id));
215+
if opt_sig.is_some_and(|sig| sig.skip_binder().safety().is_unsafe()) {
216+
self.unsafe_ops.push(("unsafe method call occurs here", expr.span));
156217
}
157218
},
158219

@@ -173,15 +234,15 @@ fn collect_unsafe_exprs<'tcx>(
173234
}
174235
))
175236
) {
176-
unsafe_ops.push(("modification of a mutable static occurs here", expr.span));
177-
collect_unsafe_exprs(cx, rhs, unsafe_ops);
178-
return Continue(Descend::No);
237+
self.unsafe_ops
238+
.push(("modification of a mutable static occurs here", expr.span));
239+
return self.visit_expr(rhs);
179240
}
180241
},
181242

182243
_ => {},
183244
}
184245

185-
Continue::<(), _>(Descend::Yes)
186-
});
246+
walk_expr(self, expr);
247+
}
187248
}

clippy_utils/src/msrvs.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ macro_rules! msrv_aliases {
2323

2424
// names may refer to stabilized feature flags or library items
2525
msrv_aliases! {
26+
1,92,0 { SAFE_RAW_PTR_TO_UNION_FIELD }
2627
1,88,0 { LET_CHAINS }
2728
1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, CONST_CHAR_IS_DIGIT, UNSIGNED_IS_MULTIPLE_OF, INTEGER_SIGN_CAST }
2829
1,85,0 { UINT_FLOAT_MIDPOINT, CONST_SIZE_OF_VAL }

tests/ui/multiple_unsafe_ops_per_block.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//@needs-asm-support
22
//@aux-build:proc_macros.rs
3+
#![feature(stmt_expr_attributes)]
34
#![expect(
45
dropping_copy_types,
56
clippy::unnecessary_operation,
@@ -209,4 +210,55 @@ async fn issue13879() {
209210
}
210211
}
211212

213+
fn issue16076() {
214+
#[derive(Clone, Copy)]
215+
union U {
216+
i: u32,
217+
f: f32,
218+
}
219+
220+
let u = U { i: 0 };
221+
222+
// Taking a raw pointer to a place is safe since Rust 1.92
223+
#[clippy::msrv = "1.92"]
224+
unsafe {
225+
_ = &raw const u.i;
226+
_ = &raw const u.i;
227+
}
228+
229+
// However it was not the case before Rust 1.92
230+
#[clippy::msrv = "1.91"]
231+
unsafe {
232+
//~^ multiple_unsafe_ops_per_block
233+
_ = &raw const u.i;
234+
_ = &raw const u.i;
235+
}
236+
237+
// Taking a reference to a union field is not safe
238+
unsafe {
239+
//~^ multiple_unsafe_ops_per_block
240+
_ = &u.i;
241+
_ = &u.i;
242+
}
243+
244+
// Check that we still check and lint the prefix of the raw pointer to a field access
245+
#[expect(clippy::deref_addrof)]
246+
unsafe {
247+
//~^ multiple_unsafe_ops_per_block
248+
_ = &raw const (*&raw const u).i;
249+
_ = &raw const (*&raw const u).i;
250+
}
251+
252+
union V {
253+
u: U,
254+
}
255+
256+
// Taking a raw pointer to a union field of an union field (etc.) is safe
257+
let v = V { u };
258+
unsafe {
259+
_ = &raw const v.u.i;
260+
_ = &raw const v.u.i;
261+
}
262+
}
263+
212264
fn main() {}

0 commit comments

Comments
 (0)