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

Tweak output of type mismatch between "then" and else if arms #57381

Merged
merged 2 commits into from
Jan 14, 2019
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
16 changes: 14 additions & 2 deletions src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,18 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
}
},
ObligationCauseCode::IfExpression { then, outer, semicolon } => {
err.span_label(then, "expected because of this");
outer.map(|sp| err.span_label(sp, "if and else have incompatible types"));
if let Some(sp) = semicolon {
err.span_suggestion_short_with_applicability(
sp,
"consider removing this semicolon",
String::new(),
Applicability::MachineApplicable,
);
}
}
_ => (),
}
}
Expand Down Expand Up @@ -1460,7 +1472,7 @@ impl<'tcx> ObligationCause<'tcx> {
}
_ => "match arms have incompatible types",
}),
IfExpression => Error0308("if and else have incompatible types"),
IfExpression { .. } => Error0308("if and else have incompatible types"),
IfExpressionWithNoElse => Error0317("if may be missing an else clause"),
MainFunctionType => Error0580("main function has wrong type"),
StartFunctionType => Error0308("start function has wrong type"),
Expand Down Expand Up @@ -1488,7 +1500,7 @@ impl<'tcx> ObligationCause<'tcx> {
hir::MatchSource::IfLetDesugar { .. } => "`if let` arms have compatible types",
_ => "match arms have compatible types",
},
IfExpression => "if and else have compatible types",
IfExpression { .. } => "if and else have compatible types",
IfExpressionWithNoElse => "if missing an else returns ()",
MainFunctionType => "`main` function has the correct type",
StartFunctionType => "`start` function has the correct type",
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
ObligationCauseCode::ExprAssignable |
ObligationCauseCode::MatchExpressionArm { .. } |
ObligationCauseCode::MatchExpressionArmPattern { .. } |
ObligationCauseCode::IfExpression |
ObligationCauseCode::IfExpression { .. } |
ObligationCauseCode::IfExpressionWithNoElse |
ObligationCauseCode::MainFunctionType |
ObligationCauseCode::StartFunctionType |
Expand Down
6 changes: 5 additions & 1 deletion src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,11 @@ pub enum ObligationCauseCode<'tcx> {
MatchExpressionArmPattern { span: Span, ty: Ty<'tcx> },

/// Computing common supertype in an if expression
IfExpression,
IfExpression {
then: Span,
outer: Option<Span>,
semicolon: Option<Span>,
},

/// Computing common supertype of an if expression with no else counter-part
IfExpressionWithNoElse,
Expand Down
6 changes: 5 additions & 1 deletion src/librustc/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,11 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
super::MatchExpressionArmPattern { span, ty } => {
tcx.lift(&ty).map(|ty| super::MatchExpressionArmPattern { span, ty })
}
super::IfExpression => Some(super::IfExpression),
super::IfExpression { then, outer, semicolon } => Some(super::IfExpression {
then,
outer,
semicolon,
}),
super::IfExpressionWithNoElse => Some(super::IfExpressionWithNoElse),
super::MainFunctionType => Some(super::MainFunctionType),
super::StartFunctionType => Some(super::StartFunctionType),
Expand Down
133 changes: 117 additions & 16 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3366,13 +3366,103 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
let coerce_to_ty = expected.coercion_target_type(self, sp);
let mut coerce: DynamicCoerceMany = CoerceMany::new(coerce_to_ty);

let if_cause = self.cause(sp, ObligationCauseCode::IfExpression);
coerce.coerce(self, &if_cause, then_expr, then_ty);
coerce.coerce(self, &self.misc(sp), then_expr, then_ty);

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

let mut outer_sp = if self.tcx.sess.source_map().is_multiline(sp) {
// The `if`/`else` isn't in one line in the output, include some context to make it
// clear it is an if/else expression:
// ```
// LL | let x = if true {
// | _____________-
// LL || 10i32
// || ----- expected because of this
// LL || } else {
// LL || 10u32
// || ^^^^^ expected i32, found u32
// LL || };
// ||_____- if and else have incompatible types
// ```
Some(sp)
} else {
// The entire expression is in one line, only point at the arms
// ```
// LL | let x = if true { 10i32 } else { 10u32 };
// | ----- ^^^^^ expected i32, found u32
// | |
// | expected because of this
// ```
None
};
let mut remove_semicolon = None;
let error_sp = if let ExprKind::Block(block, _) = &else_expr.node {
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
// Avoid overlapping spans that aren't as readable:
// ```
// 2 | let x = if true {
// | _____________-
// 3 | | 3
// | | - expected because of this
// 4 | | } else {
// | |____________^
// 5 | ||
// 6 | || };
// | || ^
// | ||_____|
// | |______if and else have incompatible types
// | expected integer, found ()
// ```
// by not pointing at the entire expression:
// ```
// 2 | let x = if true {
// | ------- if and else have incompatible types
// 3 | 3
// | - expected because of this
// 4 | } else {
// | ____________^
// 5 | |
// 6 | | };
// | |_____^ expected integer, found ()
// ```
if outer_sp.is_some() {
outer_sp = Some(self.tcx.sess.source_map().def_span(sp));
}
else_expr.span
}
} else { // shouldn't happen unless the parser has done something weird
else_expr.span
};
let then_sp = if let ExprKind::Block(block, _) = &then_expr.node {
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
outer_sp = None; // same as in `error_sp`, cleanup output
then_expr.span
}
} else { // shouldn't happen unless the parser has done something weird
then_expr.span
};

