arc_with_non_send_sync: reword and move to suspicious

This commit is contained in:
Alex Macleod 2023-07-04 16:58:02 +00:00
parent dd8e44c5a2
commit 4939a716e8
3 changed files with 73 additions and 43 deletions

View file

@ -1,12 +1,11 @@
use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::last_path_segment; use clippy_utils::last_path_segment;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use if_chain::if_chain;
use rustc_hir::{Expr, ExprKind}; use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_lint::LateLintPass; use rustc_lint::LateLintPass;
use rustc_middle::ty; use rustc_middle::ty;
use rustc_middle::ty::print::with_forced_trimmed_paths;
use rustc_middle::ty::GenericArgKind; use rustc_middle::ty::GenericArgKind;
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::sym; use rustc_span::symbol::sym;
@ -16,8 +15,8 @@ declare_clippy_lint! {
/// This lint warns when you use `Arc` with a type that does not implement `Send` or `Sync`. /// This lint warns when you use `Arc` with a type that does not implement `Send` or `Sync`.
/// ///
/// ### Why is this bad? /// ### Why is this bad?
/// Wrapping a type in Arc doesn't add thread safety to the underlying data, so data races /// `Arc<T>` is only `Send`/`Sync` when `T` is [both `Send` and `Sync`](https://doc.rust-lang.org/std/sync/struct.Arc.html#impl-Send-for-Arc%3CT%3E),
/// could occur when touching the underlying data. /// either `T` should be made `Send + Sync` or an `Rc` should be used instead of an `Arc`
/// ///
/// ### Example /// ### Example
/// ```rust /// ```rust
@ -25,16 +24,17 @@ declare_clippy_lint! {
/// # use std::sync::Arc; /// # use std::sync::Arc;
/// ///
/// fn main() { /// fn main() {
/// // This is safe, as `i32` implements `Send` and `Sync`. /// // This is fine, as `i32` implements `Send` and `Sync`.
/// let a = Arc::new(42); /// let a = Arc::new(42);
/// ///
/// // This is not safe, as `RefCell` does not implement `Sync`. /// // `RefCell` is `!Sync`, so either the `Arc` should be replaced with an `Rc`
/// // or the `RefCell` replaced with something like a `RwLock`
/// let b = Arc::new(RefCell::new(42)); /// let b = Arc::new(RefCell::new(42));
/// } /// }
/// ``` /// ```
#[clippy::version = "1.72.0"] #[clippy::version = "1.72.0"]
pub ARC_WITH_NON_SEND_SYNC, pub ARC_WITH_NON_SEND_SYNC,
correctness, suspicious,
"using `Arc` with a type that does not implement `Send` or `Sync`" "using `Arc` with a type that does not implement `Send` or `Sync`"
} }
declare_lint_pass!(ArcWithNonSendSync => [ARC_WITH_NON_SEND_SYNC]); declare_lint_pass!(ArcWithNonSendSync => [ARC_WITH_NON_SEND_SYNC]);
@ -42,35 +42,38 @@ declare_lint_pass!(ArcWithNonSendSync => [ARC_WITH_NON_SEND_SYNC]);
impl LateLintPass<'_> for ArcWithNonSendSync { impl LateLintPass<'_> for ArcWithNonSendSync {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
let ty = cx.typeck_results().expr_ty(expr); let ty = cx.typeck_results().expr_ty(expr);
if_chain! { if is_type_diagnostic_item(cx, ty, sym::Arc)
if is_type_diagnostic_item(cx, ty, sym::Arc); && let ExprKind::Call(func, [arg]) = expr.kind
if let ExprKind::Call(func, [arg]) = expr.kind; && let ExprKind::Path(func_path) = func.kind
if let ExprKind::Path(func_path) = func.kind; && last_path_segment(&func_path).ident.name == sym::new
if last_path_segment(&func_path).ident.name == sym::new; && let arg_ty = cx.typeck_results().expr_ty(arg)
if let arg_ty = cx.typeck_results().expr_ty(arg);
// make sure that the type is not and does not contain any type parameters // make sure that the type is not and does not contain any type parameters
if arg_ty.walk().all(|arg| { && arg_ty.walk().all(|arg| {
!matches!(arg.unpack(), GenericArgKind::Type(ty) if matches!(ty.kind(), ty::Param(_))) !matches!(arg.unpack(), GenericArgKind::Type(ty) if matches!(ty.kind(), ty::Param(_)))
}); })
if !cx.tcx && let Some(send) = cx.tcx.get_diagnostic_item(sym::Send)
.lang_items() && let Some(sync) = cx.tcx.lang_items().sync_trait()
.sync_trait() && let [is_send, is_sync] = [send, sync].map(|id| implements_trait(cx, arg_ty, id, &[]))
.map_or(false, |id| implements_trait(cx, arg_ty, id, &[])) || && !(is_send && is_sync)
!cx.tcx {
.get_diagnostic_item(sym::Send) span_lint_and_then(
.map_or(false, |id| implements_trait(cx, arg_ty, id, &[]));
then {
span_lint_and_help(
cx, cx,
ARC_WITH_NON_SEND_SYNC, ARC_WITH_NON_SEND_SYNC,
expr.span, expr.span,
"usage of `Arc<T>` where `T` is not `Send` or `Sync`", "usage of an `Arc` that is not `Send` or `Sync`",
None, |diag| with_forced_trimmed_paths!({
"consider using `Rc<T>` instead or wrapping `T` in a std::sync type like \ if !is_send {
`Mutex<T>`", diag.note(format!("the trait `Send` is not implemented for `{arg_ty}`"));
); }
} if !is_sync {
diag.note(format!("the trait `Sync` is not implemented for `{arg_ty}`"));
}
diag.note(format!("required for `{ty}` to implement `Send` and `Sync`"));
diag.help("consider using an `Rc` instead or wrapping the inner type with a `Mutex`");
}
));
} }
} }
} }

