Skip to content

Commit cda9709

Browse files
QwazQwaz
committed
Fix attribute handling
1 parent 3c08b6f commit cda9709

File tree

3 files changed

+197
-40
lines changed

3 files changed

+197
-40
lines changed

‎clippy_lints/src/non_send_field_in_send_ty.rs

+30-27
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note};
1+
use clippy_utils::diagnostics::span_lint_hir_and_then;
22
use clippy_utils::ty::{implements_trait, is_copy};
33
use rustc_ast::ImplPolarity;
44
use rustc_hir::{Item, ItemKind};
@@ -39,17 +39,19 @@ declare_clippy_lint! {
3939
/// ```
4040
pub NON_SEND_FIELD_IN_SEND_TY,
4141
nursery,
42-
"a field in a `Send` struct does not implement `Send`"
42+
"there is field that does not implement `Send` in a `Send` struct"
4343
}
4444

4545
declare_lint_pass!(NonSendFieldInSendTy => [NON_SEND_FIELD_IN_SEND_TY]);
4646

4747
impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
4848
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
49-
let send_trait = cx.tcx.get_diagnostic_item(sym::send_trait).unwrap();
50-
51-
// Check if we are in `Send` impl item
49+
// Checks if we are in `Send` impl item.
50+
// We start from `Send` impl instead of `check_field_def()` because
51+
// single `AdtDef` may have multiple `Send` impls due to generic
52+
// parameters, and the lint is much easier to implement in this way.
5253
if_chain! {
54+
if let Some(send_trait) = cx.tcx.get_diagnostic_item(sym::send_trait);
5355
if let ItemKind::Impl(hir_impl) = &item.kind;
5456
if let Some(trait_ref) = &hir_impl.of_trait;
5557
if let Some(trait_id) = trait_ref.trait_def_id();
@@ -63,37 +65,38 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
6365
for field in &variant.fields {
6466
let field_ty = field.ty(cx.tcx, impl_trait_substs);
6567

66-
// TODO: substs rebase_onto
67-
6868
if raw_pointer_in_ty_def(cx, field_ty)
6969
|| implements_trait(cx, field_ty, send_trait, &[])
7070
|| is_copy(cx, field_ty)
7171
{
7272
continue;
7373
}
7474

75-
if let Some(field_span) = cx.tcx.hir().span_if_local(field.did) {
76-
if is_ty_param(field_ty) {
77-
span_lint_and_help(
78-
cx,
79-
NON_SEND_FIELD_IN_SEND_TY,
80-
field_span,
81-
"a field in a `Send` struct does not implement `Send`",
82-
Some(item.span),
83-
&format!("add `{}: Send` in `Send` impl for `{}`", field_ty, self_ty),
84-
)
85-
} else {
86-
span_lint_and_note(
75+
if let Some(field_hir_id) = field
76+
.did
77+
.as_local()
78+
.map(|local_def_id| cx.tcx.hir().local_def_id_to_hir_id(local_def_id))
79+
{
80+
if let Some(field_span) = cx.tcx.hir().span_if_local(field.did) {
81+
span_lint_hir_and_then(
8782
cx,
8883
NON_SEND_FIELD_IN_SEND_TY,
84+
field_hir_id,
8985
field_span,
90-
"a field in a `Send` struct does not implement `Send`",
91-
Some(item.span),
92-
&format!(
93-
"type `{}` doesn't implement `Send` when `{}` is `Send`",
94-
field_ty, self_ty
95-
),
96-
)
86+
"non-`Send` field found in a `Send` struct",
87+
|diag| {
88+
diag.span_note(
89+
item.span,
90+
&format!(
91+
"type `{}` doesn't implement `Send` when `{}` is `Send`",
92+
field_ty, self_ty
93+
),
94+
);
95+
if is_ty_param(field_ty) {
96+
diag.help(&format!("add `{}: Send` bound", field_ty));
97+
}
98+
},
99+
);
97100
}
98101
}
99102
}
@@ -121,6 +124,6 @@ fn raw_pointer_in_ty_def<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> b
121124
}
122125

123126
/// Returns `true` if the type is a type parameter such as `T`.
124-
fn is_ty_param<'tcx>(target_ty: Ty<'tcx>) -> bool {
127+
fn is_ty_param(target_ty: Ty<'_>) -> bool {
125128
matches!(target_ty.kind(), ty::Param(_))
126129
}

‎tests/ui/non_send_field_in_send_ty.rs

+25-13
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
use std::cell::UnsafeCell;
55
use std::ptr::NonNull;
6-
use std::sync::{Arc, Mutex};
6+
use std::sync::{Arc, Mutex, MutexGuard};
77

88
// disrustor / RUSTSEC-2020-0150
99
pub struct RingBuffer<T> {
@@ -46,6 +46,22 @@ pub struct DeviceHandle<T: UsbContext> {
4646

4747
unsafe impl<T: UsbContext> Send for DeviceHandle<T> {}
4848

49+
// Other basic tests
50+
pub struct MultiField<T> {
51+
field1: T,
52+
field2: T,
53+
field3: T,
54+
}
55+
56+
unsafe impl<T> Send for MultiField<T> {}
57+
58+
pub enum MyOption<T> {
59+
MySome(T),
60+
MyNone,
61+
}
62+
63+
unsafe impl<T> Send for MyOption<T> {}
64+
4965
// Raw pointers are allowed
5066
extern "C" {
5167
type SomeFfiType;
@@ -57,7 +73,7 @@ pub struct FpTest {
5773

5874
unsafe impl Send for FpTest {}
5975

60-
// Check raw pointer false positive
76+
// Test attributes
6177
#[allow(clippy::non_send_field_in_send_ty)]
6278
pub struct AttrTest1<T>(T);
6379

@@ -76,19 +92,15 @@ unsafe impl<T> Send for AttrTest1<T> {}
7692
unsafe impl<T> Send for AttrTest2<T> {}
7793
unsafe impl<T> Send for AttrTest3<T> {}
7894

79-
pub struct MultiField<T> {
80-
field1: T,
81-
field2: T,
82-
field3: T,
95+
// Multiple non-overlapping `Send` for a single type
96+
pub struct Complex<A, B> {
97+
field1: A,
98+
field2: B,
8399
}
84100

85-
unsafe impl<T> Send for MultiField<T> {}
101+
unsafe impl<P> Send for Complex<P, u32> {}
86102

87-
pub enum MyOption<T> {
88-
MySome(T),
89-
MyNone,
90-
}
91-
92-
unsafe impl<T> Send for MyOption<T> {}
103+
// `MutexGuard` is non-Send
104+
unsafe impl<Q: Send> Send for Complex<Q, MutexGuard<'static, bool>> {}
93105

94106
fn main() {}

‎tests/ui/non_send_field_in_send_ty.stderr

+142
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
error: non-`Send` field found in a `Send` struct
2+
--> $DIR/non_send_field_in_send_ty.rs:10:5
3+
|
4+
LL | data: Vec<UnsafeCell<T>>,
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::non-send-field-in-send-ty` implied by `-D warnings`
8+
note: type `std::vec::Vec<std::cell::UnsafeCell<T>>` doesn't implement `Send` when `RingBuffer<T>` is `Send`
9+
--> $DIR/non_send_field_in_send_ty.rs:15:1
10+
|
11+
LL | unsafe impl<T> Send for RingBuffer<T> {}
12+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13+
14+
error: non-`Send` field found in a `Send` struct
15+
--> $DIR/non_send_field_in_send_ty.rs:20:5
16+
|
17+
LL | lock: Mutex<Box<T>>,
18+
| ^^^^^^^^^^^^^^^^^^^
19+
|
20+
note: type `std::sync::Mutex<std::boxed::Box<T>>` doesn't implement `Send` when `MvccRwLock<T>` is `Send`
21+
--> $DIR/non_send_field_in_send_ty.rs:23:1
22+
|
23+
LL | unsafe impl<T> Send for MvccRwLock<T> {}
24+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
25+
26+
error: non-`Send` field found in a `Send` struct
27+
--> $DIR/non_send_field_in_send_ty.rs:28:5
28+
|
29+
LL | head: Arc<RC>,
30+
| ^^^^^^^^^^^^^
31+
|
32+
note: type `std::sync::Arc<RC>` doesn't implement `Send` when `ArcGuard<RC, T>` is `Send`
33+
--> $DIR/non_send_field_in_send_ty.rs:31:1
34+
|
35+
LL | unsafe impl<RC, T: Send> Send for ArcGuard<RC, T> {}
36+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
37+
38+
error: non-`Send` field found in a `Send` struct
39+
--> $DIR/non_send_field_in_send_ty.rs:43:5
40+
|
41+
LL | context: T,
42+
| ^^^^^^^^^^
43+
|
44+
note: type `T` doesn't implement `Send` when `DeviceHandle<T>` is `Send`
45+
--> $DIR/non_send_field_in_send_ty.rs:47:1
46+
|
47+
LL | unsafe impl<T: UsbContext> Send for DeviceHandle<T> {}
48+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
49+
= help: add `T: Send` bound
50+
51+
error: non-`Send` field found in a `Send` struct
52+
--> $DIR/non_send_field_in_send_ty.rs:51:5
53+
|
54+
LL | field1: T,
55+
| ^^^^^^^^^
56+
|
57+
note: type `T` doesn't implement `Send` when `MultiField<T>` is `Send`
58+
--> $DIR/non_send_field_in_send_ty.rs:56:1
59+
|
60+
LL | unsafe impl<T> Send for MultiField<T> {}
61+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
62+
= help: add `T: Send` bound
63+
64+
error: non-`Send` field found in a `Send` struct
65+
--> $DIR/non_send_field_in_send_ty.rs:52:5
66+
|
67+
LL | field2: T,
68+
| ^^^^^^^^^
69+
|
70+
note: type `T` doesn't implement `Send` when `MultiField<T>` is `Send`
71+
--> $DIR/non_send_field_in_send_ty.rs:56:1
72+
|
73+
LL | unsafe impl<T> Send for MultiField<T> {}
74+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
75+
= help: add `T: Send` bound
76+
77+
error: non-`Send` field found in a `Send` struct
78+
--> $DIR/non_send_field_in_send_ty.rs:53:5
79+
|
80+
LL | field3: T,
81+
| ^^^^^^^^^
82+
|
83+
note: type `T` doesn't implement `Send` when `MultiField<T>` is `Send`
84+
--> $DIR/non_send_field_in_send_ty.rs:56:1
85+
|
86+
LL | unsafe impl<T> Send for MultiField<T> {}
87+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
88+
= help: add `T: Send` bound
89+
90+
error: non-`Send` field found in a `Send` struct
91+
--> $DIR/non_send_field_in_send_ty.rs:59:12
92+
|
93+
LL | MySome(T),
94+
| ^
95+
|
96+
note: type `T` doesn't implement `Send` when `MyOption<T>` is `Send`
97+
--> $DIR/non_send_field_in_send_ty.rs:63:1
98+
|
99+
LL | unsafe impl<T> Send for MyOption<T> {}
100+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
101+
= help: add `T: Send` bound
102+
103+
error: non-`Send` field found in a `Send` struct
104+
--> $DIR/non_send_field_in_send_ty.rs:88:11
105+
|
106+
LL | Enum2(T),
107+
| ^
108+
|
109+
note: type `T` doesn't implement `Send` when `AttrTest3<T>` is `Send`
110+
--> $DIR/non_send_field_in_send_ty.rs:93:1
111+
|
112+
LL | unsafe impl<T> Send for AttrTest3<T> {}
113+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
114+
= help: add `T: Send` bound
115+
116+
error: non-`Send` field found in a `Send` struct
117+
--> $DIR/non_send_field_in_send_ty.rs:97:5
118+
|
119+
LL | field1: A,
120+
| ^^^^^^^^^
121+
|
122+
note: type `P` doesn't implement `Send` when `Complex<P, u32>` is `Send`
123+
--> $DIR/non_send_field_in_send_ty.rs:101:1
124+
|
125+
LL | unsafe impl<P> Send for Complex<P, u32> {}
126+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
127+
= help: add `P: Send` bound
128+
129+
error: non-`Send` field found in a `Send` struct
130+
--> $DIR/non_send_field_in_send_ty.rs:98:5
131+
|
132+
LL | field2: B,
133+
| ^^^^^^^^^
134+
|
135+
note: type `std::sync::MutexGuard<'static, bool>` doesn't implement `Send` when `Complex<Q, std::sync::MutexGuard<'static, bool>>` is `Send`
136+
--> $DIR/non_send_field_in_send_ty.rs:104:1
137+
|
138+
LL | unsafe impl<Q: Send> Send for Complex<Q, MutexGuard<'static, bool>> {}
139+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
140+
141+
error: aborting due to 11 previous errors
142+

0 commit comments

Comments
 (0)