Rollup merge of #121146 - compiler-errors:ignore-diverging-arms, r=es…

…tebank Only point out non-diverging arms for match suggestions Fixes #121144 There is no reason to point at diverging arms, which will always coerce to whatever is the match block's evaluated type. This also removes the suggestion from #106601, since as I pointed out in #72634 (comment) the added suggestion is not firing in the right cases, but instead only when one of the match arms already *actually* evaluates to `()`. r? estebank
rust-lang · Feb 15, 2024 · 77eaa80 · 77eaa80
2 parents d6e2fc5 + 6018e21
commit 77eaa80
Show file tree

Hide file tree

Showing 9 changed files with 69 additions and 48 deletions.
diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs
@@ -79,7 +79,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             CoerceMany::with_coercion_sites(coerce_first, arms)
         };
 
-        let mut other_arms = vec![]; // Used only for diagnostics.
+        let mut prior_non_diverging_arms = vec![]; // Used only for diagnostics.
         let mut prior_arm = None;
         for arm in arms {
             if let Some(e) = &arm.guard {
@@ -118,9 +118,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         prior_arm_ty,
                         prior_arm_span,
                         scrut_span: scrut.span,
-                        scrut_hir_id: scrut.hir_id,
                         source: match_src,
-                        prior_arms: other_arms.clone(),
+                        prior_non_diverging_arms: prior_non_diverging_arms.clone(),
                         opt_suggest_box_span,
                     })),
                 ),
@@ -142,16 +141,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 false,
             );
 
-            other_arms.push(arm_span);
-            if other_arms.len() > 5 {
-                other_arms.remove(0);
-            }
-
             if !arm_ty.is_never() {
                 // When a match arm has type `!`, then it doesn't influence the expected type for
                 // the following arm. If all of the prior arms are `!`, then the influence comes
                 // from elsewhere and we shouldn't point to any previous arm.
                 prior_arm = Some((arm_block_id, arm_ty, arm_span));
+
+                prior_non_diverging_arms.push(arm_span);
+                if prior_non_diverging_arms.len() > 5 {
+                    prior_non_diverging_arms.remove(0);
+                }
             }
         }
 

diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs
@@ -777,10 +777,9 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                 prior_arm_span,
                 prior_arm_ty,
                 source,
-                ref prior_arms,
+                ref prior_non_diverging_arms,
                 opt_suggest_box_span,
                 scrut_span,
-                scrut_hir_id,
                 ..
             }) => match source {
                 hir::MatchSource::TryDesugar(scrut_hir_id) => {
@@ -817,12 +816,12 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     });
                     let source_map = self.tcx.sess.source_map();
                     let mut any_multiline_arm = source_map.is_multiline(arm_span);
-                    if prior_arms.len() <= 4 {
-                        for sp in prior_arms {
+                    if prior_non_diverging_arms.len() <= 4 {
+                        for sp in prior_non_diverging_arms {
                             any_multiline_arm |= source_map.is_multiline(*sp);
                             err.span_label(*sp, format!("this is found to be of type `{t}`"));
                         }
-                    } else if let Some(sp) = prior_arms.last() {
+                    } else if let Some(sp) = prior_non_diverging_arms.last() {
                         any_multiline_arm |= source_map.is_multiline(*sp);
                         err.span_label(
                             *sp,
@@ -848,24 +847,15 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     ) {
                         err.subdiagnostic(subdiag);
                     }
-                    if let hir::Node::Expr(m) = self.tcx.parent_hir_node(scrut_hir_id)
-                        && let hir::Node::Stmt(stmt) = self.tcx.parent_hir_node(m.hir_id)
-                        && let hir::StmtKind::Expr(_) = stmt.kind
-                    {
-                        err.span_suggestion_verbose(
-                            stmt.span.shrink_to_hi(),
-                            "consider using a semicolon here, but this will discard any values \
-                             in the match arms",
-                            ";",
-                            Applicability::MaybeIncorrect,
-                        );
-                    }
                     if let Some(ret_sp) = opt_suggest_box_span {
                         // Get return type span and point to it.
                         self.suggest_boxing_for_return_impl_trait(
                             err,
                             ret_sp,
-                            prior_arms.iter().chain(std::iter::once(&arm_span)).copied(),
+                            prior_non_diverging_arms
+                                .iter()
+                                .chain(std::iter::once(&arm_span))
+                                .copied(),
                         );
                     }
                 }

diff --git a/compiler/rustc_infer/src/infer/error_reporting/suggest.rs b/compiler/rustc_infer/src/infer/error_reporting/suggest.rs
@@ -203,10 +203,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     })
                 }
                 ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
