about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-12-04 17:19:07 +0000
committerbors <bors@rust-lang.org>2021-12-04 17:19:07 +0000
commit907f6d929487168da2ad2e86b1c25f9ef35ef8c6 (patch)
treebd7a4eb9d7076c84ca8af1c9c959c6f76b553b57
parent9eabec9f07fe2f9c21a5818a77f425f9974851e6 (diff)
parent844996b42e27a2dea559ca4a5f78328bb2b22508 (diff)
downloadrust-907f6d929487168da2ad2e86b1c25f9ef35ef8c6.tar.gz
rust-907f6d929487168da2ad2e86b1c25f9ef35ef8c6.zip
Auto merge of #8074 - Qwaz:send_nonnull, r=xFrednet
Consider NonNull as a pointer type

PR 1/2 for issue #8045. Add `NonNull` as a pointer class to suppress false positives like `UnsafeCell<NonNull<()>>`. However, this change is not sufficient to handle the cases shared in gtk-rs and Rug in the issue.

changelog: none

r? `@xFrednet`
-rw-r--r--clippy_lints/src/non_send_fields_in_send_ty.rs28
-rw-r--r--clippy_utils/src/paths.rs1
-rw-r--r--tests/ui/non_send_fields_in_send_ty.rs5
-rw-r--r--tests/ui/non_send_fields_in_send_ty.stderr20
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 9c07488bfe6..bba542ce8ca 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 3ffa548e470..6171823abbb 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 eca7f5e5655..828248d922f 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<T> {
 
 unsafe impl<T> Send for MyOption<T> {}
 
+// Test types that contain `NonNull` instead of raw pointers (#8045)
+pub struct WrappedNonNull(UnsafeCell<NonNull<()>>);
+
+unsafe impl Send for WrappedNonNull {}
+
 // Multiple type parameters
 pub struct MultiParam<A, B> {
     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 8b8a1d16d9b..3c4da36b3e0 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<A, B>` 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<A, B> Send for MultiParam<A, B> {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 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<u8>),
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = help: use a thread-safe type that implements `Send`
 
 error: this implementation is unsound, as some fields in `AttrTest3<T>` 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<T> Send for AttrTest3<T> {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 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<P, u32>` 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<P> Send for Complex<P, u32> {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 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<Q, MutexGuard<'static, bool>>` 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<Q: Send> Send for Complex<Q, MutexGuard<'static, bool>> {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 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,
    |     ^^^^^^^^^