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,
nursery,
- "there is field that does not implement `Send` in a `Send` struct"
+ "there is a field that is not safe to be sent to another thread in a `Send` struct"
}
#[derive(Copy, Clone)]
// 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 !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;
if let Some(trait_id) = trait_ref.trait_def_id();
if send_trait == trait_id;
- if let ImplPolarity::Positive = hir_impl.polarity;
+ if hir_impl.polarity == ImplPolarity::Positive;
if let Some(ty_trait_ref) = cx.tcx.impl_trait_ref(item.def_id);
if let self_ty = ty_trait_ref.self_ty();
if let ty::Adt(adt_def, impl_trait_substs) = self_ty.kind();
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),
})
}
}
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() {
/// 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,
return true;
}
- if is_copy(cx, ty) && !contains_raw_pointer(cx, ty) {
+ if is_copy(cx, ty) && !contains_pointer_like(cx, ty) {
return true;
}
.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),
}
}
-/// 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;
+ }
+ },
+ _ => (),
}
}
}