Skip to content

Commit cbd7919

Browse files
authored
fix(blocks_in_conditions): Don't lint if the block creates temporarie… (#17420)
fixes #15112 changelog: [`blocks_in_conditions`]: Don't lint when removing braces would extend the lifetime of temporaries with significant drops (e.g., `MutexGuard`)
2 parents bd9e81d + aaf7ce7 commit cbd7919

5 files changed

Lines changed: 153 additions & 2 deletions

File tree

clippy_lints/src/blocks_in_conditions.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet_block_with_applicability;
3-
use clippy_utils::{contains_return, higher, is_from_proc_macro};
3+
use clippy_utils::{contains_return, higher, is_from_proc_macro, leaks_droppable_temporary};
44
use rustc_errors::Applicability;
55
use rustc_hir::{BlockCheckMode, Expr, ExprKind, MatchSource};
66
use rustc_lint::{LateContext, LateLintPass, LintContext as _};
@@ -93,6 +93,14 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInConditions {
9393
return;
9494
}
9595

96+
// Don't lint if the block creates temporaries with significant drops that need
97+
// to be dropped at the end of the block (e.g., MutexGuard).
98+
// Removing the braces would extend the lifetime of these temporaries.
99+
// #15112
100+
if leaks_droppable_temporary(cx, ex) {
101+
return;
102+
}
103+
96104
let mut applicability = Applicability::MachineApplicable;
97105
span_lint_and_sugg(
98106
cx,

clippy_utils/src/lib.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3416,6 +3416,19 @@ pub fn leaks_droppable_temporary_with_limited_lifetime<'tcx>(cx: &LateContext<'t
34163416
.is_break()
34173417
}
34183418

3419+
/// Returns true if `expr` creates any temporary that has a significant drop and does not consume
3420+
/// it.
3421+
pub fn leaks_droppable_temporary<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
3422+
for_each_unconsumed_temporary(cx, expr, |temporary_ty| {
3423+
if temporary_ty.has_significant_drop(cx.tcx, cx.typing_env()) {
3424+
ControlFlow::Break(())
3425+
} else {
3426+
ControlFlow::Continue(())
3427+
}
3428+
})
3429+
.is_break()
3430+
}
3431+
34193432
/// Returns true if the specified `expr` requires coercion,
34203433
/// meaning that it either has a coercion or propagates a coercion from one of its sub expressions.
34213434
///

tests/ui/blocks_in_conditions.fixed

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,66 @@ fn in_closure() {
109109
}
110110
}
111111

112+
fn issue_15112() {
113+
{
114+
let v = std::sync::Mutex::new(0);
115+
match { *v.lock().unwrap() } {
116+
1 => {
117+
todo!()
118+
},
119+
2 => {
120+
todo!()
121+
},
122+
_ => {},
123+
}
124+
}
125+
126+
{
127+
// No Drop impl, so braces should be removed
128+
struct Foo;
129+
130+
impl Foo {
131+
fn foo(&self) -> i32 {
132+
0
133+
}
134+
}
135+
136+
match Foo.foo() {
137+
//~^ ERROR: omit braces around single expression condition
138+
1 => {
139+
todo!()
140+
},
141+
2 => {
142+
todo!()
143+
},
144+
_ => {},
145+
}
146+
}
147+
148+
{
149+
// Drop impl, so braces should be kept
150+
struct Bar;
151+
152+
impl Drop for Bar {
153+
fn drop(&mut self) {}
154+
}
155+
156+
impl Bar {
157+
fn bar(&self) -> i32 {
158+
0
159+
}
160+
}
161+
162+
match { Bar.bar() } {
163+
1 => {
164+
todo!()
165+
},
166+
2 => {
167+
todo!()
168+
},
169+
_ => {},
170+
}
171+
}
172+
}
173+
112174
fn main() {}

tests/ui/blocks_in_conditions.rs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,66 @@ fn in_closure() {
109109
}
110110
}
111111

112+
fn issue_15112() {
113+
{
114+
let v = std::sync::Mutex::new(0);
115+
match { *v.lock().unwrap() } {
116+
1 => {
117+
todo!()
118+
},
119+
2 => {
120+
todo!()
121+
},
122+
_ => {},
123+
}
124+
}
125+
126+
{
127+
// No Drop impl, so braces should be removed
128+
struct Foo;
129+
130+
impl Foo {
131+
fn foo(&self) -> i32 {
132+
0
133+
}
134+
}
135+
136+
match { Foo.foo() } {
137+
//~^ ERROR: omit braces around single expression condition
138+
1 => {
139+
todo!()
140+
},
141+
2 => {
142+
todo!()
143+
},
144+
_ => {},
145+
}
146+
}
147+
148+
{
149+
// Drop impl, so braces should be kept
150+
struct Bar;
151+
152+
impl Drop for Bar {
153+
fn drop(&mut self) {}
154+
}
155+
156+
impl Bar {
157+
fn bar(&self) -> i32 {
158+
0
159+
}
160+
}
161+
162+
match { Bar.bar() } {
163+
1 => {
164+
todo!()
165+
},
166+
2 => {
167+
todo!()
168+
},
169+
_ => {},
170+
}
171+
}
172+
}
173+
112174
fn main() {}

tests/ui/blocks_in_conditions.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,11 @@ error: omit braces around single expression condition
2525
LL | if { true } { 6 } else { 10 }
2626
| ^^^^^^^^ help: try: `true`
2727

28-
error: aborting due to 2 previous errors
28+
error: omit braces around single expression condition
29+
--> tests/ui/blocks_in_conditions.rs:136:15
30+
|
31+
LL | match { Foo.foo() } {
32+
| ^^^^^^^^^^^^^ help: try: `Foo.foo()`
33+
34+
error: aborting due to 3 previous errors
2935

0 commit comments

Comments
 (0)