Write lint to check for dashmaps entries being held
Signed-off-by: Marcel Müller <neikos@neikos.email>
This commit is contained in:
parent
bf589f7d28
commit
ace01a2096
8 changed files with 355 additions and 24 deletions
|
|
@ -7,12 +7,17 @@ publish = false
|
|||
[lib]
|
||||
crate-type = ["cdylib"]
|
||||
|
||||
[[example]]
|
||||
name = "ui"
|
||||
path = "ui/main.rs"
|
||||
|
||||
[dependencies]
|
||||
clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "19e305bb57a7595f2a8d81f521c0dd8bf854e739" }
|
||||
dylint_linting = "4.0.0"
|
||||
hakari = { version = "0.1", path = "../../hakari" }
|
||||
|
||||
[dev-dependencies]
|
||||
dashmap = "6"
|
||||
dylint_testing = "4.0.0"
|
||||
|
||||
[package.metadata.rust-analyzer]
|
||||
|
|
|
|||
|
|
@ -1,31 +1,29 @@
|
|||
#![feature(rustc_private)]
|
||||
#![feature(let_chains)]
|
||||
#![warn(unused_extern_crates)]
|
||||
|
||||
extern crate rustc_arena;
|
||||
extern crate rustc_ast;
|
||||
extern crate rustc_ast_pretty;
|
||||
extern crate rustc_data_structures;
|
||||
extern crate rustc_errors;
|
||||
extern crate rustc_hir;
|
||||
extern crate rustc_hir_pretty;
|
||||
extern crate rustc_index;
|
||||
extern crate rustc_infer;
|
||||
extern crate rustc_lexer;
|
||||
extern crate rustc_middle;
|
||||
extern crate rustc_mir_dataflow;
|
||||
extern crate rustc_parse;
|
||||
extern crate rustc_span;
|
||||
extern crate rustc_target;
|
||||
extern crate rustc_trait_selection;
|
||||
|
||||
use clippy_utils::diagnostics::span_lint_and_then;
|
||||
use clippy_utils::match_any_def_paths;
|
||||
use rustc_hir as hir;
|
||||
use rustc_hir::def_id::DefId;
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_lint::LateLintPass;
|
||||
use rustc_middle::mir::CoroutineLayout;
|
||||
use rustc_middle::ty::Adt;
|
||||
use rustc_middle::ty::Ty;
|
||||
use rustc_span::sym;
|
||||
|
||||
dylint_linting::declare_late_lint! {
|
||||
/// ### What it does
|
||||
/// Check whether a DashMap reference is held across an await point.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
///
|
||||
/// This can cause issues as the dashmap may deadlock
|
||||
///
|
||||
/// ### Known problems
|
||||
///
|
||||
/// Remove if none.
|
||||
|
|
@ -43,15 +41,102 @@ dylint_linting::declare_late_lint! {
|
|||
/// ```
|
||||
pub DASHMAP_REF,
|
||||
Warn,
|
||||
"description goes here"
|
||||
"Check whether a dashmap reference is held over an await point"
|
||||
}
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for DashmapRef {
|
||||
// A list of things you might check can be found here:
|
||||
// https://doc.rust-lang.org/stable/nightly-rustc/rustc_lint/trait.LateLintPass.html
|
||||
const DASHMAP_REF_GUARD: [&str; 4] = ["dashmap", "mapref", "one", "Ref"];
|
||||
const DASHMAP_REF_MUT_GUARD: [&str; 4] = ["dashmap", "mapref", "one", "RefMut"];
|
||||
const DASHMAP_ENTRY_GUARD: [&str; 4] = ["dashmap", "mapref", "entry", "Entry"];
|
||||
const DASHMAP_ITER: [&str; 3] = ["dashmap", "iter", "Iter"];
|
||||
const DASHMAP_ITER_MUT: [&str; 3] = ["dashmap", "iter", "IterMut"];
|
||||
|
||||
impl LateLintPass<'_> for DashmapRef {
|
||||
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ hir::Expr<'_>) {
|
||||
if let hir::ExprKind::Closure(hir::Closure {
|
||||
def_id,
|
||||
kind:
|
||||
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
|
||||
hir::CoroutineDesugaring::Async,
|
||||
_,
|
||||
)),
|
||||
..
|
||||
}) = expr.kind
|
||||
{
|
||||
if let Some(coroutine_layout) = cx.tcx.mir_coroutine_witnesses(*def_id) {
|
||||
check_interior_types(cx, coroutine_layout);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_interior_types(cx: &LateContext<'_>, coroutine: &CoroutineLayout<'_>) {
|
||||
for (ty_index, ty_cause) in coroutine.field_tys.iter_enumerated() {
|
||||
if let Adt(adt, _) = ty_cause.ty.kind() {
|
||||
let await_points = || {
|
||||
coroutine
|
||||
.variant_source_info
|
||||
.iter_enumerated()
|
||||
.filter_map(|(variant, source_info)| {
|
||||
coroutine.variant_fields[variant]
|
||||
.raw
|
||||
.contains(&ty_index)
|
||||
.then_some(source_info.span)
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
};
|
||||
|
||||
if is_dashmap_ref(cx, adt.did(), &ty_cause.ty) {
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
DASHMAP_REF,
|
||||
ty_cause.source_info.span,
|
||||
"this dashmap reference is held \
|
||||
across an 'await' point. Either drop it before the 'await', \
|
||||
or read it again afterwards.",
|
||||
|diag| {
|
||||
diag.span_note(
|
||||
await_points(),
|
||||
"these are all the await points this ref is held through",
|
||||
);
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn is_dashmap_ref(cx: &LateContext<'_>, def_id: DefId, ty: &Ty<'_>) -> bool {
|
||||
if let Adt(adt, args) = ty.kind() {
|
||||
if let Some(ty) = cx
|
||||
.tcx
|
||||
.is_diagnostic_item(sym::Option, adt.did())
|
||||
.then(|| args.type_at(0))
|
||||
{
|
||||
if let Adt(adt, ..) = ty.kind() {
|
||||
is_dashmap_ref(cx, adt.did(), &ty)
|
||||
} else {
|
||||
false
|
||||
}
|
||||
} else {
|
||||
match_any_def_paths(
|
||||
cx,
|
||||
def_id,
|
||||
&[
|
||||
&DASHMAP_REF_GUARD,
|
||||
&DASHMAP_REF_MUT_GUARD,
|
||||
&DASHMAP_ENTRY_GUARD,
|
||||
&DASHMAP_ITER,
|
||||
&DASHMAP_ITER_MUT,
|
||||
],
|
||||
)
|
||||
.is_some()
|
||||
}
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ui() {
|
||||
dylint_testing::ui_test(env!("CARGO_PKG_NAME"), "ui");
|
||||
dylint_testing::ui_test_example(env!("CARGO_PKG_NAME"), "ui");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1 +1,50 @@
|
|||
fn main() {}
|
||||
#[expect(unused_must_use)]
|
||||
fn main() {
|
||||
held_across();
|
||||
held_across_multiple();
|
||||
dropped_before();
|
||||
all_kinds();
|
||||
}
|
||||
|
||||
async fn held_across() {
|
||||
let map = dashmap::DashMap::<String, String>::default();
|
||||
|
||||
let reference = map.get("Hello");
|
||||
|
||||
std::future::pending::<()>().await;
|
||||
|
||||
println!("{reference:?}");
|
||||
}
|
||||
|
||||
async fn held_across_multiple() {
|
||||
let map = dashmap::DashMap::<String, String>::default();
|
||||
|
||||
let _reference = map.get("Hello");
|
||||
|
||||
std::future::pending::<()>().await;
|
||||
|
||||
std::future::pending::<()>().await;
|
||||
}
|
||||
|
||||
async fn dropped_before() {
|
||||
let map = dashmap::DashMap::<String, String>::default();
|
||||
|
||||
let _ = map.get("Hello");
|
||||
|
||||
std::future::pending::<()>().await;
|
||||
}
|
||||
|
||||
async fn all_kinds() {
|
||||
let map = dashmap::DashMap::<String, String>::default();
|
||||
|
||||
let _ref = map.get("Hello");
|
||||
let _ref_mut = map.get_mut("Hello");
|
||||
let _entry = map.entry("Hello".to_string());
|
||||
let _opt_entry = map.try_entry("Hello".to_string());
|
||||
let _iter_entry = map.iter();
|
||||
let _iter_mut_entry = map.iter_mut();
|
||||
let _direct_ref = map.get("Hello").unwrap();
|
||||
let _direct_ref_mut = map.get_mut("Hello").unwrap();
|
||||
|
||||
std::future::pending::<()>().await;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,126 @@
|
|||
warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards.
|
||||
--> $DIR/main.rs:12:9
|
||||
|
|
||||
LL | let reference = map.get("Hello");
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/main.rs:14:34
|
||||
|
|
||||
LL | std::future::pending::<()>().await;
|
||||
| ^^^^^
|
||||
= note: `#[warn(dashmap_ref)]` on by default
|
||||
|
||||
warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards.
|
||||
--> $DIR/main.rs:22:9
|
||||
|
|
||||
LL | let _reference = map.get("Hello");
|
||||
| ^^^^^^^^^^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/main.rs:24:34
|
||||
|
|
||||
LL | std::future::pending::<()>().await;
|
||||
| ^^^^^
|
||||
LL |
|
||||
LL | std::future::pending::<()>().await;
|
||||
| ^^^^^
|
||||
|
||||
warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards.
|
||||
--> $DIR/main.rs:40:9
|
||||
|
|
||||
LL | let _ref = map.get("Hello");
|
||||
| ^^^^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/main.rs:49:34
|
||||
|
|
||||
LL | std::future::pending::<()>().await;
|
||||
| ^^^^^
|
||||
|
||||
warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards.
|
||||
--> $DIR/main.rs:41:9
|
||||
|
|
||||
LL | let _ref_mut = map.get_mut("Hello");
|
||||
| ^^^^^^^^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/main.rs:49:34
|
||||
|
|
||||
LL | std::future::pending::<()>().await;
|
||||
| ^^^^^
|
||||
|
||||
warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards.
|
||||
--> $DIR/main.rs:42:9
|
||||
|
|
||||
LL | let _entry = map.entry("Hello".to_string());
|
||||
| ^^^^^^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/main.rs:49:34
|
||||
|
|
||||
LL | std::future::pending::<()>().await;
|
||||
| ^^^^^
|
||||
|
||||
warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards.
|
||||
--> $DIR/main.rs:43:9
|
||||
|
|
||||
LL | let _opt_entry = map.try_entry("Hello".to_string());
|
||||
| ^^^^^^^^^^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/main.rs:49:34
|
||||
|
|
||||
LL | std::future::pending::<()>().await;
|
||||
| ^^^^^
|
||||
|
||||
warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards.
|
||||
--> $DIR/main.rs:44:9
|
||||
|
|
||||
LL | let _iter_entry = map.iter();
|
||||
| ^^^^^^^^^^^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/main.rs:49:34
|
||||
|
|
||||
LL | std::future::pending::<()>().await;
|
||||
| ^^^^^
|
||||
|
||||
warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards.
|
||||
--> $DIR/main.rs:45:9
|
||||
|
|
||||
LL | let _iter_mut_entry = map.iter_mut();
|
||||
| ^^^^^^^^^^^^^^^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/main.rs:49:34
|
||||
|
|
||||
LL | std::future::pending::<()>().await;
|
||||
| ^^^^^
|
||||
|
||||
warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards.
|
||||
--> $DIR/main.rs:46:9
|
||||
|
|
||||
LL | let _direct_ref = map.get("Hello").unwrap();
|
||||
| ^^^^^^^^^^^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/main.rs:49:34
|
||||
|
|
||||
LL | std::future::pending::<()>().await;
|
||||
| ^^^^^
|
||||
|
||||
warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards.
|
||||
--> $DIR/main.rs:47:9
|
||||
|
|
||||
LL | let _direct_ref_mut = map.get_mut("Hello").unwrap();
|
||||
| ^^^^^^^^^^^^^^^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/main.rs:49:34
|
||||
|
|
||||
LL | std::future::pending::<()>().await;
|
||||
| ^^^^^
|
||||
|
||||
warning: 10 warnings emitted
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue