Reweork incompatible match arms error

- Point at the body expression of the match arm with the type error. - Point at the prior match arms explicitely stating the evaluated type. - Point at the entire match expr in a secondary span, instead of primary. - For type errors in the first match arm, the cause is outside of the match, treat as implicit block error to give a more appropriate error.
rust-lang · Feb 7, 2019 · 9e934e2 · 9e934e2
1 parent 825f355
commit 9e934e2
Show file tree

Hide file tree

Showing 12 changed files with 92 additions and 62 deletions.
diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs
@@ -509,22 +509,27 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                     }
                 }
             }
-            ObligationCauseCode::MatchExpressionArm { arm_span, source } => match source {
+            ObligationCauseCode::MatchExpressionArm {
+                source,
+                ref prior_arms,
+                ..
+            } => match source {
                 hir::MatchSource::IfLetDesugar { .. } => {
-                    let msg = "`if let` arm with an incompatible type";
-                    if self.tcx.sess.source_map().is_multiline(arm_span) {
-                        err.span_note(arm_span, msg);
-                    } else {
-                        err.span_label(arm_span, msg);
-                    }
+                    let msg = "`if let` arms have incompatible types";
+                    err.span_label(cause.span, msg);
                 }
                 hir::MatchSource::TryDesugar => {}
                 _ => {
-                    let msg = "match arm with an incompatible type";
-                    if self.tcx.sess.source_map().is_multiline(arm_span) {
-                        err.span_note(arm_span, msg);
-                    } else {
-                        err.span_label(arm_span, msg);
+                    let msg = "`match` arms have incompatible types";
+                    err.span_label(cause.span, msg);
+                    if prior_arms.len() < 4 {
+                        for (sp, ty) in prior_arms {
+                            err.span_label(*sp, format!("this is found to be of type `{}`", ty));
+                        }
+                    } else if let Some((sp, ty)) = prior_arms.last() {
+                        err.span_label(*sp, format!(
+                            "this and all prior arms are found to be of type `{}`", ty,
+                        ));
                     }
                 }
             },

diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs
@@ -145,6 +145,7 @@ impl<'tcx> ObligationCause<'tcx> {
             ObligationCauseCode::StartFunctionType => {
                 tcx.sess.source_map().def_span(self.span)
             }
+            ObligationCauseCode::MatchExpressionArm { arm_span, .. } => arm_span,
             _ => self.span,
         }
     }
@@ -223,6 +224,7 @@ pub enum ObligationCauseCode<'tcx> {
     MatchExpressionArm {
         arm_span: Span,
         source: hir::MatchSource,
+        prior_arms: Vec<(Span, Ty<'tcx>)>,
     },
 
     /// Computing common supertype in the pattern guard for the arms of a match expression

diff --git a/src/librustc/traits/structural_impls.rs b/src/librustc/traits/structural_impls.rs
@@ -513,10 +513,16 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
                 trait_item_def_id,
             }),
             super::ExprAssignable => Some(super::ExprAssignable),
-            super::MatchExpressionArm { arm_span, source } => Some(super::MatchExpressionArm {
+            super::MatchExpressionArm {
                 arm_span,
-                source: source,
-            }),
+                source,
+                ref prior_arms,
+            } => {
+                let prior_arms = prior_arms.iter().filter_map(|(sp, ty)| {
+                    tcx.lift(ty).map(|ty| (*sp, ty))
+                }).collect();
+                Some(super::MatchExpressionArm { arm_span, source, prior_arms })
+            }
             super::MatchExpressionArmPattern { span, ty } => {
                 tcx.lift(&ty).map(|ty| super::MatchExpressionArmPattern { span, ty })
             }

diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs
@@ -689,6 +689,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects");
             CoerceMany::with_coercion_sites(coerce_first, arms)
         };
 
