Skip to content

Commit

Permalink
Provide better spans for the match arm without tail expression
Browse files Browse the repository at this point in the history
  • Loading branch information
tirr-c committed Aug 18, 2020
1 parent b97e9b5 commit c4fb3f2
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 22 deletions.
9 changes: 9 additions & 0 deletions src/librustc_infer/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
err.span_label(span, "expected due to this");
}
ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
semi_span,
source,
ref prior_arms,
last_ty,
Expand Down Expand Up @@ -663,6 +664,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
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 }) => {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_middle/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ static_assert_size!(ObligationCauseCode<'_>, 32);
#[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)]
pub struct MatchExpressionArmCause<'tcx> {
pub arm_span: Span,
pub semi_span: Option<Span>,
pub source: hir::MatchSource,
pub prior_arms: Vec<Span>,
pub last_ty: Ty<'tcx>,
Expand Down
50 changes: 28 additions & 22 deletions src/librustc_typeck/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
} 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,
Expand All @@ -138,6 +137,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr.span,
ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
arm_span,
semi_span,
source: match_src,
prior_arms: other_arms.clone(),
last_ty: prior_arm_ty.unwrap(),
Expand Down Expand Up @@ -295,14 +295,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

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 {
Expand Down Expand Up @@ -333,26 +328,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
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
};

// 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
Expand Down Expand Up @@ -450,4 +440,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
scrut_ty
}
}

fn find_block_span(
&self,
block: &'tcx hir::Block<'tcx>,
expected_ty: Option<Ty<'tcx>>,
) -> (Span, Option<Span>) {
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)
}
}
}
42 changes: 42 additions & 0 deletions src/test/ui/match/match-incompat-type-semi.rs
Original file line number Diff line number Diff line change
@@ -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
},
};
}
74 changes: 74 additions & 0 deletions src/test/ui/match/match-incompat-type-semi.stderr
Original file line number Diff line number Diff line change
@@ -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`.

0 comments on commit c4fb3f2

Please sign in to comment.