From bf0193d5801c7155f4fb6d3bd546a2cdd762cc3c Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Tue, 24 May 2022 21:57:51 +0000 Subject: [PATCH 1/2] Suggest adding a semicolon to a closure without block This transforms `|| expr` into `|| { expr; }`. --- compiler/rustc_typeck/src/check/coercion.rs | 27 +++++++++++----- .../src/check/fn_ctxt/suggestions.rs | 31 ++++++++++++++----- .../add_semicolon_non_block_closure.rs | 11 +++++++ .../add_semicolon_non_block_closure.stderr | 16 ++++++++++ 4 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 src/test/ui/closures/add_semicolon_non_block_closure.rs create mode 100644 src/test/ui/closures/add_semicolon_non_block_closure.stderr diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 1af50191cb006..407a0267f1358 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -1500,7 +1500,8 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { coercion_error.clone(), fcx, parent_id, - expression.map(|expr| (expr, blk_id)), + expression, + Some(blk_id), ); if !fcx.tcx.features().unsized_locals { unsized_return = self.is_return_ty_unsized(fcx, blk_id); @@ -1514,6 +1515,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { coercion_error.clone(), fcx, id, + expression, None, ); if !fcx.tcx.features().unsized_locals { @@ -1564,21 +1566,30 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { ty_err: TypeError<'tcx>, fcx: &FnCtxt<'a, 'tcx>, id: hir::HirId, - expression: Option<(&'tcx hir::Expr<'tcx>, hir::HirId)>, + expression: Option<&'tcx hir::Expr<'tcx>>, + blk_id: Option, ) -> DiagnosticBuilder<'a, ErrorGuaranteed> { let mut err = fcx.report_mismatched_types(cause, expected, found, ty_err); let mut pointing_at_return_type = false; let mut fn_output = None; + let parent_id = fcx.tcx.hir().get_parent_node(id); + let parent = fcx.tcx.hir().get(parent_id); + if let Some(expr) = expression + && let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(_, _, body_id, ..), .. }) = parent + && !matches!(fcx.tcx.hir().get(body_id.hir_id), hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Block(..), .. })) + && expr.can_have_side_effects() + && !in_external_macro(fcx.tcx.sess, expr.span) + { + fcx.suggest_missing_semicolon(&mut err, expr, expected, true); + } // Verify that this is a tail expression of a function, otherwise the // label pointing out the cause for the type coercion will be wrong // as prior return coercions would not be relevant (#57664). - let parent_id = fcx.tcx.hir().get_parent_node(id); - let fn_decl = if let Some((expr, blk_id)) = expression { + let fn_decl = if let (Some(expr), Some(blk_id)) = (expression, blk_id) { pointing_at_return_type = fcx.suggest_mismatched_types_on_tail(&mut err, expr, expected, found, blk_id); - let parent = fcx.tcx.hir().get(parent_id); if let (Some(cond_expr), true, false) = ( fcx.tcx.hir().get_if_cause(expr.hir_id), expected.is_unit(), @@ -1607,7 +1618,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { }; if let Some((fn_decl, can_suggest)) = fn_decl { - if expression.is_none() { + if blk_id.is_none() { pointing_at_return_type |= fcx.suggest_missing_return_type( &mut err, &fn_decl, @@ -1625,8 +1636,8 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { let parent_id = fcx.tcx.hir().get_parent_item(id); let parent_item = fcx.tcx.hir().get_by_def_id(parent_id); - if let (Some((expr, _)), Some((fn_decl, _, _))) = - (expression, fcx.get_node_fn_decl(parent_item)) + if let (Some(expr), Some(_), Some((fn_decl, _, _))) = + (expression, blk_id, fcx.get_node_fn_decl(parent_item)) { fcx.suggest_missing_break_or_return_expr( &mut err, diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index bd58a6754481a..40b667beaac0a 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -50,7 +50,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // adding a semicolon, because there's nowhere to put it. // See issue #81943. if expr.can_have_side_effects() && !in_external_macro(self.tcx.sess, expr.span) { - self.suggest_missing_semicolon(err, expr, expected); + self.suggest_missing_semicolon(err, expr, expected, false); } let mut pointing_at_return_type = false; if let Some((fn_decl, can_suggest)) = self.get_fn_decl(blk_id) { @@ -473,11 +473,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// This routine checks if the return expression in a block would make sense on its own as a /// statement and the return type has been left as default or has been specified as `()`. If so, /// it suggests adding a semicolon. - fn suggest_missing_semicolon( + /// + /// If the expression is the expression of a closure without block (`|| expr`), a + /// block is needed to be added too (`|| { expr; }`). This is denoted by `needs_block`. + pub fn suggest_missing_semicolon( &self, err: &mut Diagnostic, expression: &'tcx hir::Expr<'tcx>, expected: Ty<'tcx>, + needs_block: bool, ) { if expected.is_unit() { // `BlockTailExpression` only relevant if the tail expr would be @@ -491,12 +495,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | ExprKind::Block(..) if expression.can_have_side_effects() => { - err.span_suggestion( - expression.span.shrink_to_hi(), - "consider using a semicolon here", - ";".to_string(), - Applicability::MachineApplicable, - ); + if needs_block { + err.multipart_suggestion( + "consider using a semicolon here", + vec![ + (expression.span.shrink_to_lo(), "{ ".to_owned()), + (expression.span.shrink_to_hi(), "; }".to_owned()), + ], + Applicability::MachineApplicable, + ); + } else { + err.span_suggestion( + expression.span.shrink_to_hi(), + "consider using a semicolon here", + ";".to_string(), + Applicability::MachineApplicable, + ); + } } _ => (), } diff --git a/src/test/ui/closures/add_semicolon_non_block_closure.rs b/src/test/ui/closures/add_semicolon_non_block_closure.rs new file mode 100644 index 0000000000000..3ae91be60c5a0 --- /dev/null +++ b/src/test/ui/closures/add_semicolon_non_block_closure.rs @@ -0,0 +1,11 @@ +fn foo(_f: impl Fn()) {} + +fn bar() -> i32 { + 1 +} + +fn main() { + foo(|| bar()) + //~^ ERROR mismatched types [E0308] + //~| HELP consider using a semicolon here +} diff --git a/src/test/ui/closures/add_semicolon_non_block_closure.stderr b/src/test/ui/closures/add_semicolon_non_block_closure.stderr new file mode 100644 index 0000000000000..ed829fc98f86f --- /dev/null +++ b/src/test/ui/closures/add_semicolon_non_block_closure.stderr @@ -0,0 +1,16 @@ +error[E0308]: mismatched types + --> $DIR/add_semicolon_non_block_closure.rs:8:12 + | +LL | fn main() { + | - expected `()` because of default return type +LL | foo(|| bar()) + | ^^^^^ expected `()`, found `i32` + | +help: consider using a semicolon here + | +LL | foo(|| { bar(); }) + | + +++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 6afaffb9c2a9e84858e552286858584bd328413e Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Mon, 30 May 2022 20:35:51 +0000 Subject: [PATCH 2/2] Check for `can_have_side_effects()` and `in_external_macro()` inside `suggest_missing_semicolon()` --- compiler/rustc_typeck/src/check/coercion.rs | 2 -- .../rustc_typeck/src/check/fn_ctxt/suggestions.rs | 13 ++++++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 407a0267f1358..d6a8659d54b58 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -1579,8 +1579,6 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { if let Some(expr) = expression && let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(_, _, body_id, ..), .. }) = parent && !matches!(fcx.tcx.hir().get(body_id.hir_id), hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Block(..), .. })) - && expr.can_have_side_effects() - && !in_external_macro(fcx.tcx.sess, expr.span) { fcx.suggest_missing_semicolon(&mut err, expr, expected, true); } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index 40b667beaac0a..76add2fb9c285 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -46,12 +46,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { blk_id: hir::HirId, ) -> bool { let expr = expr.peel_drop_temps(); - // If the expression is from an external macro, then do not suggest - // adding a semicolon, because there's nowhere to put it. - // See issue #81943. - if expr.can_have_side_effects() && !in_external_macro(self.tcx.sess, expr.span) { - self.suggest_missing_semicolon(err, expr, expected, false); - } + self.suggest_missing_semicolon(err, expr, expected, false); let mut pointing_at_return_type = false; if let Some((fn_decl, can_suggest)) = self.get_fn_decl(blk_id) { let fn_id = self.tcx.hir().get_return_block(blk_id).unwrap(); @@ -493,7 +488,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | ExprKind::If(..) | ExprKind::Match(..) | ExprKind::Block(..) - if expression.can_have_side_effects() => + if expression.can_have_side_effects() + // If the expression is from an external macro, then do not suggest + // adding a semicolon, because there's nowhere to put it. + // See issue #81943. + && !in_external_macro(self.tcx.sess, expression.span) => { if needs_block { err.multipart_suggestion(