Skip to content

Commit ea0ce7b

Browse files
flip1995flip1995
committed
Move await_holding_* lints to suspicious and improve doc
Even though the FP for that the lints were moved to pedantic isn't fixed yet, running the lintcheck tool over the most popular 279 crates didn't trigger this lint once. I would say that this lint is valuable enough, despite the known FP, to be warn-by-default. Especially since a pretty nice workaround exists.
1 parent c570941 commit ea0ce7b

File tree

4 files changed

+64
-30
lines changed

4 files changed

+64
-30
lines changed

‎clippy_lints/src/await_holding_invalid.rs‎

Lines changed: 60 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@ use rustc_span::Span;
99

1010
declare_clippy_lint! {
1111
/// ### What it does
12-
/// Checks for calls to await while holding a
13-
/// non-async-aware MutexGuard.
12+
/// Checks for calls to await while holding a non-async-aware MutexGuard.
1413
///
1514
/// ### Why is this bad?
1615
/// The Mutex types found in std::sync and parking_lot
@@ -22,77 +21,110 @@ declare_clippy_lint! {
2221
/// either by introducing a scope or an explicit call to Drop::drop.
2322
///
2423
/// ### Known problems
25-
/// Will report false positive for explicitly dropped guards ([#6446](https://github.com/rust-lang/rust-clippy/issues/6446)).
24+
/// Will report false positive for explicitly dropped guards
25+
/// ([#6446](https://github.com/rust-lang/rust-clippy/issues/6446)). A workaround for this is
26+
/// to wrap the `.lock()` call in a block instead of explicitly dropping the guard.
2627
///
2728
/// ### Example
28-
/// ```rust,ignore
29-
/// use std::sync::Mutex;
30-
///
29+
/// ```rust
30+
/// # use std::sync::Mutex;
31+
/// # async fn baz() {}
3132
/// async fn foo(x: &Mutex<u32>) {
32-
/// let guard = x.lock().unwrap();
33+
/// let mut guard = x.lock().unwrap();
3334
/// *guard += 1;
34-
/// bar.await;
35+
/// baz().await;
36+
/// }
37+
///
38+
/// async fn bar(x: &Mutex<u32>) {
39+
/// let mut guard = x.lock().unwrap();
40+
/// *guard += 1;
41+
/// drop(guard); // explicit drop
42+
/// baz().await;
3543
/// }
3644
/// ```
3745
///
3846
/// Use instead:
39-
/// ```rust,ignore
40-
/// use std::sync::Mutex;
41-
///
47+
/// ```rust
48+
/// # use std::sync::Mutex;
49+
/// # async fn baz() {}
4250
/// async fn foo(x: &Mutex<u32>) {
4351
/// {
44-
/// let guard = x.lock().unwrap();
52+
/// let mut guard = x.lock().unwrap();
4553
/// *guard += 1;
4654
/// }
47-
/// bar.await;
55+
/// baz().await;
56+
/// }
57+
///
58+
/// async fn bar(x: &Mutex<u32>) {
59+
/// {
60+
/// let mut guard = x.lock().unwrap();
61+
/// *guard += 1;
62+
/// } // guard dropped here at end of scope
63+
/// baz().await;
4864
/// }
4965
/// ```
5066
#[clippy::version = "1.45.0"]
5167
pub AWAIT_HOLDING_LOCK,
52-
pedantic,
53-
"Inside an async function, holding a MutexGuard while calling await"
68+
suspicious,
69+
"inside an async function, holding a `MutexGuard` while calling `await`"
5470
}
5571

5672
declare_clippy_lint! {
5773
/// ### What it does
58-
/// Checks for calls to await while holding a
59-
/// `RefCell` `Ref` or `RefMut`.
74+
/// Checks for calls to await while holding a `RefCell` `Ref` or `RefMut`.
6075
///
6176
/// ### Why is this bad?
6277
/// `RefCell` refs only check for exclusive mutable access
6378
/// at runtime. Holding onto a `RefCell` ref across an `await` suspension point
6479
/// risks panics from a mutable ref shared while other refs are outstanding.
6580
///
6681
/// ### Known problems
67-
/// Will report false positive for explicitly dropped refs ([#6353](https://github.com/rust-lang/rust-clippy/issues/6353)).
82+
/// Will report false positive for explicitly dropped refs
83+
/// ([#6353](https://github.com/rust-lang/rust-clippy/issues/6353)). A workaround for this is
84+
/// to wrap the `.borrow[_mut]()` call in a block instead of explicitly dropping the ref.
6885
///
6986
/// ### Example
70-
/// ```rust,ignore
71-
/// use std::cell::RefCell;
72-
///
87+
/// ```rust
88+
/// # use std::cell::RefCell;
89+
/// # async fn baz() {}
7390
/// async fn foo(x: &RefCell<u32>) {
7491
/// let mut y = x.borrow_mut();
7592
/// *y += 1;
76-
/// bar.await;
93+
/// baz().await;
94+
/// }
95+
///
96+
/// async fn bar(x: &RefCell<u32>) {
97+
/// let mut y = x.borrow_mut();
98+
/// *y += 1;
99+
/// drop(y); // explicit drop
100+
/// baz().await;
77101
/// }
78102
/// ```
79103
///
80104
/// Use instead:
81-
/// ```rust,ignore
82-
/// use std::cell::RefCell;
83-
///
105+
/// ```rust
106+
/// # use std::cell::RefCell;
107+
/// # async fn baz() {}
84108
/// async fn foo(x: &RefCell<u32>) {
85109
/// {
86110
/// let mut y = x.borrow_mut();
87111
/// *y += 1;
88112
/// }
89-
/// bar.await;
113+
/// baz().await;
114+
/// }
115+
///
116+
/// async fn bar(x: &RefCell<u32>) {
117+
/// {
118+
/// let mut y = x.borrow_mut();
119+
/// *y += 1;
120+
/// } // y dropped here at end of scope
121+
/// baz().await;
90122
/// }
91123
/// ```
92124
#[clippy::version = "1.49.0"]
93125
pub AWAIT_HOLDING_REFCELL_REF,
94-
pedantic,
95-
"Inside an async function, holding a RefCell ref while calling await"
126+
suspicious,
127+
"inside an async function, holding a `RefCell` ref while calling `await`"
96128
}
97129

98130
declare_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF]);

‎clippy_lints/src/lib.register_all.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
1414
LintId::of(attrs::DEPRECATED_SEMVER),
1515
LintId::of(attrs::MISMATCHED_TARGET_OS),
1616
LintId::of(attrs::USELESS_ATTRIBUTE),
17+
LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK),
18+
LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
1719
LintId::of(bit_mask::BAD_BIT_MASK),
1820
LintId::of(bit_mask::INEFFECTIVE_BIT_MASK),
1921
LintId::of(blacklisted_name::BLACKLISTED_NAME),

‎clippy_lints/src/lib.register_pedantic.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44

55
store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
66
LintId::of(attrs::INLINE_ALWAYS),
7-
LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK),
8-
LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
97
LintId::of(bit_mask::VERBOSE_BIT_MASK),
108
LintId::of(borrow_as_ptr::BORROW_AS_PTR),
119
LintId::of(bytecount::NAIVE_BYTECOUNT),

‎clippy_lints/src/lib.register_suspicious.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec![
66
LintId::of(assign_ops::MISREFACTORED_ASSIGN_OP),
77
LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
8+
LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK),
9+
LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
810
LintId::of(eval_order_dependence::EVAL_ORDER_DEPENDENCE),
911
LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS),
1012
LintId::of(formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),

0 commit comments

Comments
 (0)