From 231f93524d19cdcd241f314f812479c429a5c5ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 27 Oct 2023 17:19:32 +0000 Subject: [PATCH 1/2] Detect misparsed binop caused by missing semi When encountering ```rust foo() *bar = baz; ``` We currently emit potentially two errors, one for the return type of `foo` not being multiplyiable by the type of `bar`, and another for `foo() * bar` not being assignable. We now check for this case and suggest adding a semicolon in the right place. Fix #80446. --- compiler/rustc_hir_typeck/src/expr.rs | 21 +++++++++++++ compiler/rustc_hir_typeck/src/op.rs | 16 ++++++++++ .../false-binop-caused-by-missing-semi.rs | 9 ++++++ .../false-binop-caused-by-missing-semi.stderr | 31 +++++++++++++++++++ 4 files changed, 77 insertions(+) create mode 100644 tests/ui/binop/false-binop-caused-by-missing-semi.rs create mode 100644 tests/ui/binop/false-binop-caused-by-missing-semi.stderr diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index be225ceb84321..9a2f242ec4e15 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -953,12 +953,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Applicability::MachineApplicable, ); }); + self.check_for_missing_semi(lhs, &mut err); adjust_err(&mut err); err.emit(); } + /// Check if the expression that could not be assigned to was a typoed expression that + fn check_for_missing_semi(&self, expr: &'tcx hir::Expr<'tcx>, err: &mut Diagnostic) { + if let hir::ExprKind::Binary(binop, lhs, rhs) = expr.kind + && let hir::BinOpKind::Mul = binop.node + && self.tcx.sess.source_map().is_multiline(lhs.span.between(rhs.span)) + && rhs.is_syntactic_place_expr() + { + // v missing semicolon here + // foo() + // *bar = baz; + // (#80446). + err.span_suggestion_verbose( + lhs.span.shrink_to_hi(), + "you might have meant to write a semicolon here", + ";".to_string(), + Applicability::MachineApplicable, + ); + } + } + // Check if an expression `original_expr_id` comes from the condition of a while loop, /// as opposed from the body of a while loop, which we can naively check by iterating /// parents until we find a loop... diff --git a/compiler/rustc_hir_typeck/src/op.rs b/compiler/rustc_hir_typeck/src/op.rs index d0d3b0e5b73cf..92075c7832a97 100644 --- a/compiler/rustc_hir_typeck/src/op.rs +++ b/compiler/rustc_hir_typeck/src/op.rs @@ -379,6 +379,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { (err, output_def_id) } }; + if self.tcx.sess.source_map().is_multiline(lhs_expr.span.between(rhs_expr.span)) + && let IsAssign::No = is_assign + && let hir::BinOpKind::Mul = op.node + && rhs_expr.is_syntactic_place_expr() + { + // v missing semicolon here + // foo() + // *bar = baz; + // (#80446). + err.span_suggestion_verbose( + lhs_expr.span.shrink_to_hi(), + "you might have meant to write a semicolon here", + ";".to_string(), + Applicability::MaybeIncorrect, + ); + } let suggest_deref_binop = |err: &mut DiagnosticBuilder<'_, _>, lhs_deref_ty: Ty<'tcx>| { diff --git a/tests/ui/binop/false-binop-caused-by-missing-semi.rs b/tests/ui/binop/false-binop-caused-by-missing-semi.rs new file mode 100644 index 0000000000000..9cd32e1db500c --- /dev/null +++ b/tests/ui/binop/false-binop-caused-by-missing-semi.rs @@ -0,0 +1,9 @@ +fn foo() {} +fn main() { + let mut y = 42; + let x = &mut y; + foo() + *x = 0; //~ ERROR invalid left-hand side of assignment + //~^ ERROR cannot multiply `()` by `&mut {integer}` + println!("{y}"); +} diff --git a/tests/ui/binop/false-binop-caused-by-missing-semi.stderr b/tests/ui/binop/false-binop-caused-by-missing-semi.stderr new file mode 100644 index 0000000000000..8d0f15fe2ea16 --- /dev/null +++ b/tests/ui/binop/false-binop-caused-by-missing-semi.stderr @@ -0,0 +1,31 @@ +error[E0369]: cannot multiply `()` by `&mut {integer}` + --> $DIR/false-binop-caused-by-missing-semi.rs:6:5 + | +LL | foo() + | ----- () +LL | *x = 0; + | ^- &mut {integer} + | +help: you might have meant to write a semicolon here + | +LL | foo(); + | + + +error[E0070]: invalid left-hand side of assignment + --> $DIR/false-binop-caused-by-missing-semi.rs:6:8 + | +LL | / foo() +LL | | *x = 0; + | | - ^ + | |______| + | cannot assign to this expression + | +help: you might have meant to write a semicolon here + | +LL | foo(); + | + + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0070, E0369. +For more information about an error, try `rustc --explain E0070`. From 4b7aacaa4f5b2a5ceb05584f18b1a20f2572efa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 27 Oct 2023 17:27:21 +0000 Subject: [PATCH 2/2] Silence redundant error on typo resulting on binop --- compiler/rustc_hir_typeck/src/expr.rs | 8 +++++++- compiler/rustc_hir_typeck/src/op.rs | 19 +++++------------- .../false-binop-caused-by-missing-semi.fixed | 10 ++++++++++ .../false-binop-caused-by-missing-semi.rs | 3 ++- .../false-binop-caused-by-missing-semi.stderr | 20 +++---------------- 5 files changed, 27 insertions(+), 33 deletions(-) create mode 100644 tests/ui/binop/false-binop-caused-by-missing-semi.fixed diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 9a2f242ec4e15..76d1b27e7e39b 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -961,7 +961,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } /// Check if the expression that could not be assigned to was a typoed expression that - fn check_for_missing_semi(&self, expr: &'tcx hir::Expr<'tcx>, err: &mut Diagnostic) { + pub fn check_for_missing_semi( + &self, + expr: &'tcx hir::Expr<'tcx>, + err: &mut DiagnosticBuilder<'_, ErrorGuaranteed>, + ) -> bool { if let hir::ExprKind::Binary(binop, lhs, rhs) = expr.kind && let hir::BinOpKind::Mul = binop.node && self.tcx.sess.source_map().is_multiline(lhs.span.between(rhs.span)) @@ -977,7 +981,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ";".to_string(), Applicability::MachineApplicable, ); + return true; } + false } // Check if an expression `original_expr_id` comes from the condition of a while loop, diff --git a/compiler/rustc_hir_typeck/src/op.rs b/compiler/rustc_hir_typeck/src/op.rs index 92075c7832a97..f40406c67265a 100644 --- a/compiler/rustc_hir_typeck/src/op.rs +++ b/compiler/rustc_hir_typeck/src/op.rs @@ -379,21 +379,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { (err, output_def_id) } }; - if self.tcx.sess.source_map().is_multiline(lhs_expr.span.between(rhs_expr.span)) - && let IsAssign::No = is_assign - && let hir::BinOpKind::Mul = op.node - && rhs_expr.is_syntactic_place_expr() + if self.check_for_missing_semi(expr, &mut err) + && let hir::Node::Expr(expr) = self.tcx.hir().get_parent(expr.hir_id) + && let hir::ExprKind::Assign(..) = expr.kind { - // v missing semicolon here - // foo() - // *bar = baz; - // (#80446). - err.span_suggestion_verbose( - lhs_expr.span.shrink_to_hi(), - "you might have meant to write a semicolon here", - ";".to_string(), - Applicability::MaybeIncorrect, - ); + // We defer to the later error produced by `check_lhs_assignable`. + err.delay_as_bug(); } let suggest_deref_binop = diff --git a/tests/ui/binop/false-binop-caused-by-missing-semi.fixed b/tests/ui/binop/false-binop-caused-by-missing-semi.fixed new file mode 100644 index 0000000000000..b47372c906486 --- /dev/null +++ b/tests/ui/binop/false-binop-caused-by-missing-semi.fixed @@ -0,0 +1,10 @@ +// run-rustfix +fn foo() {} +fn main() { + let mut y = 42; + let x = &mut y; + foo(); + *x = 0; //~ ERROR invalid left-hand side of assignment + let _ = x; + println!("{y}"); +} diff --git a/tests/ui/binop/false-binop-caused-by-missing-semi.rs b/tests/ui/binop/false-binop-caused-by-missing-semi.rs index 9cd32e1db500c..14671de7e5111 100644 --- a/tests/ui/binop/false-binop-caused-by-missing-semi.rs +++ b/tests/ui/binop/false-binop-caused-by-missing-semi.rs @@ -1,9 +1,10 @@ +// run-rustfix fn foo() {} fn main() { let mut y = 42; let x = &mut y; foo() *x = 0; //~ ERROR invalid left-hand side of assignment - //~^ ERROR cannot multiply `()` by `&mut {integer}` + let _ = x; println!("{y}"); } diff --git a/tests/ui/binop/false-binop-caused-by-missing-semi.stderr b/tests/ui/binop/false-binop-caused-by-missing-semi.stderr index 8d0f15fe2ea16..fca042b1c57d2 100644 --- a/tests/ui/binop/false-binop-caused-by-missing-semi.stderr +++ b/tests/ui/binop/false-binop-caused-by-missing-semi.stderr @@ -1,18 +1,5 @@ -error[E0369]: cannot multiply `()` by `&mut {integer}` - --> $DIR/false-binop-caused-by-missing-semi.rs:6:5 - | -LL | foo() - | ----- () -LL | *x = 0; - | ^- &mut {integer} - | -help: you might have meant to write a semicolon here - | -LL | foo(); - | + - error[E0070]: invalid left-hand side of assignment - --> $DIR/false-binop-caused-by-missing-semi.rs:6:8 + --> $DIR/false-binop-caused-by-missing-semi.rs:7:8 | LL | / foo() LL | | *x = 0; @@ -25,7 +12,6 @@ help: you might have meant to write a semicolon here LL | foo(); | + -error: aborting due to 2 previous errors +error: aborting due to previous error -Some errors have detailed explanations: E0070, E0369. -For more information about an error, try `rustc --explain E0070`. +For more information about this error, try `rustc --explain E0070`.