diff --git a/clippy_lints/src/mixed_read_write_in_expression.rs b/clippy_lints/src/mixed_read_write_in_expression.rs index f0be7771bb1a..57ec3a1f1e63 100644 --- a/clippy_lints/src/mixed_read_write_in_expression.rs +++ b/clippy_lints/src/mixed_read_write_in_expression.rs @@ -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 { @@ -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"); } @@ -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); diff --git a/tests/ui/diverging_sub_expression.rs b/tests/ui/diverging_sub_expression.rs index e8f992e6dded..9b1619baf0e6 100644 --- a/tests/ui/diverging_sub_expression.rs +++ b/tests/ui/diverging_sub_expression.rs @@ -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 {} @@ -21,6 +22,7 @@ fn main() { } #[allow(dead_code, unused_variables)] +#[rustfmt::skip] fn foobar() { loop { let x = match 5 { @@ -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, }; } diff --git a/tests/ui/diverging_sub_expression.stderr b/tests/ui/diverging_sub_expression.stderr index 51a3b0d972e6..243a5cf5369a 100644 --- a/tests/ui/diverging_sub_expression.stderr +++ b/tests/ui/diverging_sub_expression.stderr @@ -1,5 +1,5 @@ error: sub-expression diverges - --> $DIR/diverging_sub_expression.rs:19:10 + --> $DIR/diverging_sub_expression.rs:20:10 | LL | b || diverge(); | ^^^^^^^^^ @@ -7,34 +7,66 @@ 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 diff --git a/tests/ui/never_loop.stderr b/tests/ui/never_loop.stderr index 704d448644e2..b2eafb345e33 100644 --- a/tests/ui/never_loop.stderr +++ b/tests/ui/never_loop.stderr @@ -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 | @@ -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