diff --git a/clippy_lints/src/non_send_field_in_send_ty.rs b/clippy_lints/src/non_send_field_in_send_ty.rs index d141cd2199f..0fa994cacb1 100644 --- a/clippy_lints/src/non_send_field_in_send_ty.rs +++ b/clippy_lints/src/non_send_field_in_send_ty.rs @@ -1,11 +1,14 @@ -use clippy_utils::diagnostics::span_lint_hir_and_then; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_lint_allowed; use clippy_utils::ty::{implements_trait, is_copy}; use rustc_ast::ImplPolarity; +use rustc_hir::def_id::DefId; use rustc_hir::{Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, subst::GenericArgKind, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; +use rustc_span::symbol::Symbol; +use rustc_span::{sym, Span}; declare_clippy_lint! { /// ### What it does @@ -64,51 +67,98 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy { if let self_ty = ty_trait_ref.self_ty(); if let ty::Adt(adt_def, impl_trait_substs) = self_ty.kind(); then { + let mut non_send_fields = Vec::new(); + + let hir_map = cx.tcx.hir(); for variant in &adt_def.variants { for field in &variant.fields { - let field_ty = field.ty(cx.tcx, impl_trait_substs); - - if raw_pointer_in_ty_def(cx, field_ty) - || implements_trait(cx, field_ty, send_trait, &[]) - || is_copy(cx, field_ty) - { - continue; - } - - if let Some(field_hir_id) = field - .did - .as_local() - .map(|local_def_id| cx.tcx.hir().local_def_id_to_hir_id(local_def_id)) - { - if let Some(field_span) = cx.tcx.hir().span_if_local(field.did) { - span_lint_hir_and_then( - cx, - NON_SEND_FIELD_IN_SEND_TY, - field_hir_id, - field_span, - "non-`Send` field found in a `Send` struct", - |diag| { - diag.span_note( - item.span, - &format!( - "type `{}` doesn't implement `Send` when `{}` is `Send`", - field_ty, self_ty - ), - ); - if is_ty_param(field_ty) { - diag.help(&format!("add `{}: Send` bound", field_ty)); - } - }, - ); + if_chain! { + if let Some(field_hir_id) = field + .did + .as_local() + .map(|local_def_id| hir_map.local_def_id_to_hir_id(local_def_id)); + if !is_lint_allowed(cx, NON_SEND_FIELD_IN_SEND_TY, field_hir_id); + if let field_ty = field.ty(cx.tcx, impl_trait_substs); + if !ty_allowed_in_send(cx, field_ty, send_trait); + if let Some(field_span) = hir_map.span_if_local(field.did); + then { + non_send_fields.push(NonSendField { + name: hir_map.name(field_hir_id), + span: field_span, + ty: field_ty, + generic_params: collect_generic_params(cx, field_ty), + }) } } } } + + if !non_send_fields.is_empty() { + span_lint_and_then( + cx, + NON_SEND_FIELD_IN_SEND_TY, + item.span, + &format!( + "this implementation is unsound, as some fields in `{}` are `!Send`", + self_ty + ), + |diag| { + for field in non_send_fields { + diag.span_note( + field.span, + &format!("the field `{}` has type `{}` which is not `Send`", field.name, field.ty), + ); + + match field.generic_params.len() { + 0 => diag.help("use a thread-safe type that implements `Send`"), + 1 if is_ty_param(field.ty) => diag.help(&format!("add `{}: Send` bound in `Send` impl", field.ty)), + _ => diag.help(&format!( + "add bounds on type parameter{} `{}` that satisfy `{}: Send`", + if field.generic_params.len() > 1 { "s" } else { "" }, + field.generic_params_string(), + field.ty + )), + }; + } + }, + ) + } } } } } +struct NonSendField<'tcx> { + name: Symbol, + span: Span, + ty: Ty<'tcx>, + generic_params: Vec>, +} + +impl<'tcx> NonSendField<'tcx> { + fn generic_params_string(&self) -> String { + self.generic_params + .iter() + .map(ToString::to_string) + .collect::>() + .join(", ") + } +} + +fn collect_generic_params<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Vec> { + ty.walk(cx.tcx) + .filter_map(|inner| match inner.unpack() { + GenericArgKind::Type(inner_ty) => Some(inner_ty), + _ => None, + }) + .filter(|&inner_ty| is_ty_param(inner_ty)) + .collect() +} + +fn ty_allowed_in_send<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, send_trait: DefId) -> bool { + raw_pointer_in_ty_def(cx, ty) || implements_trait(cx, ty, send_trait, &[]) || is_copy(cx, ty) +} + /// Returns `true` if the type itself is a raw pointer or has a raw pointer as a /// generic parameter, e.g., `Vec<*const u8>`. /// Note that it does not look into enum variants or struct fields. diff --git a/tests/ui/non_send_field_in_send_ty.rs b/tests/ui/non_send_field_in_send_ty.rs index b97501aa457..cf4fc1fd9b8 100644 --- a/tests/ui/non_send_field_in_send_ty.rs +++ b/tests/ui/non_send_field_in_send_ty.rs @@ -3,6 +3,7 @@ use std::cell::UnsafeCell; use std::ptr::NonNull; +use std::rc::Rc; use std::sync::{Arc, Mutex, MutexGuard}; // disrustor / RUSTSEC-2020-0150 @@ -47,6 +48,12 @@ pub struct DeviceHandle { unsafe impl Send for DeviceHandle {} // Other basic tests +pub struct NoGeneric { + rc_is_not_send: Rc, +} + +unsafe impl Send for NoGeneric {} + pub struct MultiField { field1: T, field2: T, @@ -62,6 +69,13 @@ pub enum MyOption { unsafe impl Send for MyOption {} +// Multiple type parameters +pub struct MultiParam { + vec: Vec<(A, B)>, +} + +unsafe impl Send for MultiParam {} + // Raw pointers are allowed extern "C" { type SomeFfiType; diff --git a/tests/ui/non_send_field_in_send_ty.stderr b/tests/ui/non_send_field_in_send_ty.stderr index 3a82828e0fe..52cc038b658 100644 --- a/tests/ui/non_send_field_in_send_ty.stderr +++ b/tests/ui/non_send_field_in_send_ty.stderr @@ -1,142 +1,158 @@ -error: non-`Send` field found in a `Send` struct - --> $DIR/non_send_field_in_send_ty.rs:10:5 - | -LL | data: Vec>, - | ^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: `-D clippy::non-send-field-in-send-ty` implied by `-D warnings` -note: type `std::vec::Vec>` doesn't implement `Send` when `RingBuffer` is `Send` - --> $DIR/non_send_field_in_send_ty.rs:15:1 +error: this implementation is unsound, as some fields in `RingBuffer` are `!Send` + --> $DIR/non_send_field_in_send_ty.rs:16:1 | LL | unsafe impl Send for RingBuffer {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::non-send-field-in-send-ty` implied by `-D warnings` +note: the field `data` has type `std::vec::Vec>` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:11:5 + | +LL | data: Vec>, + | ^^^^^^^^^^^^^^^^^^^^^^^^ + = help: add bounds on type parameter `T` that satisfy `std::vec::Vec>: Send` -error: non-`Send` field found in a `Send` struct - --> $DIR/non_send_field_in_send_ty.rs:20:5 - | -LL | lock: Mutex>, - | ^^^^^^^^^^^^^^^^^^^ - | -note: type `std::sync::Mutex>` doesn't implement `Send` when `MvccRwLock` is `Send` - --> $DIR/non_send_field_in_send_ty.rs:23:1 +error: this implementation is unsound, as some fields in `MvccRwLock` are `!Send` + --> $DIR/non_send_field_in_send_ty.rs:24:1 | LL | unsafe impl Send for MvccRwLock {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the field `lock` has type `std::sync::Mutex>` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:21:5 + | +LL | lock: Mutex>, + | ^^^^^^^^^^^^^^^^^^^ + = help: add bounds on type parameter `T` that satisfy `std::sync::Mutex>: Send` -error: non-`Send` field found in a `Send` struct - --> $DIR/non_send_field_in_send_ty.rs:28:5 - | -LL | head: Arc, - | ^^^^^^^^^^^^^ - | -note: type `std::sync::Arc` doesn't implement `Send` when `ArcGuard` is `Send` - --> $DIR/non_send_field_in_send_ty.rs:31:1 +error: this implementation is unsound, as some fields in `ArcGuard` are `!Send` + --> $DIR/non_send_field_in_send_ty.rs:32:1 | LL | unsafe impl Send for ArcGuard {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the field `head` has type `std::sync::Arc` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:29:5 + | +LL | head: Arc, + | ^^^^^^^^^^^^^ + = help: add bounds on type parameter `RC` that satisfy `std::sync::Arc: Send` -error: non-`Send` field found in a `Send` struct - --> $DIR/non_send_field_in_send_ty.rs:43:5 - | -LL | context: T, - | ^^^^^^^^^^ - | -note: type `T` doesn't implement `Send` when `DeviceHandle` is `Send` - --> $DIR/non_send_field_in_send_ty.rs:47:1 +error: this implementation is unsound, as some fields in `DeviceHandle` are `!Send` + --> $DIR/non_send_field_in_send_ty.rs:48:1 | LL | unsafe impl Send for DeviceHandle {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: add `T: Send` bound + | +note: the field `context` has type `T` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:44:5 + | +LL | context: T, + | ^^^^^^^^^^ + = help: add `T: Send` bound in `Send` impl -error: non-`Send` field found in a `Send` struct - --> $DIR/non_send_field_in_send_ty.rs:51:5 +error: this implementation is unsound, as some fields in `NoGeneric` are `!Send` + --> $DIR/non_send_field_in_send_ty.rs:55:1 + | +LL | unsafe impl Send for NoGeneric {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the field `rc_is_not_send` has type `std::rc::Rc` which is not `Send` + --> $DIR/non_send_field_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` + --> $DIR/non_send_field_in_send_ty.rs:63:1 + | +LL | unsafe impl Send for MultiField {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the field `field1` has type `T` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:58:5 | LL | field1: T, | ^^^^^^^^^ - | -note: type `T` doesn't implement `Send` when `MultiField` is `Send` - --> $DIR/non_send_field_in_send_ty.rs:56:1 - | -LL | unsafe impl Send for MultiField {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: add `T: Send` bound - -error: non-`Send` field found in a `Send` struct - --> $DIR/non_send_field_in_send_ty.rs:52:5 + = help: add `T: Send` bound in `Send` impl +note: the field `field2` has type `T` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:59:5 | LL | field2: T, | ^^^^^^^^^ - | -note: type `T` doesn't implement `Send` when `MultiField` is `Send` - --> $DIR/non_send_field_in_send_ty.rs:56:1 - | -LL | unsafe impl Send for MultiField {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: add `T: Send` bound - -error: non-`Send` field found in a `Send` struct - --> $DIR/non_send_field_in_send_ty.rs:53:5 + = help: add `T: Send` bound in `Send` impl +note: the field `field3` has type `T` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:60:5 | LL | field3: T, | ^^^^^^^^^ - | -note: type `T` doesn't implement `Send` when `MultiField` is `Send` - --> $DIR/non_send_field_in_send_ty.rs:56:1 - | -LL | unsafe impl Send for MultiField {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: add `T: Send` bound + = help: add `T: Send` bound in `Send` impl -error: non-`Send` field found in a `Send` struct - --> $DIR/non_send_field_in_send_ty.rs:59:12 - | -LL | MySome(T), - | ^ - | -note: type `T` doesn't implement `Send` when `MyOption` is `Send` - --> $DIR/non_send_field_in_send_ty.rs:63:1 +error: this implementation is unsound, as some fields in `MyOption` are `!Send` + --> $DIR/non_send_field_in_send_ty.rs:70:1 | LL | unsafe impl Send for MyOption {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: add `T: Send` bound + | +note: the field `0` has type `T` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:66:12 + | +LL | MySome(T), + | ^ + = help: add `T: Send` bound in `Send` impl -error: non-`Send` field found in a `Send` struct - --> $DIR/non_send_field_in_send_ty.rs:88:11 +error: this implementation is unsound, as some fields in `MultiParam` are `!Send` + --> $DIR/non_send_field_in_send_ty.rs:77:1 | -LL | Enum2(T), - | ^ +LL | unsafe impl Send for MultiParam {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -note: type `T` doesn't implement `Send` when `AttrTest3` is `Send` - --> $DIR/non_send_field_in_send_ty.rs:93:1 +note: the field `vec` has type `std::vec::Vec<(A, B)>` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:74:5 + | +LL | vec: Vec<(A, B)>, + | ^^^^^^^^^^^^^^^^ + = help: add bounds on type parameters `A, B` that satisfy `std::vec::Vec<(A, B)>: Send` + +error: this implementation is unsound, as some fields in `AttrTest3` are `!Send` + --> $DIR/non_send_field_in_send_ty.rs:107:1 | LL | unsafe impl Send for AttrTest3 {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: add `T: Send` bound + | +note: the field `0` has type `T` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:102:11 + | +LL | Enum2(T), + | ^ + = help: add `T: Send` bound in `Send` impl -error: non-`Send` field found in a `Send` struct - --> $DIR/non_send_field_in_send_ty.rs:97:5 - | -LL | field1: A, - | ^^^^^^^^^ - | -note: type `P` doesn't implement `Send` when `Complex` is `Send` - --> $DIR/non_send_field_in_send_ty.rs:101:1 +error: this implementation is unsound, as some fields in `Complex` are `!Send` + --> $DIR/non_send_field_in_send_ty.rs:115:1 | LL | unsafe impl

Send for Complex {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: add `P: Send` bound - -error: non-`Send` field found in a `Send` struct - --> $DIR/non_send_field_in_send_ty.rs:98:5 | -LL | field2: B, +note: the field `field1` has type `P` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:111:5 + | +LL | field1: A, | ^^^^^^^^^ - | -note: type `std::sync::MutexGuard<'static, bool>` doesn't implement `Send` when `Complex>` is `Send` - --> $DIR/non_send_field_in_send_ty.rs:104:1 + = help: add `P: Send` bound in `Send` impl + +error: this implementation is unsound, as some fields in `Complex>` are `!Send` + --> $DIR/non_send_field_in_send_ty.rs:118:1 | LL | unsafe impl Send for Complex> {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the field `field2` has type `std::sync::MutexGuard<'static, bool>` which is not `Send` + --> $DIR/non_send_field_in_send_ty.rs:112:5 + | +LL | field2: B, + | ^^^^^^^^^ + = help: use a thread-safe type that implements `Send` error: aborting due to 11 previous errors