Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify the purpose of the non_send lint #8075

Merged
merged 3 commits into from
Dec 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY),
LintId::of(octal_escapes::OCTAL_ESCAPES),
LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
LintId::of(missing_const_for_fn::MISSING_CONST_FOR_FN),
LintId::of(mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL),
LintId::of(mutex_atomic::MUTEX_INTEGER),
LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY),
LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
LintId::of(option_if_let_else::OPTION_IF_LET_ELSE),
LintId::of(path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
Expand Down
1 change: 0 additions & 1 deletion clippy_lints/src/lib.register_suspicious.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
LintId::of(loops::MUT_RANGE_BOUND),
LintId::of(methods::SUSPICIOUS_MAP),
LintId::of(mut_key::MUTABLE_KEY_TYPE),
LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY),
LintId::of(octal_escapes::OCTAL_ESCAPES),
LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
Expand Down
32 changes: 18 additions & 14 deletions clippy_lints/src/non_send_fields_in_send_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,28 @@ use rustc_span::sym;

declare_clippy_lint! {
/// ### What it does
/// Warns about fields in struct implementing `Send` that are neither `Send` nor `Copy`.
/// This lint warns about a `Send` implementation for a type that
/// contains fields that are not safe to be sent across threads.
/// It tries to detect fields that can cause a soundness issue
/// when sent to another thread (e.g., `Rc`) while allowing `!Send` fields
/// that are expected to exist in a `Send` type, such as raw pointers.
///
/// ### Why is this bad?
/// Sending the struct to another thread will transfer the ownership to
/// the new thread by dropping in the current thread during the transfer.
/// This causes soundness issues for non-`Send` fields, as they are also
/// dropped and might not be set up to handle this.
/// Sending the struct to another thread effectively sends all of its fields,
/// and the fields that do not implement `Send` can lead to soundness bugs
/// such as data races when accessed in a thread
/// that is different from the thread that created it.
///
/// See:
/// * [*The Rustonomicon* about *Send and Sync*](https://doc.rust-lang.org/nomicon/send-and-sync.html)
/// * [The documentation of `Send`](https://doc.rust-lang.org/std/marker/trait.Send.html)
///
/// ### Known Problems
/// Data structures that contain raw pointers may cause false positives.
/// They are sometimes safe to be sent across threads but do not implement
/// the `Send` trait. This lint has a heuristic to filter out basic cases
/// such as `Vec<*const T>`, but it's not perfect. Feel free to create an
/// issue if you have a suggestion on how this heuristic can be improved.
/// This lint relies on heuristics to distinguish types that are actually
/// unsafe to be sent across threads and `!Send` types that are expected to
/// exist in `Send` type. Its rule can filter out basic cases such as
/// `Vec<*const T>`, but it's not perfect. Feel free to create an issue if
/// you have a suggestion on how this heuristic can be improved.
///
/// ### Example
/// ```rust,ignore
Expand All @@ -46,8 +50,8 @@ declare_clippy_lint! {
/// or specify correct bounds on generic type parameters (`T: Send`).
#[clippy::version = "1.57.0"]
pub NON_SEND_FIELDS_IN_SEND_TY,
suspicious,
"there is field that does not implement `Send` in a `Send` struct"
nursery,
"there is a field that is not safe to be sent to another thread in a `Send` struct"
}

#[derive(Copy, Clone)]
Expand Down Expand Up @@ -120,14 +124,14 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
NON_SEND_FIELDS_IN_SEND_TY,
item.span,
&format!(
"this implementation is unsound, as some fields in `{}` are `!Send`",
"some fields in `{}` are not safe to be sent to another thread",
snippet(cx, hir_impl.self_ty.span, "Unknown")
),
|diag| {
for field in non_send_fields {
diag.span_note(
field.def.span,
&format!("the type of field `{}` is `!Send`", field.def.ident.name),
&format!("it is not safe to send field `{}` to another thread", field.def.ident.name),
xFrednet marked this conversation as resolved.
Show resolved Hide resolved
);

match field.generic_params.len() {
Expand Down
28 changes: 14 additions & 14 deletions tests/ui-toml/strict_non_send_fields_in_send_ty/test.stderr
Original file line number Diff line number Diff line change
@@ -1,86 +1,86 @@
error: this implementation is unsound, as some fields in `NoGeneric` are `!Send`
error: some fields in `NoGeneric` are not safe to be sent to another thread
--> $DIR/test.rs:11:1
|
LL | unsafe impl Send for NoGeneric {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::non-send-fields-in-send-ty` implied by `-D warnings`
note: the type of field `rc_is_not_send` is `!Send`
note: it is not safe to send field `rc_is_not_send` to another thread
--> $DIR/test.rs:8:5
|
LL | rc_is_not_send: Rc<String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: use a thread-safe type that implements `Send`

error: this implementation is unsound, as some fields in `MultiField<T>` are `!Send`
error: some fields in `MultiField<T>` are not safe to be sent to another thread
--> $DIR/test.rs:19:1
|
LL | unsafe impl<T> Send for MultiField<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `field1` is `!Send`
note: it is not safe to send field `field1` to another thread
--> $DIR/test.rs:14:5
|
LL | field1: T,
| ^^^^^^^^^
= help: add `T: Send` bound in `Send` impl
note: the type of field `field2` is `!Send`
note: it is not safe to send field `field2` to another thread
--> $DIR/test.rs:15:5
|
LL | field2: T,
| ^^^^^^^^^
= help: add `T: Send` bound in `Send` impl
note: the type of field `field3` is `!Send`
note: it is not safe to send field `field3` to another thread
--> $DIR/test.rs:16:5
|
LL | field3: T,
| ^^^^^^^^^
= help: add `T: Send` bound in `Send` impl

error: this implementation is unsound, as some fields in `MyOption<T>` are `!Send`
error: some fields in `MyOption<T>` are not safe to be sent to another thread
--> $DIR/test.rs:26:1
|
LL | unsafe impl<T> Send for MyOption<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `0` is `!Send`
note: it is not safe to send field `0` to another thread
--> $DIR/test.rs:22:12
|
LL | MySome(T),
| ^
= help: add `T: Send` bound in `Send` impl

error: this implementation is unsound, as some fields in `HeuristicTest` are `!Send`
error: some fields in `HeuristicTest` are not safe to be sent to another thread
--> $DIR/test.rs:41:1
|
LL | unsafe impl Send for HeuristicTest {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `field1` is `!Send`
note: it is not safe to send field `field1` to another thread
--> $DIR/test.rs:34:5
|
LL | field1: Vec<*const NonSend>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: use a thread-safe type that implements `Send`
note: the type of field `field2` is `!Send`
note: it is not safe to send field `field2` to another thread
--> $DIR/test.rs:35:5
|
LL | field2: [*const NonSend; 3],
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: use a thread-safe type that implements `Send`
note: the type of field `field3` is `!Send`
note: it is not safe to send field `field3` to another thread
--> $DIR/test.rs:36:5
|
LL | field3: (*const NonSend, *const NonSend, *const NonSend),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: use a thread-safe type that implements `Send`
note: the type of field `field4` is `!Send`
note: it is not safe to send field `field4` to another thread
--> $DIR/test.rs:37:5
|
LL | field4: (*const NonSend, Rc<u8>),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: use a thread-safe type that implements `Send`
note: the type of field `field5` is `!Send`
note: it is not safe to send field `field5` to another thread
--> $DIR/test.rs:38:5
|
LL | field5: Vec<Vec<*const NonSend>>,
Expand Down
52 changes: 26 additions & 26 deletions tests/ui/non_send_fields_in_send_ty.stderr
Original file line number Diff line number Diff line change
@@ -1,166 +1,166 @@
error: this implementation is unsound, as some fields in `RingBuffer<T>` are `!Send`
error: some fields in `RingBuffer<T>` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:16:1
|
LL | unsafe impl<T> Send for RingBuffer<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::non-send-fields-in-send-ty` implied by `-D warnings`
note: the type of field `data` is `!Send`
note: it is not safe to send field `data` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:11:5
|
LL | data: Vec<UnsafeCell<T>>,
| ^^^^^^^^^^^^^^^^^^^^^^^^
= help: add bounds on type parameter `T` that satisfy `Vec<UnsafeCell<T>>: Send`

error: this implementation is unsound, as some fields in `MvccRwLock<T>` are `!Send`
error: some fields in `MvccRwLock<T>` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:24:1
|
LL | unsafe impl<T> Send for MvccRwLock<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `lock` is `!Send`
note: it is not safe to send field `lock` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:21:5
|
LL | lock: Mutex<Box<T>>,
| ^^^^^^^^^^^^^^^^^^^
= help: add bounds on type parameter `T` that satisfy `Mutex<Box<T>>: Send`

error: this implementation is unsound, as some fields in `ArcGuard<RC, T>` are `!Send`
error: some fields in `ArcGuard<RC, T>` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:32:1
|
LL | unsafe impl<RC, T: Send> Send for ArcGuard<RC, T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `head` is `!Send`
note: it is not safe to send field `head` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:29:5
|
LL | head: Arc<RC>,
| ^^^^^^^^^^^^^
= help: add bounds on type parameter `RC` that satisfy `Arc<RC>: Send`

error: this implementation is unsound, as some fields in `DeviceHandle<T>` are `!Send`
error: some fields in `DeviceHandle<T>` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:48:1
|
LL | unsafe impl<T: UsbContext> Send for DeviceHandle<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `context` is `!Send`
note: it is not safe to send field `context` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:44:5
|
LL | context: T,
| ^^^^^^^^^^
= help: add `T: Send` bound in `Send` impl

error: this implementation is unsound, as some fields in `NoGeneric` are `!Send`
error: some fields in `NoGeneric` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:55:1
|
LL | unsafe impl Send for NoGeneric {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `rc_is_not_send` is `!Send`
note: it is not safe to send field `rc_is_not_send` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:52:5
|
LL | rc_is_not_send: Rc<String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: use a thread-safe type that implements `Send`

error: this implementation is unsound, as some fields in `MultiField<T>` are `!Send`
error: some fields in `MultiField<T>` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:63:1
|
LL | unsafe impl<T> Send for MultiField<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `field1` is `!Send`
note: it is not safe to send field `field1` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:58:5
|
LL | field1: T,
| ^^^^^^^^^
= help: add `T: Send` bound in `Send` impl
note: the type of field `field2` is `!Send`
note: it is not safe to send field `field2` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:59:5
|
LL | field2: T,
| ^^^^^^^^^
= help: add `T: Send` bound in `Send` impl
note: the type of field `field3` is `!Send`
note: it is not safe to send field `field3` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:60:5
|
LL | field3: T,
| ^^^^^^^^^
= help: add `T: Send` bound in `Send` impl

error: this implementation is unsound, as some fields in `MyOption<T>` are `!Send`
error: some fields in `MyOption<T>` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:70:1
|
LL | unsafe impl<T> Send for MyOption<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `0` is `!Send`
note: it is not safe to send field `0` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:66:12
|
LL | MySome(T),
| ^
= help: add `T: Send` bound in `Send` impl

error: this implementation is unsound, as some fields in `MultiParam<A, B>` are `!Send`
error: some fields in `MultiParam<A, B>` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:82:1
|
LL | unsafe impl<A, B> Send for MultiParam<A, B> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `vec` is `!Send`
note: it is not safe to send field `vec` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:79:5
|
LL | vec: Vec<(A, B)>,
| ^^^^^^^^^^^^^^^^
= help: add bounds on type parameters `A, B` that satisfy `Vec<(A, B)>: Send`

error: this implementation is unsound, as some fields in `HeuristicTest` are `!Send`
error: some fields in `HeuristicTest` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:100:1
|
LL | unsafe impl Send for HeuristicTest {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `field4` is `!Send`
note: it is not safe to send field `field4` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:95:5
|
LL | field4: (*const NonSend, Rc<u8>),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: use a thread-safe type that implements `Send`

error: this implementation is unsound, as some fields in `AttrTest3<T>` are `!Send`
error: some fields in `AttrTest3<T>` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:119:1
|
LL | unsafe impl<T> Send for AttrTest3<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `0` is `!Send`
note: it is not safe to send field `0` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:114:11
|
LL | Enum2(T),
| ^
= help: add `T: Send` bound in `Send` impl

error: this implementation is unsound, as some fields in `Complex<P, u32>` are `!Send`
error: some fields in `Complex<P, u32>` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:127:1
|
LL | unsafe impl<P> Send for Complex<P, u32> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `field1` is `!Send`
note: it is not safe to send field `field1` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:123:5
|
LL | field1: A,
| ^^^^^^^^^
= help: add `P: Send` bound in `Send` impl

error: this implementation is unsound, as some fields in `Complex<Q, MutexGuard<'static, bool>>` are `!Send`
error: some fields in `Complex<Q, MutexGuard<'static, bool>>` are not safe to be sent to another thread
--> $DIR/non_send_fields_in_send_ty.rs:130:1
|
LL | unsafe impl<Q: Send> Send for Complex<Q, MutexGuard<'static, bool>> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `field2` is `!Send`
note: it is not safe to send field `field2` to another thread
--> $DIR/non_send_fields_in_send_ty.rs:124:5
|
LL | field2: B,
Expand Down