From c763f833d14d31fbe63e0b26370a23a51b4f11c5 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 15 Feb 2024 14:34:42 +0000 Subject: [PATCH 1/2] Only point out non-diverging arms for match suggestions --- compiler/rustc_hir_typeck/src/_match.rs | 14 +++++------ .../src/infer/error_reporting/mod.rs | 13 ++++++---- .../src/infer/error_reporting/suggest.rs | 11 +++++--- compiler/rustc_middle/src/traits/mod.rs | 2 +- .../ui/match/dont-highlight-diverging-arms.rs | 17 +++++++++++++ .../dont-highlight-diverging-arms.stderr | 25 +++++++++++++++++++ .../match/match-arm-resolving-to-never.stderr | 5 +++- 7 files changed, 69 insertions(+), 18 deletions(-) create mode 100644 tests/ui/match/dont-highlight-diverging-arms.rs create mode 100644 tests/ui/match/dont-highlight-diverging-arms.stderr diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index 0311aa94cd485..214066814221b 100644 --- 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 { @@ -120,7 +120,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { 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 +142,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 index b953b25d6c4cf..aa92f91f24f83 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -777,7 +777,7 @@ 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, @@ -817,12 +817,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, @@ -865,7 +865,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { 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 index 248e1c0fcc878..c6f6c32fe60c2 100644 --- 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 index 8e5f026b4c27c..683610ba3b6a3 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -571,7 +571,7 @@ pub struct MatchExpressionArmCause<'tcx> { pub scrut_span: Span, pub scrut_hir_id: hir::HirId, pub source: hir::MatchSource, - pub prior_arms: Vec, + pub prior_non_diverging_arms: Vec, pub opt_suggest_box_span: Option, } diff --git a/tests/ui/match/dont-highlight-diverging-arms.rs b/tests/ui/match/dont-highlight-diverging-arms.rs new file mode 100644 index 0000000000000..dc3b4ca9caad6 --- /dev/null +++ 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 + }; +} \ No newline at end of file diff --git a/tests/ui/match/dont-highlight-diverging-arms.stderr b/tests/ui/match/dont-highlight-diverging-arms.stderr new file mode 100644 index 0000000000000..886c1af13fa37 --- /dev/null +++ b/tests/ui/match/dont-highlight-diverging-arms.stderr @@ -0,0 +1,25 @@ +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` +LL | | 3 => break 'out Some(3u16), +... | +LL | | _ => {} + | | ^^ expected `Option`, found `()` +LL | | +LL | | } + | |_________- `match` arms have incompatible types + | + = note: expected enum `Option` + found unit type `()` +help: consider using a semicolon here, but this will discard any values in the match arms + | +LL | }; + | + + +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 index 6cbdf03d4c27d..fd0c8708b8ca3 100644 --- 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 | | }; From 6018e21d8ad072d28dbd2e991dfd8295e2de321f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 15 Feb 2024 14:38:52 +0000 Subject: [PATCH 2/2] Remove a suggestion that is redundant --- compiler/rustc_hir_typeck/src/_match.rs | 1 - .../rustc_infer/src/infer/error_reporting/mod.rs | 13 ------------- compiler/rustc_middle/src/traits/mod.rs | 1 - tests/ui/match/dont-highlight-diverging-arms.rs | 2 +- .../ui/match/dont-highlight-diverging-arms.stderr | 4 ---- tests/ui/suggestions/issue-81839.stderr | 15 ++++----------- tests/ui/wf/wf-unsafe-trait-obj-match.stderr | 4 ---- 7 files changed, 5 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index 214066814221b..b0caf45b40afe 100644 --- a/compiler/rustc_hir_typeck/src/_match.rs +++ b/compiler/rustc_hir_typeck/src/_match.rs @@ -118,7 +118,6 @@ 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_non_diverging_arms: prior_non_diverging_arms.clone(), opt_suggest_box_span, diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index aa92f91f24f83..104bf4a5be873 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -780,7 +780,6 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { ref prior_non_diverging_arms, opt_suggest_box_span, scrut_span, - scrut_hir_id, .. }) => match source { hir::MatchSource::TryDesugar(scrut_hir_id) => { @@ -848,18 +847,6 @@ 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( diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 683610ba3b6a3..119e0a49acf1f 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -569,7 +569,6 @@ 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_non_diverging_arms: Vec, pub opt_suggest_box_span: Option, diff --git a/tests/ui/match/dont-highlight-diverging-arms.rs b/tests/ui/match/dont-highlight-diverging-arms.rs index dc3b4ca9caad6..0fb614fa18a6f 100644 --- a/tests/ui/match/dont-highlight-diverging-arms.rs +++ b/tests/ui/match/dont-highlight-diverging-arms.rs @@ -14,4 +14,4 @@ fn main() { None }; -} \ No newline at end of file +} diff --git a/tests/ui/match/dont-highlight-diverging-arms.stderr b/tests/ui/match/dont-highlight-diverging-arms.stderr index 886c1af13fa37..f0aaecbb7adb1 100644 --- a/tests/ui/match/dont-highlight-diverging-arms.stderr +++ b/tests/ui/match/dont-highlight-diverging-arms.stderr @@ -15,10 +15,6 @@ LL | | } | = note: expected enum `Option` found unit type `()` -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/suggestions/issue-81839.stderr b/tests/ui/suggestions/issue-81839.stderr index de1ea98554bee..34ff16c653afa 100644 --- 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 index 3b53f55ffdc1d..e30cb8ff921a0 100644 --- 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