Check for known array length in needless_range_loop

This commit is contained in:
HMPerson1
2018-10-19 16:34:16 -04:00
parent b1d0343749
commit 2e9172aea2
3 changed files with 79 additions and 7 deletions

View File

@@ -1068,7 +1068,7 @@ fn check_for_loop_range<'a, 'tcx>(
// linting condition: we only indexed one variable, and indexed it directly // linting condition: we only indexed one variable, and indexed it directly
if visitor.indexed_indirectly.is_empty() && visitor.indexed_directly.len() == 1 { if visitor.indexed_indirectly.is_empty() && visitor.indexed_directly.len() == 1 {
let (indexed, indexed_extent) = visitor let (indexed, (indexed_extent, indexed_ty)) = visitor
.indexed_directly .indexed_directly
.into_iter() .into_iter()
.next() .next()
@@ -1119,7 +1119,7 @@ fn check_for_loop_range<'a, 'tcx>(
} }
} }
if is_len_call(end, indexed) { if is_len_call(end, indexed) || is_end_eq_array_len(cx, end, limits, indexed_ty) {
String::new() String::new()
} else { } else {
match limits { match limits {
@@ -1207,6 +1207,28 @@ fn is_len_call(expr: &Expr, var: Name) -> bool {
false false
} }
fn is_end_eq_array_len(
cx: &LateContext<'_, '_>,
end: &Expr,
limits: ast::RangeLimits,
indexed_ty: Ty<'_>,
) -> bool {
if_chain! {
if let ExprKind::Lit(ref lit) = end.node;
if let ast::LitKind::Int(end_int, _) = lit.node;
if let ty::TyKind::Array(_, arr_len_const) = indexed_ty.sty;
if let Some(arr_len) = arr_len_const.assert_usize(cx.tcx);
then {
return match limits {
ast::RangeLimits::Closed => end_int + 1 >= arr_len.into(),
ast::RangeLimits::HalfOpen => end_int >= arr_len.into(),
};
}
}
false
}
fn check_for_loop_reverse_range<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arg: &'tcx Expr, expr: &'tcx Expr) { fn check_for_loop_reverse_range<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arg: &'tcx Expr, expr: &'tcx Expr) {
// if this for loop is iterating over a two-sided range... // if this for loop is iterating over a two-sided range...
if let Some(higher::Range { if let Some(higher::Range {
@@ -1678,7 +1700,7 @@ struct VarVisitor<'a, 'tcx: 'a> {
indexed_indirectly: FxHashMap<Name, Option<region::Scope>>, indexed_indirectly: FxHashMap<Name, Option<region::Scope>>,
/// subset of `indexed` of vars that are indexed directly: `v[i]` /// subset of `indexed` of vars that are indexed directly: `v[i]`
/// this will not contain cases like `v[calc_index(i)]` or `v[(i + 4) % N]` /// this will not contain cases like `v[calc_index(i)]` or `v[(i + 4) % N]`
indexed_directly: FxHashMap<Name, Option<region::Scope>>, indexed_directly: FxHashMap<Name, (Option<region::Scope>, Ty<'tcx>)>,
/// Any names that are used outside an index operation. /// Any names that are used outside an index operation.
/// Used to detect things like `&mut vec` used together with `vec[i]` /// Used to detect things like `&mut vec` used together with `vec[i]`
referenced: FxHashSet<Name>, referenced: FxHashSet<Name>,
@@ -1725,7 +1747,10 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
self.indexed_indirectly.insert(seqvar.segments[0].ident.name, Some(extent)); self.indexed_indirectly.insert(seqvar.segments[0].ident.name, Some(extent));
} }
if index_used_directly { if index_used_directly {
self.indexed_directly.insert(seqvar.segments[0].ident.name, Some(extent)); self.indexed_directly.insert(
seqvar.segments[0].ident.name,
(Some(extent), self.cx.tables.node_id_to_type(seqexpr.hir_id)),
);
} }
return false; // no need to walk further *on the variable* return false; // no need to walk further *on the variable*
} }
@@ -1734,7 +1759,10 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
self.indexed_indirectly.insert(seqvar.segments[0].ident.name, None); self.indexed_indirectly.insert(seqvar.segments[0].ident.name, None);
} }
if index_used_directly { if index_used_directly {
self.indexed_directly.insert(seqvar.segments[0].ident.name, None); self.indexed_directly.insert(
seqvar.segments[0].ident.name,
(None, self.cx.tables.node_id_to_type(seqexpr.hir_id)),
);
} }
return false; // no need to walk further *on the variable* return false; // no need to walk further *on the variable*
} }

View File

@@ -13,7 +13,7 @@ fn calc_idx(i: usize) -> usize {
} }
fn main() { fn main() {
let ns = [2, 3, 5, 7]; let ns = vec![2, 3, 5, 7];
for i in 3..10 { for i in 3..10 {
println!("{}", ns[i]); println!("{}", ns[i]);
@@ -76,4 +76,18 @@ fn main() {
for i in x..=x + 4 { for i in x..=x + 4 {
vec[i] += 1; vec[i] += 1;
} }
let arr = [1,2,3];
for i in 0..3 {
println!("{}", arr[i]);
}
for i in 0..2 {
println!("{}", arr[i]);
}
for i in 1..3 {
println!("{}", arr[i]);
}
} }

View File

@@ -50,5 +50,35 @@ help: consider using an iterator
76 | for <item> in vec.iter_mut().skip(x).take(4 + 1) { 76 | for <item> in vec.iter_mut().skip(x).take(4 + 1) {
| ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 5 previous errors error: the loop variable `i` is only used to index `arr`.
--> $DIR/needless_range_loop.rs:82:14
|
82 | for i in 0..3 {
| ^^^^
help: consider using an iterator
|
82 | for <item> in &arr {
| ^^^^^^ ^^^^
error: the loop variable `i` is only used to index `arr`.
--> $DIR/needless_range_loop.rs:86:14
|
86 | for i in 0..2 {
| ^^^^
help: consider using an iterator
|
86 | for <item> in arr.iter().take(2) {
| ^^^^^^ ^^^^^^^^^^^^^^^^^^
error: the loop variable `i` is only used to index `arr`.
--> $DIR/needless_range_loop.rs:90:14
|
90 | for i in 1..3 {
| ^^^^
help: consider using an iterator
|
90 | for <item> in arr.iter().skip(1) {
| ^^^^^^ ^^^^^^^^^^^^^^^^^^
error: aborting due to 8 previous errors