Skip to content

Commit 907f6d9

Browse files
committed
Auto merge of #8074 - Qwaz:send_nonnull, r=xFrednet
Consider NonNull as a pointer type PR 1/2 for issue #8045. Add `NonNull` as a pointer class to suppress false positives like `UnsafeCell<NonNull<()>>`. However, this change is not sufficient to handle the cases shared in gtk-rs and Rug in the issue. changelog: none r? `@xFrednet`
2 parents 9eabec9 + 844996b commit 907f6d9

File tree

4 files changed

+34
-20
lines changed

4 files changed

+34
-20
lines changed

clippy_lints/src/non_send_fields_in_send_ty.rs

+18-10
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::is_lint_allowed;
32
use clippy_utils::source::snippet;
43
use clippy_utils::ty::{implements_trait, is_copy};
4+
use clippy_utils::{is_lint_allowed, match_def_path, paths};
55
use rustc_ast::ImplPolarity;
66
use rustc_hir::def_id::DefId;
77
use rustc_hir::{FieldDef, Item, ItemKind, Node};
88
use rustc_lint::{LateContext, LateLintPass};
9+
use rustc_middle::lint::in_external_macro;
910
use rustc_middle::ty::{self, subst::GenericArgKind, Ty};
1011
use rustc_session::{declare_tool_lint, impl_lint_pass};
1112
use rustc_span::sym;
@@ -77,6 +78,7 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
7778
// single `AdtDef` may have multiple `Send` impls due to generic
7879
// parameters, and the lint is much easier to implement in this way.
7980
if_chain! {
81+
if !in_external_macro(cx.tcx.sess, item.span);
8082
if let Some(send_trait) = cx.tcx.get_diagnostic_item(sym::Send);
8183
if let ItemKind::Impl(hir_impl) = &item.kind;
8284
if let Some(trait_ref) = &hir_impl.of_trait;
@@ -181,7 +183,7 @@ fn ty_allowed_without_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty
181183
return true;
182184
}
183185

