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

Print out a more helpful type error message for do-blocks/for-loops #4141

Closed
Closed
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: 8 additions & 1 deletion src/librustc/middle/typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,20 @@ use check::fn_ctxt;
// don't.
fn suptype(fcx: @fn_ctxt, sp: span,
expected: ty::t, actual: ty::t) {
suptype_with_fn(fcx, sp, expected, actual,
|sp, e, a, s| { fcx.report_mismatched_types(sp, e, a, s) })
}

fn suptype_with_fn(fcx: @fn_ctxt, sp: span,
expected: ty::t, actual: ty::t,
handle_err: fn(span, ty::t, ty::t, &ty::type_err)) {

// n.b.: order of actual, expected is reversed
match infer::mk_subty(fcx.infcx(), false, sp,
actual, expected) {
result::Ok(()) => { /* ok */ }
result::Err(ref err) => {
fcx.report_mismatched_types(sp, expected, actual, err);
handle_err(sp, expected, actual, err);
}
}
}
Expand Down
60 changes: 44 additions & 16 deletions src/librustc/middle/typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ struct inherited {
adjustments: HashMap<ast::node_id, @ty::AutoAdjustment>
}

enum FnKind { ForLoop, DoBlock, Vanilla }

struct fn_ctxt {
// var_bindings, locals and next_var_id are shared
// with any nested functions that capture the environment
Expand All @@ -158,6 +160,11 @@ struct fn_ctxt {
// can actually be made to live as long as it needs to live.
mut region_lb: ast::node_id,

// Says whether we're inside a for loop, in a do block
// or neither. Helps with error messages involving the
// function return type.
fn_kind: FnKind,

in_scope_regions: isr_alist,

inh: @inherited,
Expand Down Expand Up @@ -187,6 +194,7 @@ fn blank_fn_ctxt(ccx: @crate_ctxt, rty: ty::t,
purity: ast::pure_fn,
mut region_lb: region_bnd,
in_scope_regions: @Nil,
fn_kind: Vanilla,
inh: blank_inherited(ccx),
ccx: ccx
}
Expand All @@ -208,7 +216,7 @@ fn check_bare_fn(ccx: @crate_ctxt,
let fty = ty::node_id_to_type(ccx.tcx, id);
match ty::get(fty).sty {
ty::ty_fn(ref fn_ty) => {
check_fn(ccx, self_info, fn_ty, decl, body, false, None)
check_fn(ccx, self_info, fn_ty, decl, body, Vanilla, None)
}
_ => ccx.tcx.sess.impossible_case(body.span,
"check_bare_fn: function type expected")
Expand All @@ -220,10 +228,13 @@ fn check_fn(ccx: @crate_ctxt,
fn_ty: &ty::FnTy,
decl: ast::fn_decl,
body: ast::blk,
indirect_ret: bool,
fn_kind: FnKind,
old_fcx: Option<@fn_ctxt>) {

let tcx = ccx.tcx;
let indirect_ret = match fn_kind {
ForLoop => true, _ => false
};

// ______________________________________________________________________
// First, we have to replace any bound regions in the fn and self
Expand Down Expand Up @@ -276,6 +287,7 @@ fn check_fn(ccx: @crate_ctxt,
purity: purity,
mut region_lb: body.node.id,
in_scope_regions: isr,
fn_kind: fn_kind,
inh: inherited,
ccx: ccx
}
Expand Down Expand Up @@ -304,7 +316,11 @@ fn check_fn(ccx: @crate_ctxt,
match body.node.expr {
Some(tail_expr) => {
let tail_expr_ty = fcx.expr_ty(tail_expr);
demand::suptype(fcx, tail_expr.span, fcx.ret_ty, tail_expr_ty);
// Special case: we print a special error if there appears
// to be do-block/for-loop confusion
demand::suptype_with_fn(fcx, tail_expr.span, fcx.ret_ty, tail_expr_ty,
|sp, e, a, s| {
fcx.report_mismatched_return_types(sp, e, a, s) });
}
None => ()
}
Expand Down Expand Up @@ -774,11 +790,27 @@ impl @fn_ctxt {
self.infcx().type_error_message(sp, mk_msg, actual_ty, err);
}

fn report_mismatched_types(sp: span, e: ty::t, a: ty::t,
fn report_mismatched_return_types(sp: span, e: ty::t, a: ty::t,
err: &ty::type_err) {
self.infcx().report_mismatched_types(sp, e, a, err);
match self.fn_kind {
ForLoop if ty::type_is_nil(e) && ty::type_is_bool(a) =>
self.tcx().sess.span_err(sp, fmt!("A for-loop body must \
return (), but it returns %s here. \
Perhaps you meant to write a `do`-block?",
ty_to_str(self.tcx(), a))),
DoBlock if ty::type_is_nil(a) =>
// If we expected bool and got ()...
self.tcx().sess.span_err(sp, fmt!("Do-block body must \
return %s, but returns () here. Perhaps you meant \
to write a `for`-loop?", ty_to_str(self.tcx(), e))),
_ => self.infcx().report_mismatched_types(sp, e, a, err)
}
}

fn report_mismatched_types(sp: span, e: ty::t, a: ty::t,
err: &ty::type_err) {
self.infcx().report_mismatched_types(sp, e, a, err)
}
}

fn do_autoderef(fcx: @fn_ctxt, sp: span, t: ty::t) -> (ty::t, uint) {
Expand Down Expand Up @@ -1087,9 +1119,9 @@ fn check_expr_with_unifier(fcx: @fn_ctxt,
DontDerefArgs => {}
}

// mismatch error happens in here
bot |= check_expr_with_assignability(fcx,
*arg, formal_ty);
fcx.write_ty(arg.id, fcx.expr_ty(*arg));

}
}
Expand Down Expand Up @@ -1414,7 +1446,7 @@ fn check_expr_with_unifier(fcx: @fn_ctxt,
ast_proto_opt: Option<ast::Proto>,
decl: ast::fn_decl,
body: ast::blk,
is_loop_body: bool,
fn_kind: FnKind,
expected: Option<ty::t>) {
let tcx = fcx.ccx.tcx;

Expand Down Expand Up @@ -1473,7 +1505,7 @@ fn check_expr_with_unifier(fcx: @fn_ctxt,
fcx.write_ty(expr.id, fty);

check_fn(fcx.ccx, None, &fn_ty, decl, body,
is_loop_body, Some(fcx));
fn_kind, Some(fcx));
}


Expand Down Expand Up @@ -2041,14 +2073,12 @@ fn check_expr_with_unifier(fcx: @fn_ctxt,
}
ast::expr_fn(proto, decl, ref body, cap_clause) => {
check_expr_fn(fcx, expr, Some(proto),
decl, (*body), false,
expected);
decl, (*body), Vanilla, expected);
capture::check_capture_clause(tcx, expr.id, cap_clause);
}
ast::expr_fn_block(decl, ref body, cap_clause) => {
check_expr_fn(fcx, expr, None,
decl, (*body), false,
expected);
decl, (*body), Vanilla, expected);
capture::check_capture_clause(tcx, expr.id, cap_clause);
}
ast::expr_loop_body(b) => {
Expand Down Expand Up @@ -2101,8 +2131,7 @@ fn check_expr_with_unifier(fcx: @fn_ctxt,
match b.node {
ast::expr_fn_block(decl, ref body, cap_clause) => {
check_expr_fn(fcx, b, None,
decl, (*body), true,
Some(inner_ty));
decl, (*body), ForLoop, Some(inner_ty));
demand::suptype(fcx, b.span, inner_ty, fcx.expr_ty(b));
capture::check_capture_clause(tcx, b.id, cap_clause);
}
Expand Down Expand Up @@ -2145,8 +2174,7 @@ fn check_expr_with_unifier(fcx: @fn_ctxt,
match b.node {
ast::expr_fn_block(decl, ref body, cap_clause) => {
check_expr_fn(fcx, b, None,
decl, (*body), true,
Some(inner_ty));
decl, (*body), DoBlock, Some(inner_ty));
demand::suptype(fcx, b.span, inner_ty, fcx.expr_ty(b));
capture::check_capture_clause(tcx, b.id, cap_clause);
}
Expand Down
4 changes: 1 addition & 3 deletions src/test/compile-fail/block-must-not-have-result-for.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// error-pattern:mismatched types: expected `()` but found `bool`

fn main() {
for vec::each(~[0]) |_i| {
for vec::each(~[0]) |_i| { //~ ERROR A for-loop body must return (), but
true
}
}
11 changes: 11 additions & 0 deletions src/test/compile-fail/issue-2817-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
fn uuid() -> uint { fail; }

fn from_str(s: ~str) -> uint { fail; }
fn to_str(u: uint) -> ~str { fail; }
fn uuid_random() -> uint { fail; }

fn main() {
for uint::range(0, 100000) |_i| { //~ ERROR A for-loop body must return (), but
false
}
}
10 changes: 10 additions & 0 deletions src/test/compile-fail/issue-2817.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
fn uuid() -> uint { fail; }

fn from_str(s: ~str) -> uint { fail; }
fn to_str(u: uint) -> ~str { fail; }
fn uuid_random() -> uint { fail; }

fn main() {
do uint::range(0, 100000) |_i| { //~ ERROR Do-block body must return bool, but
}
}