+        let mut other_arms = vec![];  // used only for diagnostics
         for (i, (arm, pats_diverge)) in arms.iter().zip(all_arm_pats_diverge).enumerate() {
             if let Some(ref g) = arm.guard {
                 self.diverges.set(pats_diverge);
@@ -709,17 +710,31 @@ https://doc.rust-lang.org/reference/types.html#trait-objects");
                 _ => false
             };
 
+            let arm_span = if let hir::ExprKind::Block(ref blk, _) = arm.body.node {
+                // Point at the block expr instead of the entire block
+                blk.expr.as_ref().map(|e| e.span).unwrap_or(arm.body.span)
+            } else {
+                arm.body.span
+            };
             if is_if_let_fallback {
                 let cause = self.cause(expr.span, ObligationCauseCode::IfExpressionWithNoElse);
                 assert!(arm_ty.is_unit());
                 coercion.coerce_forced_unit(self, &cause, &mut |_| (), true);
             } else {
-                let cause = self.cause(expr.span, ObligationCauseCode::MatchExpressionArm {
-                    arm_span: arm.body.span,
-                    source: match_src
-                });
+                let cause = if i == 0 {
+                    // The reason for the first arm to fail is not that the match arms diverge,
+                    // but rather that there's a prior obligation that doesn't hold.
+                    self.cause(arm_span, ObligationCauseCode::BlockTailExpression(arm.body.id))
+                } else {
+                    self.cause(expr.span, ObligationCauseCode::MatchExpressionArm {
+                        arm_span,
+                        source: match_src,
+                        prior_arms: other_arms.clone(),
+                    })
+                };
                 coercion.coerce(self, &cause, &arm.body, arm_ty);
             }
+            other_arms.push((arm_span, arm_ty));
         }
 
         // We won't diverge unless the discriminant or all arms diverge.