184-
if is_copy(cx, ty) && !contains_raw_pointer(cx, ty) {
186+
if is_copy(cx, ty) && !contains_pointer_like(cx, ty) {
185187
return true;
186188
}
187189

@@ -201,7 +203,7 @@ fn ty_allowed_with_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'t
201203
.all(|ty| ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait)),
202204
ty::Array(ty, _) | ty::Slice(ty) => ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait),
203205
ty::Adt(_, substs) => {
204-
if contains_raw_pointer(cx, ty) {
206+
if contains_pointer_like(cx, ty) {
205207
// descends only if ADT contains any raw pointers
206208
substs.iter().all(|generic_arg| match generic_arg.unpack() {
207209
GenericArgKind::Type(ty) => ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait),
@@ -218,14 +220,20 @@ fn ty_allowed_with_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'t
218220
}
219221
}
220222

221-
/// Checks if the type contains any raw pointers in substs (including nested ones).
222-
fn contains_raw_pointer<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> bool {
223+
/// Checks if the type contains any pointer-like types in substs (including nested ones)
224+
fn contains_pointer_like<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> bool {
223225
for ty_node in target_ty.walk(cx.tcx) {
224-
if_chain! {
225-
if let GenericArgKind::Type(inner_ty) = ty_node.unpack();
226-
if let ty::RawPtr(_) = inner_ty.kind();
227-
then {
228-
return true;
226+
if let GenericArgKind::Type(inner_ty) = ty_node.unpack() {
227+
match inner_ty.kind() {
228+
ty::RawPtr(_) => {
229+
return true;
230+
},
231+
ty::Adt(adt_def, _) => {
232+
if match_def_path(cx, adt_def.did, &paths::PTR_NON_NULL) {
233+
return true;
234+
}
235+
},
236+
_ => (),
229237
}
230238
}
231239
}

clippy_utils/src/paths.rs

+1
Original file line numberDiff line numberDiff line change
@@ -206,3 +206,4 @@ pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"];
206206
pub const WRITE_MACRO: [&str; 3] = ["core", "macros", "write"];
207207
#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros
208208
pub const WRITELN_MACRO: [&str; 3] = ["core", "macros", "writeln"];
209+
pub const PTR_NON_NULL: [&str; 4] = ["core", "ptr", "non_null", "NonNull"];

tests/ui/non_send_fields_in_send_ty.rs

+5
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,11 @@ pub enum MyOption<T> {
6969

7070
unsafe impl<T> Send for MyOption<T> {}
7171

72+
// Test types that contain `NonNull` instead of raw pointers (#8045)
73+
pub struct WrappedNonNull(UnsafeCell<NonNull<()>>);
74+
75+
unsafe impl Send for WrappedNonNull {}
76+
7277
// Multiple type parameters
7378
pub struct MultiParam<A, B> {
7479
vec: Vec<(A, B)>,

tests/ui/non_send_fields_in_send_ty.stderr

+10-10
Original file line numberDiff line numberDiff line change
@@ -103,65 +103,65 @@ LL | MySome(T),
103103
= help: add `T: Send` bound in `Send` impl
104104

105105
error: this implementation is unsound, as some fields in `MultiParam<A, B>` are `!Send`
106-
--> $DIR/non_send_fields_in_send_ty.rs:77:1
106+
--> $DIR/non_send_fields_in_send_ty.rs:82:1
107107
|
108108
LL | unsafe impl<A, B> Send for MultiParam<A, B> {}
109109
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
110110
|
111111
note: the type of field `vec` is `!Send`
112-
--> $DIR/non_send_fields_in_send_ty.rs:74:5
112+
--> $DIR/non_send_fields_in_send_ty.rs:79:5
113113
|
114114
LL | vec: Vec<(A, B)>,
115115
| ^^^^^^^^^^^^^^^^
116116
= help: add bounds on type parameters `A, B` that satisfy `Vec<(A, B)>: Send`
117117

118118
error: this implementation is unsound, as some fields in `HeuristicTest` are `!Send`
119-
--> $DIR/non_send_fields_in_send_ty.rs:95:1
119+
--> $DIR/non_send_fields_in_send_ty.rs:100:1
120120
|
121121
LL | unsafe impl Send for HeuristicTest {}
122122
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
123123
|
124124
note: the type of field `field4` is `!Send`
125-
--> $DIR/non_send_fields_in_send_ty.rs:90:5
125+
--> $DIR/non_send_fields_in_send_ty.rs:95:5
126126
|
127127
LL | field4: (*const NonSend, Rc<u8>),
128128
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
129129
= help: use a thread-safe type that implements `Send`
130130

131131
error: this implementation is unsound, as some fields in `AttrTest3<T>` are `!Send`
132-
--> $DIR/non_send_fields_in_send_ty.rs:114:1
132+
--> $DIR/non_send_fields_in_send_ty.rs:119:1
133133
|
134134
LL | unsafe impl<T> Send for AttrTest3<T> {}
135135
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
136136
|
137137
note: the type of field `0` is `!Send`
138-
--> $DIR/non_send_fields_in_send_ty.rs:109:11
138+
--> $DIR/non_send_fields_in_send_ty.rs:114:11
139139
|
140140
LL | Enum2(T),
141141
| ^
142142
= help: add `T: Send` bound in `Send` impl
143143

144144
error: this implementation is unsound, as some fields in `Complex<P, u32>` are `!Send`
145-
--> $DIR/non_send_fields_in_send_ty.rs:122:1
145+
--> $DIR/non_send_fields_in_send_ty.rs:127:1
146146
|
147147
LL | unsafe impl<P> Send for Complex<P, u32> {}
148148
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
149149
|
150150
note: the type of field `field1` is `!Send`
151-
--> $DIR/non_send_fields_in_send_ty.rs:118:5
151+
--> $DIR/non_send_fields_in_send_ty.rs:123:5
152152
|
153153
LL | field1: A,
154154
| ^^^^^^^^^
155155
= help: add `P: Send` bound in `Send` impl
156156

157157
error: this implementation is unsound, as some fields in `Complex<Q, MutexGuard<'static, bool>>` are `!Send`
158-
--> $DIR/non_send_fields_in_send_ty.rs:125:1
158+
--> $DIR/non_send_fields_in_send_ty.rs:130:1
159159
|
160160
LL | unsafe impl<Q: Send> Send for Complex<Q, MutexGuard<'static, bool>> {}
161161
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
162162
|
163163
note: the type of field `field2` is `!Send`
164-
--> $DIR/non_send_fields_in_send_ty.rs:119:5
164+
--> $DIR/non_send_fields_in_send_ty.rs:124:5
165165
|
166166
LL | field2: B,
167167
| ^^^^^^^^^

0 commit comments

Comments
 (0)