From d7a9ec2c5080dc69219e43ba8f221b4e61e47ce4 Mon Sep 17 00:00:00 2001 From: Yechan Bae Date: Thu, 23 Sep 2021 06:45:24 -0400 Subject: [PATCH] Fix attribute handling --- clippy_lints/src/non_send_field_in_send_ty.rs | 57 +++---- tests/ui/non_send_field_in_send_ty.rs | 38 +++-- tests/ui/non_send_field_in_send_ty.stderr | 142 ++++++++++++++++++ 3 files changed, 197 insertions(+), 40 deletions(-) create mode 100644 tests/ui/non_send_field_in_send_ty.stderr 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 5d6501196..07b50cb82 100644 --- a/clippy_lints/src/non_send_field_in_send_ty.rs +++ b/clippy_lints/src/non_send_field_in_send_ty.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note}; +use clippy_utils::diagnostics::span_lint_hir_and_then; use clippy_utils::ty::{implements_trait, is_copy}; use rustc_ast::ImplPolarity; use rustc_hir::{Item, ItemKind}; @@ -39,17 +39,19 @@ declare_clippy_lint! { /// ``` pub NON_SEND_FIELD_IN_SEND_TY, nursery, - "a field in a `Send` struct does not implement `Send`" + "there is field that does not implement `Send` in a `Send` struct" } declare_lint_pass!(NonSendFieldInSendTy => [NON_SEND_FIELD_IN_SEND_TY]); impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { - let send_trait = cx.tcx.get_diagnostic_item(sym::send_trait).unwrap(); - - // Check if we are in `Send` impl item + // Checks if we are in `Send` impl item. + // We start from `Send` impl instead of `check_field_def()` because + // single `AdtDef` may have multiple `Send` impls due to generic + // parameters, and the lint is much easier to implement in this way. if_chain! { + if let Some(send_trait) = cx.tcx.get_diagnostic_item(sym::send_trait); if let ItemKind::Impl(hir_impl) = &item.kind; if let Some(trait_ref) = &hir_impl.of_trait; if let Some(trait_id) = trait_ref.trait_def_id(); @@ -63,8 +65,6 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy { for field in &variant.fields { let field_ty = field.ty(cx.tcx, impl_trait_substs); - // TODO: substs rebase_onto - if raw_pointer_in_ty_def(cx, field_ty) || implements_trait(cx, field_ty, send_trait, &[]) || is_copy(cx, field_ty) @@ -72,28 +72,31 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy { continue; } - if let Some(field_span) = cx.tcx.hir().span_if_local(field.did) { - if is_ty_param(field_ty) { - span_lint_and_help( + 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, - "a field in a `Send` struct does not implement `Send`", - Some(item.span), - &format!("add `{}: Send` in `Send` impl for `{}`", field_ty, self_ty), - ) - } else { - span_lint_and_note( - cx, - NON_SEND_FIELD_IN_SEND_TY, - field_span, - "a field in a `Send` struct does not implement `Send`", - Some(item.span), - &format!( - "type `{}` doesn't implement `Send` when `{}` is `Send`", - field_ty, self_ty - ), - ) + "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)); + } + }, + ); } } } @@ -121,6 +124,6 @@ fn raw_pointer_in_ty_def<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> b } /// Returns `true` if the type is a type parameter such as `T`. -fn is_ty_param<'tcx>(target_ty: Ty<'tcx>) -> bool { +fn is_ty_param(target_ty: Ty<'_>) -> bool { matches!(target_ty.kind(), ty::Param(_)) } diff --git a/tests/ui/non_send_field_in_send_ty.rs b/tests/ui/non_send_field_in_send_ty.rs index a0c574f8e..b97501aa4 100644 --- a/tests/ui/non_send_field_in_send_ty.rs +++ b/tests/ui/non_send_field_in_send_ty.rs @@ -3,7 +3,7 @@ use std::cell::UnsafeCell; use std::ptr::NonNull; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, MutexGuard}; // disrustor / RUSTSEC-2020-0150 pub struct RingBuffer { @@ -46,6 +46,22 @@ pub struct DeviceHandle { unsafe impl Send for DeviceHandle {} +// Other basic tests +pub struct MultiField { + field1: T, + field2: T, + field3: T, +} + +unsafe impl Send for MultiField {} + +pub enum MyOption { + MySome(T), + MyNone, +} + +unsafe impl Send for MyOption {} + // Raw pointers are allowed extern "C" { type SomeFfiType; @@ -57,7 +73,7 @@ pub struct FpTest { unsafe impl Send for FpTest {} -// Check raw pointer false positive +// Test attributes #[allow(clippy::non_send_field_in_send_ty)] pub struct AttrTest1(T); @@ -76,19 +92,15 @@ unsafe impl Send for AttrTest1 {} unsafe impl Send for AttrTest2 {} unsafe impl Send for AttrTest3 {} -pub struct MultiField { - field1: T, - field2: T, - field3: T, +// Multiple non-overlapping `Send` for a single type +pub struct Complex { + field1: A, + field2: B, } -unsafe impl Send for MultiField {} +unsafe impl

Send for Complex {} -pub enum MyOption { - MySome(T), - MyNone, -} - -unsafe impl Send for MyOption {} +// `MutexGuard` is non-Send +unsafe impl Send for Complex> {} fn main() {} diff --git a/tests/ui/non_send_field_in_send_ty.stderr b/tests/ui/non_send_field_in_send_ty.stderr new file mode 100644 index 000000000..3a82828e0 --- /dev/null +++ b/tests/ui/non_send_field_in_send_ty.stderr @@ -0,0 +1,142 @@ +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 + | +LL | unsafe impl Send for RingBuffer {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +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 + | +LL | unsafe impl Send for MvccRwLock {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +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 + | +LL | unsafe impl Send for ArcGuard {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +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 + | +LL | unsafe impl Send for DeviceHandle {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: add `T: Send` bound + +error: non-`Send` field found in a `Send` struct + --> $DIR/non_send_field_in_send_ty.rs:51: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 + | +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 + | +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 + +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 + | +LL | unsafe impl Send for MyOption {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: add `T: Send` bound + +error: non-`Send` field found in a `Send` struct + --> $DIR/non_send_field_in_send_ty.rs:88:11 + | +LL | Enum2(T), + | ^ + | +note: type `T` doesn't implement `Send` when `AttrTest3` is `Send` + --> $DIR/non_send_field_in_send_ty.rs:93:1 + | +LL | unsafe impl Send for AttrTest3 {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: add `T: Send` bound + +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 + | +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: type `std::sync::MutexGuard<'static, bool>` doesn't implement `Send` when `Complex>` is `Send` + --> $DIR/non_send_field_in_send_ty.rs:104:1 + | +LL | unsafe impl Send for Complex> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 11 previous errors +