Skip to content

Commit 0bc0015

Browse files
committed
Auto merge of rust-lang#57381 - estebank:if-else-308, r=nikomatsakis
Tweak output of type mismatch between "then" and `else` `if` arms ``` error[E0308]: if and else have incompatible types --> $DIR/if-else-type-mismatch.rs:5:9 | LL | let _ = if true { | _____________- LL | | 42i32 | | ----- expected because of this LL | | } else { LL | | 42u32 | | ^^^^^ expected i32, found u32 LL | | }; | |_____- if and else have incompatible types | = note: expected type `i32` found type `u32` error[E0308]: if and else have incompatible types --> file.rs:2:38 | LL | let _ = if false { 3u8; } else { 3u8 }; | ---- ^^^ expected (), found u8 | | | | | help: consider removing this semicolon | expected because of this | = note: expected type `()` found type `u8` error[E0308]: if and else have incompatible types --> file.rs:3:37 | LL | let _ = if false { 3u8 } else { 3u8; }; | --- ^^^- | | | | | | | help: consider removing this semicolon | | expected u8, found () | expected because of this | = note: expected type `u8` found type `()` error[E0308]: if and else have incompatible types --> file.rs:4:37 | LL | let _ = if false { 3i8 } else { 3u8 }; | --- ^^^ expected i8, found u8 | | | expected because of this | = note: expected type `i8` found type `u8` ``` Fix rust-lang#57348.
2 parents a16e1a7 + 9567544 commit 0bc0015

File tree

10 files changed

+331
-28
lines changed

10 files changed

+331
-28
lines changed

src/librustc/infer/error_reporting/mod.rs

+14-2
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,18 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
511511
}
512512
}
513513
},
514+
ObligationCauseCode::IfExpression { then, outer, semicolon } => {
515+
err.span_label(then, "expected because of this");
516+
outer.map(|sp| err.span_label(sp, "if and else have incompatible types"));
517+
if let Some(sp) = semicolon {
518+
err.span_suggestion_short_with_applicability(
519+
sp,
520+
"consider removing this semicolon",
521+
String::new(),
522+
Applicability::MachineApplicable,
523+
);
524+
}
525+
}
514526
_ => (),
515527
}
516528
}
@@ -1460,7 +1472,7 @@ impl<'tcx> ObligationCause<'tcx> {
14601472
}
14611473
_ => "match arms have incompatible types",
14621474
}),
1463-
IfExpression => Error0308("if and else have incompatible types"),
1475+
IfExpression { .. } => Error0308("if and else have incompatible types"),
14641476
IfExpressionWithNoElse => Error0317("if may be missing an else clause"),
14651477
MainFunctionType => Error0580("main function has wrong type"),
14661478
StartFunctionType => Error0308("start function has wrong type"),
@@ -1488,7 +1500,7 @@ impl<'tcx> ObligationCause<'tcx> {
14881500
hir::MatchSource::IfLetDesugar { .. } => "`if let` arms have compatible types",
14891501
_ => "match arms have compatible types",
14901502
},
1491-
IfExpression => "if and else have compatible types",
1503+
IfExpression { .. } => "if and else have compatible types",
14921504
IfExpressionWithNoElse => "if missing an else returns ()",
14931505
MainFunctionType => "`main` function has the correct type",
14941506
StartFunctionType => "`start` function has the correct type",

src/librustc/traits/error_reporting.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1445,7 +1445,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
14451445
ObligationCauseCode::ExprAssignable |
14461446
ObligationCauseCode::MatchExpressionArm { .. } |
14471447
ObligationCauseCode::MatchExpressionArmPattern { .. } |
1448-
ObligationCauseCode::IfExpression |
1448+
ObligationCauseCode::IfExpression { .. } |
14491449
ObligationCauseCode::IfExpressionWithNoElse |
14501450
ObligationCauseCode::MainFunctionType |
14511451
ObligationCauseCode::StartFunctionType |

