diff --git a/src/librustc/hir/lowering/expr.rs b/src/librustc/hir/lowering/expr.rs index 50fceacaa4486..085f27dadbd1f 100644 --- a/src/librustc/hir/lowering/expr.rs +++ b/src/librustc/hir/lowering/expr.rs @@ -1039,10 +1039,9 @@ impl LoweringContext<'_> { ) -> hir::Expr { // expand let mut head = self.lower_expr(head); - let head_sp = head.span; let desugared_span = self.mark_span_with_reason( DesugaringKind::ForLoop, - head_sp, + head.span, None, ); head.span = desugared_span; @@ -1088,21 +1087,21 @@ impl LoweringContext<'_> { // `match ::std::iter::Iterator::next(&mut iter) { ... }` let match_expr = { - let iter = P(self.expr_ident(head_sp, iter, iter_pat_nid)); - let ref_mut_iter = self.expr_mut_addr_of(head_sp, iter); + let iter = P(self.expr_ident(desugared_span, iter, iter_pat_nid)); + let ref_mut_iter = self.expr_mut_addr_of(desugared_span, iter); let next_path = &[sym::iter, sym::Iterator, sym::next]; let next_expr = P(self.expr_call_std_path( - head_sp, + desugared_span, next_path, hir_vec![ref_mut_iter], )); let arms = hir_vec![pat_arm, break_arm]; - self.expr_match(head_sp, next_expr, arms, hir::MatchSource::ForLoopDesugar) + self.expr_match(desugared_span, next_expr, arms, hir::MatchSource::ForLoopDesugar) }; - let match_stmt = self.stmt_expr(head_sp, match_expr); + let match_stmt = self.stmt_expr(desugared_span, match_expr); - let next_expr = P(self.expr_ident(head_sp, next_ident, next_pat_hid)); + let next_expr = P(self.expr_ident(desugared_span, next_ident, next_pat_hid)); // `let mut __next` let next_let = self.stmt_let_pat( @@ -1117,7 +1116,7 @@ impl LoweringContext<'_> { let pat = self.lower_pat(pat); let pat_let = self.stmt_let_pat( ThinVec::new(), - head_sp, + desugared_span, Some(next_expr), pat, hir::LocalSource::ForLoopDesugar, @@ -1154,14 +1153,14 @@ impl LoweringContext<'_> { let into_iter_path = &[sym::iter, sym::IntoIterator, sym::into_iter]; P(self.expr_call_std_path( - head_sp, + desugared_span, into_iter_path, hir_vec![head], )) }; let match_expr = P(self.expr_match( - head_sp, + desugared_span, into_iter_expr, hir_vec![iter_arm], hir::MatchSource::ForLoopDesugar, @@ -1173,7 +1172,7 @@ impl LoweringContext<'_> { // surrounding scope of the `match` since the `match` is not a terminating scope. // // Also, add the attributes to the outer returned expr node. - self.expr_drop_temps(head_sp, match_expr, e.attrs.clone()) + self.expr_drop_temps(desugared_span, match_expr, e.attrs.clone()) } /// Desugar `ExprKind::Try` from: `?` into: diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 42a4a9909f8a9..a1011697ef160 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -818,6 +818,32 @@ impl<'hir> Map<'hir> { CRATE_HIR_ID } + /// When on a match arm tail expression or on a match arm, give back the enclosing `match` + /// expression. + /// + /// Used by error reporting when there's a type error in a match arm caused by the `match` + /// expression needing to be unit. + pub fn get_match_if_cause(&self, hir_id: HirId) -> Option<&Expr> { + for (_, node) in ParentHirIterator::new(hir_id, &self) { + match node { + Node::Item(_) | + Node::ForeignItem(_) | + Node::TraitItem(_) | + Node::ImplItem(_) => break, + Node::Expr(expr) => match expr.kind { + ExprKind::Match(_, _, _) => return Some(expr), + _ => {} + }, + Node::Stmt(stmt) => match stmt.kind { + StmtKind::Local(_) => break, + _ => {} + } + _ => {} + } + } + None + } + /// Returns the nearest enclosing scope. A scope is roughly an item or block. pub fn get_enclosing_scope(&self, hir_id: HirId) -> Option { for (hir_id, node) in ParentHirIterator::new(hir_id, &self) { diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 13b6b1b8aa08d..a1daed005f302 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -36,7 +36,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // 2. By expecting `bool` for `expr` we get nice diagnostics for e.g. `if x = y { .. }`. // // FIXME(60707): Consider removing hack with principled solution. - self.check_expr_has_type_or_error(discrim, self.tcx.types.bool) + self.check_expr_has_type_or_error(discrim, self.tcx.types.bool, |_| {}) } else { self.demand_discriminant_type(arms, discrim) }; @@ -106,7 +106,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if let Some(g) = &arm.guard { self.diverges.set(pats_diverge); match g { - hir::Guard::If(e) => self.check_expr_has_type_or_error(e, tcx.types.bool), + hir::Guard::If(e) => { + self.check_expr_has_type_or_error(e, tcx.types.bool, |_| {}) + } }; } @@ -442,7 +444,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { kind: TypeVariableOriginKind::TypeInference, span: discrim.span, }); - self.check_expr_has_type_or_error(discrim, discrim_ty); + self.check_expr_has_type_or_error(discrim, discrim_ty, |_| {}); discrim_ty } } diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index 564a0eac75539..56962d53a6450 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -1163,18 +1163,20 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { fcx.try_coerce(expression, expression_ty, self.expected_ty, AllowTwoPhase::No) } else { match self.expressions { - Expressions::Dynamic(ref exprs) => - fcx.try_find_coercion_lub(cause, - exprs, - self.merged_ty(), - expression, - expression_ty), - Expressions::UpFront(ref coercion_sites) => - fcx.try_find_coercion_lub(cause, - &coercion_sites[0..self.pushed], - self.merged_ty(), - expression, - expression_ty), + Expressions::Dynamic(ref exprs) => fcx.try_find_coercion_lub( + cause, + exprs, + self.merged_ty(), + expression, + expression_ty, + ), + Expressions::UpFront(ref coercion_sites) => fcx.try_find_coercion_lub( + cause, + &coercion_sites[0..self.pushed], + self.merged_ty(), + expression, + expression_ty, + ), } } } else { @@ -1216,7 +1218,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { self.pushed += 1; } } - Err(err) => { + Err(coercion_error) => { let (expected, found) = if label_expression_as_expected { // In the case where this is a "forced unit", like // `break`, we want to call the `()` "expected" @@ -1232,41 +1234,42 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { (self.final_ty.unwrap_or(self.expected_ty), expression_ty) }; - let mut db; + let mut err; match cause.code { ObligationCauseCode::ReturnNoExpression => { - db = struct_span_err!( + err = struct_span_err!( fcx.tcx.sess, cause.span, E0069, "`return;` in a function whose return type is not `()`"); - db.span_label(cause.span, "return type is not `()`"); + err.span_label(cause.span, "return type is not `()`"); } ObligationCauseCode::BlockTailExpression(blk_id) => { let parent_id = fcx.tcx.hir().get_parent_node(blk_id); - db = self.report_return_mismatched_types( + err = self.report_return_mismatched_types( cause, expected, found, - err, + coercion_error, fcx, parent_id, expression.map(|expr| (expr, blk_id)), ); } ObligationCauseCode::ReturnValue(id) => { - db = self.report_return_mismatched_types( - cause, expected, found, err, fcx, id, None); + err = self.report_return_mismatched_types( + cause, expected, found, coercion_error, fcx, id, None); } _ => { - db = fcx.report_mismatched_types(cause, expected, found, err); + err = fcx.report_mismatched_types(cause, expected, found, coercion_error); } } if let Some(augment_error) = augment_error { - augment_error(&mut db); + augment_error(&mut err); } // Error possibly reported in `check_assign` so avoid emitting error again. - db.emit_unless(expression.filter(|e| fcx.is_assign_to_bool(e, expected)).is_some()); + err.emit_unless(expression.filter(|e| fcx.is_assign_to_bool(e, expected)) + .is_some()); self.final_ty = Some(fcx.tcx.types.err); } @@ -1278,12 +1281,12 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { cause: &ObligationCause<'tcx>, expected: Ty<'tcx>, found: Ty<'tcx>, - err: TypeError<'tcx>, + ty_err: TypeError<'tcx>, fcx: &FnCtxt<'a, 'tcx>, id: hir::HirId, expression: Option<(&'tcx hir::Expr, hir::HirId)>, ) -> DiagnosticBuilder<'a> { - let mut db = fcx.report_mismatched_types(cause, expected, found, err); + let mut err = fcx.report_mismatched_types(cause, expected, found, ty_err); let mut pointing_at_return_type = false; let mut return_sp = None; @@ -1294,7 +1297,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { let parent_id = fcx.tcx.hir().get_parent_node(id); let fn_decl = if let Some((expr, blk_id)) = expression { pointing_at_return_type = fcx.suggest_mismatched_types_on_tail( - &mut db, + &mut err, expr, expected, found, @@ -1302,6 +1305,16 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { blk_id, ); let parent = fcx.tcx.hir().get(parent_id); + if let (Some(match_expr), true, false) = ( + fcx.tcx.hir().get_match_if_cause(expr.hir_id), + expected.is_unit(), + pointing_at_return_type, + ) { + if match_expr.span.desugaring_kind().is_none() { + err.span_label(match_expr.span, "expected this to be `()`"); + fcx.suggest_semicolon_at_end(match_expr.span, &mut err); + } + } fcx.get_node_fn_decl(parent).map(|(fn_decl, _, is_main)| (fn_decl, is_main)) } else { fcx.get_fn_decl(parent_id) @@ -1310,20 +1323,20 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { if let (Some((fn_decl, can_suggest)), _) = (fn_decl, pointing_at_return_type) { if expression.is_none() { pointing_at_return_type |= fcx.suggest_missing_return_type( - &mut db, &fn_decl, expected, found, can_suggest); + &mut err, &fn_decl, expected, found, can_suggest); } if !pointing_at_return_type { return_sp = Some(fn_decl.output.span()); // `impl Trait` return type } } if let (Some(sp), Some(return_sp)) = (fcx.ret_coercion_span.borrow().as_ref(), return_sp) { - db.span_label(return_sp, "expected because this return type..."); - db.span_label( *sp, format!( + err.span_label(return_sp, "expected because this return type..."); + err.span_label( *sp, format!( "...is found to be `{}` here", fcx.resolve_type_vars_with_obligations(expected), )); } - db + err } pub fn complete<'a>(self, fcx: &FnCtxt<'a, 'tcx>) -> Ty<'tcx> { diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 0cdf2fa2a5b9e..81ae85c65496c 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -53,14 +53,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, expr: &'tcx hir::Expr, expected: Ty<'tcx>, + extend_err: impl Fn(&mut DiagnosticBuilder<'_>), ) -> Ty<'tcx> { - self.check_expr_meets_expectation_or_error(expr, ExpectHasType(expected)) + self.check_expr_meets_expectation_or_error(expr, ExpectHasType(expected), extend_err) } fn check_expr_meets_expectation_or_error( &self, expr: &'tcx hir::Expr, expected: Expectation<'tcx>, + extend_err: impl Fn(&mut DiagnosticBuilder<'_>), ) -> Ty<'tcx> { let expected_ty = expected.to_option(&self).unwrap_or(self.tcx.types.bool); let mut ty = self.check_expr_with_expectation(expr, expected); @@ -88,6 +90,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ExprKind::DropTemps(expr) => expr, _ => expr, }; + extend_err(&mut err); // Error possibly reported in `check_assign` so avoid emitting error again. err.emit_unless(self.is_assign_to_bool(expr, expected_ty)); } @@ -971,7 +974,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { kind: TypeVariableOriginKind::MiscVariable, span: element.span, }); - let element_ty = self.check_expr_has_type_or_error(&element, ty); + let element_ty = self.check_expr_has_type_or_error(&element, ty, |_| {}); (element_ty, ty) } }; @@ -1058,7 +1061,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // the fields with the base_expr. This could cause us to hit errors later // when certain fields are assumed to exist that in fact do not. if !error_happened { - self.check_expr_has_type_or_error(base_expr, adt_ty); + self.check_expr_has_type_or_error(base_expr, adt_ty, |_| {}); match adt_ty.kind { ty::Adt(adt, substs) if adt.is_struct() => { let fru_field_types = adt.non_enum_variant().fields.iter().map(|f| { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 9f2c991fdd236..7560a72c89055 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3858,6 +3858,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + fn suggest_semicolon_at_end(&self, span: Span, err: &mut DiagnosticBuilder<'_>) { + err.span_suggestion_short( + span.shrink_to_hi(), + "consider using a semicolon here", + ";".to_string(), + Applicability::MachineApplicable, + ); + } + pub fn check_stmt(&self, stmt: &'tcx hir::Stmt) { // Don't do all the complex logic below for `DeclItem`. match stmt.kind { @@ -3881,7 +3890,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { hir::StmtKind::Item(_) => {} hir::StmtKind::Expr(ref expr) => { // Check with expected type of `()`. - self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit()); + + self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit(), |err| { + self.suggest_semicolon_at_end(expr.span, err); + }); } hir::StmtKind::Semi(ref expr) => { self.check_expr(&expr); diff --git a/src/test/ui/struct-literal-variant-in-if.stderr b/src/test/ui/struct-literal-variant-in-if.stderr index 85cbc787bc2db..a52ec6dc53938 100644 --- a/src/test/ui/struct-literal-variant-in-if.stderr +++ b/src/test/ui/struct-literal-variant-in-if.stderr @@ -50,7 +50,10 @@ error[E0308]: mismatched types --> $DIR/struct-literal-variant-in-if.rs:10:20 | LL | if x == E::V { field } {} - | ^^^^^ expected (), found bool + | ---------------^^^^^--- help: consider using a semicolon here + | | | + | | expected (), found bool + | expected this to be `()` | = note: expected type `()` found type `bool` diff --git a/src/test/ui/suggestions/match-needing-semi.fixed b/src/test/ui/suggestions/match-needing-semi.fixed new file mode 100644 index 0000000000000..03cbed1376ea3 --- /dev/null +++ b/src/test/ui/suggestions/match-needing-semi.fixed @@ -0,0 +1,18 @@ +// check-only +// run-rustfix + +fn main() { + match 3 { + 4 => 1, + 3 => { + 2 //~ ERROR mismatched types + } + _ => 2 + }; + match 3 { //~ ERROR mismatched types + 4 => 1, + 3 => 2, + _ => 2 + }; + let _ = (); +} diff --git a/src/test/ui/suggestions/match-needing-semi.rs b/src/test/ui/suggestions/match-needing-semi.rs new file mode 100644 index 0000000000000..f34071ac75886 --- /dev/null +++ b/src/test/ui/suggestions/match-needing-semi.rs @@ -0,0 +1,18 @@ +// check-only +// run-rustfix + +fn main() { + match 3 { + 4 => 1, + 3 => { + 2 //~ ERROR mismatched types + } + _ => 2 + } + match 3 { //~ ERROR mismatched types + 4 => 1, + 3 => 2, + _ => 2 + } + let _ = (); +} diff --git a/src/test/ui/suggestions/match-needing-semi.stderr b/src/test/ui/suggestions/match-needing-semi.stderr new file mode 100644 index 0000000000000..988945817c2ee --- /dev/null +++ b/src/test/ui/suggestions/match-needing-semi.stderr @@ -0,0 +1,36 @@ +error[E0308]: mismatched types + --> $DIR/match-needing-semi.rs:8:13 + | +LL | / match 3 { +LL | | 4 => 1, +LL | | 3 => { +LL | | 2 + | | ^ expected (), found integer +LL | | } +LL | | _ => 2 +LL | | } + | | -- help: consider using a semicolon here + | |_____| + | expected this to be `()` + | + = note: expected type `()` + found type `{integer}` + +error[E0308]: mismatched types + --> $DIR/match-needing-semi.rs:12:5 + | +LL | / match 3 { +LL | | 4 => 1, +LL | | 3 => 2, +LL | | _ => 2 +LL | | } + | | ^- help: consider using a semicolon here + | |_____| + | expected (), found integer + | + = note: expected type `()` + found type `{integer}` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`.