Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2025-02-17 14:38:46 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #12764 - lrh2000:ignore-place, r=blyxyas

Browse source 
`significant_drop_in_scrutinee`: Fix false positives due to false drops of place expressions

Place expressions do not really create temporaries, so they will not create significant drops. For example, the following code snippet is quite good (#8963):
```rust
fn main() {
    let x = std::sync::Mutex::new(vec![1, 2, 3]);
    let x_guard = x.lock().unwrap();
    match x_guard[0] {
        1 => println!("1!"),
        x => println!("{x}"),
    }
    drop(x_guard); // Some "usage"
}
```

Also, the previous logic thinks that references like `&MutexGuard<_>`/`Ref<'_, MutexGuard<'_, _>>` have significant drops, which is simply not true, so it is fixed together in this PR.

Fixes https://github.com/rust-lang/rust-clippy/issues/8963
Fixes https://github.com/rust-lang/rust-clippy/issues/9072

changelog: [`significant_drop_in_scrutinee`]: Fix false positives due to false drops of place expressions.

r? `@blyxyas`
This commit is contained in:
bors 2024-05-13 13:19:48 +00:00
commit d6991abc5a
3 changed files with 128 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

86
clippy_lints/src/matches/significant_drop_in_scrutinee.rs
View file

@ -115,45 +115,60 @@ impl<'a, 'tcx> SigDropChecker<'a, 'tcx> {
}
}
fn get_type(&self, ex: &'tcx Expr<'_>) -> Ty<'tcx> {
self.cx.typeck_results().expr_ty(ex)
fn is_sig_drop_expr(&mut self, ex: &'tcx Expr<'_>) -> bool {
!ex.is_syntactic_place_expr() && self.has_sig_drop_attr(self.cx.typeck_results().expr_ty(ex))
}
fn has_seen_type(&mut self, ty: Ty<'tcx>) -> bool {
!self.seen_types.insert(ty)
fn has_sig_drop_attr(&mut self, ty: Ty<'tcx>) -> bool {
self.seen_types.clear();
self.has_sig_drop_attr_impl(ty)
}
fn has_sig_drop_attr(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
fn has_sig_drop_attr_impl(&mut self, ty: Ty<'tcx>) -> bool {
if let Some(adt) = ty.ty_adt_def() {
if get_attr(cx.sess(), cx.tcx.get_attrs_unchecked(adt.did()), "has_significant_drop").count() > 0 {
if get_attr(
self.cx.sess(),
self.cx.tcx.get_attrs_unchecked(adt.did()),
"has_significant_drop",
)
.count()
> 0
{
return true;
}
}
match ty.kind() {
rustc_middle::ty::Adt(a, b) => {
for f in a.all_fields() {
let ty = f.ty(cx.tcx, b);
if !self.has_seen_type(ty) && self.has_sig_drop_attr(cx, ty) {
return true;
}
}
for generic_arg in *b {
if let GenericArgKind::Type(ty) = generic_arg.unpack() {
if self.has_sig_drop_attr(cx, ty) {
return true;
}
}
}
false
},
rustc_middle::ty::Array(ty, _)
| rustc_middle::ty::RawPtr(ty, _)
| rustc_middle::ty::Ref(_, ty, _)
| rustc_middle::ty::Slice(ty) => self.has_sig_drop_attr(cx, *ty),
_ => false,
if !self.seen_types.insert(ty) {
return false;
}
let result = match ty.kind() {
rustc_middle::ty::Adt(adt, args) => {
// if some field has significant drop,
adt.all_fields()
.map(|field| field.ty(self.cx.tcx, args))
.any(|ty| self.has_sig_drop_attr_impl(ty))
// or if there is no generic lifetime and..
// (to avoid false positive on `Ref<'a, MutexGuard<Foo>>`)
|| (args
.iter()
.all(|arg| !matches!(arg.unpack(), GenericArgKind::Lifetime(_)))
// some generic parameter has significant drop
// (to avoid false negative on `Box<MutexGuard<Foo>>`)
&& args
.iter()
.filter_map(|arg| match arg.unpack() {
GenericArgKind::Type(ty) => Some(ty),
_ => None,
})
.any(|ty| self.has_sig_drop_attr_impl(ty)))
},
rustc_middle::ty::Tuple(tys) => tys.iter().any(|ty| self.has_sig_drop_attr_impl(ty)),
rustc_middle::ty::Array(ty, _) | rustc_middle::ty::Slice(ty) => self.has_sig_drop_attr_impl(*ty),
_ => false,
};
result
}
}
@ -232,7 +247,7 @@ impl<'a, 'tcx> SigDropHelper<'a, 'tcx> {
if self.current_sig_drop.is_some() {
return;
}
let ty = self.sig_drop_checker.get_type(expr);
let ty = self.cx.typeck_results().expr_ty(expr);
if ty.is_ref() {
// We checked that the type was ref, so builtin_deref will return Some TypeAndMut,
// but let's avoid any chance of an ICE
@ -279,11 +294,7 @@ impl<'a, 'tcx> SigDropHelper<'a, 'tcx> {
impl<'a, 'tcx> Visitor<'tcx> for SigDropHelper<'a, 'tcx> {
fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
if !self.is_chain_end
&& self
.sig_drop_checker
.has_sig_drop_attr(self.cx, self.sig_drop_checker.get_type(ex))
{
if !self.is_chain_end && self.sig_drop_checker.is_sig_drop_expr(ex) {
self.has_significant_drop = true;
return;
}
@ -387,10 +398,7 @@ fn has_significant_drop_in_arms<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'
impl<'a, 'tcx> Visitor<'tcx> for ArmSigDropHelper<'a, 'tcx> {
fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
if self
.sig_drop_checker
.has_sig_drop_attr(self.sig_drop_checker.cx, self.sig_drop_checker.get_type(ex))
{
if self.sig_drop_checker.is_sig_drop_expr(ex) {
self.found_sig_drop_spans.insert(ex.span);
return;
}

56
tests/ui/significant_drop_in_scrutinee.rs
View file

@ -675,4 +675,60 @@ fn should_not_trigger_on_significant_iterator_drop() {
}
}
// https://github.com/rust-lang/rust-clippy/issues/9072
fn should_not_trigger_lint_if_place_expr_has_significant_drop() {
let x = Mutex::new(vec![1, 2, 3]);
let x_guard = x.lock().unwrap();
match x_guard[0] {
1 => println!("1!"),
x => println!("{x}"),
}
match x_guard.len() {
1 => println!("1!"),
x => println!("{x}"),
}
}
struct Guard<'a, T>(MutexGuard<'a, T>);
struct Ref<'a, T>(&'a T);
impl<'a, T> Guard<'a, T> {
fn guard(&self) -> &MutexGuard<T> {
&self.0
}
fn guard_ref(&self) -> Ref<MutexGuard<T>> {
Ref(&self.0)
}
fn take(self) -> Box<MutexGuard<'a, T>> {
Box::new(self.0)
}
}
fn should_not_trigger_for_significant_drop_ref() {
let mutex = Mutex::new(vec![1, 2]);
let guard = Guard(mutex.lock().unwrap());
match guard.guard().len() {
0 => println!("empty"),
_ => println!("not empty"),
}
match guard.guard_ref().0.len() {
0 => println!("empty"),
_ => println!("not empty"),
}
match guard.take().len() {
//~^ ERROR: temporary with significant `Drop` in `match` scrutinee will live until the
//~| NOTE: this might lead to deadlocks or other unexpected behavior
0 => println!("empty"),
_ => println!("not empty"),
};
}
fn main() {}

28
tests/ui/significant_drop_in_scrutinee.stderr
View file

@ -28,6 +28,9 @@ LL | match s.lock_m().get_the_value() {
LL | println!("{}", s.lock_m().get_the_value());
| ---------- another value with significant `Drop` created here
...
LL | println!("{}", s.lock_m().get_the_value());
| ---------- another value with significant `Drop` created here
...
LL | }
| - temporary lives until here
|
@ -47,6 +50,9 @@ LL | match s.lock_m_m().get_the_value() {
LL | println!("{}", s.lock_m().get_the_value());
| ---------- another value with significant `Drop` created here
...
LL | println!("{}", s.lock_m().get_the_value());
| ---------- another value with significant `Drop` created here
...
LL | }
| - temporary lives until here
|
@ -360,7 +366,7 @@ LL | match s.lock().deref().deref() {
| ^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | _ => println!("Value is {}", s.lock().deref()),
| ---------------- another value with significant `Drop` created here
| -------- another value with significant `Drop` created here
LL | };
| - temporary lives until here
|
@ -378,7 +384,7 @@ LL | match s.lock().deref().deref() {
| ^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | matcher => println!("Value is {}", s.lock().deref()),
| ---------------- another value with significant `Drop` created here
| -------- another value with significant `Drop` created here
LL | _ => println!("Value was not a match"),
LL | };
| - temporary lives until here
@ -499,5 +505,21 @@ LL ~ let value = mutex.lock().unwrap().foo();
LL ~ match value {
|
error: aborting due to 26 previous errors
error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression
--> tests/ui/significant_drop_in_scrutinee.rs:726:11
|
LL | match guard.take().len() {
| ^^^^^^^^^^^^^^^^^^
...
LL | };
| - temporary lives until here
|
= note: this might lead to deadlocks or other unexpected behavior
help: try moving the temporary above the match
|
LL ~ let value = guard.take().len();
LL ~ match value {
|
error: aborting due to 27 previous errors