diff --git a/src/test/ui/if/if-let-arm-types.rs b/src/test/ui/if/if-let-arm-types.rs
@@ -1,10 +1,11 @@
 fn main() {
-    if let Some(b) = None { //~ ERROR: `if let` arms have incompatible types
-        //~^ expected (), found integer
-        //~| expected type `()`
-        //~| found type `{integer}`
+    if let Some(b) = None {
+        //~^ NOTE if let` arms have incompatible types
         ()
     } else {
         1
     };
+    //~^^ ERROR: `if let` arms have incompatible types
+    //~| NOTE expected (), found integer
+    //~| NOTE expected type `()`
 }
diff --git a/src/test/ui/if/if-let-arm-types.stderr b/src/test/ui/if/if-let-arm-types.stderr
@@ -1,25 +1,17 @@
 error[E0308]: `if let` arms have incompatible types
-  --> $DIR/if-let-arm-types.rs:2:5
+  --> $DIR/if-let-arm-types.rs:6:9
    |
-LL | /     if let Some(b) = None { //~ ERROR: `if let` arms have incompatible types
-LL | |         //~^ expected (), found integer
-LL | |         //~| expected type `()`
-LL | |         //~| found type `{integer}`
-...  |
+LL | /     if let Some(b) = None {
+LL | |         //~^ NOTE if let` arms have incompatible types
+LL | |         ()
+LL | |     } else {
 LL | |         1
+   | |         ^ expected (), found integer
 LL | |     };
-   | |_____^ expected (), found integer
+   | |_____- `if let` arms have incompatible types
    |
    = note: expected type `()`
               found type `{integer}`
-note: `if let` arm with an incompatible type
-  --> $DIR/if-let-arm-types.rs:7:12
-   |
-LL |       } else {
-   |  ____________^
-LL | |         1
-LL | |     };
-   | |_____^
 
 error: aborting due to previous error
 

diff --git a/src/test/ui/issues/issue-11319.rs b/src/test/ui/issues/issue-11319.rs
@@ -1,12 +1,14 @@
 fn main() {
     match Some(10) {
-    //~^ ERROR match arms have incompatible types
-    //~| expected type `bool`
-    //~| found type `()`
-    //~| expected bool, found ()
+    //~^ NOTE `match` arms have incompatible types
         Some(5) => false,
+        //~^ NOTE this is found to be of type `bool`
         Some(2) => true,
+        //~^ NOTE this is found to be of type `bool`
         None    => (),
+        //~^ ERROR match arms have incompatible types
+        //~| NOTE expected bool, found ()
+        //~| NOTE expected type `bool`
         _       => true
     }
 }
diff --git a/src/test/ui/issues/issue-11319.stderr b/src/test/ui/issues/issue-11319.stderr
@@ -1,16 +1,20 @@
 error[E0308]: match arms have incompatible types
-  --> $DIR/issue-11319.rs:2:5
+  --> $DIR/issue-11319.rs:8:20
    |
 LL | /     match Some(10) {
-LL | |     //~^ ERROR match arms have incompatible types
-LL | |     //~| expected type `bool`
-LL | |     //~| found type `()`
-...  |
+LL | |     //~^ NOTE `match` arms have incompatible types
+LL | |         Some(5) => false,
+   | |                    ----- this is found to be of type `bool`
+LL | |         //~^ NOTE this is found to be of type `bool`
+LL | |         Some(2) => true,
+   | |                    ---- this is found to be of type `bool`
+LL | |         //~^ NOTE this is found to be of type `bool`
 LL | |         None    => (),
-   | |                    -- match arm with an incompatible type
+   | |                    ^^ expected bool, found ()
+...  |
 LL | |         _       => true
 LL | |     }
-   | |_____^ expected bool, found ()
+   | |_____- `match` arms have incompatible types
    |
    = note: expected type `bool`
               found type `()`

diff --git a/src/test/ui/issues/issue-17728.rs b/src/test/ui/issues/issue-17728.rs
@@ -97,7 +97,7 @@ impl Debug for Player {
 }
 
 fn str_to_direction(to_parse: &str) -> RoomDirection {
-    match to_parse { //~ ERROR match arms have incompatible types
+    match to_parse {
         "w" | "west" => RoomDirection::West,
         "e" | "east" => RoomDirection::East,
         "n" | "north" => RoomDirection::North,
@@ -108,6 +108,7 @@ fn str_to_direction(to_parse: &str) -> RoomDirection {
         "down" => RoomDirection::Down,
         _ => None
     }
+        //~^^ ERROR match arms have incompatible types
 }
 
 fn main() {

diff --git a/src/test/ui/issues/issue-17728.stderr b/src/test/ui/issues/issue-17728.stderr
@@ -10,17 +10,19 @@ LL |             Some(entry) => Ok(entry),
    |                            ^^^^^^^^^ ...but data from `room` is returned here
 
 error[E0308]: match arms have incompatible types
-  --> $DIR/issue-17728.rs:100:5
+  --> $DIR/issue-17728.rs:109:14
    |
-LL | /     match to_parse { //~ ERROR match arms have incompatible types
+LL | /     match to_parse {
 LL | |         "w" | "west" => RoomDirection::West,
 LL | |         "e" | "east" => RoomDirection::East,
 LL | |         "n" | "north" => RoomDirection::North,
 ...  |
+LL | |         "down" => RoomDirection::Down,
+   | |                   ------------------- this and all prior arms are found to be of type `RoomDirection`
 LL | |         _ => None
-   | |              ---- match arm with an incompatible type
+   | |              ^^^^ expected enum `RoomDirection`, found enum `std::option::Option`
 LL | |     }
-   | |_____^ expected enum `RoomDirection`, found enum `std::option::Option`
+   | |_____- `match` arms have incompatible types
    |
    = note: expected type `RoomDirection`
               found type `std::option::Option<_>`

diff --git a/src/test/ui/issues/issue-24036.rs b/src/test/ui/issues/issue-24036.rs
@@ -6,11 +6,11 @@ fn closure_to_loc() {
 
 fn closure_from_match() {
     let x = match 1usize {
-    //~^ ERROR match arms have incompatible types
         1 => |c| c + 1,
         2 => |c| c - 1,
         _ => |c| c - 1
     };
+    //~^^^ ERROR match arms have incompatible types
 }
 
 fn main() { }
diff --git a/src/test/ui/issues/issue-24036.stderr b/src/test/ui/issues/issue-24036.stderr
@@ -10,20 +10,20 @@ LL |     x = |c| c + 1;
    = help: consider boxing your closure and/or using it as a trait object
 
 error[E0308]: match arms have incompatible types
-  --> $DIR/issue-24036.rs:8:13
+  --> $DIR/issue-24036.rs:10:14
    |
 LL |       let x = match 1usize {
-   |  _____________^
-LL | |     //~^ ERROR match arms have incompatible types
+   |  _____________-
 LL | |         1 => |c| c + 1,
+   | |              --------- this is found to be of type `[closure@$DIR/issue-24036.rs:9:14: 9:23]`
 LL | |         2 => |c| c - 1,
-   | |              --------- match arm with an incompatible type
+   | |              ^^^^^^^^^ expected closure, found a different closure
 LL | |         _ => |c| c - 1
 LL | |     };
-   | |_____^ expected closure, found a different closure
+   | |_____- `match` arms have incompatible types
    |
-   = note: expected type `[closure@$DIR/issue-24036.rs:10:14: 10:23]`
-              found type `[closure@$DIR/issue-24036.rs:11:14: 11:23]`
+   = note: expected type `[closure@$DIR/issue-24036.rs:9:14: 9:23]`
+              found type `[closure@$DIR/issue-24036.rs:10:14: 10:23]`
    = note: no two closures, even if identical, have the same type
    = help: consider boxing your closure and/or using it as a trait object