View file

@ -12,9 +12,13 @@ fn issue11076<T>() {
} }
fn main() { fn main() {
// This is safe, as `i32` implements `Send` and `Sync`. let _ = Arc::new(42);
let a = Arc::new(42);
// This is not safe, as `RefCell` does not implement `Sync`. // !Sync
let b = Arc::new(RefCell::new(42)); let _ = Arc::new(RefCell::new(42));
let mutex = Mutex::new(1);
// !Send
let _ = Arc::new(mutex.lock().unwrap());
// !Send + !Sync
let _ = Arc::new(&42 as *const i32);
} }

View file

@ -1,11 +1,34 @@
error: usage of `Arc<T>` where `T` is not `Send` or `Sync` error: usage of an `Arc` that is not `Send` or `Sync`
--> $DIR/arc_with_non_send_sync.rs:19:13 --> $DIR/arc_with_non_send_sync.rs:18:13
| |
LL | let b = Arc::new(RefCell::new(42)); LL | let _ = Arc::new(RefCell::new(42));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
= help: consider using `Rc<T>` instead or wrapping `T` in a std::sync type like `Mutex<T>` = note: the trait `Sync` is not implemented for `RefCell<i32>`
= note: required for `Arc<RefCell<i32>>` to implement `Send` and `Sync`
= help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
= note: `-D clippy::arc-with-non-send-sync` implied by `-D warnings` = note: `-D clippy::arc-with-non-send-sync` implied by `-D warnings`
error: aborting due to previous error error: usage of an `Arc` that is not `Send` or `Sync`
--> $DIR/arc_with_non_send_sync.rs:21:13
|
LL | let _ = Arc::new(mutex.lock().unwrap());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the trait `Send` is not implemented for `MutexGuard<'_, i32>`
= note: required for `Arc<MutexGuard<'_, i32>>` to implement `Send` and `Sync`
= help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
error: usage of an `Arc` that is not `Send` or `Sync`
--> $DIR/arc_with_non_send_sync.rs:23:13
|
LL | let _ = Arc::new(&42 as *const i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the trait `Send` is not implemented for `*const i32`
= note: the trait `Sync` is not implemented for `*const i32`
= note: required for `Arc<*const i32>` to implement `Send` and `Sync`
= help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
error: aborting due to 3 previous errors