From 8bbd8b0b9263718b4c0c6c86b5ab2fb038aa3f5b Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 7 Mar 2016 23:24:11 +0100 Subject: [PATCH 1/2] Fix ICE in for_loop with globals --- src/loops.rs | 25 ++++++++++++++++++------- tests/compile-fail/for_loop.rs | 15 +++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/loops.rs b/src/loops.rs index e41402ef8..462ca7c49 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -345,9 +345,11 @@ 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 - let pat_extent = cx.tcx.region_maps.var_scope(pat.id); - if cx.tcx.region_maps.is_subscope_of(indexed_extent, pat_extent) { - return; + 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, // indexed variables + indexed: HashMap>, // indexed variables, the extent is None for global nonindex: bool, // has the var been used otherwise? } @@ -687,9 +689,18 @@ 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) { - let extent = self.cx.tcx.region_maps.var_scope(def.base_def.var_id()); - self.indexed.insert(seqvar.segments[0].identifier.name, extent); - return; // no need to walk further + 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, 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 + } + _ => (), + } } } } diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index 0853ae83c..bbdf9d8f1 100644 --- a/tests/compile-fail/for_loop.rs +++ b/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); From 55a584ac84387cf56bf2a4af05ecd2643cce5980 Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 7 Mar 2016 23:24:49 +0100 Subject: [PATCH 2/2] Bump to 0.0.48 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 5fede91ea..7f19858ff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy" -version = "0.0.47" +version = "0.0.48" authors = [ "Manish Goregaokar ", "Andre Bogus ",