Skip to content

Commit a032189

Browse files
committed
Fix significant drop detection
1 parent 20b085d commit a032189

File tree

2 files changed

+40
-26
lines changed

2 files changed

+40
-26
lines changed

clippy_lints/src/matches/significant_drop_in_scrutinee.rs

+32-24
Original file line numberDiff line numberDiff line change
@@ -119,41 +119,49 @@ impl<'a, 'tcx> SigDropChecker<'a, 'tcx> {
119119
self.cx.typeck_results().expr_ty(ex)
120120
}
121121

122-
fn has_seen_type(&mut self, ty: Ty<'tcx>) -> bool {
123-
!self.seen_types.insert(ty)
122+
fn has_sig_drop_attr(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
123+
self.seen_types.clear();
124+
self.has_sig_drop_attr_impl(cx, ty)
124125
}
125126

126-
fn has_sig_drop_attr(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
127+
fn has_sig_drop_attr_impl(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
127128
if let Some(adt) = ty.ty_adt_def() {
128129
if get_attr(cx.sess(), cx.tcx.get_attrs_unchecked(adt.did()), "has_significant_drop").count() > 0 {
129130
return true;
130131
}
131132
}
132133

133-
match ty.kind() {
134-
rustc_middle::ty::Adt(a, b) => {
135-
for f in a.all_fields() {
136-
let ty = f.ty(cx.tcx, b);
137-
if !self.has_seen_type(ty) && self.has_sig_drop_attr(cx, ty) {
138-
return true;
139-
}
140-
}
134+
if !self.seen_types.insert(ty) {
135+
return false;
136+
}
141137

142-
for generic_arg in *b {
143-
if let GenericArgKind::Type(ty) = generic_arg.unpack() {
144-
if self.has_sig_drop_attr(cx, ty) {
145-
return true;
146-
}
147-
}
148-
}
149-
false
138+
let result = match ty.kind() {
139+
rustc_middle::ty::Adt(adt, args) => {
140+
// if some field has significant drop,
141+
adt.all_fields()
142+
.map(|field| field.ty(cx.tcx, args))
143+
.any(|ty| self.has_sig_drop_attr(cx, ty))
144+
// or if there is no generic lifetime and..
145+
// (to avoid false positive on `Ref<'a, MutexGuard<Foo>>`)
146+
|| (args
147+
.iter()
148+
.all(|arg| !matches!(arg.unpack(), GenericArgKind::Lifetime(_)))
149+
// some generic parameter has significant drop
150+
// (to avoid false negative on `Box<MutexGuard<Foo>>`)
151+
&& args
152+
.iter()
153+
.filter_map(|arg| match arg.unpack() {
154+
GenericArgKind::Type(ty) => Some(ty),
155+
_ => None,
156+
})
157+
.any(|ty| self.has_sig_drop_attr(cx, ty)))
150158
},
151-
rustc_middle::ty::Array(ty, _)
152-
| rustc_middle::ty::RawPtr(ty, _)
153-
| rustc_middle::ty::Ref(_, ty, _)
154-
| rustc_middle::ty::Slice(ty) => self.has_sig_drop_attr(cx, *ty),
159+
rustc_middle::ty::Tuple(tys) => tys.iter().any(|ty| self.has_sig_drop_attr(cx, ty)),
160+
rustc_middle::ty::Array(ty, _) | rustc_middle::ty::Slice(ty) => self.has_sig_drop_attr(cx, *ty),
155161
_ => false,
156-
}
162+
};
163+
164+
result
157165
}
158166
}
159167

tests/ui/significant_drop_in_scrutinee.stderr

+8-2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ LL | match s.lock_m().get_the_value() {
2828
LL | println!("{}", s.lock_m().get_the_value());
2929
| ---------- another value with significant `Drop` created here
3030
...
31+
LL | println!("{}", s.lock_m().get_the_value());
32+
| ---------- another value with significant `Drop` created here
33+
...
3134
LL | }
3235
| - temporary lives until here
3336
|
@@ -47,6 +50,9 @@ LL | match s.lock_m_m().get_the_value() {
4750
LL | println!("{}", s.lock_m().get_the_value());
4851
| ---------- another value with significant `Drop` created here
4952
...
53+
LL | println!("{}", s.lock_m().get_the_value());
54+
| ---------- another value with significant `Drop` created here
55+
...
5056
LL | }
5157
| - temporary lives until here
5258
|
@@ -360,7 +366,7 @@ LL | match s.lock().deref().deref() {
360366
| ^^^^^^^^^^^^^^^^^^^^^^^^
361367
...
362368
LL | _ => println!("Value is {}", s.lock().deref()),
363-
| ---------------- another value with significant `Drop` created here
369+
| -------- another value with significant `Drop` created here
364370
LL | };
365371
| - temporary lives until here
366372
|
@@ -378,7 +384,7 @@ LL | match s.lock().deref().deref() {
378384
| ^^^^^^^^^^^^^^^^^^^^^^^^
379385
...
380386
LL | matcher => println!("Value is {}", s.lock().deref()),
381-
| ---------------- another value with significant `Drop` created here
387+
| -------- another value with significant `Drop` created here
382388
LL | _ => println!("Value was not a match"),
383389
LL | };
384390
| - temporary lives until here

0 commit comments

Comments
 (0)