Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide better spans for the match arm without tail expression #75656

Merged
merged 1 commit into from
Aug 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
estebank marked this conversation as resolved.
Show resolved Hide resolved

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`.