about summary refs log tree commit diff
diff options
context:
space:
mode:
authorTim Chevalier <chevalier@alum.wellesley.edu>2012-04-19 18:04:41 -0700
committerTim Chevalier <chevalier@alum.wellesley.edu>2012-04-19 18:14:38 -0700
commitcdc8722f9503efdac0c619cfed1958192fe59eb9 (patch)
treea503c1669e4fb8f7191d1245eaf610f9b38b721a
parent594d22e7e24a630957aefa29f9da9a92e3c31587 (diff)
Add a lint pass to check for while true { ... } loops
And suggest changing them to loop { ... }. Had to fix the few
remaining while true loops (in core::io). Closes #1962.
-rw-r--r--src/libcore/io.rs4
-rw-r--r--src/rustc/middle/lint.rs40
-rw-r--r--src/test/compile-fail/issue-1962.rs8
3 files changed, 47 insertions, 5 deletions
diff --git a/src/libcore/io.rs b/src/libcore/io.rs
index 01edaeabe05..b44ec02c74b 100644
--- a/src/libcore/io.rs
+++ b/src/libcore/io.rs
@@ -110,7 +110,7 @@ impl reader_util for reader {
 
     fn read_line() -> str {
         let mut buf: [u8] = [];
-        while true {
+        loop {
             let ch = self.read_byte();
             if ch == -1 || ch == 10 { break; }
             buf += [ch as u8];
@@ -120,7 +120,7 @@ impl reader_util for reader {
 
     fn read_c_str() -> str {
         let mut buf: [u8] = [];
-        while true {
+        loop {
             let ch = self.read_byte();
             if ch < 1 { break; } else { buf += [ch as u8]; }
         }
diff --git a/src/rustc/middle/lint.rs b/src/rustc/middle/lint.rs
index f8820d9f3c0..7adff107f1f 100644
--- a/src/rustc/middle/lint.rs
+++ b/src/rustc/middle/lint.rs
@@ -5,6 +5,7 @@ import syntax::attr;
 import syntax::codemap::span;
 import std::map::{map,hashmap,hash_from_strs};
 import io::writer_util;
+import syntax::print::pprust::expr_to_str;
 
 export lint, ctypes, unused_imports;
 export level, ignore, warn, error;
@@ -12,17 +13,18 @@ export lookup_lint, lint_dict, get_lint_dict, check_crate;
 
 #[doc="
 
-A 'lint' check is a kind of miscallaneous constraint that a user _might_ want
+A 'lint' check is a kind of miscellaneous constraint that a user _might_ want
 to enforce, but might reasonably want to permit as well, on a module-by-module
 basis. They contrast with static constraints enforced by other phases of the
 compiler, which are generally required to hold in order to compile the program
-correctly at all.
+at all.
 
 "]
 
 enum lint {
     ctypes,
     unused_imports,
+    while_true
 }
 
 enum level {
@@ -35,6 +37,10 @@ type lint_spec = @{lint: lint,
 
 type lint_dict = hashmap<str,lint_spec>;
 
+/*
+  Pass names should not contain a '-', as the compiler normalizes
+  '-' to '_' in command-line flags
+ */
 fn get_lint_dict() -> lint_dict {
     let v = [
         ("ctypes",
@@ -45,7 +51,12 @@ fn get_lint_dict() -> lint_dict {
         ("unused_imports",
          @{lint: unused_imports,
            desc: "imports that are never used",
-           default: ignore})
+           default: ignore}),
+
+        ("while_true",
+         @{lint: while_true,
+           desc: "suggest using loop { } instead of while(true) { }",
+           default: warn})
     ];
     hash_from_strs(v)
 }
@@ -165,11 +176,34 @@ fn check_item(cx: ctxt, i: @ast::item) {
             alt lint {
               ctypes { check_item_ctypes(cx, level, i); }
               unused_imports { check_item_unused_imports(cx, level, i); }
+              while_true { check_item_while_true(cx, level, i); }
             }
         }
     }
 }
 
+fn check_item_while_true(cx: ctxt, level: level, it: @ast::item) {
+    let visit = visit::mk_simple_visitor(@{
+        visit_expr: fn@(e: @ast::expr) {
+           alt e.node {
+             ast::expr_while(cond, _) {
+                alt cond.node {
+                    ast::expr_lit(@{node: ast::lit_bool(true),_}) {
+                            cx.span_lint(
+                              level, e.span,
+                              "Denote infinite loops with loop { ... }");
+                    }
+                    _ {}
+                }
+             }
+             _ {}
+          }
+        }
+        with *visit::default_simple_visitor()
+    });
+    visit::visit_item(it, (), visit);
+}
+
 fn check_item_unused_imports(_cx: ctxt, _level: level, _it: @ast::item) {
     // FIXME: Don't know how to check this in lint yet, it's currently being
     // done over in resolve. When resolve is rewritten, do it here instead.
diff --git a/src/test/compile-fail/issue-1962.rs b/src/test/compile-fail/issue-1962.rs
new file mode 100644
index 00000000000..d80906584de
--- /dev/null
+++ b/src/test/compile-fail/issue-1962.rs
@@ -0,0 +1,8 @@
+// compile-flags: -W err-while-true
+fn main() {
+  let mut i = 0;
+  while true  { //! ERROR Denote infinite loops with loop
+    i += 1;
+    if i == 5 { break; }
+  }
+}