better parsing of condition in while loop for mutability

allow condition to be a block: by calling visit_expr of the visitor directly on the condition instead of walk_expr on the whole expression, we bypass the match to ExprWhile that calls visit_expr on the condition and visit_block on the body. This allow to re-enable visit_block in the visitor, as it won't be called on the while body
allow condition to use static variables: maintain a list of static variables used, and if they are mutable
This commit is contained in:
François Mockers
2018-05-27 23:59:07 +02:00
parent fc008aa14c
commit 5379fc1b28
2 changed files with 35 additions and 4 deletions

View File

@@ -2154,9 +2154,10 @@ fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, b
let mut mut_var_visitor = VarCollectorVisitor { let mut mut_var_visitor = VarCollectorVisitor {
cx, cx,
ids: HashMap::new(), ids: HashMap::new(),
def_ids: HashMap::new(),
skip: false, skip: false,
}; };
walk_expr(&mut mut_var_visitor, expr); mut_var_visitor.visit_expr(cond);
if mut_var_visitor.skip { if mut_var_visitor.skip {
return; return;
} }
@@ -2172,7 +2173,7 @@ fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, b
if delegate.skip { if delegate.skip {
return; return;
} }
if !delegate.used_mutably.iter().any(|(_, v)| *v) { if !(delegate.used_mutably.iter().any(|(_, v)| *v) || mut_var_visitor.def_ids.iter().any(|(_, v)| *v)) {
span_lint( span_lint(
cx, cx,
WHILE_IMMUTABLE_CONDITION, WHILE_IMMUTABLE_CONDITION,
@@ -2189,6 +2190,7 @@ fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, b
struct VarCollectorVisitor<'a, 'tcx: 'a> { struct VarCollectorVisitor<'a, 'tcx: 'a> {
cx: &'a LateContext<'a, 'tcx>, cx: &'a LateContext<'a, 'tcx>,
ids: HashMap<NodeId, bool>, ids: HashMap<NodeId, bool>,
def_ids: HashMap<def_id::DefId, bool>,
skip: bool, skip: bool,
} }
@@ -2203,6 +2205,9 @@ impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> {
Def::Local(node_id) | Def::Upvar(node_id, ..) => { Def::Local(node_id) | Def::Upvar(node_id, ..) => {
self.ids.insert(node_id, false); self.ids.insert(node_id, false);
}, },
Def::Static(def_id, mutable) => {
self.def_ids.insert(def_id, mutable);
},
_ => {}, _ => {},
} }
} }
@@ -2221,8 +2226,6 @@ impl<'a, 'tcx> Visitor<'tcx> for VarCollectorVisitor<'a, 'tcx> {
} }
} }
fn visit_block(&mut self, _b: &'tcx Block) {}
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
NestedVisitorMap::None NestedVisitorMap::None
} }

View File

@@ -0,0 +1,28 @@
#![allow(dead_code)]
/// Issue: https://github.com/rust-lang-nursery/rust-clippy/issues/2596
pub fn loop_on_block_condition(u: &mut isize) {
while { *u < 0 } {
*u += 1;
}
}
/// https://github.com/rust-lang-nursery/rust-clippy/issues/2584
fn loop_with_unsafe_condition(ptr: *const u8) {
let mut len = 0;
while unsafe { *ptr.offset(len) } != 0 {
len += 1;
}
}
/// https://github.com/rust-lang-nursery/rust-clippy/issues/2710
static mut RUNNING: bool = true;
fn loop_on_static_condition() {
unsafe {
while RUNNING {
RUNNING = false;
}
}
}
fn main() {}