2021-10-07 04:21:30 -05:00
|
|
|
use clippy_utils::diagnostics::span_lint_and_then;
|
|
|
|
use clippy_utils::source::snippet;
|
|
|
|
use clippy_utils::ty::{implements_trait, is_copy};
|
2023-09-26 22:56:38 -05:00
|
|
|
use clippy_utils::is_lint_allowed;
|
2021-10-07 04:21:30 -05:00
|
|
|
use rustc_ast::ImplPolarity;
|
|
|
|
use rustc_hir::def_id::DefId;
|
|
|
|
use rustc_hir::{FieldDef, Item, ItemKind, Node};
|
|
|
|
use rustc_lint::{LateContext, LateLintPass};
|
2021-12-06 05:33:31 -06:00
|
|
|
use rustc_middle::lint::in_external_macro;
|
2023-07-11 16:35:29 -05:00
|
|
|
use rustc_middle::ty::{self, GenericArgKind, Ty};
|
2021-10-07 04:21:30 -05:00
|
|
|
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
|
|
|
use rustc_span::sym;
|
|
|
|
|
|
|
|
declare_clippy_lint! {
|
|
|
|
/// ### What it does
|
2021-12-17 06:40:22 -06:00
|
|
|
/// 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.
|
2021-10-07 04:21:30 -05:00
|
|
|
///
|
|
|
|
/// ### Why is this bad?
|
2021-12-17 06:40:22 -06:00
|
|
|
/// 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.
|
2021-10-07 04:21:30 -05:00
|
|
|
///
|
|
|
|
/// 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
|
2021-12-17 06:40:22 -06:00
|
|
|
/// 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.
|
2021-10-07 04:21:30 -05:00
|
|
|
///
|
|
|
|
/// ### Example
|
|
|
|
/// ```rust,ignore
|
|
|
|
/// struct ExampleStruct<T> {
|
|
|
|
/// rc_is_not_send: Rc<String>,
|
|
|
|
/// unbounded_generic_field: T,
|
|
|
|
/// }
|
|
|
|
///
|
|
|
|
/// // This impl is unsound because it allows sending `!Send` types through `ExampleStruct`
|
|
|
|
/// unsafe impl<T> Send for ExampleStruct<T> {}
|
|
|
|
/// ```
|
|
|
|
/// 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`).
|
2021-12-06 05:33:31 -06:00
|
|
|
#[clippy::version = "1.57.0"]
|
2021-10-07 04:21:30 -05:00
|
|
|
pub NON_SEND_FIELDS_IN_SEND_TY,
|
2021-12-17 06:40:22 -06:00
|
|
|
nursery,
|
|
|
|
"there is a field that is not safe to be sent to another thread in a `Send` struct"
|
2021-10-07 04:21:30 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
#[derive(Copy, Clone)]
|
|
|
|
pub struct NonSendFieldInSendTy {
|
|
|
|
enable_raw_pointer_heuristic: bool,
|
|
|
|
}
|
|
|
|
|
|
|
|
impl NonSendFieldInSendTy {
|
|
|
|
pub fn new(enable_raw_pointer_heuristic: bool) -> Self {
|
|
|
|
Self {
|
|
|
|
enable_raw_pointer_heuristic,
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
impl_lint_pass!(NonSendFieldInSendTy => [NON_SEND_FIELDS_IN_SEND_TY]);
|
|
|
|
|
|
|
|
impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
|
|
|
|
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
|
|
|
|
let ty_allowed_in_send = if self.enable_raw_pointer_heuristic {
|
|
|
|
ty_allowed_with_raw_pointer_heuristic
|
|
|
|
} else {
|
|
|
|
ty_allowed_without_raw_pointer_heuristic
|
|
|
|
};
|
|
|
|
|
|
|
|
// Checks if we are in `Send` impl item.
|
|
|
|
// We start from `Send` impl instead of `check_field_def()` because
|
|
|
|
// single `AdtDef` may have multiple `Send` impls due to generic
|
|
|
|
// parameters, and the lint is much easier to implement in this way.
|
|
|
|
if_chain! {
|
2021-12-06 05:33:31 -06:00
|
|
|
if !in_external_macro(cx.tcx.sess, item.span);
|
2021-10-07 04:21:30 -05:00
|
|
|
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 hir_impl.polarity == ImplPolarity::Positive;
|
2023-01-10 15:57:22 -06:00
|
|
|
if let Some(ty_trait_ref) = cx.tcx.impl_trait_ref(item.owner_id);
|
2023-07-11 16:35:29 -05:00
|
|
|
if let self_ty = ty_trait_ref.instantiate_identity().self_ty();
|
|
|
|
if let ty::Adt(adt_def, impl_trait_args) = self_ty.kind();
|
2021-10-07 04:21:30 -05:00
|
|
|
then {
|
|
|
|
let mut non_send_fields = Vec::new();
|
|
|
|
|
|
|
|
let hir_map = cx.tcx.hir();
|
2022-03-04 14:28:41 -06:00
|
|
|
for variant in adt_def.variants() {
|
2021-10-07 04:21:30 -05:00
|
|
|
for field in &variant.fields {
|
|
|
|
if_chain! {
|
|
|
|
if let Some(field_hir_id) = field
|
|
|
|
.did
|
|
|
|
.as_local()
|
|
|
|
.map(|local_def_id| hir_map.local_def_id_to_hir_id(local_def_id));
|
|
|
|
if !is_lint_allowed(cx, NON_SEND_FIELDS_IN_SEND_TY, field_hir_id);
|
2023-07-11 16:35:29 -05:00
|
|
|
if let field_ty = field.ty(cx.tcx, impl_trait_args);
|
2021-10-07 04:21:30 -05:00
|
|
|
if !ty_allowed_in_send(cx, field_ty, send_trait);
|
|
|
|
if let Node::Field(field_def) = hir_map.get(field_hir_id);
|
|
|
|
then {
|
|
|
|
non_send_fields.push(NonSendField {
|
|
|
|
def: field_def,
|
|
|
|
ty: field_ty,
|
2022-01-11 21:19:52 -06:00
|
|
|
generic_params: collect_generic_params(field_ty),
|
2021-10-07 04:21:30 -05:00
|
|
|
})
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if !non_send_fields.is_empty() {
|
|
|
|
span_lint_and_then(
|
|
|
|
cx,
|
|
|
|
NON_SEND_FIELDS_IN_SEND_TY,
|
|
|
|
item.span,
|
|
|
|
&format!(
|
2021-12-17 06:40:22 -06:00
|
|
|
"some fields in `{}` are not safe to be sent to another thread",
|
2021-10-07 04:21:30 -05:00
|
|
|
snippet(cx, hir_impl.self_ty.span, "Unknown")
|
|
|
|
),
|
|
|
|
|diag| {
|
|
|
|
for field in non_send_fields {
|
|
|
|
diag.span_note(
|
|
|
|
field.def.span,
|
Restrict `From<S>` for `{D,Subd}iagnosticMessage`.
Currently a `{D,Subd}iagnosticMessage` can be created from any type that
impls `Into<String>`. That includes `&str`, `String`, and `Cow<'static,
str>`, which are reasonable. It also includes `&String`, which is pretty
weird, and results in many places making unnecessary allocations for
patterns like this:
```
self.fatal(&format!(...))
```
This creates a string with `format!`, takes a reference, passes the
reference to `fatal`, which does an `into()`, which clones the
reference, doing a second allocation. Two allocations for a single
string, bleh.
This commit changes the `From` impls so that you can only create a
`{D,Subd}iagnosticMessage` from `&str`, `String`, or `Cow<'static,
str>`. This requires changing all the places that currently create one
from a `&String`. Most of these are of the `&format!(...)` form
described above; each one removes an unnecessary static `&`, plus an
allocation when executed. There are also a few places where the existing
use of `&String` was more reasonable; these now just use `clone()` at
the call site.
As well as making the code nicer and more efficient, this is a step
towards possibly using `Cow<'static, str>` in
`{D,Subd}iagnosticMessage::{Str,Eager}`. That would require changing
the `From<&'a str>` impls to `From<&'static str>`, which is doable, but
I'm not yet sure if it's worthwhile.
2023-04-19 22:26:58 -05:00
|
|
|
format!("it is not safe to send field `{}` to another thread", field.def.ident.name),
|
2021-10-07 04:21:30 -05:00
|
|
|
);
|
|
|
|
|
|
|
|
match field.generic_params.len() {
|
|
|
|
0 => diag.help("use a thread-safe type that implements `Send`"),
|
Restrict `From<S>` for `{D,Subd}iagnosticMessage`.
Currently a `{D,Subd}iagnosticMessage` can be created from any type that
impls `Into<String>`. That includes `&str`, `String`, and `Cow<'static,
str>`, which are reasonable. It also includes `&String`, which is pretty
weird, and results in many places making unnecessary allocations for
patterns like this:
```
self.fatal(&format!(...))
```
This creates a string with `format!`, takes a reference, passes the
reference to `fatal`, which does an `into()`, which clones the
reference, doing a second allocation. Two allocations for a single
string, bleh.
This commit changes the `From` impls so that you can only create a
`{D,Subd}iagnosticMessage` from `&str`, `String`, or `Cow<'static,
str>`. This requires changing all the places that currently create one
from a `&String`. Most of these are of the `&format!(...)` form
described above; each one removes an unnecessary static `&`, plus an
allocation when executed. There are also a few places where the existing
use of `&String` was more reasonable; these now just use `clone()` at
the call site.
As well as making the code nicer and more efficient, this is a step
towards possibly using `Cow<'static, str>` in
`{D,Subd}iagnosticMessage::{Str,Eager}`. That would require changing
the `From<&'a str>` impls to `From<&'static str>`, which is doable, but
I'm not yet sure if it's worthwhile.
2023-04-19 22:26:58 -05:00
|
|
|
1 if is_ty_param(field.ty) => diag.help(format!("add `{}: Send` bound in `Send` impl", field.ty)),
|
|
|
|
_ => diag.help(format!(
|
2021-10-07 04:21:30 -05:00
|
|
|
"add bounds on type parameter{} `{}` that satisfy `{}: Send`",
|
|
|
|
if field.generic_params.len() > 1 { "s" } else { "" },
|
|
|
|
field.generic_params_string(),
|
|
|
|
snippet(cx, field.def.ty.span, "Unknown"),
|
|
|
|
)),
|
|
|
|
};
|
|
|
|
}
|
|
|
|
},
|
|
|
|
);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
struct NonSendField<'tcx> {
|
|
|
|
def: &'tcx FieldDef<'tcx>,
|
|
|
|
ty: Ty<'tcx>,
|
|
|
|
generic_params: Vec<Ty<'tcx>>,
|
|
|
|
}
|
|
|
|
|
|
|
|
impl<'tcx> NonSendField<'tcx> {
|
|
|
|
fn generic_params_string(&self) -> String {
|
|
|
|
self.generic_params
|
|
|
|
.iter()
|
|
|
|
.map(ToString::to_string)
|
|
|
|
.collect::<Vec<_>>()
|
|
|
|
.join(", ")
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
/// Given a type, collect all of its generic parameters.
|
|
|
|
/// Example: `MyStruct<P, Box<Q, R>>` => `vec![P, Q, R]`
|
2022-01-11 21:19:52 -06:00
|
|
|
fn collect_generic_params(ty: Ty<'_>) -> Vec<Ty<'_>> {
|
|
|
|
ty.walk()
|
2021-10-07 04:21:30 -05:00
|
|
|
.filter_map(|inner| match inner.unpack() {
|
|
|
|
GenericArgKind::Type(inner_ty) => Some(inner_ty),
|
|
|
|
_ => None,
|
|
|
|
})
|
|
|
|
.filter(|&inner_ty| is_ty_param(inner_ty))
|
|
|
|
.collect()
|
|
|
|
}
|
|
|
|
|
|
|
|
/// Be more strict when the heuristic is disabled
|
|
|
|
fn ty_allowed_without_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, send_trait: DefId) -> bool {
|
|
|
|
if implements_trait(cx, ty, send_trait, &[]) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2021-12-06 05:33:31 -06:00
|
|
|
if is_copy(cx, ty) && !contains_pointer_like(cx, ty) {
|
2021-10-07 04:21:30 -05:00
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
false
|
|
|
|
}
|
|
|
|
|
|
|
|
/// 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 implements_trait(cx, ty, send_trait, &[]) || is_copy(cx, ty) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
// The type is known to be `!Send` and `!Copy`
|
|
|
|
match ty.kind() {
|
2022-02-07 09:06:55 -06:00
|
|
|
ty::Tuple(fields) => fields
|
|
|
|
.iter()
|
2021-10-07 04:21:30 -05:00
|
|
|
.all(|ty| ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait)),
|
Overhaul `TyS` and `Ty`.
Specifically, change `Ty` from this:
```
pub type Ty<'tcx> = &'tcx TyS<'tcx>;
```
to this
```
pub struct Ty<'tcx>(Interned<'tcx, TyS<'tcx>>);
```
There are two benefits to this.
- It's now a first class type, so we can define methods on it. This
means we can move a lot of methods away from `TyS`, leaving `TyS` as a
barely-used type, which is appropriate given that it's not meant to
be used directly.
- The uniqueness requirement is now explicit, via the `Interned` type.
E.g. the pointer-based `Eq` and `Hash` comes from `Interned`, rather
than via `TyS`, which wasn't obvious at all.
Much of this commit is boring churn. The interesting changes are in
these files:
- compiler/rustc_middle/src/arena.rs
- compiler/rustc_middle/src/mir/visit.rs
- compiler/rustc_middle/src/ty/context.rs
- compiler/rustc_middle/src/ty/mod.rs
Specifically:
- Most mentions of `TyS` are removed. It's very much a dumb struct now;
`Ty` has all the smarts.
- `TyS` now has `crate` visibility instead of `pub`.
- `TyS::make_for_test` is removed in favour of the static `BOOL_TY`,
which just works better with the new structure.
- The `Eq`/`Ord`/`Hash` impls are removed from `TyS`. `Interned`s impls
of `Eq`/`Hash` now suffice. `Ord` is now partly on `Interned`
(pointer-based, for the `Equal` case) and partly on `TyS`
(contents-based, for the other cases).
- There are many tedious sigil adjustments, i.e. adding or removing `*`
or `&`. They seem to be unavoidable.
2022-01-24 21:13:38 -06:00
|
|
|
ty::Array(ty, _) | ty::Slice(ty) => ty_allowed_with_raw_pointer_heuristic(cx, *ty, send_trait),
|
2023-07-11 16:35:29 -05:00
|
|
|
ty::Adt(_, args) => {
|
2021-12-06 05:33:31 -06:00
|
|
|
if contains_pointer_like(cx, ty) {
|
2021-10-07 04:21:30 -05:00
|
|
|
// descends only if ADT contains any raw pointers
|
2023-07-11 16:35:29 -05:00
|
|
|
args.iter().all(|generic_arg| match generic_arg.unpack() {
|
2021-10-07 04:21:30 -05:00
|
|
|
GenericArgKind::Type(ty) => ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait),
|
|
|
|
// Lifetimes and const generics are not solid part of ADT and ignored
|
|
|
|
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => true,
|
|
|
|
})
|
|
|
|
} else {
|
|
|
|
false
|
|
|
|
}
|
|
|
|
},
|
|
|
|
// Raw pointers are `!Send` but allowed by the heuristic
|
|
|
|
ty::RawPtr(_) => true,
|
|
|
|
_ => false,
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2023-07-11 16:35:29 -05:00
|
|
|
/// Checks if the type contains any pointer-like types in args (including nested ones)
|
2021-12-06 05:33:31 -06:00
|
|
|
fn contains_pointer_like<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> bool {
|
2022-01-11 21:19:52 -06:00
|
|
|
for ty_node in target_ty.walk() {
|
2021-12-06 05:33:31 -06:00
|
|
|
if let GenericArgKind::Type(inner_ty) = ty_node.unpack() {
|
|
|
|
match inner_ty.kind() {
|
|
|
|
ty::RawPtr(_) => {
|
|
|
|
return true;
|
|
|
|
},
|
|
|
|
ty::Adt(adt_def, _) => {
|
2023-09-26 22:56:38 -05:00
|
|
|
if cx.tcx.is_diagnostic_item(sym::NonNull, adt_def.did()) {
|
2021-12-06 05:33:31 -06:00
|
|
|
return true;
|
|
|
|
}
|
|
|
|
},
|
|
|
|
_ => (),
|
2021-10-07 04:21:30 -05:00
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
false
|
|
|
|
}
|
|
|
|
|
|
|
|
/// Returns `true` if the type is a type parameter such as `T`.
|
|
|
|
fn is_ty_param(target_ty: Ty<'_>) -> bool {
|
|
|
|
matches!(target_ty.kind(), ty::Param(_))
|
|
|
|
}
|