Skip to content

Commit

Permalink
Improve error message for breaks in blocks
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alexcrichton committed Nov 12, 2013
1 parent 0cc5e6c commit 5fdbcc4
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 48 deletions.
86 changes: 43 additions & 43 deletions src/librustc/middle/check_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}
}
19 changes: 15 additions & 4 deletions src/test/compile-fail/break-outside-loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
}
2 changes: 1 addition & 1 deletion src/test/compile-fail/return-in-block-function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@

fn main() {
let _x = || {
return //~ ERROR: `return` in block function
return //~ ERROR: `return` in a closure
};
}

9 comments on commit 5fdbcc4

@bors
Copy link
Contributor

@bors bors commented on 5fdbcc4 Nov 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from brson
at alexcrichton@5fdbcc4

@bors
Copy link
Contributor

@bors bors commented on 5fdbcc4 Nov 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging alexcrichton/rust/no-xray = 5fdbcc4 into auto

@bors
Copy link
Contributor

@bors bors commented on 5fdbcc4 Nov 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alexcrichton/rust/no-xray = 5fdbcc4 merged ok, testing candidate = 9a831566

@bors
Copy link
Contributor

@bors bors commented on 5fdbcc4 Nov 12, 2013

@bors
Copy link
Contributor

@bors bors commented on 5fdbcc4 Nov 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from brson
at alexcrichton@5fdbcc4

@bors
Copy link
Contributor

@bors bors commented on 5fdbcc4 Nov 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging alexcrichton/rust/no-xray = 5fdbcc4 into auto

@bors
Copy link
Contributor

@bors bors commented on 5fdbcc4 Nov 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alexcrichton/rust/no-xray = 5fdbcc4 merged ok, testing candidate = 3b0d486

@bors
Copy link
Contributor

@bors bors commented on 5fdbcc4 Nov 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 5fdbcc4 Nov 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 3b0d486

Please sign in to comment.