From fb2aeafba012c3df7e8c9cc9d4fbf1b29e492547 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Fri, 7 Dec 2012 21:14:20 -0800 Subject: [PATCH] Print out a more helpful type error message for do-blocks/for-loops If a do-block body has the wrong type, or a for-loop body has a non-() type, suggest that the user might have meant the other one. As per #2817 --- src/librustc/middle/typeck/check/demand.rs | 9 ++- src/librustc/middle/typeck/check/mod.rs | 60 ++++++++++++++----- .../block-must-not-have-result-for.rs | 4 +- src/test/compile-fail/issue-2817-2.rs | 11 ++++ src/test/compile-fail/issue-2817.rs | 10 ++++ 5 files changed, 74 insertions(+), 20 deletions(-) create mode 100644 src/test/compile-fail/issue-2817-2.rs create mode 100644 src/test/compile-fail/issue-2817.rs diff --git a/src/librustc/middle/typeck/check/demand.rs b/src/librustc/middle/typeck/check/demand.rs index ee34ce64612a3..a7e1da6ed7df7 100644 --- a/src/librustc/middle/typeck/check/demand.rs +++ b/src/librustc/middle/typeck/check/demand.rs @@ -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); } } } diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index db16d3d15a421..3ef7c011f2201 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -133,6 +133,8 @@ struct inherited { adjustments: HashMap } +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 @@ -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, @@ -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 } @@ -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") @@ -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 @@ -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 } @@ -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 => () } @@ -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) { @@ -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)); } } @@ -1414,7 +1446,7 @@ fn check_expr_with_unifier(fcx: @fn_ctxt, ast_proto_opt: Option, decl: ast::fn_decl, body: ast::blk, - is_loop_body: bool, + fn_kind: FnKind, expected: Option) { let tcx = fcx.ccx.tcx; @@ -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)); } @@ -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) => { @@ -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); } @@ -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); } diff --git a/src/test/compile-fail/block-must-not-have-result-for.rs b/src/test/compile-fail/block-must-not-have-result-for.rs index 41a2182ba2b26..ebede037fdfc4 100644 --- a/src/test/compile-fail/block-must-not-have-result-for.rs +++ b/src/test/compile-fail/block-must-not-have-result-for.rs @@ -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 } } \ No newline at end of file diff --git a/src/test/compile-fail/issue-2817-2.rs b/src/test/compile-fail/issue-2817-2.rs new file mode 100644 index 0000000000000..aa9bfc826d2d6 --- /dev/null +++ b/src/test/compile-fail/issue-2817-2.rs @@ -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 + } +} \ No newline at end of file diff --git a/src/test/compile-fail/issue-2817.rs b/src/test/compile-fail/issue-2817.rs new file mode 100644 index 0000000000000..ea41f29dcdc67 --- /dev/null +++ b/src/test/compile-fail/issue-2817.rs @@ -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 + } +} \ No newline at end of file