Skip to content

Improve the error message for missing else clauses in if expressions #18015

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

Merged
merged 1 commit into from Oct 16, 2014
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
62 changes: 33 additions & 29 deletions src/librustc/middle/typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2999,35 +2999,36 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
expected: Expectation) {
check_expr_has_type(fcx, cond_expr, ty::mk_bool());

// Disregard "castable to" expectations because they
// can lead us astray. Consider for example `if cond
// {22} else {c} as u8` -- if we propagate the
// "castable to u8" constraint to 22, it will pick the
// type 22u8, which is overly constrained (c might not
// be a u8). In effect, the problem is that the
// "castable to" expectation is not the tightest thing
// we can say, so we want to drop it in this case.
// The tightest thing we can say is "must unify with
// else branch". Note that in the case of a "has type"
// constraint, this limitation does not hold.

// If the expected type is just a type variable, then don't use
// an expected type. Otherwise, we might write parts of the type
// when checking the 'then' block which are incompatible with the
// 'else' branch.
let expected = match expected.only_has_type() {
ExpectHasType(ety) => {
match infer::resolve_type(fcx.infcx(), Some(sp), ety, force_tvar) {
Ok(rty) if !ty::type_is_ty_var(rty) => ExpectHasType(rty),
_ => NoExpectation
}
}
_ => NoExpectation
};
check_block_with_expected(fcx, then_blk, expected);
let then_ty = fcx.node_ty(then_blk.id);

let branches_ty = match opt_else_expr {
Some(ref else_expr) => {
// Disregard "castable to" expectations because they
// can lead us astray. Consider for example `if cond
// {22} else {c} as u8` -- if we propagate the
// "castable to u8" constraint to 22, it will pick the
// type 22u8, which is overly constrained (c might not
// be a u8). In effect, the problem is that the
// "castable to" expectation is not the tightest thing
// we can say, so we want to drop it in this case.
// The tightest thing we can say is "must unify with
// else branch". Note that in the case of a "has type"
// constraint, this limitation does not hold.

// If the expected type is just a type variable, then don't use
// an expected type. Otherwise, we might write parts of the type
// when checking the 'then' block which are incompatible with the
// 'else' branch.
let expected = match expected.only_has_type() {
ExpectHasType(ety) => {
match infer::resolve_type(fcx.infcx(), Some(sp), ety, force_tvar) {
Ok(rty) if !ty::type_is_ty_var(rty) => ExpectHasType(rty),
_ => NoExpectation
}
}
_ => NoExpectation
};
check_block_with_expected(fcx, then_blk, expected);
let then_ty = fcx.node_ty(then_blk.id);
check_expr_with_expectation(fcx, &**else_expr, expected);
let else_ty = fcx.expr_ty(&**else_expr);
infer::common_supertype(fcx.infcx(),
Expand All @@ -3037,8 +3038,11 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
else_ty)
}
None => {
check_block_no_value(fcx, then_blk);
ty::mk_nil()
infer::common_supertype(fcx.infcx(),
infer::IfExpressionWithNoElse(sp),
false,
then_ty,
ty::mk_nil())
}
};

Expand Down
4 changes: 4 additions & 0 deletions src/librustc/middle/typeck/infer/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ impl<'a, 'tcx> ErrorReporting for InferCtxt<'a, 'tcx> {
infer::RelateOutputImplTypes(_) => "mismatched types",
infer::MatchExpressionArm(_, _) => "match arms have incompatible types",
infer::IfExpression(_) => "if and else have incompatible types",
infer::IfExpressionWithNoElse(_) => "if may be missing an else clause",
};

self.tcx.sess.span_err(
Expand Down Expand Up @@ -1486,6 +1487,9 @@ impl<'a, 'tcx> ErrorReportingHelpers for InferCtxt<'a, 'tcx> {
infer::IfExpression(_) => {
format!("if and else have compatible types")
}
infer::IfExpressionWithNoElse(_) => {
format!("if may be missing an else clause")
}
};

match self.values_str(&trace.values) {
Expand Down
7 changes: 7 additions & 0 deletions src/librustc/middle/typeck/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ pub enum TypeOrigin {

// Computing common supertype in an if expression
IfExpression(Span),

// Computing common supertype of an if expression with no else counter-part
IfExpressionWithNoElse(Span)
}

/// See `error_reporting.rs` for more details
Expand Down Expand Up @@ -1001,6 +1004,7 @@ impl TypeOrigin {
RelateOutputImplTypes(span) => span,
MatchExpressionArm(match_span, _) => match_span,
IfExpression(span) => span,
IfExpressionWithNoElse(span) => span
}
}
}
Expand Down Expand Up @@ -1030,6 +1034,9 @@ impl Repr for TypeOrigin {
IfExpression(a) => {
format!("IfExpression({})", a.repr(tcx))
}
IfExpressionWithNoElse(a) => {
format!("IfExpressionWithNoElse({})", a.repr(tcx))
}
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/test/compile-fail/if-without-else-result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// error-pattern:mismatched types: expected `()`, found `bool`

extern crate debug;

fn main() {
let a = if true { true };
//~^ ERROR if may be missing an else clause: expected `()`, found `bool` (expected (), found bool)
println!("{:?}", a);
}
18 changes: 18 additions & 0 deletions src/test/compile-fail/issue-4201.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn main() {
let a = if true {
0
} else if false {
//~^ ERROR if may be missing an else clause: expected `()`, found `<generic integer #1>`
1
};
}