Look into tuple, array, ADT args in raw pointer heuristic

Author: Qwaz

clippy_lints/src/non_send_field_in_send_ty.rs

@@ -121,7 +121,7 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
                                 };
                             }
                         },
-                    )
+                    );
                 }
             }
         }
@@ -145,6 +145,8 @@ impl<'tcx> NonSendField<'tcx> {
     }
 }
 
+/// Given a type, collect all of its generic parameters.
+/// Example: MyStruct<P, Box<Q, R>> => vec![P, Q, R]
 fn collect_generic_params<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Vec<Ty<'tcx>> {
     ty.walk(cx.tcx)
         .filter_map(|inner| match inner.unpack() {
@@ -155,14 +157,47 @@ fn collect_generic_params<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Vec<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)
+/// Determine if the given type is allowed in an ADT that implements `Send`
+fn ty_allowed_in_send(cx: &LateContext<'tcx>, ty: Ty<'tcx>, send_trait: DefId) -> bool {
+    // TODO: check configuration and call `ty_implements_send_or_copy()` or
+    // `ty_allowed_with_raw_pointer_heuristic()`
+    ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait)
 }
 
-/// 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.
-fn raw_pointer_in_ty_def<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> bool {
+/// Determine if the given type is `Send` or `Copy`
+fn ty_implements_send_or_copy(cx: &LateContext<'tcx>, ty: Ty<'tcx>, send_trait: DefId) -> bool {
+    implements_trait(cx, ty, send_trait, &[]) || is_copy(cx, ty)
+}
+
+/// Heuristic to allow cases like `Vec<*const u8>`
+fn ty_allowed_with_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, send_trait: DefId) -> bool {
+    if ty_implements_send_or_copy(cx, ty, send_trait) {
+        true
+    } else {
+        // The type is known to be `!Send` and `!Copy`
+        match ty.kind() {
+            ty::Tuple(_) => ty
+                .tuple_fields()
+                .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) {
+                    // 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),
+                        GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => true,
+                    })
+                } else {
+                    false
+                }
+            },
+            ty::RawPtr(_) => true,
+            _ => false,
+        }
+    }
+}
+
+fn contains_raw_pointer<'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();

tests/ui/non_send_field_in_send_ty.rs

@@ -76,16 +76,23 @@ pub struct MultiParam<A, B> {
 
 unsafe impl<A, B> Send for MultiParam<A, B> {}
 
-// Raw pointers are allowed
+// Tests for raw pointer heuristic
 extern "C" {
-    type SomeFfiType;
+    type NonSend;
 }
 
-pub struct FpTest {
-    vec: Vec<*const SomeFfiType>,
+pub struct HeuristicTest {
+    // raw pointers are allowed
+    field1: Vec<*const NonSend>,
+    field2: [*const NonSend; 3],
+    field3: (*const NonSend, *const NonSend, *const NonSend),
+    // not allowed when it contains concrete `!Send` field
+    field4: (*const NonSend, Rc<u8>),
+    // nested raw pointer is also allowed
+    field5: Vec<Vec<*const NonSend>>,
 }
 
-unsafe impl Send for FpTest {}
+unsafe impl Send for HeuristicTest {}
 
 // Test attributes
 #[allow(clippy::non_send_field_in_send_ty)]

tests/ui/non_send_field_in_send_ty.stderr

@@ -115,44 +115,57 @@ 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 `HeuristicTest` are `!Send`
+  --> $DIR/non_send_field_in_send_ty.rs:95:1
+   |
+LL | unsafe impl Send for HeuristicTest {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: the field `field4` has type `(*const NonSend, std::rc::Rc<u8>)` which is not `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:90: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_field_in_send_ty.rs:107:1
+  --> $DIR/non_send_field_in_send_ty.rs:114:1
    |
 LL | unsafe impl<T> Send for AttrTest3<T> {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: the field `0` has type `T` which is not `Send`
-  --> $DIR/non_send_field_in_send_ty.rs:102:11
+  --> $DIR/non_send_field_in_send_ty.rs:109: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_field_in_send_ty.rs:115:1
+  --> $DIR/non_send_field_in_send_ty.rs:122:1
    |
 LL | unsafe impl<P> Send for Complex<P, u32> {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: the field `field1` has type `P` which is not `Send`
-  --> $DIR/non_send_field_in_send_ty.rs:111:5
+  --> $DIR/non_send_field_in_send_ty.rs:118:5
    |
 LL |     field1: A,
    |     ^^^^^^^^^
    = help: add `P: Send` bound in `Send` impl
 
 error: this implementation is unsound, as some fields in `Complex<Q, std::sync::MutexGuard<'static, bool>>` are `!Send`
-  --> $DIR/non_send_field_in_send_ty.rs:118:1
+  --> $DIR/non_send_field_in_send_ty.rs:125:1
    |
 LL | unsafe impl<Q: Send> Send for Complex<Q, MutexGuard<'static, bool>> {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 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
+  --> $DIR/non_send_field_in_send_ty.rs:119:5
    |
 LL |     field2: B,
    |     ^^^^^^^^^
    = help: use a thread-safe type that implements `Send`
 
-error: aborting due to 11 previous errors
+error: aborting due to 12 previous errors