mirror of
https://github.com/rust-lang/rust.git
synced 2024-12-04 20:54:13 +00:00
Add lint for holding RefCell Ref across an await
This commit is contained in:
parent
74530adcba
commit
57bf80f776
83
clippy_lints/src/await_holding_refcell_ref.rs
Normal file
83
clippy_lints/src/await_holding_refcell_ref.rs
Normal file
@ -0,0 +1,83 @@
|
||||
use crate::utils::{match_def_path, paths, span_lint_and_note};
|
||||
use rustc_hir::def_id::DefId;
|
||||
use rustc_hir::{AsyncGeneratorKind, Body, BodyId, GeneratorKind};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty::GeneratorInteriorTypeCause;
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
use rustc_span::Span;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for calls to await while holding a
|
||||
/// `RefCell` `Ref` or `RefMut`.
|
||||
///
|
||||
/// **Why is this bad?** `RefCell` refs only check for exclusive mutable access
|
||||
/// at runtime. Holding onto a `RefCell` ref across an `await` suspension point
|
||||
/// risks panics from a mutable ref shared while other refs are outstanding.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
///
|
||||
/// ```rust,ignore
|
||||
/// use std::cell::RefCell;
|
||||
///
|
||||
/// async fn foo(x: &RefCell<u32>) {
|
||||
/// let b = x.borrow_mut()();
|
||||
/// *ref += 1;
|
||||
/// bar.await;
|
||||
/// }
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```rust,ignore
|
||||
/// use std::cell::RefCell;
|
||||
///
|
||||
/// async fn foo(x: &RefCell<u32>) {
|
||||
/// {
|
||||
/// let b = x.borrow_mut();
|
||||
/// *ref += 1;
|
||||
/// }
|
||||
/// bar.await;
|
||||
/// }
|
||||
/// ```
|
||||
pub AWAIT_HOLDING_REFCELL_REF,
|
||||
pedantic,
|
||||
"Inside an async function, holding a RefCell ref while calling await"
|
||||
}
|
||||
|
||||
declare_lint_pass!(AwaitHoldingRefCellRef => [AWAIT_HOLDING_REFCELL_REF]);
|
||||
|
||||
impl LateLintPass<'_> for AwaitHoldingRefCellRef {
|
||||
fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) {
|
||||
use AsyncGeneratorKind::{Block, Closure, Fn};
|
||||
if let Some(GeneratorKind::Async(Block | Closure | Fn)) = body.generator_kind {
|
||||
let body_id = BodyId {
|
||||
hir_id: body.value.hir_id,
|
||||
};
|
||||
let def_id = cx.tcx.hir().body_owner_def_id(body_id);
|
||||
let typeck_results = cx.tcx.typeck(def_id);
|
||||
check_interior_types(cx, &typeck_results.generator_interior_types, body.value.span);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_interior_types(cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorTypeCause<'_>], span: Span) {
|
||||
for ty_cause in ty_causes {
|
||||
if let rustc_middle::ty::Adt(adt, _) = ty_cause.ty.kind() {
|
||||
if is_refcell_ref(cx, adt.did) {
|
||||
span_lint_and_note(
|
||||
cx,
|
||||
AWAIT_HOLDING_REFCELL_REF,
|
||||
ty_cause.span,
|
||||
"this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await.",
|
||||
ty_cause.scope_span.or(Some(span)),
|
||||
"these are all the await points this ref is held through",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn is_refcell_ref(cx: &LateContext<'_>, def_id: DefId) -> bool {
|
||||
match_def_path(cx, def_id, &paths::REFCELL_REF) || match_def_path(cx, def_id, &paths::REFCELL_REFMUT)
|
||||
}
|
@ -161,6 +161,7 @@ mod async_yields_async;
|
||||
mod atomic_ordering;
|
||||
mod attrs;
|
||||
mod await_holding_lock;
|
||||
mod await_holding_refcell_ref;
|
||||
mod bit_mask;
|
||||
mod blacklisted_name;
|
||||
mod blocks_in_if_conditions;
|
||||
@ -510,6 +511,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
&attrs::UNKNOWN_CLIPPY_LINTS,
|
||||
&attrs::USELESS_ATTRIBUTE,
|
||||
&await_holding_lock::AWAIT_HOLDING_LOCK,
|
||||
&await_holding_refcell_ref::AWAIT_HOLDING_REFCELL_REF,
|
||||
&bit_mask::BAD_BIT_MASK,
|
||||
&bit_mask::INEFFECTIVE_BIT_MASK,
|
||||
&bit_mask::VERBOSE_BIT_MASK,
|
||||
@ -906,6 +908,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
// end register lints, do not remove this comment, it’s used in `update_lints`
|
||||
|
||||
store.register_late_pass(|| box await_holding_lock::AwaitHoldingLock);
|
||||
store.register_late_pass(|| box await_holding_refcell_ref::AwaitHoldingRefCellRef);
|
||||
store.register_late_pass(|| box serde_api::SerdeAPI);
|
||||
store.register_late_pass(|| box utils::internal_lints::CompilerLintFunctions::new());
|
||||
store.register_late_pass(|| box utils::internal_lints::LintWithoutLintPass::default());
|
||||
@ -1189,6 +1192,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
|
||||
LintId::of(&attrs::INLINE_ALWAYS),
|
||||
LintId::of(&await_holding_lock::AWAIT_HOLDING_LOCK),
|
||||
LintId::of(&await_holding_refcell_ref::AWAIT_HOLDING_REFCELL_REF),
|
||||
LintId::of(&bit_mask::VERBOSE_BIT_MASK),
|
||||
LintId::of(&checked_conversions::CHECKED_CONVERSIONS),
|
||||
LintId::of(&copies::MATCH_SAME_ARMS),
|
||||
|
@ -93,6 +93,8 @@ pub const RANGE_ARGUMENT_TRAIT: [&str; 3] = ["core", "ops", "RangeBounds"];
|
||||
pub const RC: [&str; 3] = ["alloc", "rc", "Rc"];
|
||||
pub const RC_PTR_EQ: [&str; 4] = ["alloc", "rc", "Rc", "ptr_eq"];
|
||||
pub const RECEIVER: [&str; 4] = ["std", "sync", "mpsc", "Receiver"];
|
||||
pub const REFCELL_REF: [&str; 3] = ["core", "cell", "Ref"];
|
||||
pub const REFCELL_REFMUT: [&str; 3] = ["core", "cell", "RefMut"];
|
||||
pub const REGEX_BUILDER_NEW: [&str; 5] = ["regex", "re_builder", "unicode", "RegexBuilder", "new"];
|
||||
pub const REGEX_BYTES_BUILDER_NEW: [&str; 5] = ["regex", "re_builder", "bytes", "RegexBuilder", "new"];
|
||||
pub const REGEX_BYTES_NEW: [&str; 4] = ["regex", "re_bytes", "Regex", "new"];
|
||||
|
71
tests/ui/await_holding_refcell_ref.rs
Normal file
71
tests/ui/await_holding_refcell_ref.rs
Normal file
@ -0,0 +1,71 @@
|
||||
// edition:2018
|
||||
#![warn(clippy::await_holding_refcell_ref)]
|
||||
|
||||
use std::cell::RefCell;
|
||||
|
||||
async fn bad(x: &RefCell<u32>) -> u32 {
|
||||
let b = x.borrow();
|
||||
baz().await
|
||||
}
|
||||
|
||||
async fn bad_mut(x: &RefCell<u32>) -> u32 {
|
||||
let b = x.borrow_mut();
|
||||
baz().await
|
||||
}
|
||||
|
||||
async fn good(x: &RefCell<u32>) -> u32 {
|
||||
{
|
||||
let b = x.borrow_mut();
|
||||
let y = *b + 1;
|
||||
}
|
||||
baz().await;
|
||||
let b = x.borrow_mut();
|
||||
47
|
||||
}
|
||||
|
||||
async fn baz() -> u32 {
|
||||
42
|
||||
}
|
||||
|
||||
async fn also_bad(x: &RefCell<u32>) -> u32 {
|
||||
let first = baz().await;
|
||||
|
||||
let b = x.borrow_mut();
|
||||
|
||||
let second = baz().await;
|
||||
|
||||
let third = baz().await;
|
||||
|
||||
first + second + third
|
||||
}
|
||||
|
||||
async fn not_good(x: &RefCell<u32>) -> u32 {
|
||||
let first = baz().await;
|
||||
|
||||
let second = {
|
||||
let b = x.borrow_mut();
|
||||
baz().await
|
||||
};
|
||||
|
||||
let third = baz().await;
|
||||
|
||||
first + second + third
|
||||
}
|
||||
|
||||
#[allow(clippy::manual_async_fn)]
|
||||
fn block_bad(x: &RefCell<u32>) -> impl std::future::Future<Output = u32> + '_ {
|
||||
async move {
|
||||
let b = x.borrow_mut();
|
||||
baz().await
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let m = RefCell::new(100);
|
||||
good(&m);
|
||||
bad(&m);
|
||||
bad_mut(&m);
|
||||
also_bad(&m);
|
||||
not_good(&m);
|
||||
block_bad(&m);
|
||||
}
|
77
tests/ui/await_holding_refcell_ref.stderr
Normal file
77
tests/ui/await_holding_refcell_ref.stderr
Normal file
@ -0,0 +1,77 @@
|
||||
error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await.
|
||||
--> $DIR/await_holding_refcell_ref.rs:7:9
|
||||
|
|
||||
LL | let b = x.borrow();
|
||||
| ^
|
||||
|
|
||||
= note: `-D clippy::await-holding-refcell-ref` implied by `-D warnings`
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/await_holding_refcell_ref.rs:7:5
|
||||
|
|
||||
LL | / let b = x.borrow();
|
||||
LL | | baz().await
|
||||
LL | | }
|
||||
| |_^
|
||||
|
||||
error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await.
|
||||
--> $DIR/await_holding_refcell_ref.rs:12:9
|
||||
|
|
||||
LL | let b = x.borrow_mut();
|
||||
| ^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/await_holding_refcell_ref.rs:12:5
|
||||
|
|
||||
LL | / let b = x.borrow_mut();
|
||||
LL | | baz().await
|
||||
LL | | }
|
||||
| |_^
|
||||
|
||||
error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await.
|
||||
--> $DIR/await_holding_refcell_ref.rs:33:9
|
||||
|
|
||||
LL | let b = x.borrow_mut();
|
||||
| ^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/await_holding_refcell_ref.rs:33:5
|
||||
|
|
||||
LL | / let b = x.borrow_mut();
|
||||
LL | |
|
||||
LL | | let second = baz().await;
|
||||
LL | |
|
||||
... |
|
||||
LL | | first + second + third
|
||||
LL | | }
|
||||
| |_^
|
||||
|
||||
error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await.
|
||||
--> $DIR/await_holding_refcell_ref.rs:46:13
|
||||
|
|
||||
LL | let b = x.borrow_mut();
|
||||
| ^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/await_holding_refcell_ref.rs:46:9
|
||||
|
|
||||
LL | / let b = x.borrow_mut();
|
||||
LL | | baz().await
|
||||
LL | | };
|
||||
| |_____^
|
||||
|
||||
error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await.
|
||||
--> $DIR/await_holding_refcell_ref.rs:58:13
|
||||
|
|
||||
LL | let b = x.borrow_mut();
|
||||
| ^
|
||||
|
|
||||
note: these are all the await points this ref is held through
|
||||
--> $DIR/await_holding_refcell_ref.rs:58:9
|
||||
|
|
||||
LL | / let b = x.borrow_mut();
|
||||
LL | | baz().await
|
||||
LL | | }
|
||||
| |_____^
|
||||
|
||||
error: aborting due to 5 previous errors
|
||||
|
Loading…
Reference in New Issue
Block a user