]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/non_send_fields_in_send_ty.rs
Split out `infalliable_detructuring_match`
[rust.git] / clippy_lints / src / non_send_fields_in_send_ty.rs
index 7ebf84d400f569c3f891bc7d417afeeb2d741167..ab1559c85d8b1f431946778eeb03991ffa925aa9 100644 (file)
@@ -1,35 +1,40 @@
 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;
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Warns about fields in struct implementing `Send` that are neither `Send` nor `Copy`.
+    /// This lint warns about a `Send` implementation for a type that
+    /// contains fields that are not safe to be sent across threads.
+    /// It tries to detect fields that can cause a soundness issue
+    /// when sent to another thread (e.g., `Rc`) while allowing `!Send` fields
+    /// that are expected to exist in a `Send` type, such as raw pointers.
     ///
     /// ### Why is this bad?
-    /// Sending the struct to another thread will transfer the ownership to
-    /// the new thread by dropping in the current thread during the transfer.
-    /// This causes soundness issues for non-`Send` fields, as they are also
-    /// dropped and might not be set up to handle this.
+    /// Sending the struct to another thread effectively sends all of its fields,
+    /// and the fields that do not implement `Send` can lead to soundness bugs
+    /// such as data races when accessed in a thread
+    /// that is different from the thread that created it.
     ///
     /// See:
     /// * [*The Rustonomicon* about *Send and Sync*](https://doc.rust-lang.org/nomicon/send-and-sync.html)
     /// * [The documentation of `Send`](https://doc.rust-lang.org/std/marker/trait.Send.html)
     ///
     /// ### Known Problems
-    /// Data structures that contain raw pointers may cause false positives.
-    /// They are sometimes safe to be sent across threads but do not implement
-    /// the `Send` trait. This lint has a heuristic to filter out basic cases
-    /// such as `Vec<*const T>`, but it's not perfect. Feel free to create an
-    /// issue if you have a suggestion on how this heuristic can be improved.
+    /// This lint relies on heuristics to distinguish types that are actually
+    /// unsafe to be sent across threads and `!Send` types that are expected to
+    /// exist in  `Send` type. Its rule can filter out basic cases such as
+    /// `Vec<*const T>`, but it's not perfect. Feel free to create an issue if
+    /// you have a suggestion on how this heuristic can be improved.
     ///
     /// ### Example
     /// ```rust,ignore
     /// ```
     /// Use thread-safe types like [`std::sync::Arc`](https://doc.rust-lang.org/std/sync/struct.Arc.html)
     /// or specify correct bounds on generic type parameters (`T: Send`).
+    #[clippy::version = "1.57.0"]
     pub NON_SEND_FIELDS_IN_SEND_TY,
-    suspicious,
-    "there is field that does not implement `Send` in a `Send` struct"
+    nursery,
+    "there is a field that is not safe to be sent to another thread in a `Send` struct"
 }
 
 #[derive(Copy, Clone)]
@@ -76,6 +82,7 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
         // 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;
@@ -104,7 +111,7 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
                                 non_send_fields.push(NonSendField {
                                     def: field_def,
                                     ty: field_ty,
-                                    generic_params: collect_generic_params(cx, field_ty),
+                                    generic_params: collect_generic_params(field_ty),
                                 })
                             }
                         }
@@ -117,14 +124,14 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
                         NON_SEND_FIELDS_IN_SEND_TY,
                         item.span,
                         &format!(
-                            "this implementation is unsound, as some fields in `{}` are `!Send`",
+                            "some fields in `{}` are not safe to be sent to another thread",
                             snippet(cx, hir_impl.self_ty.span, "Unknown")
                         ),
                         |diag| {
                             for field in non_send_fields {
                                 diag.span_note(
                                     field.def.span,
-                                    &format!("the type of field `{}` is `!Send`", field.def.ident.name),
+                                    &format!("it is not safe to send field `{}` to another thread", field.def.ident.name),
                                 );
 
                                 match field.generic_params.len() {
@@ -164,8 +171,8 @@ fn generic_params_string(&self) -> String {
 
 /// 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)
+fn collect_generic_params(ty: Ty<'_>) -> Vec<Ty<'_>> {
+    ty.walk()
         .filter_map(|inner| match inner.unpack() {
             GenericArgKind::Type(inner_ty) => Some(inner_ty),
             _ => None,
@@ -180,7 +187,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;
     }
 
@@ -200,7 +207,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),
@@ -217,14 +224,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 {
-    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;
+/// 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() {
+        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;
+                    }
+                },
+                _ => (),
             }
         }
     }