From 844996b42e27a2dea559ca4a5f78328bb2b22508 Mon Sep 17 00:00:00 2001 From: Yechan Bae Date: Fri, 3 Dec 2021 19:11:40 -0500 Subject: [PATCH] Consider NonNull as a pointer type --- .../src/non_send_fields_in_send_ty.rs | 28 ++++++++++++------- clippy_utils/src/paths.rs | 1 + tests/ui/non_send_fields_in_send_ty.rs | 5 ++++ tests/ui/non_send_fields_in_send_ty.stderr | 20 ++++++------- 4 files changed, 34 insertions(+), 20 deletions(-) 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 9c07488bfe6e..bba542ce8ca7 100644 --- a/clippy_lints/src/non_send_fields_in_send_ty.rs +++ b/clippy_lints/src/non_send_fields_in_send_ty.rs @@ -1,11 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::is_lint_allowed; use clippy_utils::source::snippet; use clippy_utils::ty::{implements_trait, is_copy}; +use clippy_utils::{is_lint_allowed, match_def_path, paths}; use rustc_ast::ImplPolarity; use rustc_hir::def_id::DefId; use rustc_hir::{FieldDef, Item, ItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::lint::in_external_macro; use rustc_middle::ty::{self, subst::GenericArgKind, Ty}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::sym; @@ -77,6 +78,7 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy { // 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 !in_external_macro(cx.tcx.sess, item.span); if let Some(send_trait) = cx.tcx.get_diagnostic_item(sym::Send); if let ItemKind::Impl(hir_impl) = &item.kind; if let Some(trait_ref) = &hir_impl.of_trait; @@ -181,7 +183,7 @@ fn ty_allowed_without_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty return true; } - if is_copy(cx, ty) && !contains_raw_pointer(cx, ty) { + if is_copy(cx, ty) && !contains_pointer_like(cx, ty) { return true; } @@ -201,7 +203,7 @@ fn ty_allowed_with_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'t .all(|ty| ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait)), ty::Array(ty, _) | ty::Slice(ty) => ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait), ty::Adt(_, substs) => { - if contains_raw_pointer(cx, ty) { + if contains_pointer_like(cx, ty) { // descends only if ADT contains any raw pointers substs.iter().all(|generic_arg| match generic_arg.unpack() { GenericArgKind::Type(ty) => ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait), @@ -218,14 +220,20 @@ fn ty_allowed_with_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'t } } -/// Checks if the type contains any raw pointers in substs (including nested ones). -fn contains_raw_pointer<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> bool { +/// Checks if the type contains any pointer-like types in substs (including nested ones) +fn contains_pointer_like<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> bool { for ty_node in target_ty.walk(cx.tcx) { - if_chain! { - if let GenericArgKind::Type(inner_ty) = ty_node.unpack(); - if let ty::RawPtr(_) = inner_ty.kind(); - then { - return true; + if let GenericArgKind::Type(inner_ty) = ty_node.unpack() { + match inner_ty.kind() { + ty::RawPtr(_) => { + return true; + }, + ty::Adt(adt_def, _) => { + if match_def_path(cx, adt_def.did, &paths::PTR_NON_NULL) { + return true; + } + }, + _ => (), } } } diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 3ffa548e4701..6171823abbbd 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -206,3 +206,4 @@ pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"]; pub const WRITE_MACRO: [&str; 3] = ["core", "macros", "write"]; #[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros pub const WRITELN_MACRO: [&str; 3] = ["core", "macros", "writeln"]; +pub const PTR_NON_NULL: [&str; 4] = ["core", "ptr", "non_null", "NonNull"]; diff --git a/tests/ui/non_send_fields_in_send_ty.rs b/tests/ui/non_send_fields_in_send_ty.rs index eca7f5e56559..828248d922f8 100644 --- a/tests/ui/non_send_fields_in_send_ty.rs +++ b/tests/ui/non_send_fields_in_send_ty.rs @@ -69,6 +69,11 @@ pub enum MyOption { unsafe impl Send for MyOption {} +// Test types that contain `NonNull` instead of raw pointers (#8045) +pub struct WrappedNonNull(UnsafeCell>); + +unsafe impl Send for WrappedNonNull {} + // Multiple type parameters pub struct MultiParam { vec: Vec<(A, B)>, diff --git a/tests/ui/non_send_fields_in_send_ty.stderr b/tests/ui/non_send_fields_in_send_ty.stderr index 8b8a1d16d9bb..3c4da36b3e05 100644 --- a/tests/ui/non_send_fields_in_send_ty.stderr +++ b/tests/ui/non_send_fields_in_send_ty.stderr @@ -103,65 +103,65 @@ LL | MySome(T), = help: add `T: Send` bound in `Send` impl error: this implementation is unsound, as some fields in `MultiParam` are `!Send` - --> $DIR/non_send_fields_in_send_ty.rs:77:1 + --> $DIR/non_send_fields_in_send_ty.rs:82:1 | LL | unsafe impl Send for MultiParam {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the type of field `vec` is `!Send` - --> $DIR/non_send_fields_in_send_ty.rs:74:5 + --> $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` - --> $DIR/non_send_fields_in_send_ty.rs:95:1 + --> $DIR/non_send_fields_in_send_ty.rs:100:1 | LL | unsafe impl Send for HeuristicTest {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the type of field `field4` is `!Send` - --> $DIR/non_send_fields_in_send_ty.rs:90:5 + --> $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` - --> $DIR/non_send_fields_in_send_ty.rs:114:1 + --> $DIR/non_send_fields_in_send_ty.rs:119:1 | LL | unsafe impl Send for AttrTest3 {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the type of field `0` is `!Send` - --> $DIR/non_send_fields_in_send_ty.rs:109:11 + --> $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` - --> $DIR/non_send_fields_in_send_ty.rs:122:1 + --> $DIR/non_send_fields_in_send_ty.rs:127:1 | LL | unsafe impl

Send for Complex {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the type of field `field1` is `!Send` - --> $DIR/non_send_fields_in_send_ty.rs:118:5 + --> $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` - --> $DIR/non_send_fields_in_send_ty.rs:125:1 + --> $DIR/non_send_fields_in_send_ty.rs:130:1 | LL | unsafe impl Send for Complex> {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the type of field `field2` is `!Send` - --> $DIR/non_send_fields_in_send_ty.rs:119:5 + --> $DIR/non_send_fields_in_send_ty.rs:124:5 | LL | field2: B, | ^^^^^^^^^