Skip to content

Fix diverging_sub_expression not checking body of block #10785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion clippy_lints/src/mixed_read_write_in_expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ struct DivergenceVisitor<'a, 'tcx> {
impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
fn maybe_walk_expr(&mut self, e: &'tcx Expr<'_>) {
match e.kind {
ExprKind::Closure { .. } => {},
ExprKind::Closure(..) | ExprKind::If(..) | ExprKind::Loop(..) => {},
ExprKind::Match(e, arms, _) => {
self.visit_expr(e);
for arm in arms {
Expand All @@ -128,6 +128,7 @@ impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
_ => walk_expr(self, e),
}
}

fn report_diverging_sub_expr(&mut self, e: &Expr<'_>) {
span_lint(self.cx, DIVERGING_SUB_EXPRESSION, e.span, "sub-expression diverges");
}
Expand All @@ -136,6 +137,15 @@ impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
impl<'a, 'tcx> Visitor<'tcx> for DivergenceVisitor<'a, 'tcx> {
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
match e.kind {
// fix #10776
ExprKind::Block(block, ..) => match (block.stmts, block.expr) {
([], Some(e)) => self.visit_expr(e),
([stmt], None) => match stmt.kind {
StmtKind::Expr(e) | StmtKind::Semi(e) => self.visit_expr(e),
_ => {},
},
_ => {},
},
ExprKind::Continue(_) | ExprKind::Break(_, _) | ExprKind::Ret(_) => self.report_diverging_sub_expr(e),
ExprKind::Call(func, _) => {
let typ = self.cx.typeck_results().expr_ty(func);
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/diverging_sub_expression.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![warn(clippy::diverging_sub_expression)]
#![allow(clippy::match_same_arms, clippy::overly_complex_bool_expr)]
#![allow(clippy::nonminimal_bool)]
#[allow(clippy::empty_loop)]
fn diverge() -> ! {
loop {}
Expand All @@ -21,6 +22,7 @@ fn main() {
}

#[allow(dead_code, unused_variables)]
#[rustfmt::skip]
fn foobar() {
loop {
let x = match 5 {
Expand All @@ -35,6 +37,20 @@ fn foobar() {
99 => return,
_ => true || panic!("boo"),
},
// lint blocks as well
15 => true || { return; },
16 => false || { return; },
// ... and when it's a single expression
17 => true || { return },
18 => false || { return },
// ... but not when there's both an expression and a statement
19 => true || { _ = 1; return },
20 => false || { _ = 1; return },
// ... or multiple statements
21 => true || { _ = 1; return; },
22 => false || { _ = 1; return; },
23 => true || { return; true },
24 => true || { return; true },
_ => true || break,
};
}
Expand Down
46 changes: 39 additions & 7 deletions tests/ui/diverging_sub_expression.stderr
Original file line number Diff line number Diff line change
@@ -1,40 +1,72 @@
error: sub-expression diverges
--> $DIR/diverging_sub_expression.rs:19:10
--> $DIR/diverging_sub_expression.rs:20:10
|
LL | b || diverge();
| ^^^^^^^^^
|
= note: `-D clippy::diverging-sub-expression` implied by `-D warnings`

error: sub-expression diverges
--> $DIR/diverging_sub_expression.rs:20:10
--> $DIR/diverging_sub_expression.rs:21:10
|
LL | b || A.foo();
| ^^^^^^^

error: sub-expression diverges
--> $DIR/diverging_sub_expression.rs:29:26
--> $DIR/diverging_sub_expression.rs:31:26
|
LL | 6 => true || return,
| ^^^^^^

error: sub-expression diverges
--> $DIR/diverging_sub_expression.rs:30:26
--> $DIR/diverging_sub_expression.rs:32:26
|
LL | 7 => true || continue,
| ^^^^^^^^

error: sub-expression diverges
--> $DIR/diverging_sub_expression.rs:33:26
--> $DIR/diverging_sub_expression.rs:35:26
|
LL | 3 => true || diverge(),
| ^^^^^^^^^

error: sub-expression diverges
--> $DIR/diverging_sub_expression.rs:38:26
--> $DIR/diverging_sub_expression.rs:38:30
|
LL | _ => true || panic!("boo"),
| ^^^^^^^^^^^^^
|
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)

error: sub-expression diverges
--> $DIR/diverging_sub_expression.rs:41:29
|
LL | 15 => true || { return; },
| ^^^^^^

error: sub-expression diverges
--> $DIR/diverging_sub_expression.rs:42:30
|
LL | 16 => false || { return; },
| ^^^^^^

error: sub-expression diverges
--> $DIR/diverging_sub_expression.rs:44:29
|
LL | 17 => true || { return },
| ^^^^^^

error: sub-expression diverges
--> $DIR/diverging_sub_expression.rs:45:30
|
LL | 18 => false || { return },
| ^^^^^^

error: sub-expression diverges
--> $DIR/diverging_sub_expression.rs:54:26
|
LL | _ => true || break,
| ^^^^^

error: aborting due to 6 previous errors
error: aborting due to 11 previous errors

10 changes: 9 additions & 1 deletion tests/ui/never_loop.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@ LL | | }
LL | | }
| |_____^

error: sub-expression diverges
--> $DIR/never_loop.rs:247:17
|
LL | break 'a;
| ^^^^^^^^
|
= note: `-D clippy::diverging-sub-expression` implied by `-D warnings`

error: this loop never actually loops
--> $DIR/never_loop.rs:278:13
|
Expand All @@ -139,5 +147,5 @@ help: if you need the first element of the iterator, try writing
LL | if let Some(_) = (0..20).next() {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 12 previous errors
error: aborting due to 13 previous errors