src/librustc/traits/mod.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,11 @@ pub enum ObligationCauseCode<'tcx> {
229229
MatchExpressionArmPattern { span: Span, ty: Ty<'tcx> },
230230

231231
/// Computing common supertype in an if expression
232-
IfExpression,
232+
IfExpression {
233+
then: Span,
234+
outer: Option<Span>,
235+
semicolon: Option<Span>,
236+
},
233237

234238
/// Computing common supertype of an if expression with no else counter-part
235239
IfExpressionWithNoElse,

src/librustc/traits/structural_impls.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,11 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
520520
super::MatchExpressionArmPattern { span, ty } => {
521521
tcx.lift(&ty).map(|ty| super::MatchExpressionArmPattern { span, ty })
522522
}
523-
super::IfExpression => Some(super::IfExpression),
523+
super::IfExpression { then, outer, semicolon } => Some(super::IfExpression {
524+
then,
525+
outer,
526+
semicolon,
527+
}),
524528
super::IfExpressionWithNoElse => Some(super::IfExpressionWithNoElse),
525529
super::MainFunctionType => Some(super::MainFunctionType),
526530
super::StartFunctionType => Some(super::StartFunctionType),

src/librustc_typeck/check/mod.rs

+117-16
Original file line numberDiff line numberDiff line change
@@ -3366,13 +3366,103 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
33663366
let coerce_to_ty = expected.coercion_target_type(self, sp);
33673367
let mut coerce: DynamicCoerceMany = CoerceMany::new(coerce_to_ty);
33683368

3369-
let if_cause = self.cause(sp, ObligationCauseCode::IfExpression);
3370-
coerce.coerce(self, &if_cause, then_expr, then_ty);
3369+
coerce.coerce(self, &self.misc(sp), then_expr, then_ty);
33713370

33723371
if let Some(else_expr) = opt_else_expr {
33733372
let else_ty = self.check_expr_with_expectation(else_expr, expected);
33743373
let else_diverges = self.diverges.get();
33753374

3375+
let mut outer_sp = if self.tcx.sess.source_map().is_multiline(sp) {
3376+
// The `if`/`else` isn't in one line in the output, include some context to make it
3377+
// clear it is an if/else expression:
3378+
// ```
3379+
// LL | let x = if true {
3380+
// | _____________-
3381+
// LL || 10i32
3382+
// || ----- expected because of this
3383+
// LL || } else {
3384+
// LL || 10u32
3385+
// || ^^^^^ expected i32, found u32
3386+
// LL || };
3387+
// ||_____- if and else have incompatible types
3388+
// ```
3389+
Some(sp)
3390+
} else {
3391+
// The entire expression is in one line, only point at the arms
3392+
// ```
3393+
// LL | let x = if true { 10i32 } else { 10u32 };
3394+
// | ----- ^^^^^ expected i32, found u32
3395+
// | |
3396+
// | expected because of this
3397+
// ```
3398+
None
3399+
};
3400+
let mut remove_semicolon = None;
3401+
let error_sp = if let ExprKind::Block(block, _) = &else_expr.node {
3402+
if let Some(expr) = &block.expr {
3403+
expr.span
3404+
} else if let Some(stmt) = block.stmts.last() {
3405+
// possibly incorrect trailing `;` in the else arm
3406+
remove_semicolon = self.could_remove_semicolon(block, then_ty);
3407+
stmt.span
3408+
} else { // empty block, point at its entirety
3409+
// Avoid overlapping spans that aren't as readable:
3410+
// ```
3411+
// 2 | let x = if true {
3412+
// | _____________-
3413+
// 3 | | 3
3414+
// | | - expected because of this
3415+
// 4 | | } else {
3416+
// | |____________^
3417+
// 5 | ||
3418+
// 6 | || };
3419+
// | || ^
3420+
// | ||_____|
3421+
// | |______if and else have incompatible types
3422+
// | expected integer, found ()
3423+
// ```
3424+
// by not pointing at the entire expression:
3425+
// ```
3426+
// 2 | let x = if true {
3427+
// | ------- if and else have incompatible types
3428+
// 3 | 3
3429+
// | - expected because of this
3430+
// 4 | } else {
3431+
// | ____________^
3432+
// 5 | |
3433+
// 6 | | };
3434+
// | |_____^ expected integer, found ()
3435+
// ```
3436+
if outer_sp.is_some() {
3437+
outer_sp = Some(self.tcx.sess.source_map().def_span(sp));
3438+
}
3439+
else_expr.span
3440+
}
3441+
} else { // shouldn't happen unless the parser has done something weird
3442+
else_expr.span
3443+
};
3444+
let then_sp = if let ExprKind::Block(block, _) = &then_expr.node {
3445+
if let Some(expr) = &block.expr {
3446+
expr.span
3447+
} else if let Some(stmt) = block.stmts.last() {
3448+
// possibly incorrect trailing `;` in the else arm
3449+
remove_semicolon = remove_semicolon.or(
3450+
self.could_remove_semicolon(block, else_ty));
3451+
stmt.span
3452+
} else { // empty block, point at its entirety
3453+
outer_sp = None; // same as in `error_sp`, cleanup output
3454+
then_expr.span
3455+
}
3456+
} else { // shouldn't happen unless the parser has done something weird
3457+
then_expr.span
3458+
};
3459+
3460+
let if_cause = self.cause(error_sp, ObligationCauseCode::IfExpression {
3461+
then: then_sp,
3462+
outer: outer_sp,
3463+
semicolon: remove_semicolon,
3464+
});
3465+
33763466
coerce.coerce(self, &if_cause, else_expr, else_ty);
33773467

33783468
// We won't diverge unless both branches do (or the condition does).
@@ -5144,7 +5234,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
51445234
}
51455235
}
51465236

