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 rust-lang#2817

Author: catamorphism

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);
       }
     }
 }

src/librustc/middle/typeck/check/mod.rs

@@ -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
@@ -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<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;
 
@@ -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);
           }

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
     }
 }

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
+    }
+}

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
+    }
+}