Skip to content

Commit

Permalink
Auto merge of #4040 - mikerite:fix-build-20190428, r=Manishearth
Browse files Browse the repository at this point in the history 
Fix breakage due to rust-lang/rust#60225

Wrote this up before I saw that Manish already started on a fix in #4038. It no doubt contains errors. Feel free to close.
  • Loading branch information
@bors
bors committed Apr 28, 2019
2 parents 910d538 + 5ad79c2 commit 09a0422
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 74 deletions.
93 changes: 43 additions & 50 deletions clippy_lints/src/loops.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,8 @@ fn never_loop_expr(expr: &Expr, main_loop_id: HirId) -> NeverLoopResult {
| ExprKind::Field(ref e, _)
| ExprKind::AddrOf(_, ref e)
| ExprKind::Struct(_, _, Some(ref e))
| ExprKind::Repeat(ref e, _) => never_loop_expr(e, main_loop_id),
| ExprKind::Repeat(ref e, _)
| ExprKind::Use(ref e) => never_loop_expr(e, main_loop_id),
ExprKind::Array(ref es) | ExprKind::MethodCall(_, _, ref es) | ExprKind::Tup(ref es) => {
never_loop_expr_all(&mut es.iter(), main_loop_id)
},
Expand Down Expand Up @@ -1458,54 +1459,46 @@ fn check_for_loop_explicit_counter<'a, 'tcx>(

// For each candidate, check the parent block to see if
// it's initialized to zero at the start of the loop.
let map = &cx.tcx.hir();
let expr_node_id = expr.hir_id;
let parent_scope = map
.get_enclosing_scope(expr_node_id)
.and_then(|id| map.get_enclosing_scope(id));
if let Some(parent_id) = parent_scope {
if let Node::Block(block) = map.get_by_hir_id(parent_id) {
for (id, _) in visitor.states.iter().filter(|&(_, v)| *v == VarState::IncrOnce) {
let mut visitor2 = InitializeVisitor {
cx,
end_expr: expr,
var_id: *id,
state: VarState::IncrOnce,
name: None,
depth: 0,
past_loop: false,
};
walk_block(&mut visitor2, block);

if visitor2.state == VarState::Warn {
if let Some(name) = visitor2.name {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
EXPLICIT_COUNTER_LOOP,
expr.span,
&format!("the variable `{}` is used as a loop counter.", name),
"consider using",
format!(
"for ({}, {}) in {}.enumerate()",
name,
snippet_with_applicability(cx, pat.span, "item", &mut applicability),
if higher::range(cx, arg).is_some() {
format!(
"({})",
snippet_with_applicability(cx, arg.span, "_", &mut applicability)
)
} else {
format!(
"{}",
sugg::Sugg::hir_with_applicability(cx, arg, "_", &mut applicability)
.maybe_par()
)
}
),
applicability,
);
}
if let Some(block) = get_enclosing_block(&cx, expr.hir_id) {
for (id, _) in visitor.states.iter().filter(|&(_, v)| *v == VarState::IncrOnce) {
let mut visitor2 = InitializeVisitor {
cx,
end_expr: expr,
var_id: *id,
state: VarState::IncrOnce,
name: None,
depth: 0,
past_loop: false,
};
walk_block(&mut visitor2, block);

if visitor2.state == VarState::Warn {
if let Some(name) = visitor2.name {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
EXPLICIT_COUNTER_LOOP,
expr.span,
&format!("the variable `{}` is used as a loop counter.", name),
"consider using",
format!(
"for ({}, {}) in {}.enumerate()",
name,
snippet_with_applicability(cx, pat.span, "item", &mut applicability),
if higher::range(cx, arg).is_some() {
format!(
"({})",
snippet_with_applicability(cx, arg.span, "_", &mut applicability)
)
} else {
format!(
"{}",
sugg::Sugg::hir_with_applicability(cx, arg, "_", &mut applicability).maybe_par()
)
}
),
applicability,
);
}
}
}
Expand Down Expand Up @@ -2042,7 +2035,7 @@ fn is_simple_break_expr(expr: &Expr) -> bool {
// To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be
// incremented exactly once in the loop body, and initialized to zero
// at the start of the loop.
#[derive(PartialEq)]
#[derive(Debug, PartialEq)]
enum VarState {
Initial, // Not examined yet
IncrOnce, // Incremented exactly once, may be a loop counter
Expand Down
6 changes: 6 additions & 0 deletions clippy_lints/src/utils/author.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,12 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
ExprKind::Err => {
println!("Err = {}", current);
},
ExprKind::Use(ref expr) => {
let expr_pat = self.next("expr");
println!("Use(ref {}) = {};", expr_pat, current);
self.current = expr_pat;
self.visit_expr(expr);
},
}
}

Expand Down
6 changes: 6 additions & 0 deletions clippy_lints/src/utils/hir_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
&& self.eq_block(lb, rb)
&& both(ll, rl, |l, r| l.ident.as_str() == r.ident.as_str())
},
(&ExprKind::Use(ref le), &ExprKind::Use(ref re)) => self.eq_expr(le, re),
_ => false,
}
}
Expand Down Expand Up @@ -606,6 +607,11 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
}
},
ExprKind::Err => {},
ExprKind::Use(ref e) => {
let c: fn(_) -> _ = ExprKind::Use;
c.hash(&mut self.s);
self.hash_expr(e);
},
}
}

Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/utils/inspector.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,10 @@ fn print_expr(cx: &LateContext<'_, '_>, expr: &hir::Expr, indent: usize) {
hir::ExprKind::Err => {
println!("{}Err", ind);
},
hir::ExprKind::Use(ref e) => {
println!("{}Use", ind);
print_expr(cx, e, indent + 1);
},
}
}

Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/sugg.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ impl<'a> Sugg<'a> {
| hir::ExprKind::Struct(..)
| hir::ExprKind::Tup(..)
| hir::ExprKind::While(..)
| hir::ExprKind::Use(_)
| hir::ExprKind::Err => Sugg::NonParen(snippet),
hir::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet),
hir::ExprKind::AssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet),
Expand Down
42 changes: 18 additions & 24 deletions tests/ui/author/for_loop.stdout
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
if_chain! {
if let ExprKind::Block(ref block) = expr.node;
if let StmtKind::Local(ref local) = block.node;
if let Some(ref init) = local.init;
if let ExprKind::Match(ref expr, ref arms, MatchSource::ForLoopDesugar) = init.node;
if let ExprKind::Call(ref func, ref args) = expr.node;
if let ExprKind::Use(ref expr) = expr.node;
if let ExprKind::Match(ref expr1, ref arms, MatchSource::ForLoopDesugar) = expr.node;
if let ExprKind::Call(ref func, ref args) = expr1.node;
if let ExprKind::Path(ref path) = func.node;
if match_qpath(path, &["{{root}}", "std", "iter", "IntoIterator", "into_iter"]);
if args.len() == 1;
Expand All @@ -13,12 +11,12 @@ if_chain! {
// unimplemented: field checks
if arms.len() == 1;
if let ExprKind::Loop(ref body, ref label, LoopSource::ForLoop) = arms[0].body.node;
if let StmtKind::Local(ref local1) = body.node;
if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local1.pat.node;
if let StmtKind::Local(ref local) = body.node;
if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local.pat.node;
if name.node.as_str() == "__next";
if let StmtKind::Expr(ref e, _) = local1.pat.node
if let ExprKind::Match(ref expr1, ref arms1, MatchSource::ForLoopDesugar) = e.node;
if let ExprKind::Call(ref func1, ref args1) = expr1.node;
if let StmtKind::Expr(ref e, _) = local.pat.node
if let ExprKind::Match(ref expr2, ref arms1, MatchSource::ForLoopDesugar) = e.node;
if let ExprKind::Call(ref func1, ref args1) = expr2.node;
if let ExprKind::Path(ref path2) = func1.node;
if match_qpath(path2, &["{{root}}", "std", "iter", "Iterator", "next"]);
if args1.len() == 1;
Expand All @@ -40,27 +38,23 @@ if_chain! {
if arms1[1].pats.len() == 1;
if let PatKind::Path(ref path7) = arms1[1].pats[0].node;
if match_qpath(path7, &["{{root}}", "std", "option", "Option", "None"]);
if let StmtKind::Local(ref local2) = path7.node;
if let Some(ref init1) = local2.init;
if let ExprKind::Path(ref path8) = init1.node;
if let StmtKind::Local(ref local1) = path7.node;
if let Some(ref init) = local1.init;
if let ExprKind::Path(ref path8) = init.node;
if match_qpath(path8, &["__next"]);
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name1, None) = local2.pat.node;
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name1, None) = local1.pat.node;
if name1.node.as_str() == "y";
if let StmtKind::Expr(ref e1, _) = local2.pat.node
if let ExprKind::Block(ref block1) = e1.node;
if let StmtKind::Local(ref local3) = block1.node;
if let Some(ref init2) = local3.init;
if let ExprKind::Path(ref path9) = init2.node;
if let StmtKind::Expr(ref e1, _) = local1.pat.node
if let ExprKind::Block(ref block) = e1.node;
if let StmtKind::Local(ref local2) = block.node;
if let Some(ref init1) = local2.init;
if let ExprKind::Path(ref path9) = init1.node;
if match_qpath(path9, &["y"]);
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name2, None) = local3.pat.node;
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name2, None) = local2.pat.node;
if name2.node.as_str() == "z";
if arms[0].pats.len() == 1;
if let PatKind::Binding(BindingAnnotation::Mutable, _, name3, None) = arms[0].pats[0].node;
if name3.node.as_str() == "iter";
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name4, None) = local.pat.node;
if name4.node.as_str() == "_result";
if let ExprKind::Path(ref path10) = local.pat.node;
if match_qpath(path10, &["_result"]);
then {
// report your lint here
}
Expand Down

0 comments on commit 09a0422

Please sign in to comment.