From 34812e82d066dc1b3ef89df4272300662374f907 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sun, 7 Feb 2016 18:10:03 +0100 Subject: [PATCH] Use const_eval in loops --- src/loops.rs | 45 ++++++++++++++++++++++------------ tests/compile-fail/for_loop.rs | 22 ++++++++++++++--- 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/loops.rs b/src/loops.rs index 4620d6a3e818..cecf47daf556 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -1,11 +1,12 @@ -use rustc::lint::*; -use rustc_front::hir::*; use reexport::*; -use rustc_front::intravisit::{Visitor, walk_expr, walk_block, walk_decl}; -use rustc::middle::ty; -use rustc::middle::def::Def; -use consts::{constant_simple, Constant}; use rustc::front::map::Node::NodeBlock; +use rustc::lint::*; +use rustc::middle::const_eval::EvalHint::ExprTypeChecked; +use rustc::middle::const_eval::{ConstVal, eval_const_expr_partial}; +use rustc::middle::def::Def; +use rustc::middle::ty; +use rustc_front::hir::*; +use rustc_front::intravisit::{Visitor, walk_expr, walk_block, walk_decl}; use std::borrow::Cow; use std::collections::{HashSet, HashMap}; @@ -421,22 +422,36 @@ fn check_for_loop_reverse_range(cx: &LateContext, arg: &Expr, expr: &Expr) { // if this for loop is iterating over a two-sided range... if let ExprRange(Some(ref start_expr), Some(ref stop_expr)) = arg.node { // ...and both sides are compile-time constant integers... - if let Some(start_idx @ Constant::Int(..)) = constant_simple(start_expr) { - if let Some(stop_idx @ Constant::Int(..)) = constant_simple(stop_expr) { + if let Ok(start_idx) = eval_const_expr_partial(&cx.tcx, start_expr, ExprTypeChecked, None) { + if let Ok(stop_idx) = eval_const_expr_partial(&cx.tcx, stop_expr, ExprTypeChecked, None) { // ...and the start index is greater than the stop index, // this loop will never run. This is often confusing for developers // who think that this will iterate from the larger value to the // smaller value. - if start_idx > stop_idx { - span_help_and_lint(cx, + let (sup, eq) = match (start_idx, stop_idx) { + (ConstVal::Int(start_idx), ConstVal::Int(stop_idx)) => (start_idx > stop_idx, start_idx == stop_idx), + (ConstVal::Uint(start_idx), ConstVal::Uint(stop_idx)) => (start_idx > stop_idx, start_idx == stop_idx), + _ => (false, false), + }; + + if sup { + let start_snippet = snippet(cx, start_expr.span, "_"); + let stop_snippet = snippet(cx, stop_expr.span, "_"); + + span_lint_and_then(cx, REVERSE_RANGE_LOOP, expr.span, "this range is empty so this for loop will never run", - &format!("Consider using `({}..{}).rev()` if you are attempting to iterate \ - over this range in reverse", - stop_idx, - start_idx)); - } else if start_idx == stop_idx { + |db| { + db.span_suggestion(expr.span, + "consider using the following if \ + you are attempting to iterate \ + over this range in reverse", + format!("({}..{}).rev()` ", + stop_snippet, + start_snippet)); + }); + } else if eq { // if they are equal, it's also problematic - this loop // will never run. span_lint(cx, diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index e361ebe777f4..4609c840836c 100644 --- a/tests/compile-fail/for_loop.rs +++ b/tests/compile-fail/for_loop.rs @@ -65,7 +65,7 @@ fn for_loop_over_option_and_result() { break; } - // while let false positive for Option + // while let false positive for Result while let Ok(x) = result { println!("{}", x); break; @@ -85,8 +85,10 @@ impl Unrelated { #[deny(needless_range_loop, explicit_iter_loop, iter_next_loop, reverse_range_loop, explicit_counter_loop)] #[deny(unused_collect)] -#[allow(linkedlist,shadow_unrelated,unnecessary_mut_passed, cyclomatic_complexity)] +#[allow(linkedlist, shadow_unrelated, unnecessary_mut_passed, cyclomatic_complexity)] fn main() { + const MAX_LEN: usize = 42; + let mut vec = vec![1, 2, 3, 4]; let vec2 = vec![1, 2, 3, 4]; for i in 0..vec.len() { @@ -111,6 +113,11 @@ fn main() { println!("{}", vec[i]); } + for i in 0..MAX_LEN { + //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(MAX_LEN)` + println!("{}", vec[i]); + } + for i in 5..10 { //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(10).skip(5)` println!("{}", vec[i]); @@ -126,7 +133,16 @@ fn main() { println!("{} {}", vec[i], i); } - for i in 10..0 { //~ERROR this range is empty so this for loop will never run + for i in 10..0 { + //~^ERROR this range is empty so this for loop will never run + //~|HELP consider + //~|SUGGESTION (0..10).rev() + println!("{}", i); + } + + for i in MAX_LEN..0 { //~ERROR this range is empty so this for loop will never run + //~|HELP consider + //~|SUGGESTION (0..MAX_LEN).rev() println!("{}", i); }