Merge pull request #747 from mcarton/fix-ice

Fix ICE in for_loop with globals

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "clippy"
-version = "0.0.47"
+version = "0.0.48"
 authors = [
 	"Manish Goregaokar <manishsmail@gmail.com>",
 	"Andre Bogus <bogusandre@gmail.com>",

src/loops.rs

@@ -345,10 +345,12 @@ fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, ex
                                                        .unwrap_or_else(|| unreachable!() /* len == 1 */);
 
                 // ensure that the indexed variable was declared before the loop, see #601
+                if let Some(indexed_extent) = indexed_extent {
                     let pat_extent = cx.tcx.region_maps.var_scope(pat.id);
                     if cx.tcx.region_maps.is_subscope_of(indexed_extent, pat_extent) {
                         return;
                     }
+                }
 
                 let starts_at_zero = is_integer_literal(start, 0);
 
@@ -669,7 +671,7 @@ fn recover_for_loop(expr: &Expr) -> Option<(&Pat, &Expr, &Expr)> {
 struct VarVisitor<'v, 't: 'v> {
     cx: &'v LateContext<'v, 't>, // context reference
     var: Name, // var name to look for as index
-    indexed: HashMap<Name, CodeExtent>, // indexed variables
+    indexed: HashMap<Name, Option<CodeExtent>>, // indexed variables, the extent is None for global
     nonindex: bool, // has the var been used otherwise?
 }
 
@@ -687,10 +689,19 @@ impl<'v, 't> Visitor<'v> for VarVisitor<'v, 't> {
                     ], {
                         let def_map = self.cx.tcx.def_map.borrow();
                         if let Some(def) = def_map.get(&seqexpr.id) {
+                            match def.base_def {
+                                Def::Local(..) | Def::Upvar(..) => {
                                     let extent = self.cx.tcx.region_maps.var_scope(def.base_def.var_id());
-                            self.indexed.insert(seqvar.segments[0].identifier.name, extent);
+                                    self.indexed.insert(seqvar.segments[0].identifier.name, Some(extent));
                                     return;  // no need to walk further
                                 }
+                                Def::Static(..) | Def::Const(..) => {
+                                    self.indexed.insert(seqvar.segments[0].identifier.name, None);
+                                    return;  // no need to walk further
+                                }
+                                _ => (),
+                            }
+                        }
                     }
                 }
                 // we are not indexing anything, record that

tests/compile-fail/for_loop.rs

@@ -3,6 +3,9 @@
 
 use std::collections::*;
 
+static STATIC: [usize; 4] = [ 0,  1,  8, 16 ];
+const CONST: [usize; 4] = [ 0,  1,  8, 16 ];
+
 #[deny(clippy)]
 fn for_loop_over_option_and_result() {
     let option = Some(1);
@@ -95,6 +98,18 @@ fn main() {
         //~^ ERROR `i` is only used to index `vec`. Consider using `for item in &vec`
         println!("{}", vec[i]);
     }
+
+    // ICE #746
+    for j in 0..4 {
+        //~^ ERROR `j` is only used to index `STATIC`
+        println!("{:?}", STATIC[j]);
+    }
+
+    for j in 0..4 {
+        //~^ ERROR `j` is only used to index `CONST`
+        println!("{:?}", CONST[j]);
+    }
+
     for i in 0..vec.len() {
         //~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate()`
         println!("{} {}", vec[i], i);