-                    prior_arms,
+                    prior_non_diverging_arms,
                     ..
                 }) => {
-                    if let [.., arm_span] = &prior_arms[..] {
+                    if let [.., arm_span] = &prior_non_diverging_arms[..] {
                         Some(ConsiderAddingAwait::BothFuturesSugg {
                             first: arm_span.shrink_to_hi(),
                             second: exp_span.shrink_to_hi(),
@@ -234,11 +234,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     Some(ConsiderAddingAwait::FutureSugg { span: then_span.shrink_to_hi() })
                 }
                 ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
-                    ref prior_arms,
+                    ref prior_non_diverging_arms,
                     ..
                 }) => Some({
                     ConsiderAddingAwait::FutureSuggMultiple {
-                        spans: prior_arms.iter().map(|arm| arm.shrink_to_hi()).collect(),
+                        spans: prior_non_diverging_arms
+                            .iter()
+                            .map(|arm| arm.shrink_to_hi())
+                            .collect(),
                     }
                 }),
                 _ => None,

diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs
@@ -569,9 +569,8 @@ pub struct MatchExpressionArmCause<'tcx> {
     pub prior_arm_ty: Ty<'tcx>,
     pub prior_arm_span: Span,
     pub scrut_span: Span,
-    pub scrut_hir_id: hir::HirId,
     pub source: hir::MatchSource,
-    pub prior_arms: Vec<Span>,
+    pub prior_non_diverging_arms: Vec<Span>,
     pub opt_suggest_box_span: Option<Span>,
 }
 

diff --git a/tests/ui/match/dont-highlight-diverging-arms.rs b/tests/ui/match/dont-highlight-diverging-arms.rs
@@ -0,0 +1,17 @@
+fn main() {
+    let m = 42u32;
+
+    let value = 'out: {
+        match m {
+            1 => break 'out Some(1u16),
+            2 => Some(2u16),
+            3 => break 'out Some(3u16),
+            4 => break 'out Some(4u16),
+            5 => break 'out Some(5u16),
+            _ => {}
+            //~^ ERROR  `match` arms have incompatible types
+        }
+
+        None
+    };
+}
diff --git a/tests/ui/match/dont-highlight-diverging-arms.stderr b/tests/ui/match/dont-highlight-diverging-arms.stderr
@@ -0,0 +1,21 @@
+error[E0308]: `match` arms have incompatible types
+  --> $DIR/dont-highlight-diverging-arms.rs:11:18
+   |
+LL | /         match m {
+LL | |             1 => break 'out Some(1u16),
+LL | |             2 => Some(2u16),
+   | |                  ---------- this is found to be of type `Option<u16>`
+LL | |             3 => break 'out Some(3u16),
+...  |
+LL | |             _ => {}
+   | |                  ^^ expected `Option<u16>`, found `()`
+LL | |
+LL | |         }
+   | |_________- `match` arms have incompatible types
+   |
+   = note:   expected enum `Option<u16>`
+           found unit type `()`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0308`.
diff --git a/tests/ui/match/match-arm-resolving-to-never.stderr b/tests/ui/match/match-arm-resolving-to-never.stderr
@@ -3,11 +3,14 @@ error[E0308]: `match` arms have incompatible types
    |
 LL | /     match E::F {
 LL | |         E::A => 1,
+   | |                 - this is found to be of type `{integer}`
 LL | |         E::B => 2,
+   | |                 - this is found to be of type `{integer}`
 LL | |         E::C => 3,
+   | |                 - this is found to be of type `{integer}`
 LL | |         E::D => 4,
+   | |                 - this is found to be of type `{integer}`
 LL | |         E::E => unimplemented!(""),
-   | |                 ------------------ this and all prior arms are found to be of type `{integer}`
 LL | |         E::F => "",
    | |                 ^^ expected integer, found `&str`
 LL | |     };

diff --git a/tests/ui/suggestions/issue-81839.stderr b/tests/ui/suggestions/issue-81839.stderr
@@ -4,22 +4,15 @@ error[E0308]: `match` arms have incompatible types
 LL | /     match num {
 LL | |         1 => {
 LL | |             cx.answer_str("hi");
-   | |             -------------------- this is found to be of type `()`
+   | |             --------------------
+   | |             |                  |
+   | |             |                  help: consider removing this semicolon
+   | |             this is found to be of type `()`
 LL | |         }
 LL | |         _ => cx.answer_str("hi"),
    | |              ^^^^^^^^^^^^^^^^^^^ expected `()`, found future
 LL | |     }
    | |_____- `match` arms have incompatible types
-   |
-help: consider removing this semicolon
-   |
-LL -             cx.answer_str("hi");
-LL +             cx.answer_str("hi")
-   |
-help: consider using a semicolon here, but this will discard any values in the match arms
-   |
-LL |     };
-   |      +
 
 error: aborting due to 1 previous error
 

diff --git a/tests/ui/wf/wf-unsafe-trait-obj-match.stderr b/tests/ui/wf/wf-unsafe-trait-obj-match.stderr
@@ -11,10 +11,6 @@ LL | |     }
    |
    = note: expected reference `&S`
               found reference `&R`
-help: consider using a semicolon here, but this will discard any values in the match arms
-   |
-LL |     };
-   |      +
 
 error[E0038]: the trait `Trait` cannot be made into an object
   --> $DIR/wf-unsafe-trait-obj-match.rs:26:21