Rollup merge of rust-lang#64825 - estebank:match-unit, r=Centril

Centril · web-flow · commit 8c3fd1c7ff43 · 2019-09-29T19:42:29.000+02:00
Point at enclosing match when expecting `()` in arm When encountering code like the following: ```rust fn main() { match 3 { 4 => 1, 3 => { println!("Yep it maches."); 2 } _ => 2 } println!("Bye!") } ``` point at the enclosing `match` expression and suggest ignoring the returned value: ``` 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} ``` Fix rust-lang#40799.
diff --git a/src/librustc/hir/lowering/expr.rs b/src/librustc/hir/lowering/expr.rs
@@ -1037,10 +1037,9 @@ impl LoweringContext<'_> {
     ) -> hir::Expr {
         // expand <head>
         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;
@@ -1086,21 +1085,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(
@@ -1115,7 +1114,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,
@@ -1152,14 +1151,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,
@@ -1171,7 +1170,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: `<expr>?` into:
diff --git 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<HirId> {
         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
@@ -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
@@ -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,14 +1297,24 @@ 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,
                 cause.span,
                 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
@@ -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
@@ -3879,6 +3879,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 {
@@ -3902,7 +3911,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
@@ -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
diff --git a/src/test/ui/suggestions/match-needing-semi.rs b/src/test/ui/suggestions/match-needing-semi.rs
diff --git a/src/test/ui/suggestions/match-needing-semi.stderr b/src/test/ui/suggestions/match-needing-semi.stderr

Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,10 @@ error[E0308]: mismatched types`
`50`	`50`	`--> $DIR/struct-literal-variant-in-if.rs:10:20`
`51`	`51`	`\|`
`52`	`52`	`LL \| if x == E::V { field } {}`
`53`		`- \| ^^^^^ expected (), found bool`
	`53`	`+ \| ---------------^^^^^--- help: consider using a semicolon here`
	`54`	`+ \| \| \|`
	`55`	`+ \| \| expected (), found bool`
	`56`	+ \| expected this to be `()`
`54`	`57`	`\|`
`55`	`58`	= note: expected type `()`
`56`	`59`	found type `bool`