let if_cause = self.cause(error_sp, ObligationCauseCode::IfExpression {
then: then_sp,
outer: outer_sp,
semicolon: remove_semicolon,
});

coerce.coerce(self, &if_cause, else_expr, else_ty);

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


/// A common error is to add an extra semicolon:
///
/// ```
Expand All @@ -5156,31 +5245,43 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
/// This routine checks if the final statement in a block is an
/// expression with an explicit semicolon whose type is compatible
/// with `expected_ty`. If so, it suggests removing the semicolon.
fn consider_hint_about_removing_semicolon(&self,
blk: &'gcx hir::Block,
expected_ty: Ty<'tcx>,
err: &mut DiagnosticBuilder) {
fn consider_hint_about_removing_semicolon(
&self,
blk: &'gcx hir::Block,
expected_ty: Ty<'tcx>,
err: &mut DiagnosticBuilder,
) {
if let Some(span_semi) = self.could_remove_semicolon(blk, expected_ty) {
err.span_suggestion_with_applicability(
span_semi,
"consider removing this semicolon",
String::new(),
Applicability::MachineApplicable,
);
}
}

fn could_remove_semicolon(
&self,
blk: &'gcx hir::Block,
expected_ty: Ty<'tcx>,
) -> Option<Span> {
// Be helpful when the user wrote `{... expr;}` and
// taking the `;` off is enough to fix the error.
let last_stmt = match blk.stmts.last() {
Some(s) => s,
None => return,
None => return None,
};
let last_expr = match last_stmt.node {
hir::StmtKind::Semi(ref e, _) => e,
_ => return,
_ => return None,
};
let last_expr_ty = self.node_ty(last_expr.hir_id);
if self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err() {
return;
return None;
}
let original_span = original_sp(last_stmt.span, blk.span);
let span_semi = original_span.with_lo(original_span.hi() - BytePos(1));
err.span_suggestion_with_applicability(
span_semi,
"consider removing this semicolon",
String::new(),
Applicability::MachineApplicable);
Some(original_span.with_lo(original_span.hi() - BytePos(1)))
}

// Instantiates the given path, which must refer to an item with the given
Expand Down
46 changes: 46 additions & 0 deletions src/test/ui/if-else-type-mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
fn main() {
let _ = if true {
1i32
} else {
2u32
};
//~^^ ERROR if and else have incompatible types
let _ = if true { 42i32 } else { 42u32 };
//~^ ERROR if and else have incompatible types
let _ = if true {
3u32;
} else {
4u32
};
//~^^ ERROR if and else have incompatible types
let _ = if true {
5u32
} else {
6u32;
};
//~^^ ERROR if and else have incompatible types
let _ = if true {
7i32;
} else {
8u32
};
//~^^ ERROR if and else have incompatible types
let _ = if true {
9i32
} else {
10u32;
};
//~^^ ERROR if and else have incompatible types
let _ = if true {

} else {
11u32
};
//~^^ ERROR if and else have incompatible types
let _ = if true {
12i32
} else {

};
//~^^^ ERROR if and else have incompatible types
}
Loading