From c4fb3f297b36e902369a3f62214274149865e6e2 Mon Sep 17 00:00:00 2001 From: Wonwoo Choi Date: Tue, 18 Aug 2020 15:04:26 +0900 Subject: [PATCH] Provide better spans for the match arm without tail expression --- .../infer/error_reporting/mod.rs | 9 +++ src/librustc_middle/traits/mod.rs | 1 + src/librustc_typeck/check/_match.rs | 50 +++++++------ src/test/ui/match/match-incompat-type-semi.rs | 42 +++++++++++ .../ui/match/match-incompat-type-semi.stderr | 74 +++++++++++++++++++ 5 files changed, 154 insertions(+), 22 deletions(-) create mode 100644 src/test/ui/match/match-incompat-type-semi.rs create mode 100644 src/test/ui/match/match-incompat-type-semi.stderr diff --git a/src/librustc_infer/infer/error_reporting/mod.rs b/src/librustc_infer/infer/error_reporting/mod.rs index 2b2c42207e4..8212958510a 100644 --- a/src/librustc_infer/infer/error_reporting/mod.rs +++ b/src/librustc_infer/infer/error_reporting/mod.rs @@ -609,6 +609,7 @@ fn note_error_origin( err.span_label(span, "expected due to this"); } ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause { + semi_span, source, ref prior_arms, last_ty, @@ -663,6 +664,14 @@ fn note_error_origin( format!("this and all prior arms are found to be of type `{}`", t), ); } + if let Some(sp) = semi_span { + err.span_suggestion_short( + sp, + "consider removing this semicolon", + String::new(), + Applicability::MachineApplicable, + ); + } } }, ObligationCauseCode::IfExpression(box IfExpressionCause { then, outer, semicolon }) => { diff --git a/src/librustc_middle/traits/mod.rs b/src/librustc_middle/traits/mod.rs index ea9c8b7a415..f86403fa502 100644 --- a/src/librustc_middle/traits/mod.rs +++ b/src/librustc_middle/traits/mod.rs @@ -345,6 +345,7 @@ pub fn peel_derives(&self) -> &Self { #[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)] pub struct MatchExpressionArmCause<'tcx> { pub arm_span: Span, + pub semi_span: Option, pub source: hir::MatchSource, pub prior_arms: Vec, pub last_ty: Ty<'tcx>, diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 9e23f5df3c6..40088bc0690 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -124,11 +124,10 @@ pub fn check_match( } } } else { - let arm_span = if let hir::ExprKind::Block(blk, _) = &arm.body.kind { - // Point at the block expr instead of the entire block - blk.expr.as_ref().map(|e| e.span).unwrap_or(arm.body.span) + let (arm_span, semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind { + self.find_block_span(blk, prior_arm_ty) } else { - arm.body.span + (arm.body.span, None) }; let (span, code) = match i { // The reason for the first arm to fail is not that the match arms diverge, @@ -138,6 +137,7 @@ pub fn check_match( expr.span, ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause { arm_span, + semi_span, source: match_src, prior_arms: other_arms.clone(), last_ty: prior_arm_ty.unwrap(), @@ -295,14 +295,9 @@ fn if_cause( let mut remove_semicolon = None; let error_sp = if let ExprKind::Block(block, _) = &else_expr.kind { - if let Some(expr) = &block.expr { - expr.span - } else if let Some(stmt) = block.stmts.last() { - // possibly incorrect trailing `;` in the else arm - remove_semicolon = self.could_remove_semicolon(block, then_ty); - stmt.span - } else { - // empty block; point at its entirety + let (error_sp, semi_sp) = self.find_block_span(block, Some(then_ty)); + remove_semicolon = semi_sp; + if block.expr.is_none() && block.stmts.is_empty() { // Avoid overlapping spans that aren't as readable: // ``` // 2 | let x = if true { @@ -333,8 +328,8 @@ fn if_cause( if outer_sp.is_some() { outer_sp = Some(self.tcx.sess.source_map().guess_head_span(span)); } - else_expr.span } + error_sp } else { // shouldn't happen unless the parser has done something weird else_expr.span @@ -342,17 +337,12 @@ fn if_cause( // Compute `Span` of `then` part of `if`-expression. let then_sp = if let ExprKind::Block(block, _) = &then_expr.kind { - if let Some(expr) = &block.expr { - expr.span - } else if let Some(stmt) = block.stmts.last() { - // possibly incorrect trailing `;` in the else arm - remove_semicolon = remove_semicolon.or(self.could_remove_semicolon(block, else_ty)); - stmt.span - } else { - // empty block; point at its entirety + let (then_sp, semi_sp) = self.find_block_span(block, Some(else_ty)); + remove_semicolon = remove_semicolon.or(semi_sp); + if block.expr.is_none() && block.stmts.is_empty() { outer_sp = None; // same as in `error_sp`; cleanup output - then_expr.span } + then_sp } else { // shouldn't happen unless the parser has done something weird then_expr.span @@ -450,4 +440,20 @@ fn demand_scrutinee_type( scrut_ty } } + + fn find_block_span( + &self, + block: &'tcx hir::Block<'tcx>, + expected_ty: Option>, + ) -> (Span, Option) { + if let Some(expr) = &block.expr { + (expr.span, None) + } else if let Some(stmt) = block.stmts.last() { + // possibly incorrect trailing `;` in the else arm + (stmt.span, expected_ty.and_then(|ty| self.could_remove_semicolon(block, ty))) + } else { + // empty block; point at its entirety + (block.span, None) + } + } } diff --git a/src/test/ui/match/match-incompat-type-semi.rs b/src/test/ui/match/match-incompat-type-semi.rs new file mode 100644 index 00000000000..9ab40fa3cce --- /dev/null +++ b/src/test/ui/match/match-incompat-type-semi.rs @@ -0,0 +1,42 @@ +// Diagnostic enhancement explained in issue #75418. +// Point at the last statement in the block if there's no tail expression, +// and suggest removing the semicolon if appropriate. + +fn main() { + let _ = match Some(42) { + Some(x) => { + x + }, + None => { + 0; + //~^ ERROR incompatible types + //~| HELP consider removing this semicolon + }, + }; + + let _ = if let Some(x) = Some(42) { + x + } else { + 0; + //~^ ERROR incompatible types + //~| HELP consider removing this semicolon + }; + + let _ = match Some(42) { + Some(x) => { + x + }, + None => { + (); + //~^ ERROR incompatible types + }, + }; + + let _ = match Some(42) { + Some(x) => { + x + }, + None => { //~ ERROR incompatible types + }, + }; +} diff --git a/src/test/ui/match/match-incompat-type-semi.stderr b/src/test/ui/match/match-incompat-type-semi.stderr new file mode 100644 index 00000000000..701f15fdc4b --- /dev/null +++ b/src/test/ui/match/match-incompat-type-semi.stderr @@ -0,0 +1,74 @@ +error[E0308]: `match` arms have incompatible types + --> $DIR/match-incompat-type-semi.rs:11:13 + | +LL | let _ = match Some(42) { + | _____________- +LL | | Some(x) => { +LL | | x + | | - this is found to be of type `{integer}` +LL | | }, +LL | | None => { +LL | | 0; + | | ^- + | | || + | | |help: consider removing this semicolon + | | expected integer, found `()` +... | +LL | | }, +LL | | }; + | |_____- `match` arms have incompatible types + +error[E0308]: `if` and `else` have incompatible types + --> $DIR/match-incompat-type-semi.rs:20:9 + | +LL | let _ = if let Some(x) = Some(42) { + | _____________- +LL | | x + | | - expected because of this +LL | | } else { +LL | | 0; + | | ^- + | | || + | | |help: consider removing this semicolon + | | expected integer, found `()` +LL | | +LL | | +LL | | }; + | |_____- `if` and `else` have incompatible types + +error[E0308]: `match` arms have incompatible types + --> $DIR/match-incompat-type-semi.rs:30:13 + | +LL | let _ = match Some(42) { + | _____________- +LL | | Some(x) => { +LL | | x + | | - this is found to be of type `{integer}` +LL | | }, +LL | | None => { +LL | | (); + | | ^^^ expected integer, found `()` +LL | | +LL | | }, +LL | | }; + | |_____- `match` arms have incompatible types + +error[E0308]: `match` arms have incompatible types + --> $DIR/match-incompat-type-semi.rs:39:17 + | +LL | let _ = match Some(42) { + | _____________- +LL | | Some(x) => { +LL | | x + | | - this is found to be of type `{integer}` +LL | | }, +LL | | None => { + | |_________________^ +LL | || }, + | ||_________^ expected integer, found `()` +LL | | }; + | |_____- `match` arms have incompatible types + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0308`. -- 2.44.0