5147-
51485237
/// A common error is to add an extra semicolon:
51495238
///
51505239
/// ```
@@ -5156,31 +5245,43 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
51565245
/// This routine checks if the final statement in a block is an
51575246
/// expression with an explicit semicolon whose type is compatible
51585247
/// with `expected_ty`. If so, it suggests removing the semicolon.
5159-
fn consider_hint_about_removing_semicolon(&self,
5160-
blk: &'gcx hir::Block,
5161-
expected_ty: Ty<'tcx>,
5162-
err: &mut DiagnosticBuilder) {
5248+
fn consider_hint_about_removing_semicolon(
5249+
&self,
5250+
blk: &'gcx hir::Block,
5251+
expected_ty: Ty<'tcx>,
5252+
err: &mut DiagnosticBuilder,
5253+
) {
5254+
if let Some(span_semi) = self.could_remove_semicolon(blk, expected_ty) {
5255+
err.span_suggestion_with_applicability(
5256+
span_semi,
5257+
"consider removing this semicolon",
5258+
String::new(),
5259+
Applicability::MachineApplicable,
5260+
);
5261+
}
5262+
}
5263+
5264+
fn could_remove_semicolon(
5265+
&self,
5266+
blk: &'gcx hir::Block,
5267+
expected_ty: Ty<'tcx>,
5268+
) -> Option<Span> {
51635269
// Be helpful when the user wrote `{... expr;}` and
51645270
// taking the `;` off is enough to fix the error.
51655271
let last_stmt = match blk.stmts.last() {
51665272
Some(s) => s,
5167-
None => return,
5273+
None => return None,
51685274
};
51695275
let last_expr = match last_stmt.node {
51705276
hir::StmtKind::Semi(ref e, _) => e,
5171-
_ => return,
5277+
_ => return None,
51725278
};
51735279
let last_expr_ty = self.node_ty(last_expr.hir_id);
51745280
if self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err() {
5175-
return;
5281+
return None;
51765282
}
51775283
let original_span = original_sp(last_stmt.span, blk.span);
5178-
let span_semi = original_span.with_lo(original_span.hi() - BytePos(1));
5179-
err.span_suggestion_with_applicability(
5180-
span_semi,
5181-
"consider removing this semicolon",
5182-
String::new(),
5183-
Applicability::MachineApplicable);
5284+
Some(original_span.with_lo(original_span.hi() - BytePos(1)))
51845285
}
51855286

51865287
// Instantiates the given path, which must refer to an item with the given

src/test/ui/if-else-type-mismatch.rs

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
fn main() {
2+
let _ = if true {
3+
1i32
4+
} else {
5+
2u32
6+
};
7+
//~^^ ERROR if and else have incompatible types
8+
let _ = if true { 42i32 } else { 42u32 };
9+
//~^ ERROR if and else have incompatible types
10+
let _ = if true {
11+
3u32;
12+
} else {
13+
4u32
14+
};
15+
//~^^ ERROR if and else have incompatible types
16+
let _ = if true {
17+
5u32
18+
} else {
19+
6u32;
20+
};
21+
//~^^ ERROR if and else have incompatible types
22+
let _ = if true {
23+
7i32;
24+
} else {
25+
8u32
26+
};
27+
//~^^ ERROR if and else have incompatible types
28+
let _ = if true {
29+
9i32
30+
} else {
31+
10u32;
32+
};
33+
//~^^ ERROR if and else have incompatible types
34+
let _ = if true {
35+
36+
} else {
37+
11u32
38+
};
39+
//~^^ ERROR if and else have incompatible types
40+
let _ = if true {
41+
12i32
42+
} else {
43+
44+
};
45+
//~^^^ ERROR if and else have incompatible types
46+
}

0 commit comments

Comments
 (0)