diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index b7b5f059de6f..85ca2cf308d9 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -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), diff --git a/clippy_lints/src/lib.register_nursery.rs b/clippy_lints/src/lib.register_nursery.rs index 59182fd8175d..e3cf06700183 100644 --- a/clippy_lints/src/lib.register_nursery.rs +++ b/clippy_lints/src/lib.register_nursery.rs @@ -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), diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs index 414bfc42fdfc..10f8ae4b7f7f 100644 --- a/clippy_lints/src/lib.register_suspicious.rs +++ b/clippy_lints/src/lib.register_suspicious.rs @@ -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), diff --git a/clippy_lints/src/non_send_fields_in_send_ty.rs b/clippy_lints/src/non_send_fields_in_send_ty.rs index bba542ce8ca7..203f03d3603c 100644 --- a/clippy_lints/src/non_send_fields_in_send_ty.rs +++ b/clippy_lints/src/non_send_fields_in_send_ty.rs @@ -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 @@ -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)] @@ -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), ); match field.generic_params.len() { diff --git a/tests/ui-toml/strict_non_send_fields_in_send_ty/test.stderr b/tests/ui-toml/strict_non_send_fields_in_send_ty/test.stderr index b07f9dd3df30..49eecf18b4c4 100644 --- a/tests/ui-toml/strict_non_send_fields_in_send_ty/test.stderr +++ b/tests/ui-toml/strict_non_send_fields_in_send_ty/test.stderr @@ -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, | ^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: use a thread-safe type that implements `Send` -error: this implementation is unsound, as some fields in `MultiField` are `!Send` +error: some fields in `MultiField` are not safe to be sent to another thread --> $DIR/test.rs:19:1 | LL | unsafe impl Send for MultiField {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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` are `!Send` +error: some fields in `MyOption` are not safe to be sent to another thread --> $DIR/test.rs:26:1 | LL | unsafe impl Send for MyOption {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = 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>, diff --git a/tests/ui/non_send_fields_in_send_ty.stderr b/tests/ui/non_send_fields_in_send_ty.stderr index 3c4da36b3e05..60df4e226e4f 100644 --- a/tests/ui/non_send_fields_in_send_ty.stderr +++ b/tests/ui/non_send_fields_in_send_ty.stderr @@ -1,166 +1,166 @@ -error: this implementation is unsound, as some fields in `RingBuffer` are `!Send` +error: some fields in `RingBuffer` are not safe to be sent to another thread --> $DIR/non_send_fields_in_send_ty.rs:16:1 | LL | unsafe impl Send for RingBuffer {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = 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>, | ^^^^^^^^^^^^^^^^^^^^^^^^ = help: add bounds on type parameter `T` that satisfy `Vec>: Send` -error: this implementation is unsound, as some fields in `MvccRwLock` are `!Send` +error: some fields in `MvccRwLock` are not safe to be sent to another thread --> $DIR/non_send_fields_in_send_ty.rs:24:1 | LL | unsafe impl Send for MvccRwLock {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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>, | ^^^^^^^^^^^^^^^^^^^ = help: add bounds on type parameter `T` that satisfy `Mutex>: Send` -error: this implementation is unsound, as some fields in `ArcGuard` are `!Send` +error: some fields in `ArcGuard` are not safe to be sent to another thread --> $DIR/non_send_fields_in_send_ty.rs:32:1 | LL | unsafe impl Send for ArcGuard {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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, | ^^^^^^^^^^^^^ = help: add bounds on type parameter `RC` that satisfy `Arc: Send` -error: this implementation is unsound, as some fields in `DeviceHandle` are `!Send` +error: some fields in `DeviceHandle` are not safe to be sent to another thread --> $DIR/non_send_fields_in_send_ty.rs:48:1 | LL | unsafe impl Send for DeviceHandle {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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, | ^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: use a thread-safe type that implements `Send` -error: this implementation is unsound, as some fields in `MultiField` are `!Send` +error: some fields in `MultiField` are not safe to be sent to another thread --> $DIR/non_send_fields_in_send_ty.rs:63:1 | LL | unsafe impl Send for MultiField {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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` are `!Send` +error: some fields in `MyOption` are not safe to be sent to another thread --> $DIR/non_send_fields_in_send_ty.rs:70:1 | LL | unsafe impl Send for MyOption {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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` are `!Send` +error: some fields in `MultiParam` are not safe to be sent to another thread --> $DIR/non_send_fields_in_send_ty.rs:82:1 | LL | unsafe impl Send for MultiParam {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: use a thread-safe type that implements `Send` -error: this implementation is unsound, as some fields in `AttrTest3` are `!Send` +error: some fields in `AttrTest3` are not safe to be sent to another thread --> $DIR/non_send_fields_in_send_ty.rs:119:1 | LL | unsafe impl Send for AttrTest3 {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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` are `!Send` +error: some fields in `Complex` are not safe to be sent to another thread --> $DIR/non_send_fields_in_send_ty.rs:127:1 | LL | unsafe impl

Send for Complex {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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>` are `!Send` +error: some fields in `Complex>` are not safe to be sent to another thread --> $DIR/non_send_fields_in_send_ty.rs:130:1 | LL | unsafe impl Send for Complex> {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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,