From 5fdbcc4020a440e18a3c570ffad5d2bb089c08db Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Mon, 11 Nov 2013 11:29:15 -0800
Subject: [PATCH] Improve error message for breaks in blocks

Before it was always stated that it was a "break outside of a loop" when you
could very well be in a loop, but just in a block instead.

Closes #3064
---
 src/librustc/middle/check_loop.rs             | 86 +++++++++----------
 src/test/compile-fail/break-outside-loop.rs   | 19 +++-
 .../compile-fail/return-in-block-function.rs  |  2 +-
 3 files changed, 59 insertions(+), 48 deletions(-)

diff --git a/src/librustc/middle/check_loop.rs b/src/librustc/middle/check_loop.rs
index a6aab151e5ad9..8a320f88649a0 100644
--- a/src/librustc/middle/check_loop.rs
+++ b/src/librustc/middle/check_loop.rs
@@ -8,68 +8,68 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-
 use middle::ty;
 
-use syntax::ast::*;
-use syntax::visit;
+use syntax::ast;
+use syntax::codemap::Span;
 use syntax::visit::Visitor;
+use syntax::visit;
 
-#[deriving(Clone)]
-pub struct Context {
-    in_loop: bool,
-    can_ret: bool
+#[deriving(Clone, Eq)]
+enum Context {
+    Normal, Loop, Closure
 }
 
 struct CheckLoopVisitor {
     tcx: ty::ctxt,
 }
 
-pub fn check_crate(tcx: ty::ctxt, crate: &Crate) {
-    visit::walk_crate(&mut CheckLoopVisitor { tcx: tcx },
-                      crate,
-                      Context { in_loop: false, can_ret: true });
+pub fn check_crate(tcx: ty::ctxt, crate: &ast::Crate) {
+    visit::walk_crate(&mut CheckLoopVisitor { tcx: tcx }, crate, Normal)
 }
 
 impl Visitor<Context> for CheckLoopVisitor {
-    fn visit_item(&mut self, i:@item, _cx:Context) {
-        visit::walk_item(self, i, Context {
-                                    in_loop: false,
-                                    can_ret: true
-                                  });
+    fn visit_item(&mut self, i: @ast::item, _cx: Context) {
+        visit::walk_item(self, i, Normal);
     }
 
-    fn visit_expr(&mut self, e:@Expr, cx:Context) {
-
-            match e.node {
-              ExprWhile(e, ref b) => {
+    fn visit_expr(&mut self, e: @ast::Expr, cx:Context) {
+        match e.node {
+            ast::ExprWhile(e, ref b) => {
                 self.visit_expr(e, cx);
-                self.visit_block(b, Context { in_loop: true,.. cx });
-              }
-              ExprLoop(ref b, _) => {
-                self.visit_block(b, Context { in_loop: true,.. cx });
-              }
-              ExprFnBlock(_, ref b) | ExprProc(_, ref b) => {
-                self.visit_block(b, Context { in_loop: false, can_ret: false });
-              }
-              ExprBreak(_) => {
-                if !cx.in_loop {
-                    self.tcx.sess.span_err(e.span, "`break` outside of loop");
-                }
-              }
-              ExprAgain(_) => {
-                if !cx.in_loop {
-                    self.tcx.sess.span_err(e.span, "`loop` outside of loop");
-                }
-              }
-              ExprRet(oe) => {
-                if !cx.can_ret {
-                    self.tcx.sess.span_err(e.span, "`return` in block function");
+                self.visit_block(b, Loop);
+            }
+            ast::ExprLoop(ref b, _) => {
+                self.visit_block(b, Loop);
+            }
+            ast::ExprFnBlock(_, ref b) | ast::ExprProc(_, ref b) => {
+                self.visit_block(b, Closure);
+            }
+            ast::ExprBreak(_) => self.require_loop("break", cx, e.span),
+            ast::ExprAgain(_) => self.require_loop("continue", cx, e.span),
+            ast::ExprRet(oe) => {
+                if cx == Closure {
+                    self.tcx.sess.span_err(e.span, "`return` in a closure");
                 }
                 visit::walk_expr_opt(self, oe, cx);
-              }
-              _ => visit::walk_expr(self, e, cx)
             }
+            _ => visit::walk_expr(self, e, cx)
+        }
+    }
+}
 
+impl CheckLoopVisitor {
+    fn require_loop(&self, name: &str, cx: Context, span: Span) {
+        match cx {
+            Loop => {}
+            Closure => {
+                self.tcx.sess.span_err(span, format!("`{}` inside of a closure",
+                                                     name));
+            }
+            Normal => {
+                self.tcx.sess.span_err(span, format!("`{}` outside of loop",
+                                                     name));
+            }
+        }
     }
 }
diff --git a/src/test/compile-fail/break-outside-loop.rs b/src/test/compile-fail/break-outside-loop.rs
index b3154c9742aca..06281a5e2884b 100644
--- a/src/test/compile-fail/break-outside-loop.rs
+++ b/src/test/compile-fail/break-outside-loop.rs
@@ -8,15 +8,26 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// error-pattern:`break` outside of loop
-
 struct Foo {
     t: ~str
 }
 
+fn cond() -> bool { true }
+
+fn foo(_: ||) {}
+
 fn main() {
-    let pth = break;
+    let pth = break; //~ ERROR: `break` outside of loop
+    if cond() { continue } //~ ERROR: `continue` outside of loop
 
-    let rs: Foo = Foo{t: pth};
+    while cond() {
+        if cond() { break }
+        if cond() { continue }
+        do foo {
+            if cond() { break } //~ ERROR: `break` inside of a closure
+            if cond() { continue } //~ ERROR: `continue` inside of a closure
+        }
+    }
 
+    let rs: Foo = Foo{t: pth};
 }
diff --git a/src/test/compile-fail/return-in-block-function.rs b/src/test/compile-fail/return-in-block-function.rs
index 75a72f97204ef..f231810cbf117 100644
--- a/src/test/compile-fail/return-in-block-function.rs
+++ b/src/test/compile-fail/return-in-block-function.rs
@@ -10,6 +10,6 @@
 
 fn main() {
     let _x = || {
-        return //~ ERROR: `return` in block function
+        return //~ ERROR: `return` in a closure
     };
 }