From dc89bb1135afc31fc9ee2272e627192c04354d22 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 10 Jun 2020 14:21:58 +1200
Subject: [PATCH 01/35] Use if_chain in Increment/InitializeVisitor

---
 clippy_lints/src/loops.rs | 43 +++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 7f998c63f497..9f3be26e6723 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -2162,15 +2162,16 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
                 match parent.kind {
                     ExprKind::AssignOp(op, ref lhs, ref rhs) => {
                         if lhs.hir_id == expr.hir_id {
-                            if op.node == BinOpKind::Add && is_integer_const(self.cx, rhs, 1) {
-                                *state = match *state {
-                                    VarState::Initial if self.depth == 0 => VarState::IncrOnce,
-                                    _ => VarState::DontWarn,
-                                };
+                            *state = if op.node == BinOpKind::Add
+                                && is_integer_const(self.cx, rhs, 1)
+                                && *state == VarState::Initial
+                                && self.depth == 0
+                            {
+                                VarState::IncrOnce
                             } else {
-                                // Assigned some other value
-                                *state = VarState::DontWarn;
-                            }
+                                // Assigned some other value or assigned multiple times
+                                VarState::DontWarn
+                            };
                         }
                     },
                     ExprKind::Assign(ref lhs, _, _) if lhs.hir_id == expr.hir_id => *state = VarState::DontWarn,
@@ -2212,18 +2213,20 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
 
     fn visit_stmt(&mut self, stmt: &'tcx Stmt<'_>) {
         // Look for declarations of the variable
-        if let StmtKind::Local(ref local) = stmt.kind {
-            if local.pat.hir_id == self.var_id {
-                if let PatKind::Binding(.., ident, _) = local.pat.kind {
-                    self.name = Some(ident.name);
-
-                    self.state = local.init.as_ref().map_or(VarState::Declared, |init| {
-                        if is_integer_const(&self.cx, init, 0) {
-                            VarState::Warn
-                        } else {
-                            VarState::Declared
-                        }
-                    })
+        if_chain! {
+            if let StmtKind::Local(ref local) = stmt.kind;
+            if local.pat.hir_id == self.var_id;
+            if let PatKind::Binding(.., ident, _) = local.pat.kind;
+            then {
+                self.name = Some(ident.name);
+                self.state = if_chain! {
+                    if let Some(ref init) = local.init;
+                    if is_integer_const(&self.cx, init, 0);
+                    then {
+                        VarState::Warn
+                    } else {
+                        VarState::Declared
+                    }
                 }
             }
         }

From 116f30dc33d9e3744f257f2f7f5467acfbff178b Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 10 Jun 2020 14:25:06 +1200
Subject: [PATCH 02/35] Use else blocks instead of return statements in
 Increment/InitializeVisitor

---
 clippy_lints/src/loops.rs | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 9f3be26e6723..a59d3ef8708b 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -2181,16 +2181,17 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
                     _ => (),
                 }
             }
+
+            walk_expr(self, expr);
         } else if is_loop(expr) || is_conditional(expr) {
             self.depth += 1;
             walk_expr(self, expr);
             self.depth -= 1;
-            return;
         } else if let ExprKind::Continue(_) = expr.kind {
             self.done = true;
-            return;
+        } else {
+            walk_expr(self, expr);
         }
-        walk_expr(self, expr);
     }
     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
         NestedVisitorMap::None
@@ -2272,16 +2273,16 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
                 self.state = VarState::DontWarn;
                 return;
             }
+            walk_expr(self, expr);
         } else if !self.past_loop && is_loop(expr) {
             self.state = VarState::DontWarn;
-            return;
         } else if is_conditional(expr) {
             self.depth += 1;
             walk_expr(self, expr);
             self.depth -= 1;
-            return;
+        } else {
+            walk_expr(self, expr);
         }
-        walk_expr(self, expr);
     }
 
     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {

From b2d5b89a1de15df9052fdf44d01b174add82837f Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 10 Jun 2020 14:41:09 +1200
Subject: [PATCH 03/35] Check if it's after the loop earlier

---
 clippy_lints/src/loops.rs | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index a59d3ef8708b..20e75546d710 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -2250,6 +2250,11 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
 
         // If node is the desired variable, see how it's used
         if var_def_id(self.cx, expr) == Some(self.var_id) {
+            if self.past_loop {
+                self.state = VarState::DontWarn;
+                return;
+            }
+
             if let Some(parent) = get_parent_expr(self.cx, expr) {
                 match parent.kind {
                     ExprKind::AssignOp(_, ref lhs, _) if lhs.hir_id == expr.hir_id => {
@@ -2269,10 +2274,6 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
                 }
             }
 
-            if self.past_loop {
-                self.state = VarState::DontWarn;
-                return;
-            }
             walk_expr(self, expr);
         } else if !self.past_loop && is_loop(expr) {
             self.state = VarState::DontWarn;

From 31cb1109648bf4242cab47571343578244e7fb9d Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 10 Jun 2020 14:48:26 +1200
Subject: [PATCH 04/35] add concinient methods to Increment/InitializeVisitor

---
 clippy_lints/src/loops.rs | 63 +++++++++++++++++++++++++++------------
 1 file changed, 44 insertions(+), 19 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 20e75546d710..61f0059cca47 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -1536,31 +1536,17 @@ fn check_for_loop_explicit_counter<'tcx>(
     expr: &'tcx Expr<'_>,
 ) {
     // Look for variables that are incremented once per loop iteration.
-    let mut visitor = IncrementVisitor {
-        cx,
-        states: FxHashMap::default(),
-        depth: 0,
-        done: false,
-    };
+    let mut visitor = IncrementVisitor::new(cx);
     walk_expr(&mut visitor, body);
 
     // For each candidate, check the parent block to see if
     // it's initialized to zero at the start of the loop.
     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,
-            };
+        for id in visitor.into_results() {
+            let mut visitor2 = InitializeVisitor::new(cx, expr, id);
             walk_block(&mut visitor2, block);
 
-            if visitor2.state == VarState::Warn {
-                if let Some(name) = visitor2.name {
+            if let Some(name) = visitor2.get_result() {
                     let mut applicability = Applicability::MachineApplicable;
 
                     // for some reason this is the only way to get the `Span`
@@ -1585,7 +1571,6 @@ fn check_for_loop_explicit_counter<'tcx>(
                         ),
                         applicability,
                     );
-                }
             }
         }
     }
@@ -2142,6 +2127,24 @@ struct IncrementVisitor<'a, 'tcx> {
     done: bool,
 }
 
+impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
+    fn new(cx: &'a LateContext<'a, 'tcx>) -> Self {
+        Self {
+            cx,
+            states: FxHashMap::default(),
+            depth: 0,
+            done: false,
+        }
+    }
+
+    fn into_results(self) -> impl Iterator<Item = HirId> {
+        self.states
+            .into_iter()
+            .filter(|(_, state)| *state == VarState::IncrOnce)
+            .map(|(id, _)| id)
+    }
+}
+
 impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
     type Map = Map<'tcx>;
 
@@ -2209,6 +2212,28 @@ struct InitializeVisitor<'a, 'tcx> {
     past_loop: bool,
 }
 
+impl<'a, 'tcx> InitializeVisitor<'a, 'tcx> {
+    fn new(cx: &'a LateContext<'a, 'tcx>, end_expr: &'tcx Expr<'tcx>, var_id: HirId) -> Self {
+        Self {
+            cx,
+            end_expr,
+            var_id,
+            state: VarState::IncrOnce,
+            name: None,
+            depth: 0,
+            past_loop: false,
+        }
+    }
+
+    fn get_result(&self) -> Option<Name> {
+        if self.state == VarState::Warn {
+            self.name
+        } else {
+            None
+        }
+    }
+}
+
 impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
     type Map = Map<'tcx>;
 

From c599e2fcfaaedb12b560f4136bab3d0b450acf8f Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 10 Jun 2020 15:05:51 +1200
Subject: [PATCH 05/35] Split VarState

---
 clippy_lints/src/loops.rs | 87 +++++++++++++++++++++------------------
 1 file changed, 48 insertions(+), 39 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 61f0059cca47..2b7830e7cadb 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -2107,23 +2107,18 @@ 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(Debug, PartialEq)]
-enum VarState {
+enum IncrementVisitorVarState {
     Initial,  // Not examined yet
     IncrOnce, // Incremented exactly once, may be a loop counter
-    Declared, // Declared but not (yet) initialized to zero
-    Warn,
     DontWarn,
 }
 
 /// Scan a for loop for variables that are incremented exactly once and not used after that.
 struct IncrementVisitor<'a, 'tcx> {
-    cx: &'a LateContext<'tcx>,          // context reference
-    states: FxHashMap<HirId, VarState>, // incremented variables
-    depth: u32,                         // depth of conditional expressions
+    cx: &'a LateContext<'tcx>,                          // context reference
+    states: FxHashMap<HirId, IncrementVisitorVarState>, // incremented variables
+    depth: u32,                                         // depth of conditional expressions
     done: bool,
 }
 
@@ -2140,7 +2135,7 @@ impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
     fn into_results(self) -> impl Iterator<Item = HirId> {
         self.states
             .into_iter()
-            .filter(|(_, state)| *state == VarState::IncrOnce)
+            .filter(|(_, state)| *state == IncrementVisitorVarState::IncrOnce)
             .map(|(id, _)| id)
     }
 }
@@ -2156,9 +2151,9 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
         // If node is a variable
         if let Some(def_id) = var_def_id(self.cx, expr) {
             if let Some(parent) = get_parent_expr(self.cx, expr) {
-                let state = self.states.entry(def_id).or_insert(VarState::Initial);
-                if *state == VarState::IncrOnce {
-                    *state = VarState::DontWarn;
+                let state = self.states.entry(def_id).or_insert(IncrementVisitorVarState::Initial);
+                if *state == IncrementVisitorVarState::IncrOnce {
+                    *state = IncrementVisitorVarState::DontWarn;
                     return;
                 }
 
@@ -2167,19 +2162,21 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
                         if lhs.hir_id == expr.hir_id {
                             *state = if op.node == BinOpKind::Add
                                 && is_integer_const(self.cx, rhs, 1)
-                                && *state == VarState::Initial
+                                && *state == IncrementVisitorVarState::Initial
                                 && self.depth == 0
                             {
-                                VarState::IncrOnce
+                                IncrementVisitorVarState::IncrOnce
                             } else {
                                 // Assigned some other value or assigned multiple times
-                                VarState::DontWarn
+                                IncrementVisitorVarState::DontWarn
                             };
                         }
                     },
-                    ExprKind::Assign(ref lhs, _, _) if lhs.hir_id == expr.hir_id => *state = VarState::DontWarn,
+                    ExprKind::Assign(ref lhs, _, _) if lhs.hir_id == expr.hir_id => {
+                        *state = IncrementVisitorVarState::DontWarn
+                    },
                     ExprKind::AddrOf(BorrowKind::Ref, mutability, _) if mutability == Mutability::Mut => {
-                        *state = VarState::DontWarn
+                        *state = IncrementVisitorVarState::DontWarn
                     },
                     _ => (),
                 }
@@ -2201,13 +2198,20 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
     }
 }
 
-/// Checks whether a variable is initialized to zero at the start of a loop.
+enum InitializeVisitorState {
+    Initial,          // Not examined yet
+    Declared(Symbol), // Declared but not (yet) initialized
+    Initialized { name: Symbol },
+    DontWarn,
+}
+
+/// Checks whether a variable is initialized to zero at the start of a loop and not modified
+/// and used after the loop.
 struct InitializeVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,  // context reference
     end_expr: &'tcx Expr<'tcx>, // the for loop. Stop scanning here.
     var_id: HirId,
-    state: VarState,
-    name: Option<Symbol>,
+    state: InitializeVisitorState,
     depth: u32, // depth of conditional expressions
     past_loop: bool,
 }
@@ -2218,16 +2222,15 @@ impl<'a, 'tcx> InitializeVisitor<'a, 'tcx> {
             cx,
             end_expr,
             var_id,
-            state: VarState::IncrOnce,
-            name: None,
+            state: InitializeVisitorState::Initial,
             depth: 0,
             past_loop: false,
         }
     }
 
     fn get_result(&self) -> Option<Name> {
-        if self.state == VarState::Warn {
-            self.name
+        if let InitializeVisitorState::Initialized { name } = self.state {
+            Some(name)
         } else {
             None
         }
@@ -2244,23 +2247,24 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
             if local.pat.hir_id == self.var_id;
             if let PatKind::Binding(.., ident, _) = local.pat.kind;
             then {
-                self.name = Some(ident.name);
                 self.state = if_chain! {
                     if let Some(ref init) = local.init;
                     if is_integer_const(&self.cx, init, 0);
                     then {
-                        VarState::Warn
-                    } else {
-                        VarState::Declared
+                    InitializeVisitorState::Initialized {
+                        name: ident.name
                     }
+                } else {
+                    InitializeVisitorState::Declared(ident.name)
                 }
             }
         }
+        }
         walk_stmt(self, stmt);
     }
 
     fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
-        if self.state == VarState::DontWarn {
+        if matches!(self.state, InitializeVisitorState::DontWarn) {
             return;
         }
         if expr.hir_id == self.end_expr.hir_id {
@@ -2269,31 +2273,36 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
         }
         // No need to visit expressions before the variable is
         // declared
-        if self.state == VarState::IncrOnce {
+        if matches!(self.state, InitializeVisitorState::Initial) {
             return;
         }
 
         // If node is the desired variable, see how it's used
         if var_def_id(self.cx, expr) == Some(self.var_id) {
             if self.past_loop {
-                self.state = VarState::DontWarn;
+                self.state = InitializeVisitorState::DontWarn;
                 return;
             }
 
             if let Some(parent) = get_parent_expr(self.cx, expr) {
                 match parent.kind {
                     ExprKind::AssignOp(_, ref lhs, _) if lhs.hir_id == expr.hir_id => {
-                        self.state = VarState::DontWarn;
+                        self.state = InitializeVisitorState::DontWarn;
                     },
                     ExprKind::Assign(ref lhs, ref rhs, _) if lhs.hir_id == expr.hir_id => {
-                        self.state = if is_integer_const(&self.cx, rhs, 0) && self.depth == 0 {
-                            VarState::Warn
-                        } else {
-                            VarState::DontWarn
+                        self.state = if_chain! {
+                            if is_integer_const(&self.cx, rhs, 0) && self.depth == 0;
+                            if let InitializeVisitorState::Declared(name)
+                                | InitializeVisitorState::Initialized { name, ..} = self.state;
+                            then {
+                                InitializeVisitorState::Initialized { name }
+                            } else {
+                                InitializeVisitorState::DontWarn
+                            }
                         }
                     },
                     ExprKind::AddrOf(BorrowKind::Ref, mutability, _) if mutability == Mutability::Mut => {
-                        self.state = VarState::DontWarn
+                        self.state = InitializeVisitorState::DontWarn
                     },
                     _ => (),
                 }
@@ -2301,7 +2310,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
 
             walk_expr(self, expr);
         } else if !self.past_loop && is_loop(expr) {
-            self.state = VarState::DontWarn;
+            self.state = InitializeVisitorState::DontWarn;
         } else if is_conditional(expr) {
             self.depth += 1;
             walk_expr(self, expr);

From 13c207d3756754c54a6b20d852087616d5abfbf4 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 10 Jun 2020 15:14:57 +1200
Subject: [PATCH 06/35] Generalise `InitializeVisitor`

---
 clippy_lints/src/loops.rs     | 36 +++++++++++++++++++----------------
 clippy_lints/src/utils/mod.rs |  2 +-
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 2b7830e7cadb..bf067c70a7ec 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -1528,6 +1528,9 @@ fn check_arg_type(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) {
     }
 }
 
+// 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.
 fn check_for_loop_explicit_counter<'tcx>(
     cx: &LateContext<'tcx>,
     pat: &'tcx Pat<'_>,
@@ -1546,7 +1549,10 @@ fn check_for_loop_explicit_counter<'tcx>(
             let mut visitor2 = InitializeVisitor::new(cx, expr, id);
             walk_block(&mut visitor2, block);
 
-            if let Some(name) = visitor2.get_result() {
+            if_chain! {
+                if let Some((name, initializer)) = visitor2.get_result();
+                if is_integer_const(cx, initializer, 0);
+                then {
                     let mut applicability = Applicability::MachineApplicable;
 
                     // for some reason this is the only way to get the `Span`
@@ -1571,6 +1577,7 @@ fn check_for_loop_explicit_counter<'tcx>(
                         ),
                         applicability,
                     );
+                }
             }
         }
     }
@@ -2198,20 +2205,20 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
     }
 }
 
-enum InitializeVisitorState {
+enum InitializeVisitorState<'hir> {
     Initial,          // Not examined yet
     Declared(Symbol), // Declared but not (yet) initialized
-    Initialized { name: Symbol },
+    Initialized { name: Symbol, initializer: &'hir Expr<'hir> },
     DontWarn,
 }
 
-/// Checks whether a variable is initialized to zero at the start of a loop and not modified
+/// Checks whether a variable is initialized at the start of a loop and not modified
 /// and used after the loop.
 struct InitializeVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,  // context reference
     end_expr: &'tcx Expr<'tcx>, // the for loop. Stop scanning here.
     var_id: HirId,
-    state: InitializeVisitorState,
+    state: InitializeVisitorState<'tcx>,
     depth: u32, // depth of conditional expressions
     past_loop: bool,
 }
@@ -2228,9 +2235,9 @@ impl<'a, 'tcx> InitializeVisitor<'a, 'tcx> {
         }
     }
 
-    fn get_result(&self) -> Option<Name> {
-        if let InitializeVisitorState::Initialized { name } = self.state {
-            Some(name)
+    fn get_result(&self) -> Option<(Name, &'tcx Expr<'tcx>)> {
+        if let InitializeVisitorState::Initialized { name, initializer } = self.state {
+            Some((name, initializer))
         } else {
             None
         }
@@ -2247,19 +2254,16 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
             if local.pat.hir_id == self.var_id;
             if let PatKind::Binding(.., ident, _) = local.pat.kind;
             then {
-                self.state = if_chain! {
-                    if let Some(ref init) = local.init;
-                    if is_integer_const(&self.cx, init, 0);
-                    then {
+                self.state = if let Some(ref init) = local.init {
                     InitializeVisitorState::Initialized {
-                        name: ident.name
+                        initializer: init,
+                        name: ident.name,
                     }
                 } else {
                     InitializeVisitorState::Declared(ident.name)
                 }
             }
         }
-        }
         walk_stmt(self, stmt);
     }
 
@@ -2291,11 +2295,11 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
                     },
                     ExprKind::Assign(ref lhs, ref rhs, _) if lhs.hir_id == expr.hir_id => {
                         self.state = if_chain! {
-                            if is_integer_const(&self.cx, rhs, 0) && self.depth == 0;
+                            if self.depth == 0;
                             if let InitializeVisitorState::Declared(name)
                                 | InitializeVisitorState::Initialized { name, ..} = self.state;
                             then {
-                                InitializeVisitorState::Initialized { name }
+                                InitializeVisitorState::Initialized { initializer: rhs, name }
                             } else {
                                 InitializeVisitorState::DontWarn
                             }
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index 92cb31fcf854..a0ddcce111e1 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -707,7 +707,7 @@ fn reindent_multiline_inner(s: &str, ignore_first: bool, indent: Option<usize>,
 }
 
 /// Gets the parent expression, if any –- this is useful to constrain a lint.
-pub fn get_parent_expr<'c>(cx: &'c LateContext<'_>, e: &Expr<'_>) -> Option<&'c Expr<'c>> {
+pub fn get_parent_expr<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
     let map = &cx.tcx.hir();
     let hir_id = e.hir_id;
     let parent_id = map.get_parent_node(hir_id);

From 9573a0d378033a81e55ca834a5d305d3cf2be24d Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 10 Jun 2020 15:15:20 +1200
Subject: [PATCH 07/35] Rename variables

---
 clippy_lints/src/loops.rs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index bf067c70a7ec..1c316c00f433 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -1539,18 +1539,18 @@ fn check_for_loop_explicit_counter<'tcx>(
     expr: &'tcx Expr<'_>,
 ) {
     // Look for variables that are incremented once per loop iteration.
-    let mut visitor = IncrementVisitor::new(cx);
-    walk_expr(&mut visitor, body);
+    let mut increment_visitor = IncrementVisitor::new(cx);
+    walk_expr(&mut increment_visitor, body);
 
     // For each candidate, check the parent block to see if
     // it's initialized to zero at the start of the loop.
     if let Some(block) = get_enclosing_block(&cx, expr.hir_id) {
-        for id in visitor.into_results() {
-            let mut visitor2 = InitializeVisitor::new(cx, expr, id);
-            walk_block(&mut visitor2, block);
+        for id in increment_visitor.into_results() {
+            let mut initialize_visitor = InitializeVisitor::new(cx, expr, id);
+            walk_block(&mut initialize_visitor, block);
 
             if_chain! {
-                if let Some((name, initializer)) = visitor2.get_result();
+                if let Some((name, initializer)) = initialize_visitor.get_result();
                 if is_integer_const(cx, initializer, 0);
                 then {
                     let mut applicability = Applicability::MachineApplicable;

From 1026b42f0694eb9239b5cebe80be743d5ded0da5 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 10 Jun 2020 15:32:24 +1200
Subject: [PATCH 08/35] Rename a struct and variables

---
 clippy_lints/src/loops.rs | 65 ++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 1c316c00f433..d9896f7fd6b1 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -820,9 +820,9 @@ impl Offset {
     }
 }
 
-struct FixedOffsetVar<'hir> {
-    var: &'hir Expr<'hir>,
-    offset: Offset,
+struct IndexExpr<'hir> {
+    base: &'hir Expr<'hir>,
+    idx_offset: Offset,
 }
 
 fn is_slice_like<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'_>) -> bool {
@@ -845,14 +845,14 @@ fn fetch_cloned_expr<'tcx>(expr: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> {
     }
 }
 
-fn get_offset<'tcx>(cx: &LateContext<'tcx>, idx: &Expr<'_>, var: HirId) -> Option<Offset> {
-    fn extract_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, var: HirId) -> Option<String> {
+fn get_offset<'tcx>(cx: &LateContext<'tcx>, idx: &Expr<'_>, start: HirId) -> Option<Offset> {
+    fn extract_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, start: HirId) -> Option<String> {
         match &e.kind {
             ExprKind::Lit(l) => match l.node {
                 ast::LitKind::Int(x, _ty) => Some(x.to_string()),
                 _ => None,
             },
-            ExprKind::Path(..) if !same_var(cx, e, var) => Some(snippet_opt(cx, e.span).unwrap_or_else(|| "??".into())),
+            ExprKind::Path(..) if !same_var(cx, e, start) => Some(snippet_opt(cx, e.span).unwrap_or_else(|| "??".into())),
             _ => None,
         }
     }
@@ -860,20 +860,20 @@ fn get_offset<'tcx>(cx: &LateContext<'tcx>, idx: &Expr<'_>, var: HirId) -> Optio
     match idx.kind {
         ExprKind::Binary(op, lhs, rhs) => match op.node {
             BinOpKind::Add => {
-                let offset_opt = if same_var(cx, lhs, var) {
-                    extract_offset(cx, rhs, var)
-                } else if same_var(cx, rhs, var) {
-                    extract_offset(cx, lhs, var)
+                let offset_opt = if same_var(cx, lhs, start) {
+                    extract_offset(cx, rhs, start)
+                } else if same_var(cx, rhs, start) {
+                    extract_offset(cx, lhs, start)
                 } else {
                     None
                 };
 
                 offset_opt.map(Offset::positive)
             },
-            BinOpKind::Sub if same_var(cx, lhs, var) => extract_offset(cx, rhs, var).map(Offset::negative),
+            BinOpKind::Sub if same_var(cx, lhs, start) => extract_offset(cx, rhs, start).map(Offset::negative),
             _ => None,
         },
-        ExprKind::Path(..) if same_var(cx, idx, var) => Some(Offset::positive("0".into())),
+        ExprKind::Path(..) if same_var(cx, idx, start) => Some(Offset::positive("0".into())),
         _ => None,
     }
 }
@@ -916,8 +916,8 @@ fn build_manual_memcpy_suggestion<'tcx>(
     start: &Expr<'_>,
     end: &Expr<'_>,
     limits: ast::RangeLimits,
-    dst_var: FixedOffsetVar<'_>,
-    src_var: FixedOffsetVar<'_>,
+    dst: IndexExpr<'_>,
+    src: IndexExpr<'_>,
 ) -> String {
     fn print_sum(arg1: &str, arg2: &Offset) -> String {
         match (arg1, &arg2.value[..], arg2.sign) {
@@ -944,13 +944,13 @@ fn build_manual_memcpy_suggestion<'tcx>(
         }
     }
 
-    let print_limit = |end: &Expr<'_>, offset: Offset, var: &Expr<'_>| {
+    let print_limit = |end: &Expr<'_>, offset: Offset, base: &Expr<'_>| {
         if_chain! {
             if let ExprKind::MethodCall(method, _, len_args, _) = end.kind;
             if method.ident.name == sym!(len);
             if len_args.len() == 1;
             if let Some(arg) = len_args.get(0);
-            if var_def_id(cx, arg) == var_def_id(cx, var);
+            if var_def_id(cx, arg) == var_def_id(cx, base);
             then {
                 match offset.sign {
                     OffsetSign::Negative => format!("({} - {})", snippet(cx, end.span, "<src>.len()"), offset.value),
@@ -971,25 +971,26 @@ fn build_manual_memcpy_suggestion<'tcx>(
     };
 
     let start_str = snippet(cx, start.span, "").to_string();
-    let dst_offset = print_offset(&start_str, &dst_var.offset);
-    let dst_limit = print_limit(end, dst_var.offset, dst_var.var);
-    let src_offset = print_offset(&start_str, &src_var.offset);
-    let src_limit = print_limit(end, src_var.offset, src_var.var);
+    let dst_offset = print_offset(&start_str, &dst.idx_offset);
+    let dst_limit = print_limit(end, dst.idx_offset, dst.base);
+    let src_offset = print_offset(&start_str, &src.idx_offset);
+    let src_limit = print_limit(end, src.idx_offset, src.base);
 
-    let dst_var_name = snippet_opt(cx, dst_var.var.span).unwrap_or_else(|| "???".into());
-    let src_var_name = snippet_opt(cx, src_var.var.span).unwrap_or_else(|| "???".into());
+    let dst_base_str = snippet_opt(cx, dst.base.span).unwrap_or_else(|| "???".into());
+    let src_base_str = snippet_opt(cx, src.base.span).unwrap_or_else(|| "???".into());
 
     let dst = if dst_offset == "" && dst_limit == "" {
-        dst_var_name
+        dst_base_str
     } else {
-        format!("{}[{}..{}]", dst_var_name, dst_offset, dst_limit)
+        format!("{}[{}..{}]", dst_base_str, dst_offset, dst_limit)
     };
 
     format!(
         "{}.clone_from_slice(&{}[{}..{}])",
-        dst, src_var_name, src_offset, src_limit
+        dst, src_base_str, src_offset, src_limit
     )
 }
+
 /// Checks for for loops that sequentially copy items from one slice-like
 /// object to another.
 fn detect_manual_memcpy<'tcx>(
@@ -1014,18 +1015,18 @@ fn detect_manual_memcpy<'tcx>(
                     o.and_then(|(lhs, rhs)| {
                         let rhs = fetch_cloned_expr(rhs);
                         if_chain! {
-                            if let ExprKind::Index(seqexpr_left, idx_left) = lhs.kind;
-                            if let ExprKind::Index(seqexpr_right, idx_right) = rhs.kind;
-                            if is_slice_like(cx, cx.typeck_results().expr_ty(seqexpr_left))
-                                && is_slice_like(cx, cx.typeck_results().expr_ty(seqexpr_right));
+                            if let ExprKind::Index(base_left, idx_left) = lhs.kind;
+                            if let ExprKind::Index(base_right, idx_right) = rhs.kind;
+                            if is_slice_like(cx, cx.typeck_results().expr_ty(base_left))
+                                && is_slice_like(cx, cx.typeck_results().expr_ty(base_right));
                             if let Some(offset_left) = get_offset(cx, &idx_left, canonical_id);
                             if let Some(offset_right) = get_offset(cx, &idx_right, canonical_id);
 
                             // Source and destination must be different
-                            if var_def_id(cx, seqexpr_left) != var_def_id(cx, seqexpr_right);
+                            if var_def_id(cx, base_left) != var_def_id(cx, base_right);
                             then {
-                                Some((FixedOffsetVar { var: seqexpr_left, offset: offset_left },
-                                    FixedOffsetVar { var: seqexpr_right, offset: offset_right }))
+                                Some((IndexExpr { base: base_left, idx_offset: offset_left },
+                                    IndexExpr { base: base_right, idx_offset: offset_right }))
                             } else {
                                 None
                             }

From b4b4da162f19e9a4c63854a2b5a6167b83f9d8b9 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 10 Jun 2020 15:51:23 +1200
Subject: [PATCH 09/35] Introduce Start and StartKind

---
 clippy_lints/src/loops.rs | 67 +++++++++++++++++++++++++++++++--------
 1 file changed, 53 insertions(+), 14 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index d9896f7fd6b1..157dff59d449 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -818,13 +818,29 @@ impl Offset {
             sign: OffsetSign::Positive,
         }
     }
+
+    fn empty() -> Self {
+        Self::positive("0".into())
+    }
+}
+
+#[derive(Debug, Clone, Copy)]
+enum StartKind<'hir> {
+    Range,
+    Counter { initializer: &'hir Expr<'hir> },
 }
 
 struct IndexExpr<'hir> {
     base: &'hir Expr<'hir>,
+    idx: StartKind<'hir>,
     idx_offset: Offset,
 }
 
+struct Start<'hir> {
+    id: HirId,
+    kind: StartKind<'hir>,
+}
+
 fn is_slice_like<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'_>) -> bool {
     let is_slice = match ty.kind() {
         ty::Ref(_, subty, _) => is_slice_like(cx, subty),
@@ -845,14 +861,32 @@ fn fetch_cloned_expr<'tcx>(expr: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> {
     }
 }
 
-fn get_offset<'tcx>(cx: &LateContext<'tcx>, idx: &Expr<'_>, start: HirId) -> Option<Offset> {
-    fn extract_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, start: HirId) -> Option<String> {
+fn get_offset<'tcx>(
+    cx: &LateContext<'tcx>,
+    idx: &Expr<'_>,
+    starts: &[Start<'tcx>],
+) -> Option<(StartKind<'tcx>, Offset)> {
+    fn extract_start<'tcx>(
+        cx: &LateContext<'tcx>,
+        expr: &Expr<'_>,
+        starts: &[Start<'tcx>],
+    ) -> Option<StartKind<'tcx>> {
+        starts.iter().find(|var| same_var(cx, expr, var.id)).map(|v| v.kind)
+    }
+
+    fn extract_offset<'tcx>(
+        cx: &LateContext<'tcx>,
+        e: &Expr<'_>,
+        starts: &[Start<'tcx>],
+    ) -> Option<String> {
         match &e.kind {
             ExprKind::Lit(l) => match l.node {
                 ast::LitKind::Int(x, _ty) => Some(x.to_string()),
                 _ => None,
             },
-            ExprKind::Path(..) if !same_var(cx, e, start) => Some(snippet_opt(cx, e.span).unwrap_or_else(|| "??".into())),
+            ExprKind::Path(..) if extract_start(cx, e, starts).is_none() => {
+                Some(snippet_opt(cx, e.span).unwrap_or_else(|| "??".into()))
+            },
             _ => None,
         }
     }
@@ -860,20 +894,21 @@ fn get_offset<'tcx>(cx: &LateContext<'tcx>, idx: &Expr<'_>, start: HirId) -> Opt
     match idx.kind {
         ExprKind::Binary(op, lhs, rhs) => match op.node {
             BinOpKind::Add => {
-                let offset_opt = if same_var(cx, lhs, start) {
-                    extract_offset(cx, rhs, start)
-                } else if same_var(cx, rhs, start) {
-                    extract_offset(cx, lhs, start)
+                let offset_opt = if let Some(s) = extract_start(cx, lhs, starts) {
+                    extract_offset(cx, rhs, starts).map(|o| (s, o))
+                } else if let Some(s) = extract_start(cx, rhs, starts) {
+                    extract_offset(cx, lhs, starts).map(|o| (s, o))
                 } else {
                     None
                 };
 
-                offset_opt.map(Offset::positive)
+                offset_opt.map(|(s, o)| (s, Offset::positive(o)))
             },
-            BinOpKind::Sub if same_var(cx, lhs, start) => extract_offset(cx, rhs, start).map(Offset::negative),
+            BinOpKind::Sub => extract_start(cx, lhs, starts)
+                .and_then(|s| extract_offset(cx, rhs, starts).map(|o| (s, Offset::negative(o)))),
             _ => None,
         },
-        ExprKind::Path(..) if same_var(cx, idx, start) => Some(Offset::positive("0".into())),
+        ExprKind::Path(..) => extract_start(cx, idx, starts).map(|s| (s, Offset::empty())),
         _ => None,
     }
 }
@@ -1008,6 +1043,10 @@ fn detect_manual_memcpy<'tcx>(
     {
         // the var must be a single name
         if let PatKind::Binding(_, canonical_id, _, _) = pat.kind {
+            let mut starts = vec![Start {
+                id: canonical_id,
+                kind: StartKind::Range,
+            }];
             // The only statements in the for loops can be indexed assignments from
             // indexed retrievals.
             let big_sugg = get_assignments(body)
@@ -1019,14 +1058,14 @@ fn detect_manual_memcpy<'tcx>(
                             if let ExprKind::Index(base_right, idx_right) = rhs.kind;
                             if is_slice_like(cx, cx.typeck_results().expr_ty(base_left))
                                 && is_slice_like(cx, cx.typeck_results().expr_ty(base_right));
-                            if let Some(offset_left) = get_offset(cx, &idx_left, canonical_id);
-                            if let Some(offset_right) = get_offset(cx, &idx_right, canonical_id);
+                            if let Some((start_left, offset_left)) = get_offset(cx, &idx_left, &starts);
+                            if let Some((start_right, offset_right)) = get_offset(cx, &idx_right, &starts);
 
                             // Source and destination must be different
                             if var_def_id(cx, base_left) != var_def_id(cx, base_right);
                             then {
-                                Some((IndexExpr { base: base_left, idx_offset: offset_left },
-                                    IndexExpr { base: base_right, idx_offset: offset_right }))
+                                Some((IndexExpr { base: base_left, idx: start_left, idx_offset: offset_left },
+                                    IndexExpr { base: base_right, idx: start_right, idx_offset: offset_right }))
                             } else {
                                 None
                             }

From 720f19f2ec4282f636889b35beabf31272e3b1b2 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 10 Jun 2020 15:58:12 +1200
Subject: [PATCH 10/35] Implement detecting `manual_memcpy` with loop counters

---
 clippy_lints/src/loops.rs | 90 ++++++++++++++++++++++++++++-----------
 1 file changed, 66 insertions(+), 24 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 157dff59d449..c036d63c3351 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -913,37 +913,62 @@ fn get_offset<'tcx>(
     }
 }
 
-fn get_assignments<'tcx>(body: &'tcx Expr<'tcx>) -> impl Iterator<Item = Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>> {
-    fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> {
-        if let ExprKind::Assign(lhs, rhs, _) = e.kind {
-            Some((lhs, rhs))
-        } else {
-            None
-        }
+fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> {
+    if let ExprKind::Assign(lhs, rhs, _) = e.kind {
+        Some((lhs, rhs))
+    } else {
+        None
     }
+}
 
-    // This is one of few ways to return different iterators
-    // derived from: https://stackoverflow.com/questions/29760668/conditionally-iterate-over-one-of-several-possible-iterators/52064434#52064434
-    let mut iter_a = None;
-    let mut iter_b = None;
+fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
+    cx: &'a LateContext<'a, 'tcx>,
+    stmts: &'tcx [Stmt<'tcx>],
+    expr: Option<&'tcx Expr<'tcx>>,
+    loop_counters: &'c [Start<'tcx>],
+) -> impl Iterator<Item = Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>> + 'c {
+    stmts
+        .iter()
+        .filter_map(move |stmt| match stmt.kind {
+            StmtKind::Local(..) | StmtKind::Item(..) => None,
+            StmtKind::Expr(e) | StmtKind::Semi(e) => if_chain! {
+                if let ExprKind::AssignOp(_, var, _) = e.kind;
+                // skip StartKind::Range
+                if loop_counters.iter().skip(1).any(|counter| Some(counter.id) == var_def_id(cx, var));
+                then { None } else { Some(e) }
+            },
+        })
+        .chain(expr.into_iter())
+        .map(get_assignment)
+}
 
-    if let ExprKind::Block(b, _) = body.kind {
-        let Block { stmts, expr, .. } = *b;
+fn get_loop_counters<'a, 'tcx>(
+    cx: &'a LateContext<'a, 'tcx>,
+    body: &'tcx Block<'tcx>,
+    expr: &'tcx Expr<'_>,
+) -> Option<impl Iterator<Item = Start<'tcx>> + 'a> {
+    // Look for variables that are incremented once per loop iteration.
+    let mut increment_visitor = IncrementVisitor::new(cx);
+    walk_block(&mut increment_visitor, body);
 
-        iter_a = stmts
-            .iter()
-            .filter_map(|stmt| match stmt.kind {
-                StmtKind::Local(..) | StmtKind::Item(..) => None,
-                StmtKind::Expr(e) | StmtKind::Semi(e) => Some(e),
+    // For each candidate, check the parent block to see if
+    // it's initialized to zero at the start of the loop.
+    if let Some(block) = get_enclosing_block(&cx, expr.hir_id) {
+        increment_visitor
+            .into_results()
+            .filter_map(move |var_id| {
+                let mut initialize_visitor = InitializeVisitor::new(cx, expr, var_id);
+                walk_block(&mut initialize_visitor, block);
+
+                initialize_visitor.get_result().map(|(_, initializer)| Start {
+                    id: var_id,
+                    kind: StartKind::Counter { initializer },
+                })
             })
-            .chain(expr.into_iter())
-            .map(get_assignment)
             .into()
     } else {
-        iter_b = Some(get_assignment(body))
+        None
     }
-
-    iter_a.into_iter().flatten().chain(iter_b.into_iter())
 }
 
 fn build_manual_memcpy_suggestion<'tcx>(
@@ -1047,9 +1072,26 @@ fn detect_manual_memcpy<'tcx>(
                 id: canonical_id,
                 kind: StartKind::Range,
             }];
+
+            // This is one of few ways to return different iterators
+            // derived from: https://stackoverflow.com/questions/29760668/conditionally-iterate-over-one-of-several-possible-iterators/52064434#52064434
+            let mut iter_a = None;
+            let mut iter_b = None;
+
+            if let ExprKind::Block(block, _) = body.kind {
+                if let Some(loop_counters) = get_loop_counters(cx, block, expr) {
+                    starts.extend(loop_counters);
+                }
+                iter_a = Some(get_assignments(cx, block.stmts, block.expr, &starts));
+            } else {
+                iter_b = Some(get_assignment(body));
+            }
+
             // The only statements in the for loops can be indexed assignments from
             // indexed retrievals.
-            let big_sugg = get_assignments(body)
+            let assignments = iter_a.into_iter().flatten().chain(iter_b.into_iter());
+
+            let big_sugg = assignments
                 .map(|o| {
                     o.and_then(|(lhs, rhs)| {
                         let rhs = fetch_cloned_expr(rhs);

From de56279cd9047832216e1d1c06dc45375bf01b31 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Thu, 11 Jun 2020 11:06:13 +1200
Subject: [PATCH 11/35] Implement building the `manual_memcpy` sugggestion with
 loop counters

---
 clippy_lints/src/loops.rs      | 185 +++++++++++++++++++++++++--------
 clippy_lints/src/utils/sugg.rs |  78 +++++++++++++-
 2 files changed, 213 insertions(+), 50 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index c036d63c3351..bd2ae47b7234 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -800,19 +800,19 @@ enum OffsetSign {
 }
 
 struct Offset {
-    value: String,
+    value: MinifyingSugg<'static>,
     sign: OffsetSign,
 }
 
 impl Offset {
-    fn negative(value: String) -> Self {
+    fn negative(value: MinifyingSugg<'static>) -> Self {
         Self {
             value,
             sign: OffsetSign::Negative,
         }
     }
 
-    fn positive(value: String) -> Self {
+    fn positive(value: MinifyingSugg<'static>) -> Self {
         Self {
             value,
             sign: OffsetSign::Positive,
@@ -820,7 +820,88 @@ impl Offset {
     }
 
     fn empty() -> Self {
-        Self::positive("0".into())
+        Self::positive(MinifyingSugg::non_paren("0"))
+    }
+}
+
+fn apply_offset(lhs: &MinifyingSugg<'static>, rhs: &Offset) -> MinifyingSugg<'static> {
+    match rhs.sign {
+        OffsetSign::Positive => lhs + &rhs.value,
+        OffsetSign::Negative => lhs - &rhs.value,
+    }
+}
+
+#[derive(Clone)]
+struct MinifyingSugg<'a>(sugg::Sugg<'a>);
+
+impl std::fmt::Display for MinifyingSugg<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
+        std::fmt::Display::fmt(&self.0, f)
+    }
+}
+
+impl<'a> MinifyingSugg<'a> {
+    fn as_str(&self) -> &str {
+        let sugg::Sugg::NonParen(s) | sugg::Sugg::MaybeParen(s) | sugg::Sugg::BinOp(_, s) = &self.0;
+        s.as_ref()
+    }
+
+    fn hir(cx: &LateContext<'_, '_>, expr: &Expr<'_>, default: &'a str) -> Self {
+        Self(sugg::Sugg::hir(cx, expr, default))
+    }
+
+    fn maybe_par(self) -> Self {
+        Self(self.0.maybe_par())
+    }
+
+    fn non_paren(str: impl Into<std::borrow::Cow<'a, str>>) -> Self {
+        Self(sugg::Sugg::NonParen(str.into()))
+    }
+}
+
+impl std::ops::Add for &MinifyingSugg<'static> {
+    type Output = MinifyingSugg<'static>;
+    fn add(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
+        match (self.as_str(), rhs.as_str()) {
+            ("0", _) => rhs.clone(),
+            (_, "0") => self.clone(),
+            (_, _) => MinifyingSugg(&self.0 + &rhs.0),
+        }
+    }
+}
+
+impl std::ops::Sub for &MinifyingSugg<'static> {
+    type Output = MinifyingSugg<'static>;
+    fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
+        match (self.as_str(), rhs.as_str()) {
+            (_, "0") => self.clone(),
+            ("0", _) => MinifyingSugg(sugg::make_unop("-", rhs.0.clone())),
+            (x, y) if x == y => MinifyingSugg::non_paren("0"),
+            (_, _) => MinifyingSugg(&self.0 - &rhs.0),
+        }
+    }
+}
+
+impl std::ops::Add<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
+    type Output = MinifyingSugg<'static>;
+    fn add(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
+        match (self.as_str(), rhs.as_str()) {
+            ("0", _) => rhs.clone(),
+            (_, "0") => self,
+            (_, _) => MinifyingSugg(self.0 + &rhs.0),
+        }
+    }
+}
+
+impl std::ops::Sub<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
+    type Output = MinifyingSugg<'static>;
+    fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
+        match (self.as_str(), rhs.as_str()) {
+            (_, "0") => self,
+            ("0", _) => MinifyingSugg(sugg::make_unop("-", rhs.0.clone())),
+            (x, y) if x == y => MinifyingSugg::non_paren("0"),
+            (_, _) => MinifyingSugg(self.0 - &rhs.0),
+        }
     }
 }
 
@@ -878,14 +959,15 @@ fn get_offset<'tcx>(
         cx: &LateContext<'tcx>,
         e: &Expr<'_>,
         starts: &[Start<'tcx>],
-    ) -> Option<String> {
+    ) -> Option<MinifyingSugg<'static>> {
         match &e.kind {
             ExprKind::Lit(l) => match l.node {
-                ast::LitKind::Int(x, _ty) => Some(x.to_string()),
+                ast::LitKind::Int(x, _ty) => Some(MinifyingSugg::non_paren(x.to_string())),
                 _ => None,
             },
             ExprKind::Path(..) if extract_start(cx, e, starts).is_none() => {
-                Some(snippet_opt(cx, e.span).unwrap_or_else(|| "??".into()))
+                // `e` is always non paren as it's a `Path`
+                Some(MinifyingSugg::non_paren(snippet(cx, e.span, "???")))
             },
             _ => None,
         }
@@ -979,32 +1061,15 @@ fn build_manual_memcpy_suggestion<'tcx>(
     dst: IndexExpr<'_>,
     src: IndexExpr<'_>,
 ) -> String {
-    fn print_sum(arg1: &str, arg2: &Offset) -> String {
-        match (arg1, &arg2.value[..], arg2.sign) {
-            ("0", "0", _) => "0".into(),
-            ("0", x, OffsetSign::Positive) | (x, "0", _) => x.into(),
-            ("0", x, OffsetSign::Negative) => format!("-{}", x),
-            (x, y, OffsetSign::Positive) => format!("({} + {})", x, y),
-            (x, y, OffsetSign::Negative) => {
-                if x == y {
-                    "0".into()
-                } else {
-                    format!("({} - {})", x, y)
-                }
-            },
-        }
-    }
-
-    fn print_offset(start_str: &str, inline_offset: &Offset) -> String {
-        let offset = print_sum(start_str, inline_offset);
+    fn print_offset(offset: MinifyingSugg<'static>) -> MinifyingSugg<'static> {
         if offset.as_str() == "0" {
-            "".into()
+            MinifyingSugg::non_paren("")
         } else {
             offset
         }
     }
 
-    let print_limit = |end: &Expr<'_>, offset: Offset, base: &Expr<'_>| {
+    let print_limit = |end: &Expr<'_>, end_str: &str, base: &Expr<'_>, sugg: MinifyingSugg<'static>| -> MinifyingSugg<'static> {
         if_chain! {
             if let ExprKind::MethodCall(method, _, len_args, _) = end.kind;
             if method.ident.name == sym!(len);
@@ -1012,42 +1077,72 @@ fn build_manual_memcpy_suggestion<'tcx>(
             if let Some(arg) = len_args.get(0);
             if var_def_id(cx, arg) == var_def_id(cx, base);
             then {
-                match offset.sign {
-                    OffsetSign::Negative => format!("({} - {})", snippet(cx, end.span, "<src>.len()"), offset.value),
-                    OffsetSign::Positive => "".into(),
+                if sugg.as_str() == end_str {
+                    MinifyingSugg::non_paren("")
+                } else {
+                    sugg
                 }
             } else {
-                let end_str = match limits {
+                match limits {
                     ast::RangeLimits::Closed => {
-                        let end = sugg::Sugg::hir(cx, end, "<count>");
-                        format!("{}", end + sugg::ONE)
+                        sugg + &MinifyingSugg::non_paren("1")
                     },
-                    ast::RangeLimits::HalfOpen => format!("{}", snippet(cx, end.span, "..")),
-                };
-
-                print_sum(&end_str, &offset)
+                    ast::RangeLimits::HalfOpen => sugg,
+                }
             }
         }
     };
 
-    let start_str = snippet(cx, start.span, "").to_string();
-    let dst_offset = print_offset(&start_str, &dst.idx_offset);
-    let dst_limit = print_limit(end, dst.idx_offset, dst.base);
-    let src_offset = print_offset(&start_str, &src.idx_offset);
-    let src_limit = print_limit(end, src.idx_offset, src.base);
+    let start_str = MinifyingSugg::hir(cx, start, "");
+    let end_str = MinifyingSugg::hir(cx, end, "");
+
+    let print_offset_and_limit = |idx_expr: &IndexExpr<'_>| match idx_expr.idx {
+        StartKind::Range => (
+            print_offset(apply_offset(&start_str, &idx_expr.idx_offset)),
+            print_limit(
+                end,
+                end_str.as_str(),
+                idx_expr.base,
+                apply_offset(&end_str, &idx_expr.idx_offset),
+            ),
+        ),
+        StartKind::Counter { initializer } => {
+            let counter_start = MinifyingSugg::hir(cx, initializer, "");
+            (
+                print_offset(apply_offset(&counter_start, &idx_expr.idx_offset)),
+                print_limit(
+                    end,
+                    end_str.as_str(),
+                    idx_expr.base,
+                    apply_offset(&end_str, &idx_expr.idx_offset) + &counter_start - &start_str,
+                ),
+            )
+        },
+    };
+
+    let (dst_offset, dst_limit) = print_offset_and_limit(&dst);
+    let (src_offset, src_limit) = print_offset_and_limit(&src);
 
     let dst_base_str = snippet_opt(cx, dst.base.span).unwrap_or_else(|| "???".into());
     let src_base_str = snippet_opt(cx, src.base.span).unwrap_or_else(|| "???".into());
 
-    let dst = if dst_offset == "" && dst_limit == "" {
+    let dst = if dst_offset.as_str() == "" && dst_limit.as_str() == "" {
         dst_base_str
     } else {
-        format!("{}[{}..{}]", dst_base_str, dst_offset, dst_limit)
+        format!(
+            "{}[{}..{}]",
+            dst_base_str,
+            dst_offset.maybe_par(),
+            dst_limit.maybe_par()
+        )
     };
 
     format!(
         "{}.clone_from_slice(&{}[{}..{}])",
-        dst, src_base_str, src_offset, src_limit
+        dst,
+        src_base_str,
+        src_offset.maybe_par(),
+        src_limit.maybe_par()
     )
 }
 
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index ec8b7e59b597..50d48650a093 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -36,6 +36,46 @@ impl Display for Sugg<'_> {
     }
 }
 
+// It's impossible to derive Clone due to the lack of the impl Clone for AssocOp
+impl Clone for Sugg<'_> {
+    fn clone(&self) -> Self {
+        /// manually cloning AssocOp
+        fn clone_assoc_op(this: &AssocOp) -> AssocOp {
+            match this {
+                AssocOp::Add => AssocOp::Add,
+                AssocOp::Subtract => AssocOp::Subtract,
+                AssocOp::Multiply => AssocOp::Multiply,
+                AssocOp::Divide => AssocOp::Divide,
+                AssocOp::Modulus => AssocOp::Modulus,
+                AssocOp::LAnd => AssocOp::LAnd,
+                AssocOp::LOr => AssocOp::LOr,
+                AssocOp::BitXor => AssocOp::BitXor,
+                AssocOp::BitAnd => AssocOp::BitAnd,
+                AssocOp::BitOr => AssocOp::BitOr,
+                AssocOp::ShiftLeft => AssocOp::ShiftLeft,
+                AssocOp::ShiftRight => AssocOp::ShiftRight,
+                AssocOp::Equal => AssocOp::Equal,
+                AssocOp::Less => AssocOp::Less,
+                AssocOp::LessEqual => AssocOp::LessEqual,
+                AssocOp::NotEqual => AssocOp::NotEqual,
+                AssocOp::Greater => AssocOp::Greater,
+                AssocOp::GreaterEqual => AssocOp::GreaterEqual,
+                AssocOp::Assign => AssocOp::Assign,
+                AssocOp::AssignOp(t) => AssocOp::AssignOp(*t),
+                AssocOp::As => AssocOp::As,
+                AssocOp::DotDot => AssocOp::DotDot,
+                AssocOp::DotDotEq => AssocOp::DotDotEq,
+                AssocOp::Colon => AssocOp::Colon,
+            }
+        }
+        match self {
+            Sugg::NonParen(x) => Sugg::NonParen(x.clone()),
+            Sugg::MaybeParen(x) => Sugg::MaybeParen(x.clone()),
+            Sugg::BinOp(op, x) => Sugg::BinOp(clone_assoc_op(op), x.clone()),
+        }
+    }
+}
+
 #[allow(clippy::wrong_self_convention)] // ok, because of the function `as_ty` method
 impl<'a> Sugg<'a> {
     /// Prepare a suggestion from an expression.
@@ -267,21 +307,49 @@ impl<'a> Sugg<'a> {
     }
 }
 
-impl<'a, 'b> std::ops::Add<Sugg<'b>> for Sugg<'a> {
+impl std::ops::Add for Sugg<'_> {
     type Output = Sugg<'static>;
-    fn add(self, rhs: Sugg<'b>) -> Sugg<'static> {
+    fn add(self, rhs: Sugg<'_>) -> Sugg<'static> {
         make_binop(ast::BinOpKind::Add, &self, &rhs)
     }
 }
 
-impl<'a, 'b> std::ops::Sub<Sugg<'b>> for Sugg<'a> {
+impl std::ops::Sub for Sugg<'_> {
     type Output = Sugg<'static>;
-    fn sub(self, rhs: Sugg<'b>) -> Sugg<'static> {
+    fn sub(self, rhs: Sugg<'_>) -> Sugg<'static> {
         make_binop(ast::BinOpKind::Sub, &self, &rhs)
     }
 }
 
-impl<'a> std::ops::Not for Sugg<'a> {
+impl std::ops::Add<&Sugg<'_>> for Sugg<'_> {
+    type Output = Sugg<'static>;
+    fn add(self, rhs: &Sugg<'_>) -> Sugg<'static> {
+        make_binop(ast::BinOpKind::Add, &self, rhs)
+    }
+}
+
+impl std::ops::Sub<&Sugg<'_>> for Sugg<'_> {
+    type Output = Sugg<'static>;
+    fn sub(self, rhs: &Sugg<'_>) -> Sugg<'static> {
+        make_binop(ast::BinOpKind::Sub, &self, rhs)
+    }
+}
+
+impl std::ops::Add for &Sugg<'_> {
+    type Output = Sugg<'static>;
+    fn add(self, rhs: &Sugg<'_>) -> Sugg<'static> {
+        make_binop(ast::BinOpKind::Add, self, rhs)
+    }
+}
+
+impl std::ops::Sub for &Sugg<'_> {
+    type Output = Sugg<'static>;
+    fn sub(self, rhs: &Sugg<'_>) -> Sugg<'static> {
+        make_binop(ast::BinOpKind::Sub, self, rhs)
+    }
+}
+
+impl std::ops::Not for Sugg<'_> {
     type Output = Sugg<'static>;
     fn not(self) -> Sugg<'static> {
         make_unop("!", self)

From 8da6cfd17b7707d678d01a6688572169015ea83e Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Thu, 11 Jun 2020 12:11:06 +1200
Subject: [PATCH 12/35] fmt

---
 clippy_lints/src/loops.rs | 41 ++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index bd2ae47b7234..2aee3b93e825 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -1069,29 +1069,30 @@ fn build_manual_memcpy_suggestion<'tcx>(
         }
     }
 
-    let print_limit = |end: &Expr<'_>, end_str: &str, base: &Expr<'_>, sugg: MinifyingSugg<'static>| -> MinifyingSugg<'static> {
-        if_chain! {
-            if let ExprKind::MethodCall(method, _, len_args, _) = end.kind;
-            if method.ident.name == sym!(len);
-            if len_args.len() == 1;
-            if let Some(arg) = len_args.get(0);
-            if var_def_id(cx, arg) == var_def_id(cx, base);
-            then {
-                if sugg.as_str() == end_str {
-                    MinifyingSugg::non_paren("")
+    let print_limit =
+        |end: &Expr<'_>, end_str: &str, base: &Expr<'_>, sugg: MinifyingSugg<'static>| -> MinifyingSugg<'static> {
+            if_chain! {
+                if let ExprKind::MethodCall(method, _, len_args, _) = end.kind;
+                if method.ident.name == sym!(len);
+                if len_args.len() == 1;
+                if let Some(arg) = len_args.get(0);
+                if var_def_id(cx, arg) == var_def_id(cx, base);
+                then {
+                    if sugg.as_str() == end_str {
+                        MinifyingSugg::non_paren("")
+                    } else {
+                        sugg
+                    }
                 } else {
-                    sugg
-                }
-            } else {
-                match limits {
-                    ast::RangeLimits::Closed => {
-                        sugg + &MinifyingSugg::non_paren("1")
-                    },
-                    ast::RangeLimits::HalfOpen => sugg,
+                    match limits {
+                        ast::RangeLimits::Closed => {
+                            sugg + &MinifyingSugg::non_paren("1")
+                        },
+                        ast::RangeLimits::HalfOpen => sugg,
+                    }
                 }
             }
-        }
-    };
+        };
 
     let start_str = MinifyingSugg::hir(cx, start, "");
     let end_str = MinifyingSugg::hir(cx, end, "");

From d9a88be0b05c2cfec0679caf428d129c9d46256d Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Thu, 18 Jun 2020 10:24:12 +1200
Subject: [PATCH 13/35] Rename `get_offset` and its private items

---
 clippy_lints/src/loops.rs | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 2aee3b93e825..98c411f5ae60 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -942,20 +942,20 @@ fn fetch_cloned_expr<'tcx>(expr: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> {
     }
 }
 
-fn get_offset<'tcx>(
+fn get_details_from_idx<'tcx>(
     cx: &LateContext<'tcx>,
     idx: &Expr<'_>,
     starts: &[Start<'tcx>],
 ) -> Option<(StartKind<'tcx>, Offset)> {
-    fn extract_start<'tcx>(
+    fn get_start<'tcx>(
         cx: &LateContext<'tcx>,
-        expr: &Expr<'_>,
+        e: &Expr<'_>,
         starts: &[Start<'tcx>],
     ) -> Option<StartKind<'tcx>> {
-        starts.iter().find(|var| same_var(cx, expr, var.id)).map(|v| v.kind)
+        starts.iter().find(|var| same_var(cx, e, var.id)).map(|v| v.kind)
     }
 
-    fn extract_offset<'tcx>(
+    fn get_offset<'tcx>(
         cx: &LateContext<'tcx>,
         e: &Expr<'_>,
         starts: &[Start<'tcx>],
@@ -965,7 +965,7 @@ fn get_offset<'tcx>(
                 ast::LitKind::Int(x, _ty) => Some(MinifyingSugg::non_paren(x.to_string())),
                 _ => None,
             },
-            ExprKind::Path(..) if extract_start(cx, e, starts).is_none() => {
+            ExprKind::Path(..) if get_start(cx, e, starts).is_none() => {
                 // `e` is always non paren as it's a `Path`
                 Some(MinifyingSugg::non_paren(snippet(cx, e.span, "???")))
             },
@@ -976,21 +976,22 @@ fn get_offset<'tcx>(
     match idx.kind {
         ExprKind::Binary(op, lhs, rhs) => match op.node {
             BinOpKind::Add => {
-                let offset_opt = if let Some(s) = extract_start(cx, lhs, starts) {
-                    extract_offset(cx, rhs, starts).map(|o| (s, o))
-                } else if let Some(s) = extract_start(cx, rhs, starts) {
-                    extract_offset(cx, lhs, starts).map(|o| (s, o))
+                let offset_opt = if let Some(s) = get_start(cx, lhs, starts) {
+                    get_offset(cx, rhs, starts).map(|o| (s, o))
+                } else if let Some(s) = get_start(cx, rhs, starts) {
+                    get_offset(cx, lhs, starts).map(|o| (s, o))
                 } else {
                     None
                 };
 
                 offset_opt.map(|(s, o)| (s, Offset::positive(o)))
             },
-            BinOpKind::Sub => extract_start(cx, lhs, starts)
-                .and_then(|s| extract_offset(cx, rhs, starts).map(|o| (s, Offset::negative(o)))),
+            BinOpKind::Sub => {
+                get_start(cx, lhs, starts).and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, Offset::negative(o))))
+            },
             _ => None,
         },
-        ExprKind::Path(..) => extract_start(cx, idx, starts).map(|s| (s, Offset::empty())),
+        ExprKind::Path(..) => get_start(cx, idx, starts).map(|s| (s, Offset::empty())),
         _ => None,
     }
 }
@@ -1196,8 +1197,8 @@ fn detect_manual_memcpy<'tcx>(
                             if let ExprKind::Index(base_right, idx_right) = rhs.kind;
                             if is_slice_like(cx, cx.typeck_results().expr_ty(base_left))
                                 && is_slice_like(cx, cx.typeck_results().expr_ty(base_right));
-                            if let Some((start_left, offset_left)) = get_offset(cx, &idx_left, &starts);
-                            if let Some((start_right, offset_right)) = get_offset(cx, &idx_right, &starts);
+                            if let Some((start_left, offset_left)) = get_details_from_idx(cx, &idx_left, &starts);
+                            if let Some((start_right, offset_right)) = get_details_from_idx(cx, &idx_right, &starts);
 
                             // Source and destination must be different
                             if var_def_id(cx, base_left) != var_def_id(cx, base_right);

From 774e82a566c6c3349700a8bece44d6a8c6755039 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Thu, 11 Jun 2020 11:19:44 +1200
Subject: [PATCH 14/35] Add the tests for `manual_memcpy` with loop counters

---
 tests/ui/manual_memcpy.rs     | 54 +++++++++++++++++++++++++++++++
 tests/ui/manual_memcpy.stderr | 60 +++++++++++++++++++++++++++++++++--
 2 files changed, 111 insertions(+), 3 deletions(-)

diff --git a/tests/ui/manual_memcpy.rs b/tests/ui/manual_memcpy.rs
index 0083f94798fe..87bfb4fdd163 100644
--- a/tests/ui/manual_memcpy.rs
+++ b/tests/ui/manual_memcpy.rs
@@ -115,6 +115,60 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
     }
 }
 
+#[allow(clippy::needless_range_loop, clippy::explicit_counter_loop)]
+pub fn manual_copy_with_counters(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
+    let mut count = 0;
+    for i in 3..src.len() {
+        dst[i] = src[count];
+        count += 1;
+    }
+
+    let mut count = 0;
+    for i in 3..src.len() {
+        dst[count] = src[i];
+        count += 1;
+    }
+
+    let mut count = 3;
+    for i in 0..src.len() {
+        dst[count] = src[i];
+        count += 1;
+    }
+
+    let mut count = 3;
+    for i in 0..src.len() {
+        dst[i] = src[count];
+        count += 1;
+    }
+
+    let mut count = 0;
+    for i in 3..(3 + src.len()) {
+        dst[i] = src[count];
+        count += 1;
+    }
+
+    let mut count = 3;
+    for i in 5..src.len() {
+        dst[i] = src[count - 2];
+        count += 1;
+    }
+
+    let mut count = 5;
+    for i in 3..10 {
+        dst[i] = src[count];
+        count += 1;
+    }
+
+    let mut count = 3;
+    let mut count = 30;
+    for i in 0..src.len() {
+        dst[count] = src[i];
+        dst2[count] = src[i];
+        count += 1;
+        count += 1;
+    }
+}
+
 #[warn(clippy::needless_range_loop, clippy::manual_memcpy)]
 pub fn manual_clone(src: &[String], dst: &mut [String]) {
     for i in 0..src.len() {
diff --git a/tests/ui/manual_memcpy.stderr b/tests/ui/manual_memcpy.stderr
index bad84a589008..a5698b08103b 100644
--- a/tests/ui/manual_memcpy.stderr
+++ b/tests/ui/manual_memcpy.stderr
@@ -16,7 +16,7 @@ error: it looks like you're manually copying between slices
   --> $DIR/manual_memcpy.rs:17:14
    |
 LL |     for i in 0..src.len() {
-   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..])`
+   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..(src.len() + 10)])`
 
 error: it looks like you're manually copying between slices
   --> $DIR/manual_memcpy.rs:22:14
@@ -79,10 +79,64 @@ LL |     for i in 0..0 {
    |              ^^^^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:120:14
+  --> $DIR/manual_memcpy.rs:121:14
+   |
+LL |     for i in 3..src.len() {
+   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)])`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/manual_memcpy.rs:127:14
+   |
+LL |     for i in 3..src.len() {
+   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..])`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/manual_memcpy.rs:133:14
+   |
+LL |     for i in 0..src.len() {
+   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..])`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/manual_memcpy.rs:139:14
+   |
+LL |     for i in 0..src.len() {
+   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)])`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/manual_memcpy.rs:145:14
+   |
+LL |     for i in 3..(3 + src.len()) {
+   |              ^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..(3 + src.len())].clone_from_slice(&src[..((3 + src.len()) - 3)])`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/manual_memcpy.rs:151:14
+   |
+LL |     for i in 5..src.len() {
+   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)])`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/manual_memcpy.rs:157:14
+   |
+LL |     for i in 3..10 {
+   |              ^^^^^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)])`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/manual_memcpy.rs:164:14
+   |
+LL |     for i in 0..src.len() {
+   |              ^^^^^^^^^^^^
+   |
+help: try replacing the loop by
+   |
+LL |     for i in dst[3..(src.len() + 3)].clone_from_slice(&src[..])
+LL |     dst2[30..(src.len() + 30)].clone_from_slice(&src[..]) {
+   |
+
+error: it looks like you're manually copying between slices
+  --> $DIR/manual_memcpy.rs:174:14
    |
 LL |     for i in 0..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`
 
-error: aborting due to 13 previous errors
+error: aborting due to 21 previous errors
 

From 9aad38bf614c3fb6d306f5dec4a0af606bb3c9c8 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Thu, 18 Jun 2020 10:48:44 +1200
Subject: [PATCH 15/35] Update `manual_memcpy.stderr` to reflect additional
 parentheses

---
 tests/ui/manual_memcpy.stderr | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/ui/manual_memcpy.stderr b/tests/ui/manual_memcpy.stderr
index a5698b08103b..e6bef9981a3d 100644
--- a/tests/ui/manual_memcpy.stderr
+++ b/tests/ui/manual_memcpy.stderr
@@ -58,13 +58,13 @@ error: it looks like you're manually copying between slices
   --> $DIR/manual_memcpy.rs:94:14
    |
 LL |     for i in from..from + src.len() {
-   |              ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + src.len()].clone_from_slice(&src[..(from + src.len() - from)])`
+   |              ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..(from + src.len())].clone_from_slice(&src[..(from + src.len() - from)])`
 
 error: it looks like you're manually copying between slices
   --> $DIR/manual_memcpy.rs:98:14
    |
 LL |     for i in from..from + 3 {
-   |              ^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + 3].clone_from_slice(&src[..(from + 3 - from)])`
+   |              ^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..(from + 3)].clone_from_slice(&src[..(from + 3 - from)])`
 
 error: it looks like you're manually copying between slices
   --> $DIR/manual_memcpy.rs:103:14
@@ -106,7 +106,7 @@ error: it looks like you're manually copying between slices
   --> $DIR/manual_memcpy.rs:145:14
    |
 LL |     for i in 3..(3 + src.len()) {
-   |              ^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..(3 + src.len())].clone_from_slice(&src[..((3 + src.len()) - 3)])`
+   |              ^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)])`
 
 error: it looks like you're manually copying between slices
   --> $DIR/manual_memcpy.rs:151:14

From 4ea4a972500a8ddecfc737d51eec960324dcb02f Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Thu, 18 Jun 2020 12:31:13 +1200
Subject: [PATCH 16/35] Add tests for bitwise operations

---
 tests/ui/manual_memcpy.rs     | 8 ++++++++
 tests/ui/manual_memcpy.stderr | 8 +++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/tests/ui/manual_memcpy.rs b/tests/ui/manual_memcpy.rs
index 87bfb4fdd163..4846ab5eaaa3 100644
--- a/tests/ui/manual_memcpy.rs
+++ b/tests/ui/manual_memcpy.rs
@@ -167,6 +167,14 @@ pub fn manual_copy_with_counters(src: &[i32], dst: &mut [i32], dst2: &mut [i32])
         count += 1;
         count += 1;
     }
+
+    // make sure parentheses are added properly to bitwise operators, which have lower precedence than
+    // arithmetric ones
+    let mut count = 0 << 1;
+    for i in 0..1 << 1 {
+        dst[count] = src[i + 2];
+        count += 1;
+    }
 }
 
 #[warn(clippy::needless_range_loop, clippy::manual_memcpy)]
diff --git a/tests/ui/manual_memcpy.stderr b/tests/ui/manual_memcpy.stderr
index e6bef9981a3d..d189bde0508e 100644
--- a/tests/ui/manual_memcpy.stderr
+++ b/tests/ui/manual_memcpy.stderr
@@ -135,8 +135,14 @@ LL |     dst2[30..(src.len() + 30)].clone_from_slice(&src[..]) {
 error: it looks like you're manually copying between slices
   --> $DIR/manual_memcpy.rs:174:14
    |
+LL |     for i in 0..1 << 1 {
+   |              ^^^^^^^^^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)])`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/manual_memcpy.rs:182:14
+   |
 LL |     for i in 0..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`
 
-error: aborting due to 21 previous errors
+error: aborting due to 22 previous errors
 

From eb3ffe6ed2999e9d3385be0ff981e30082ea0d2c Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Tue, 23 Jun 2020 18:51:05 +1200
Subject: [PATCH 17/35] make use of macros in operator overloading

---
 clippy_lints/src/utils/sugg.rs | 55 ++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index 50d48650a093..aada4122e78a 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -13,6 +13,7 @@ use rustc_span::{BytePos, Pos};
 use std::borrow::Cow;
 use std::convert::TryInto;
 use std::fmt::Display;
+use std::ops::{Add, Sub, Not};
 
 /// A helper type to build suggestion correctly handling parenthesis.
 pub enum Sugg<'a> {
@@ -307,49 +308,53 @@ impl<'a> Sugg<'a> {
     }
 }
 
-impl std::ops::Add for Sugg<'_> {
-    type Output = Sugg<'static>;
-    fn add(self, rhs: Sugg<'_>) -> Sugg<'static> {
-        make_binop(ast::BinOpKind::Add, &self, &rhs)
-    }
-}
+// Copied from the rust standart library, and then edited
+macro_rules! forward_binop_impls_to_ref {
+    (impl $imp:ident, $method:ident for $t:ty, type Output = $o:ty) => {
+        impl $imp<$t> for &$t {
+            type Output = $o;
 
-impl std::ops::Sub for Sugg<'_> {
-    type Output = Sugg<'static>;
-    fn sub(self, rhs: Sugg<'_>) -> Sugg<'static> {
-        make_binop(ast::BinOpKind::Sub, &self, &rhs)
-    }
-}
+            fn $method(self, other: $t) -> $o {
+                $imp::$method(self, &other)
+            }
+        }
 
-impl std::ops::Add<&Sugg<'_>> for Sugg<'_> {
-    type Output = Sugg<'static>;
-    fn add(self, rhs: &Sugg<'_>) -> Sugg<'static> {
-        make_binop(ast::BinOpKind::Add, &self, rhs)
-    }
-}
+        impl $imp<&$t> for $t {
+            type Output = $o;
 
-impl std::ops::Sub<&Sugg<'_>> for Sugg<'_> {
-    type Output = Sugg<'static>;
-    fn sub(self, rhs: &Sugg<'_>) -> Sugg<'static> {
-        make_binop(ast::BinOpKind::Sub, &self, rhs)
+            fn $method(self, other: &$t) -> $o {
+                $imp::$method(&self, other)
+            }
+        }
+
+        impl $imp for $t {
+            type Output = $o;
+
+            fn $method(self, other: $t) -> $o {
+                $imp::$method(&self, &other)
+            }
+        }
     }
 }
 
-impl std::ops::Add for &Sugg<'_> {
+impl Add for &Sugg<'_> {
     type Output = Sugg<'static>;
     fn add(self, rhs: &Sugg<'_>) -> Sugg<'static> {
         make_binop(ast::BinOpKind::Add, self, rhs)
     }
 }
 
-impl std::ops::Sub for &Sugg<'_> {
+impl Sub for &Sugg<'_> {
     type Output = Sugg<'static>;
     fn sub(self, rhs: &Sugg<'_>) -> Sugg<'static> {
         make_binop(ast::BinOpKind::Sub, self, rhs)
     }
 }
 
-impl std::ops::Not for Sugg<'_> {
+forward_binop_impls_to_ref!(impl Add, add for Sugg<'_>, type Output = Sugg<'static>);
+forward_binop_impls_to_ref!(impl Sub, sub for Sugg<'_>, type Output = Sugg<'static>);
+
+impl Not for Sugg<'_> {
     type Output = Sugg<'static>;
     fn not(self) -> Sugg<'static> {
         make_unop("!", self)

From 10d7a18f72155f03dbd27b872a52b5dd45def8db Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Tue, 23 Jun 2020 22:37:36 +1200
Subject: [PATCH 18/35] fmt

---
 clippy_lints/src/utils/sugg.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index aada4122e78a..8f2aa724d20b 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -13,7 +13,7 @@ use rustc_span::{BytePos, Pos};
 use std::borrow::Cow;
 use std::convert::TryInto;
 use std::fmt::Display;
-use std::ops::{Add, Sub, Not};
+use std::ops::{Add, Not, Sub};
 
 /// A helper type to build suggestion correctly handling parenthesis.
 pub enum Sugg<'a> {
@@ -334,7 +334,7 @@ macro_rules! forward_binop_impls_to_ref {
                 $imp::$method(&self, &other)
             }
         }
-    }
+    };
 }
 
 impl Add for &Sugg<'_> {

From 174065fc98ef9335ea45a234aa18286cdf6c3934 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 24 Jun 2020 11:33:49 +1200
Subject: [PATCH 19/35] fix the multiple counters test

---
 tests/ui/manual_memcpy.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/ui/manual_memcpy.rs b/tests/ui/manual_memcpy.rs
index 4846ab5eaaa3..8318fd898112 100644
--- a/tests/ui/manual_memcpy.rs
+++ b/tests/ui/manual_memcpy.rs
@@ -160,12 +160,12 @@ pub fn manual_copy_with_counters(src: &[i32], dst: &mut [i32], dst2: &mut [i32])
     }
 
     let mut count = 3;
-    let mut count = 30;
+    let mut count2 = 30;
     for i in 0..src.len() {
         dst[count] = src[i];
-        dst2[count] = src[i];
-        count += 1;
+        dst2[count2] = src[i];
         count += 1;
+        count2 += 1;
     }
 
     // make sure parentheses are added properly to bitwise operators, which have lower precedence than

From 44187383f4724bd7e4b2b220235e93438043947a Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 24 Jun 2020 12:41:46 +1200
Subject: [PATCH 20/35] Use operator overloading instead of direct calls of
 `make_binop`

---
 clippy_lints/src/loops.rs      | 4 ++--
 clippy_lints/src/utils/sugg.rs | 9 ++++++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 98c411f5ae60..5928d12f30be 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -875,7 +875,7 @@ impl std::ops::Sub for &MinifyingSugg<'static> {
     fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
         match (self.as_str(), rhs.as_str()) {
             (_, "0") => self.clone(),
-            ("0", _) => MinifyingSugg(sugg::make_unop("-", rhs.0.clone())),
+            ("0", _) => MinifyingSugg(-(rhs.0.clone())),
             (x, y) if x == y => MinifyingSugg::non_paren("0"),
             (_, _) => MinifyingSugg(&self.0 - &rhs.0),
         }
@@ -898,7 +898,7 @@ impl std::ops::Sub<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
     fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
         match (self.as_str(), rhs.as_str()) {
             (_, "0") => self,
-            ("0", _) => MinifyingSugg(sugg::make_unop("-", rhs.0.clone())),
+            ("0", _) => MinifyingSugg(-(rhs.0.clone())),
             (x, y) if x == y => MinifyingSugg::non_paren("0"),
             (_, _) => MinifyingSugg(self.0 - &rhs.0),
         }
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index 8f2aa724d20b..1f448f86d72e 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -13,7 +13,7 @@ use rustc_span::{BytePos, Pos};
 use std::borrow::Cow;
 use std::convert::TryInto;
 use std::fmt::Display;
-use std::ops::{Add, Not, Sub};
+use std::ops::{Add, Neg, Not, Sub};
 
 /// A helper type to build suggestion correctly handling parenthesis.
 pub enum Sugg<'a> {
@@ -354,6 +354,13 @@ impl Sub for &Sugg<'_> {
 forward_binop_impls_to_ref!(impl Add, add for Sugg<'_>, type Output = Sugg<'static>);
 forward_binop_impls_to_ref!(impl Sub, sub for Sugg<'_>, type Output = Sugg<'static>);
 
+impl Neg for Sugg<'_> {
+    type Output = Sugg<'static>;
+    fn neg(self) -> Sugg<'static> {
+        make_unop("-", self)
+    }
+}
+
 impl Not for Sugg<'_> {
     type Output = Sugg<'static>;
     fn not(self) -> Sugg<'static> {

From f410df3c4810e80b6703dcfdbc4d48f812eb4889 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Sat, 27 Jun 2020 18:56:38 +1200
Subject: [PATCH 21/35] make clippy happy (`needless_pass_by_value`,
 `filter_map` and `find_map`)

---
 clippy_lints/src/loops.rs | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 5928d12f30be..f684e7de8f83 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -952,7 +952,13 @@ fn get_details_from_idx<'tcx>(
         e: &Expr<'_>,
         starts: &[Start<'tcx>],
     ) -> Option<StartKind<'tcx>> {
-        starts.iter().find(|var| same_var(cx, e, var.id)).map(|v| v.kind)
+        starts.iter().find_map(|start| {
+            if same_var(cx, e, start.id) {
+                Some(start.kind)
+            } else {
+                None
+            }
+        })
     }
 
     fn get_offset<'tcx>(
@@ -1059,8 +1065,8 @@ fn build_manual_memcpy_suggestion<'tcx>(
     start: &Expr<'_>,
     end: &Expr<'_>,
     limits: ast::RangeLimits,
-    dst: IndexExpr<'_>,
-    src: IndexExpr<'_>,
+    dst: &IndexExpr<'_>,
+    src: &IndexExpr<'_>,
 ) -> String {
     fn print_offset(offset: MinifyingSugg<'static>) -> MinifyingSugg<'static> {
         if offset.as_str() == "0" {
@@ -1211,7 +1217,7 @@ fn detect_manual_memcpy<'tcx>(
                         }
                     })
                 })
-                .map(|o| o.map(|(dst, src)| build_manual_memcpy_suggestion(cx, start, end, limits, dst, src)))
+                .map(|o| o.map(|(dst, src)| build_manual_memcpy_suggestion(cx, start, end, limits, &dst, &src)))
                 .collect::<Option<Vec<_>>>()
                 .filter(|v| !v.is_empty())
                 .map(|v| v.join("\n    "));
@@ -2319,10 +2325,13 @@ impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
     }
 
     fn into_results(self) -> impl Iterator<Item = HirId> {
-        self.states
-            .into_iter()
-            .filter(|(_, state)| *state == IncrementVisitorVarState::IncrOnce)
-            .map(|(id, _)| id)
+        self.states.into_iter().filter_map(|(id, state)| {
+            if state == IncrementVisitorVarState::IncrOnce {
+                Some(id)
+            } else {
+                None
+            }
+        })
     }
 }
 

From ce653d65a8f08d785c5061e26570b4e59372e325 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Wed, 1 Jul 2020 12:22:16 +1200
Subject: [PATCH 22/35] use `#[derive]` instead of the manual implementation

---
 clippy_lints/src/utils/sugg.rs | 41 +---------------------------------
 1 file changed, 1 insertion(+), 40 deletions(-)

diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index 1f448f86d72e..062b273c0f40 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -16,6 +16,7 @@ use std::fmt::Display;
 use std::ops::{Add, Neg, Not, Sub};
 
 /// A helper type to build suggestion correctly handling parenthesis.
+#[derive(Clone)]
 pub enum Sugg<'a> {
     /// An expression that never needs parenthesis such as `1337` or `[0; 42]`.
     NonParen(Cow<'a, str>),
@@ -37,46 +38,6 @@ impl Display for Sugg<'_> {
     }
 }
 
-// It's impossible to derive Clone due to the lack of the impl Clone for AssocOp
-impl Clone for Sugg<'_> {
-    fn clone(&self) -> Self {
-        /// manually cloning AssocOp
-        fn clone_assoc_op(this: &AssocOp) -> AssocOp {
-            match this {
-                AssocOp::Add => AssocOp::Add,
-                AssocOp::Subtract => AssocOp::Subtract,
-                AssocOp::Multiply => AssocOp::Multiply,
-                AssocOp::Divide => AssocOp::Divide,
-                AssocOp::Modulus => AssocOp::Modulus,
-                AssocOp::LAnd => AssocOp::LAnd,
-                AssocOp::LOr => AssocOp::LOr,
-                AssocOp::BitXor => AssocOp::BitXor,
-                AssocOp::BitAnd => AssocOp::BitAnd,
-                AssocOp::BitOr => AssocOp::BitOr,
-                AssocOp::ShiftLeft => AssocOp::ShiftLeft,
-                AssocOp::ShiftRight => AssocOp::ShiftRight,
-                AssocOp::Equal => AssocOp::Equal,
-                AssocOp::Less => AssocOp::Less,
-                AssocOp::LessEqual => AssocOp::LessEqual,
-                AssocOp::NotEqual => AssocOp::NotEqual,
-                AssocOp::Greater => AssocOp::Greater,
-                AssocOp::GreaterEqual => AssocOp::GreaterEqual,
-                AssocOp::Assign => AssocOp::Assign,
-                AssocOp::AssignOp(t) => AssocOp::AssignOp(*t),
-                AssocOp::As => AssocOp::As,
-                AssocOp::DotDot => AssocOp::DotDot,
-                AssocOp::DotDotEq => AssocOp::DotDotEq,
-                AssocOp::Colon => AssocOp::Colon,
-            }
-        }
-        match self {
-            Sugg::NonParen(x) => Sugg::NonParen(x.clone()),
-            Sugg::MaybeParen(x) => Sugg::MaybeParen(x.clone()),
-            Sugg::BinOp(op, x) => Sugg::BinOp(clone_assoc_op(op), x.clone()),
-        }
-    }
-}
-
 #[allow(clippy::wrong_self_convention)] // ok, because of the function `as_ty` method
 impl<'a> Sugg<'a> {
     /// Prepare a suggestion from an expression.

From e855fe3620c0a6981a4238df548fa5c2f36a24c9 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Sun, 16 Aug 2020 14:01:56 +1200
Subject: [PATCH 23/35] Reflect the changes that has been made and fmt

---
 clippy_lints/src/loops.rs | 45 ++++++++++++++++-----------------------
 1 file changed, 18 insertions(+), 27 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index f684e7de8f83..4ff567ffb0ec 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -846,7 +846,7 @@ impl<'a> MinifyingSugg<'a> {
         s.as_ref()
     }
 
-    fn hir(cx: &LateContext<'_, '_>, expr: &Expr<'_>, default: &'a str) -> Self {
+    fn hir(cx: &LateContext<'_>, expr: &Expr<'_>, default: &'a str) -> Self {
         Self(sugg::Sugg::hir(cx, expr, default))
     }
 
@@ -947,11 +947,7 @@ fn get_details_from_idx<'tcx>(
     idx: &Expr<'_>,
     starts: &[Start<'tcx>],
 ) -> Option<(StartKind<'tcx>, Offset)> {
-    fn get_start<'tcx>(
-        cx: &LateContext<'tcx>,
-        e: &Expr<'_>,
-        starts: &[Start<'tcx>],
-    ) -> Option<StartKind<'tcx>> {
+    fn get_start<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, starts: &[Start<'tcx>]) -> Option<StartKind<'tcx>> {
         starts.iter().find_map(|start| {
             if same_var(cx, e, start.id) {
                 Some(start.kind)
@@ -982,13 +978,9 @@ fn get_details_from_idx<'tcx>(
     match idx.kind {
         ExprKind::Binary(op, lhs, rhs) => match op.node {
             BinOpKind::Add => {
-                let offset_opt = if let Some(s) = get_start(cx, lhs, starts) {
-                    get_offset(cx, rhs, starts).map(|o| (s, o))
-                } else if let Some(s) = get_start(cx, rhs, starts) {
-                    get_offset(cx, lhs, starts).map(|o| (s, o))
-                } else {
-                    None
-                };
+                let offset_opt = get_start(cx, lhs, starts)
+                    .and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, o)))
+                    .or_else(|| get_start(cx, rhs, starts).and_then(|s| get_offset(cx, lhs, starts).map(|o| (s, o))));
 
                 offset_opt.map(|(s, o)| (s, Offset::positive(o)))
             },
@@ -1011,7 +1003,7 @@ fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx
 }
 
 fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
-    cx: &'a LateContext<'a, 'tcx>,
+    cx: &'a LateContext<'tcx>,
     stmts: &'tcx [Stmt<'tcx>],
     expr: Option<&'tcx Expr<'tcx>>,
     loop_counters: &'c [Start<'tcx>],
@@ -1032,7 +1024,7 @@ fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
 }
 
 fn get_loop_counters<'a, 'tcx>(
-    cx: &'a LateContext<'a, 'tcx>,
+    cx: &'a LateContext<'tcx>,
     body: &'tcx Block<'tcx>,
     expr: &'tcx Expr<'_>,
 ) -> Option<impl Iterator<Item = Start<'tcx>> + 'a> {
@@ -1042,7 +1034,7 @@ fn get_loop_counters<'a, 'tcx>(
 
     // For each candidate, check the parent block to see if
     // it's initialized to zero at the start of the loop.
-    if let Some(block) = get_enclosing_block(&cx, expr.hir_id) {
+    get_enclosing_block(&cx, expr.hir_id).and_then(|block| {
         increment_visitor
             .into_results()
             .filter_map(move |var_id| {
@@ -1055,9 +1047,7 @@ fn get_loop_counters<'a, 'tcx>(
                 })
             })
             .into()
-    } else {
-        None
-    }
+    })
 }
 
 fn build_manual_memcpy_suggestion<'tcx>(
@@ -2315,7 +2305,7 @@ struct IncrementVisitor<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
-    fn new(cx: &'a LateContext<'a, 'tcx>) -> Self {
+    fn new(cx: &'a LateContext<'tcx>) -> Self {
         Self {
             cx,
             states: FxHashMap::default(),
@@ -2396,7 +2386,10 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
 enum InitializeVisitorState<'hir> {
     Initial,          // Not examined yet
     Declared(Symbol), // Declared but not (yet) initialized
-    Initialized { name: Symbol, initializer: &'hir Expr<'hir> },
+    Initialized {
+        name: Symbol,
+        initializer: &'hir Expr<'hir>,
+    },
     DontWarn,
 }
 
@@ -2412,7 +2405,7 @@ struct InitializeVisitor<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> InitializeVisitor<'a, 'tcx> {
-    fn new(cx: &'a LateContext<'a, 'tcx>, end_expr: &'tcx Expr<'tcx>, var_id: HirId) -> Self {
+    fn new(cx: &'a LateContext<'tcx>, end_expr: &'tcx Expr<'tcx>, var_id: HirId) -> Self {
         Self {
             cx,
             end_expr,
@@ -2423,7 +2416,7 @@ impl<'a, 'tcx> InitializeVisitor<'a, 'tcx> {
         }
     }
 
-    fn get_result(&self) -> Option<(Name, &'tcx Expr<'tcx>)> {
+    fn get_result(&self) -> Option<(Symbol, &'tcx Expr<'tcx>)> {
         if let InitializeVisitorState::Initialized { name, initializer } = self.state {
             Some((name, initializer))
         } else {
@@ -2442,14 +2435,12 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
             if local.pat.hir_id == self.var_id;
             if let PatKind::Binding(.., ident, _) = local.pat.kind;
             then {
-                self.state = if let Some(ref init) = local.init {
+                self.state = local.init.map_or(InitializeVisitorState::Declared(ident.name), |init| {
                     InitializeVisitorState::Initialized {
                         initializer: init,
                         name: ident.name,
                     }
-                } else {
-                    InitializeVisitorState::Declared(ident.name)
-                }
+                })
             }
         }
         walk_stmt(self, stmt);

From 4918e7ad62259e4c35a0b9d6ac0f0e98e51ff7b1 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Sun, 27 Sep 2020 14:19:43 +1300
Subject: [PATCH 24/35] Replace `snippet_opt` + `unwrap_or_else` with `snippet`

---
 clippy_lints/src/loops.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 4ff567ffb0ec..647537933f72 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -5,9 +5,8 @@ use crate::utils::usage::{is_unused, mutated_variables};
 use crate::utils::{
     get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
     is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item, last_path_segment, match_trait_method,
-    match_type, match_var, multispan_sugg, qpath_res, snippet, snippet_opt, snippet_with_applicability,
-    snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg,
-    SpanlessEq,
+    match_type, match_var, multispan_sugg, qpath_res, snippet, snippet_with_applicability, snippet_with_macro_callsite,
+    span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq,
 };
 use if_chain::if_chain;
 use rustc_ast::ast;
@@ -1121,8 +1120,8 @@ fn build_manual_memcpy_suggestion<'tcx>(
     let (dst_offset, dst_limit) = print_offset_and_limit(&dst);
     let (src_offset, src_limit) = print_offset_and_limit(&src);
 
-    let dst_base_str = snippet_opt(cx, dst.base.span).unwrap_or_else(|| "???".into());
-    let src_base_str = snippet_opt(cx, src.base.span).unwrap_or_else(|| "???".into());
+    let dst_base_str = snippet(cx, dst.base.span, "???");
+    let src_base_str = snippet(cx, src.base.span, "???");
 
     let dst = if dst_offset.as_str() == "" && dst_limit.as_str() == "" {
         dst_base_str
@@ -1133,6 +1132,7 @@ fn build_manual_memcpy_suggestion<'tcx>(
             dst_offset.maybe_par(),
             dst_limit.maybe_par()
         )
+        .into()
     };
 
     format!(

From 5c71352b18ee7a48e825aefd2862b8e0d16ea45b Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Sun, 27 Sep 2020 14:52:06 +1300
Subject: [PATCH 25/35] Prevent unnecessary lints from triggering

---
 clippy_lints/src/loops.rs     | 12 ++++++++----
 tests/ui/manual_memcpy.rs     |  1 -
 tests/ui/manual_memcpy.stderr | 20 ++++++++++----------
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 647537933f72..f2df53aee4f6 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -768,12 +768,14 @@ fn check_for_loop<'tcx>(
     body: &'tcx Expr<'_>,
     expr: &'tcx Expr<'_>,
 ) {
-    check_for_loop_range(cx, pat, arg, body, expr);
+    let is_manual_memcpy_triggered = detect_manual_memcpy(cx, pat, arg, body, expr);
+    if !is_manual_memcpy_triggered {
+        check_for_loop_range(cx, pat, arg, body, expr);
+        check_for_loop_explicit_counter(cx, pat, arg, body, expr);
+    }
     check_for_loop_arg(cx, pat, arg, expr);
-    check_for_loop_explicit_counter(cx, pat, arg, body, expr);
     check_for_loop_over_map_kv(cx, pat, arg, body, expr);
     check_for_mut_range_bound(cx, arg, body);
-    detect_manual_memcpy(cx, pat, arg, body, expr);
     detect_same_item_push(cx, pat, arg, body, expr);
 }
 
@@ -1152,7 +1154,7 @@ fn detect_manual_memcpy<'tcx>(
     arg: &'tcx Expr<'_>,
     body: &'tcx Expr<'_>,
     expr: &'tcx Expr<'_>,
-) {
+) -> bool {
     if let Some(higher::Range {
         start: Some(start),
         end: Some(end),
@@ -1222,9 +1224,11 @@ fn detect_manual_memcpy<'tcx>(
                     big_sugg,
                     Applicability::Unspecified,
                 );
+                return true;
             }
         }
     }
+    false
 }
 
 // Scans the body of the for loop and determines whether lint should be given
diff --git a/tests/ui/manual_memcpy.rs b/tests/ui/manual_memcpy.rs
index 8318fd898112..84758275dd74 100644
--- a/tests/ui/manual_memcpy.rs
+++ b/tests/ui/manual_memcpy.rs
@@ -115,7 +115,6 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
     }
 }
 
-#[allow(clippy::needless_range_loop, clippy::explicit_counter_loop)]
 pub fn manual_copy_with_counters(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
     let mut count = 0;
     for i in 3..src.len() {
diff --git a/tests/ui/manual_memcpy.stderr b/tests/ui/manual_memcpy.stderr
index d189bde0508e..464b18984fbc 100644
--- a/tests/ui/manual_memcpy.stderr
+++ b/tests/ui/manual_memcpy.stderr
@@ -79,49 +79,49 @@ LL |     for i in 0..0 {
    |              ^^^^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:121:14
+  --> $DIR/manual_memcpy.rs:120:14
    |
 LL |     for i in 3..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:127:14
+  --> $DIR/manual_memcpy.rs:126:14
    |
 LL |     for i in 3..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:133:14
+  --> $DIR/manual_memcpy.rs:132:14
    |
 LL |     for i in 0..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:139:14
+  --> $DIR/manual_memcpy.rs:138:14
    |
 LL |     for i in 0..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:145:14
+  --> $DIR/manual_memcpy.rs:144:14
    |
 LL |     for i in 3..(3 + src.len()) {
    |              ^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:151:14
+  --> $DIR/manual_memcpy.rs:150:14
    |
 LL |     for i in 5..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:157:14
+  --> $DIR/manual_memcpy.rs:156:14
    |
 LL |     for i in 3..10 {
    |              ^^^^^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:164:14
+  --> $DIR/manual_memcpy.rs:163:14
    |
 LL |     for i in 0..src.len() {
    |              ^^^^^^^^^^^^
@@ -133,13 +133,13 @@ LL |     dst2[30..(src.len() + 30)].clone_from_slice(&src[..]) {
    |
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:174:14
+  --> $DIR/manual_memcpy.rs:173:14
    |
 LL |     for i in 0..1 << 1 {
    |              ^^^^^^^^^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:182:14
+  --> $DIR/manual_memcpy.rs:181:14
    |
 LL |     for i in 0..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`

From 99aceebf1c7cb382e18d66914bd9f576e529aa99 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Sun, 27 Sep 2020 16:38:41 +1300
Subject: [PATCH 26/35] Use the spans of the entire `for` loops for suggestions

---
 clippy_lints/src/loops.rs     |  23 ++--
 tests/ui/manual_memcpy.stderr | 196 ++++++++++++++++++++++------------
 2 files changed, 140 insertions(+), 79 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index f2df53aee4f6..7c8b6f483bcb 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -779,6 +779,17 @@ fn check_for_loop<'tcx>(
     detect_same_item_push(cx, pat, arg, body, expr);
 }
 
+// this function assumes the given expression is a `for` loop.
+fn get_span_of_entire_for_loop(expr: &Expr<'_>) -> Span {
+    // for some reason this is the only way to get the `Span`
+    // of the entire `for` loop
+    if let ExprKind::Match(_, arms, _) = &expr.kind {
+        arms[0].body.span
+    } else {
+        unreachable!()
+    }
+}
+
 fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool {
     if_chain! {
         if let ExprKind::Path(qpath) = &expr.kind;
@@ -1138,7 +1149,7 @@ fn build_manual_memcpy_suggestion<'tcx>(
     };
 
     format!(
-        "{}.clone_from_slice(&{}[{}..{}])",
+        "{}.clone_from_slice(&{}[{}..{}]);",
         dst,
         src_base_str,
         src_offset.maybe_par(),
@@ -1218,7 +1229,7 @@ fn detect_manual_memcpy<'tcx>(
                 span_lint_and_sugg(
                     cx,
                     MANUAL_MEMCPY,
-                    expr.span,
+                    get_span_of_entire_for_loop(expr),
                     "it looks like you're manually copying between slices",
                     "try replacing the loop by",
                     big_sugg,
@@ -1734,13 +1745,7 @@ fn check_for_loop_explicit_counter<'tcx>(
                 then {
                     let mut applicability = Applicability::MachineApplicable;
 
-                    // for some reason this is the only way to get the `Span`
-                    // of the entire `for` loop
-                    let for_span = if let ExprKind::Match(_, arms, _) = &expr.kind {
-                        arms[0].body.span
-                    } else {
-                        unreachable!()
-                    };
+                    let for_span = get_span_of_entire_for_loop(expr);
 
                     span_lint_and_sugg(
                         cx,
diff --git a/tests/ui/manual_memcpy.stderr b/tests/ui/manual_memcpy.stderr
index 464b18984fbc..db62ed90d97d 100644
--- a/tests/ui/manual_memcpy.stderr
+++ b/tests/ui/manual_memcpy.stderr
@@ -1,148 +1,204 @@
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:7:14
+  --> $DIR/manual_memcpy.rs:7:5
    |
-LL |     for i in 0..src.len() {
-   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`
+LL | /     for i in 0..src.len() {
+LL | |         dst[i] = src[i];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);`
    |
    = note: `-D clippy::manual-memcpy` implied by `-D warnings`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:12:14
+  --> $DIR/manual_memcpy.rs:12:5
    |
-LL |     for i in 0..src.len() {
-   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[10..(src.len() + 10)].clone_from_slice(&src[..])`
+LL | /     for i in 0..src.len() {
+LL | |         dst[i + 10] = src[i];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[10..(src.len() + 10)].clone_from_slice(&src[..]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:17:14
+  --> $DIR/manual_memcpy.rs:17:5
    |
-LL |     for i in 0..src.len() {
-   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..(src.len() + 10)])`
+LL | /     for i in 0..src.len() {
+LL | |         dst[i] = src[i + 10];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..(src.len() + 10)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:22:14
+  --> $DIR/manual_memcpy.rs:22:5
    |
-LL |     for i in 11..src.len() {
-   |              ^^^^^^^^^^^^^ help: try replacing the loop by: `dst[11..src.len()].clone_from_slice(&src[(11 - 10)..(src.len() - 10)])`
+LL | /     for i in 11..src.len() {
+LL | |         dst[i] = src[i - 10];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[11..src.len()].clone_from_slice(&src[(11 - 10)..(src.len() - 10)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:27:14
+  --> $DIR/manual_memcpy.rs:27:5
    |
-LL |     for i in 0..dst.len() {
-   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst.clone_from_slice(&src[..dst.len()])`
+LL | /     for i in 0..dst.len() {
+LL | |         dst[i] = src[i];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst.clone_from_slice(&src[..dst.len()]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:40:14
+  --> $DIR/manual_memcpy.rs:40:5
    |
-LL |     for i in 10..256 {
-   |              ^^^^^^^
+LL | /     for i in 10..256 {
+LL | |         dst[i] = src[i - 5];
+LL | |         dst2[i + 500] = src[i]
+LL | |     }
+   | |_____^
    |
 help: try replacing the loop by
    |
-LL |     for i in dst[10..256].clone_from_slice(&src[(10 - 5)..(256 - 5)])
-LL |     dst2[(10 + 500)..(256 + 500)].clone_from_slice(&src[10..256]) {
+LL |     dst[10..256].clone_from_slice(&src[(10 - 5)..(256 - 5)]);
+LL |     dst2[(10 + 500)..(256 + 500)].clone_from_slice(&src[10..256]);
    |
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:52:14
+  --> $DIR/manual_memcpy.rs:52:5
    |
-LL |     for i in 10..LOOP_OFFSET {
-   |              ^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[(10 + LOOP_OFFSET)..(LOOP_OFFSET + LOOP_OFFSET)].clone_from_slice(&src[(10 - some_var)..(LOOP_OFFSET - some_var)])`
+LL | /     for i in 10..LOOP_OFFSET {
+LL | |         dst[i + LOOP_OFFSET] = src[i - some_var];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[(10 + LOOP_OFFSET)..(LOOP_OFFSET + LOOP_OFFSET)].clone_from_slice(&src[(10 - some_var)..(LOOP_OFFSET - some_var)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:65:14
+  --> $DIR/manual_memcpy.rs:65:5
    |
-LL |     for i in 0..src_vec.len() {
-   |              ^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..])`
+LL | /     for i in 0..src_vec.len() {
+LL | |         dst_vec[i] = src_vec[i];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:94:14
+  --> $DIR/manual_memcpy.rs:94:5
    |
-LL |     for i in from..from + src.len() {
-   |              ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..(from + src.len())].clone_from_slice(&src[..(from + src.len() - from)])`
+LL | /     for i in from..from + src.len() {
+LL | |         dst[i] = src[i - from];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[from..(from + src.len())].clone_from_slice(&src[..(from + src.len() - from)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:98:14
+  --> $DIR/manual_memcpy.rs:98:5
    |
-LL |     for i in from..from + 3 {
-   |              ^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..(from + 3)].clone_from_slice(&src[..(from + 3 - from)])`
+LL | /     for i in from..from + 3 {
+LL | |         dst[i] = src[i - from];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[from..(from + 3)].clone_from_slice(&src[..(from + 3 - from)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:103:14
+  --> $DIR/manual_memcpy.rs:103:5
    |
-LL |     for i in 0..5 {
-   |              ^^^^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5])`
+LL | /     for i in 0..5 {
+LL | |         dst[i - 0] = src[i];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:108:14
+  --> $DIR/manual_memcpy.rs:108:5
    |
-LL |     for i in 0..0 {
-   |              ^^^^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0])`
+LL | /     for i in 0..0 {
+LL | |         dst[i] = src[i];
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:120:14
+  --> $DIR/manual_memcpy.rs:120:5
    |
-LL |     for i in 3..src.len() {
-   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)])`
+LL | /     for i in 3..src.len() {
+LL | |         dst[i] = src[count];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:126:14
+  --> $DIR/manual_memcpy.rs:126:5
    |
-LL |     for i in 3..src.len() {
-   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..])`
+LL | /     for i in 3..src.len() {
+LL | |         dst[count] = src[i];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:132:14
+  --> $DIR/manual_memcpy.rs:132:5
    |
-LL |     for i in 0..src.len() {
-   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..])`
+LL | /     for i in 0..src.len() {
+LL | |         dst[count] = src[i];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:138:14
+  --> $DIR/manual_memcpy.rs:138:5
    |
-LL |     for i in 0..src.len() {
-   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)])`
+LL | /     for i in 0..src.len() {
+LL | |         dst[i] = src[count];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:144:14
+  --> $DIR/manual_memcpy.rs:144:5
    |
-LL |     for i in 3..(3 + src.len()) {
-   |              ^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)])`
+LL | /     for i in 3..(3 + src.len()) {
+LL | |         dst[i] = src[count];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:150:14
+  --> $DIR/manual_memcpy.rs:150:5
    |
-LL |     for i in 5..src.len() {
-   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)])`
+LL | /     for i in 5..src.len() {
+LL | |         dst[i] = src[count - 2];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:156:14
+  --> $DIR/manual_memcpy.rs:156:5
    |
-LL |     for i in 3..10 {
-   |              ^^^^^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)])`
+LL | /     for i in 3..10 {
+LL | |         dst[i] = src[count];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:163:14
+  --> $DIR/manual_memcpy.rs:163:5
    |
-LL |     for i in 0..src.len() {
-   |              ^^^^^^^^^^^^
+LL | /     for i in 0..src.len() {
+LL | |         dst[count] = src[i];
+LL | |         dst2[count2] = src[i];
+LL | |         count += 1;
+LL | |         count2 += 1;
+LL | |     }
+   | |_____^
    |
 help: try replacing the loop by
    |
-LL |     for i in dst[3..(src.len() + 3)].clone_from_slice(&src[..])
-LL |     dst2[30..(src.len() + 30)].clone_from_slice(&src[..]) {
+LL |     dst[3..(src.len() + 3)].clone_from_slice(&src[..]);
+LL |     dst2[30..(src.len() + 30)].clone_from_slice(&src[..]);
    |
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:173:14
+  --> $DIR/manual_memcpy.rs:173:5
    |
-LL |     for i in 0..1 << 1 {
-   |              ^^^^^^^^^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)])`
+LL | /     for i in 0..1 << 1 {
+LL | |         dst[count] = src[i + 2];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:181:14
+  --> $DIR/manual_memcpy.rs:181:5
    |
-LL |     for i in 0..src.len() {
-   |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`
+LL | /     for i in 0..src.len() {
+LL | |         dst[i] = src[i].clone();
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);`
 
 error: aborting due to 22 previous errors
 

From 9725f00f4dd089b875a5d5306ff906494e041f38 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Mon, 28 Sep 2020 02:27:55 +1300
Subject: [PATCH 27/35] Use the `From` trait to make `MinifyingSugg`

---
 clippy_lints/src/loops.rs      | 84 +++++++++++++++-------------------
 clippy_lints/src/utils/sugg.rs |  6 ++-
 2 files changed, 42 insertions(+), 48 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 7c8b6f483bcb..215700fed8cd 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -817,22 +817,22 @@ struct Offset {
 }
 
 impl Offset {
-    fn negative(value: MinifyingSugg<'static>) -> Self {
+    fn negative(value: Sugg<'static>) -> Self {
         Self {
-            value,
+            value: value.into(),
             sign: OffsetSign::Negative,
         }
     }
 
-    fn positive(value: MinifyingSugg<'static>) -> Self {
+    fn positive(value: Sugg<'static>) -> Self {
         Self {
-            value,
+            value: value.into(),
             sign: OffsetSign::Positive,
         }
     }
 
     fn empty() -> Self {
-        Self::positive(MinifyingSugg::non_paren("0"))
+        Self::positive(sugg::ZERO)
     }
 }
 
@@ -844,30 +844,22 @@ fn apply_offset(lhs: &MinifyingSugg<'static>, rhs: &Offset) -> MinifyingSugg<'st
 }
 
 #[derive(Clone)]
-struct MinifyingSugg<'a>(sugg::Sugg<'a>);
-
-impl std::fmt::Display for MinifyingSugg<'_> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
-        std::fmt::Display::fmt(&self.0, f)
-    }
-}
+struct MinifyingSugg<'a>(Sugg<'a>);
 
 impl<'a> MinifyingSugg<'a> {
     fn as_str(&self) -> &str {
-        let sugg::Sugg::NonParen(s) | sugg::Sugg::MaybeParen(s) | sugg::Sugg::BinOp(_, s) = &self.0;
+        let Sugg::NonParen(s) | Sugg::MaybeParen(s) | Sugg::BinOp(_, s) = &self.0;
         s.as_ref()
     }
 
-    fn hir(cx: &LateContext<'_>, expr: &Expr<'_>, default: &'a str) -> Self {
-        Self(sugg::Sugg::hir(cx, expr, default))
-    }
-
-    fn maybe_par(self) -> Self {
-        Self(self.0.maybe_par())
+    fn into_sugg(self) -> Sugg<'a> {
+        self.0
     }
+}
 
-    fn non_paren(str: impl Into<std::borrow::Cow<'a, str>>) -> Self {
-        Self(sugg::Sugg::NonParen(str.into()))
+impl<'a> From<Sugg<'a>> for MinifyingSugg<'a> {
+    fn from(sugg: Sugg<'a>) -> Self {
+        Self(sugg)
     }
 }
 
@@ -877,7 +869,7 @@ impl std::ops::Add for &MinifyingSugg<'static> {
         match (self.as_str(), rhs.as_str()) {
             ("0", _) => rhs.clone(),
             (_, "0") => self.clone(),
-            (_, _) => MinifyingSugg(&self.0 + &rhs.0),
+            (_, _) => (&self.0 + &rhs.0).into(),
         }
     }
 }
@@ -887,9 +879,9 @@ impl std::ops::Sub for &MinifyingSugg<'static> {
     fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
         match (self.as_str(), rhs.as_str()) {
             (_, "0") => self.clone(),
-            ("0", _) => MinifyingSugg(-(rhs.0.clone())),
-            (x, y) if x == y => MinifyingSugg::non_paren("0"),
-            (_, _) => MinifyingSugg(&self.0 - &rhs.0),
+            ("0", _) => (-rhs.0.clone()).into(),
+            (x, y) if x == y => sugg::ZERO.into(),
+            (_, _) => (&self.0 - &rhs.0).into(),
         }
     }
 }
@@ -900,7 +892,7 @@ impl std::ops::Add<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
         match (self.as_str(), rhs.as_str()) {
             ("0", _) => rhs.clone(),
             (_, "0") => self,
-            (_, _) => MinifyingSugg(self.0 + &rhs.0),
+            (_, _) => (self.0 + &rhs.0).into(),
         }
     }
 }
@@ -910,9 +902,9 @@ impl std::ops::Sub<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
     fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
         match (self.as_str(), rhs.as_str()) {
             (_, "0") => self,
-            ("0", _) => MinifyingSugg(-(rhs.0.clone())),
-            (x, y) if x == y => MinifyingSugg::non_paren("0"),
-            (_, _) => MinifyingSugg(self.0 - &rhs.0),
+            ("0", _) => (-rhs.0.clone()).into(),
+            (x, y) if x == y => sugg::ZERO.into(),
+            (_, _) => (self.0 - &rhs.0).into(),
         }
     }
 }
@@ -969,19 +961,15 @@ fn get_details_from_idx<'tcx>(
         })
     }
 
-    fn get_offset<'tcx>(
-        cx: &LateContext<'tcx>,
-        e: &Expr<'_>,
-        starts: &[Start<'tcx>],
-    ) -> Option<MinifyingSugg<'static>> {
+    fn get_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, starts: &[Start<'tcx>]) -> Option<Sugg<'static>> {
         match &e.kind {
             ExprKind::Lit(l) => match l.node {
-                ast::LitKind::Int(x, _ty) => Some(MinifyingSugg::non_paren(x.to_string())),
+                ast::LitKind::Int(x, _ty) => Some(Sugg::NonParen(x.to_string().into())),
                 _ => None,
             },
             ExprKind::Path(..) if get_start(cx, e, starts).is_none() => {
                 // `e` is always non paren as it's a `Path`
-                Some(MinifyingSugg::non_paren(snippet(cx, e.span, "???")))
+                Some(Sugg::NonParen(snippet(cx, e.span, "???")))
             },
             _ => None,
         }
@@ -1072,7 +1060,7 @@ fn build_manual_memcpy_suggestion<'tcx>(
 ) -> String {
     fn print_offset(offset: MinifyingSugg<'static>) -> MinifyingSugg<'static> {
         if offset.as_str() == "0" {
-            MinifyingSugg::non_paren("")
+            sugg::EMPTY.into()
         } else {
             offset
         }
@@ -1088,14 +1076,14 @@ fn build_manual_memcpy_suggestion<'tcx>(
                 if var_def_id(cx, arg) == var_def_id(cx, base);
                 then {
                     if sugg.as_str() == end_str {
-                        MinifyingSugg::non_paren("")
+                        sugg::EMPTY.into()
                     } else {
                         sugg
                     }
                 } else {
                     match limits {
                         ast::RangeLimits::Closed => {
-                            sugg + &MinifyingSugg::non_paren("1")
+                            sugg + &sugg::ONE.into()
                         },
                         ast::RangeLimits::HalfOpen => sugg,
                     }
@@ -1103,29 +1091,31 @@ fn build_manual_memcpy_suggestion<'tcx>(
             }
         };
 
-    let start_str = MinifyingSugg::hir(cx, start, "");
-    let end_str = MinifyingSugg::hir(cx, end, "");
+    let start_str = Sugg::hir(cx, start, "").into();
+    let end_str: MinifyingSugg<'_> = Sugg::hir(cx, end, "").into();
 
     let print_offset_and_limit = |idx_expr: &IndexExpr<'_>| match idx_expr.idx {
         StartKind::Range => (
-            print_offset(apply_offset(&start_str, &idx_expr.idx_offset)),
+            print_offset(apply_offset(&start_str, &idx_expr.idx_offset)).into_sugg(),
             print_limit(
                 end,
                 end_str.as_str(),
                 idx_expr.base,
                 apply_offset(&end_str, &idx_expr.idx_offset),
-            ),
+            )
+            .into_sugg(),
         ),
         StartKind::Counter { initializer } => {
-            let counter_start = MinifyingSugg::hir(cx, initializer, "");
+            let counter_start = Sugg::hir(cx, initializer, "").into();
             (
-                print_offset(apply_offset(&counter_start, &idx_expr.idx_offset)),
+                print_offset(apply_offset(&counter_start, &idx_expr.idx_offset)).into_sugg(),
                 print_limit(
                     end,
                     end_str.as_str(),
                     idx_expr.base,
                     apply_offset(&end_str, &idx_expr.idx_offset) + &counter_start - &start_str,
-                ),
+                )
+                .into_sugg(),
             )
         },
     };
@@ -1136,7 +1126,7 @@ fn build_manual_memcpy_suggestion<'tcx>(
     let dst_base_str = snippet(cx, dst.base.span, "???");
     let src_base_str = snippet(cx, src.base.span, "???");
 
-    let dst = if dst_offset.as_str() == "" && dst_limit.as_str() == "" {
+    let dst = if dst_offset == sugg::EMPTY && dst_limit == sugg::EMPTY {
         dst_base_str
     } else {
         format!(
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index 062b273c0f40..0b2cb667bf41 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -16,7 +16,7 @@ use std::fmt::Display;
 use std::ops::{Add, Neg, Not, Sub};
 
 /// A helper type to build suggestion correctly handling parenthesis.
-#[derive(Clone)]
+#[derive(Clone, PartialEq)]
 pub enum Sugg<'a> {
     /// An expression that never needs parenthesis such as `1337` or `[0; 42]`.
     NonParen(Cow<'a, str>),
@@ -27,8 +27,12 @@ pub enum Sugg<'a> {
     BinOp(AssocOp, Cow<'a, str>),
 }
 
+/// Literal constant `0`, for convenience.
+pub const ZERO: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("0"));
 /// Literal constant `1`, for convenience.
 pub const ONE: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("1"));
+/// a constant represents an empty string, for convenience.
+pub const EMPTY: Sugg<'static> = Sugg::NonParen(Cow::Borrowed(""));
 
 impl Display for Sugg<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {

From ec94bd6cb45e337fd10ca19fadb9a71ecb67db5f Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Mon, 28 Sep 2020 02:43:45 +1300
Subject: [PATCH 28/35] split up the `manual_memcpy` test

---
 tests/ui/manual_memcpy/with_loop_counters.rs  |  64 ++++++++++
 .../manual_memcpy/with_loop_counters.stderr   |  93 ++++++++++++++
 .../without_loop_counters.rs}                 |  61 ---------
 .../without_loop_counters.stderr}             | 117 +++---------------
 4 files changed, 171 insertions(+), 164 deletions(-)
 create mode 100644 tests/ui/manual_memcpy/with_loop_counters.rs
 create mode 100644 tests/ui/manual_memcpy/with_loop_counters.stderr
 rename tests/ui/{manual_memcpy.rs => manual_memcpy/without_loop_counters.rs} (68%)
 rename tests/ui/{manual_memcpy.stderr => manual_memcpy/without_loop_counters.stderr} (50%)

diff --git a/tests/ui/manual_memcpy/with_loop_counters.rs b/tests/ui/manual_memcpy/with_loop_counters.rs
new file mode 100644
index 000000000000..a49ba9eb10af
--- /dev/null
+++ b/tests/ui/manual_memcpy/with_loop_counters.rs
@@ -0,0 +1,64 @@
+#![warn(clippy::needless_range_loop, clippy::manual_memcpy)]
+
+pub fn manual_copy_with_counters(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
+    let mut count = 0;
+    for i in 3..src.len() {
+        dst[i] = src[count];
+        count += 1;
+    }
+
+    let mut count = 0;
+    for i in 3..src.len() {
+        dst[count] = src[i];
+        count += 1;
+    }
+
+    let mut count = 3;
+    for i in 0..src.len() {
+        dst[count] = src[i];
+        count += 1;
+    }
+
+    let mut count = 3;
+    for i in 0..src.len() {
+        dst[i] = src[count];
+        count += 1;
+    }
+
+    let mut count = 0;
+    for i in 3..(3 + src.len()) {
+        dst[i] = src[count];
+        count += 1;
+    }
+
+    let mut count = 3;
+    for i in 5..src.len() {
+        dst[i] = src[count - 2];
+        count += 1;
+    }
+
+    let mut count = 5;
+    for i in 3..10 {
+        dst[i] = src[count];
+        count += 1;
+    }
+
+    let mut count = 3;
+    let mut count2 = 30;
+    for i in 0..src.len() {
+        dst[count] = src[i];
+        dst2[count2] = src[i];
+        count += 1;
+        count2 += 1;
+    }
+
+    // make sure parentheses are added properly to bitwise operators, which have lower precedence than
+    // arithmetric ones
+    let mut count = 0 << 1;
+    for i in 0..1 << 1 {
+        dst[count] = src[i + 2];
+        count += 1;
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/manual_memcpy/with_loop_counters.stderr b/tests/ui/manual_memcpy/with_loop_counters.stderr
new file mode 100644
index 000000000000..24393ad9b4d4
--- /dev/null
+++ b/tests/ui/manual_memcpy/with_loop_counters.stderr
@@ -0,0 +1,93 @@
+error: it looks like you're manually copying between slices
+  --> $DIR/with_loop_counters.rs:5:5
+   |
+LL | /     for i in 3..src.len() {
+LL | |         dst[i] = src[count];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)]);`
+   |
+   = note: `-D clippy::manual-memcpy` implied by `-D warnings`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/with_loop_counters.rs:11:5
+   |
+LL | /     for i in 3..src.len() {
+LL | |         dst[count] = src[i];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..]);`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/with_loop_counters.rs:17:5
+   |
+LL | /     for i in 0..src.len() {
+LL | |         dst[count] = src[i];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..]);`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/with_loop_counters.rs:23:5
+   |
+LL | /     for i in 0..src.len() {
+LL | |         dst[i] = src[count];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)]);`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/with_loop_counters.rs:29:5
+   |
+LL | /     for i in 3..(3 + src.len()) {
+LL | |         dst[i] = src[count];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)]);`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/with_loop_counters.rs:35:5
+   |
+LL | /     for i in 5..src.len() {
+LL | |         dst[i] = src[count - 2];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)]);`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/with_loop_counters.rs:41:5
+   |
+LL | /     for i in 3..10 {
+LL | |         dst[i] = src[count];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)]);`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/with_loop_counters.rs:48:5
+   |
+LL | /     for i in 0..src.len() {
+LL | |         dst[count] = src[i];
+LL | |         dst2[count2] = src[i];
+LL | |         count += 1;
+LL | |         count2 += 1;
+LL | |     }
+   | |_____^
+   |
+help: try replacing the loop by
+   |
+LL |     dst[3..(src.len() + 3)].clone_from_slice(&src[..]);
+LL |     dst2[30..(src.len() + 30)].clone_from_slice(&src[..]);
+   |
+
+error: it looks like you're manually copying between slices
+  --> $DIR/with_loop_counters.rs:58:5
+   |
+LL | /     for i in 0..1 << 1 {
+LL | |         dst[count] = src[i + 2];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)]);`
+
+error: aborting due to 9 previous errors
+
diff --git a/tests/ui/manual_memcpy.rs b/tests/ui/manual_memcpy/without_loop_counters.rs
similarity index 68%
rename from tests/ui/manual_memcpy.rs
rename to tests/ui/manual_memcpy/without_loop_counters.rs
index 84758275dd74..0083f94798fe 100644
--- a/tests/ui/manual_memcpy.rs
+++ b/tests/ui/manual_memcpy/without_loop_counters.rs
@@ -115,67 +115,6 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
     }
 }
 
-pub fn manual_copy_with_counters(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
-    let mut count = 0;
-    for i in 3..src.len() {
-        dst[i] = src[count];
-        count += 1;
-    }
-
-    let mut count = 0;
-    for i in 3..src.len() {
-        dst[count] = src[i];
-        count += 1;
-    }
-
-    let mut count = 3;
-    for i in 0..src.len() {
-        dst[count] = src[i];
-        count += 1;
-    }
-
-    let mut count = 3;
-    for i in 0..src.len() {
-        dst[i] = src[count];
-        count += 1;
-    }
-
-    let mut count = 0;
-    for i in 3..(3 + src.len()) {
-        dst[i] = src[count];
-        count += 1;
-    }
-
-    let mut count = 3;
-    for i in 5..src.len() {
-        dst[i] = src[count - 2];
-        count += 1;
-    }
-
-    let mut count = 5;
-    for i in 3..10 {
-        dst[i] = src[count];
-        count += 1;
-    }
-
-    let mut count = 3;
-    let mut count2 = 30;
-    for i in 0..src.len() {
-        dst[count] = src[i];
-        dst2[count2] = src[i];
-        count += 1;
-        count2 += 1;
-    }
-
-    // make sure parentheses are added properly to bitwise operators, which have lower precedence than
-    // arithmetric ones
-    let mut count = 0 << 1;
-    for i in 0..1 << 1 {
-        dst[count] = src[i + 2];
-        count += 1;
-    }
-}
-
 #[warn(clippy::needless_range_loop, clippy::manual_memcpy)]
 pub fn manual_clone(src: &[String], dst: &mut [String]) {
     for i in 0..src.len() {
diff --git a/tests/ui/manual_memcpy.stderr b/tests/ui/manual_memcpy/without_loop_counters.stderr
similarity index 50%
rename from tests/ui/manual_memcpy.stderr
rename to tests/ui/manual_memcpy/without_loop_counters.stderr
index db62ed90d97d..54b966f6e541 100644
--- a/tests/ui/manual_memcpy.stderr
+++ b/tests/ui/manual_memcpy/without_loop_counters.stderr
@@ -1,5 +1,5 @@
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:7:5
+  --> $DIR/without_loop_counters.rs:7:5
    |
 LL | /     for i in 0..src.len() {
 LL | |         dst[i] = src[i];
@@ -9,7 +9,7 @@ LL | |     }
    = note: `-D clippy::manual-memcpy` implied by `-D warnings`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:12:5
+  --> $DIR/without_loop_counters.rs:12:5
    |
 LL | /     for i in 0..src.len() {
 LL | |         dst[i + 10] = src[i];
@@ -17,7 +17,7 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst[10..(src.len() + 10)].clone_from_slice(&src[..]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:17:5
+  --> $DIR/without_loop_counters.rs:17:5
    |
 LL | /     for i in 0..src.len() {
 LL | |         dst[i] = src[i + 10];
@@ -25,7 +25,7 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..(src.len() + 10)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:22:5
+  --> $DIR/without_loop_counters.rs:22:5
    |
 LL | /     for i in 11..src.len() {
 LL | |         dst[i] = src[i - 10];
@@ -33,7 +33,7 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst[11..src.len()].clone_from_slice(&src[(11 - 10)..(src.len() - 10)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:27:5
+  --> $DIR/without_loop_counters.rs:27:5
    |
 LL | /     for i in 0..dst.len() {
 LL | |         dst[i] = src[i];
@@ -41,7 +41,7 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst.clone_from_slice(&src[..dst.len()]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:40:5
+  --> $DIR/without_loop_counters.rs:40:5
    |
 LL | /     for i in 10..256 {
 LL | |         dst[i] = src[i - 5];
@@ -56,7 +56,7 @@ LL |     dst2[(10 + 500)..(256 + 500)].clone_from_slice(&src[10..256]);
    |
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:52:5
+  --> $DIR/without_loop_counters.rs:52:5
    |
 LL | /     for i in 10..LOOP_OFFSET {
 LL | |         dst[i + LOOP_OFFSET] = src[i - some_var];
@@ -64,7 +64,7 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst[(10 + LOOP_OFFSET)..(LOOP_OFFSET + LOOP_OFFSET)].clone_from_slice(&src[(10 - some_var)..(LOOP_OFFSET - some_var)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:65:5
+  --> $DIR/without_loop_counters.rs:65:5
    |
 LL | /     for i in 0..src_vec.len() {
 LL | |         dst_vec[i] = src_vec[i];
@@ -72,7 +72,7 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:94:5
+  --> $DIR/without_loop_counters.rs:94:5
    |
 LL | /     for i in from..from + src.len() {
 LL | |         dst[i] = src[i - from];
@@ -80,7 +80,7 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst[from..(from + src.len())].clone_from_slice(&src[..(from + src.len() - from)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:98:5
+  --> $DIR/without_loop_counters.rs:98:5
    |
 LL | /     for i in from..from + 3 {
 LL | |         dst[i] = src[i - from];
@@ -88,7 +88,7 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst[from..(from + 3)].clone_from_slice(&src[..(from + 3 - from)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:103:5
+  --> $DIR/without_loop_counters.rs:103:5
    |
 LL | /     for i in 0..5 {
 LL | |         dst[i - 0] = src[i];
@@ -96,7 +96,7 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:108:5
+  --> $DIR/without_loop_counters.rs:108:5
    |
 LL | /     for i in 0..0 {
 LL | |         dst[i] = src[i];
@@ -104,101 +104,12 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:120:5
-   |
-LL | /     for i in 3..src.len() {
-LL | |         dst[i] = src[count];
-LL | |         count += 1;
-LL | |     }
-   | |_____^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)]);`
-
-error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:126:5
-   |
-LL | /     for i in 3..src.len() {
-LL | |         dst[count] = src[i];
-LL | |         count += 1;
-LL | |     }
-   | |_____^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..]);`
-
-error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:132:5
-   |
-LL | /     for i in 0..src.len() {
-LL | |         dst[count] = src[i];
-LL | |         count += 1;
-LL | |     }
-   | |_____^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..]);`
-
-error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:138:5
-   |
-LL | /     for i in 0..src.len() {
-LL | |         dst[i] = src[count];
-LL | |         count += 1;
-LL | |     }
-   | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)]);`
-
-error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:144:5
-   |
-LL | /     for i in 3..(3 + src.len()) {
-LL | |         dst[i] = src[count];
-LL | |         count += 1;
-LL | |     }
-   | |_____^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)]);`
-
-error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:150:5
-   |
-LL | /     for i in 5..src.len() {
-LL | |         dst[i] = src[count - 2];
-LL | |         count += 1;
-LL | |     }
-   | |_____^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)]);`
-
-error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:156:5
-   |
-LL | /     for i in 3..10 {
-LL | |         dst[i] = src[count];
-LL | |         count += 1;
-LL | |     }
-   | |_____^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)]);`
-
-error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:163:5
-   |
-LL | /     for i in 0..src.len() {
-LL | |         dst[count] = src[i];
-LL | |         dst2[count2] = src[i];
-LL | |         count += 1;
-LL | |         count2 += 1;
-LL | |     }
-   | |_____^
-   |
-help: try replacing the loop by
-   |
-LL |     dst[3..(src.len() + 3)].clone_from_slice(&src[..]);
-LL |     dst2[30..(src.len() + 30)].clone_from_slice(&src[..]);
-   |
-
-error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:173:5
-   |
-LL | /     for i in 0..1 << 1 {
-LL | |         dst[count] = src[i + 2];
-LL | |         count += 1;
-LL | |     }
-   | |_____^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)]);`
-
-error: it looks like you're manually copying between slices
-  --> $DIR/manual_memcpy.rs:181:5
+  --> $DIR/without_loop_counters.rs:120:5
    |
 LL | /     for i in 0..src.len() {
 LL | |         dst[i] = src[i].clone();
 LL | |     }
    | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);`
 
-error: aborting due to 22 previous errors
+error: aborting due to 13 previous errors
 

From 388384177e89db60280dc275e4239e97e61f6a59 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Fri, 2 Oct 2020 13:30:50 +1300
Subject: [PATCH 29/35] document `MinifyingSugg` and `Offset`

...and also swap their position
---
 clippy_lints/src/loops.rs | 82 +++++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 38 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 215700fed8cd..9a1ba4907cda 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -805,44 +805,10 @@ fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool {
     }
 }
 
-#[derive(Clone, Copy)]
-enum OffsetSign {
-    Positive,
-    Negative,
-}
-
-struct Offset {
-    value: MinifyingSugg<'static>,
-    sign: OffsetSign,
-}
-
-impl Offset {
-    fn negative(value: Sugg<'static>) -> Self {
-        Self {
-            value: value.into(),
-            sign: OffsetSign::Negative,
-        }
-    }
-
-    fn positive(value: Sugg<'static>) -> Self {
-        Self {
-            value: value.into(),
-            sign: OffsetSign::Positive,
-        }
-    }
-
-    fn empty() -> Self {
-        Self::positive(sugg::ZERO)
-    }
-}
-
-fn apply_offset(lhs: &MinifyingSugg<'static>, rhs: &Offset) -> MinifyingSugg<'static> {
-    match rhs.sign {
-        OffsetSign::Positive => lhs + &rhs.value,
-        OffsetSign::Negative => lhs - &rhs.value,
-    }
-}
-
+/// a wrapper of `Sugg`. Besides what `Sugg` do, this removes unnecessary `0`;
+/// and also, it avoids subtracting a variable from the same one by replacing it with `0`.
+/// it exists for the convenience of the overloaded operators while normal functions can do the
+/// same.
 #[derive(Clone)]
 struct MinifyingSugg<'a>(Sugg<'a>);
 
@@ -909,6 +875,46 @@ impl std::ops::Sub<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
     }
 }
 
+/// a wrapper around `MinifyingSugg`, which carries a operator like currying
+/// so that the suggested code become more efficient (e.g. `foo + -bar` `foo - bar`).
+struct Offset {
+    value: MinifyingSugg<'static>,
+    sign: OffsetSign,
+}
+
+#[derive(Clone, Copy)]
+enum OffsetSign {
+    Positive,
+    Negative,
+}
+
+impl Offset {
+    fn negative(value: Sugg<'static>) -> Self {
+        Self {
+            value: value.into(),
+            sign: OffsetSign::Negative,
+        }
+    }
+
+    fn positive(value: Sugg<'static>) -> Self {
+        Self {
+            value: value.into(),
+            sign: OffsetSign::Positive,
+        }
+    }
+
+    fn empty() -> Self {
+        Self::positive(sugg::ZERO)
+    }
+}
+
+fn apply_offset(lhs: &MinifyingSugg<'static>, rhs: &Offset) -> MinifyingSugg<'static> {
+    match rhs.sign {
+        OffsetSign::Positive => lhs + &rhs.value,
+        OffsetSign::Negative => lhs - &rhs.value,
+    }
+}
+
 #[derive(Debug, Clone, Copy)]
 enum StartKind<'hir> {
     Range,

From 94d7b82340792af6e5c9b7c105dca1f2fef1b495 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Fri, 2 Oct 2020 14:04:46 +1300
Subject: [PATCH 30/35] simplify the code

---
 clippy_lints/src/loops.rs | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 9a1ba4907cda..2ae5a9acb75c 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -973,10 +973,7 @@ fn get_details_from_idx<'tcx>(
                 ast::LitKind::Int(x, _ty) => Some(Sugg::NonParen(x.to_string().into())),
                 _ => None,
             },
-            ExprKind::Path(..) if get_start(cx, e, starts).is_none() => {
-                // `e` is always non paren as it's a `Path`
-                Some(Sugg::NonParen(snippet(cx, e.span, "???")))
-            },
+            ExprKind::Path(..) if get_start(cx, e, starts).is_none() => Some(Sugg::hir(cx, e, "???")),
             _ => None,
         }
     }
@@ -1010,8 +1007,7 @@ fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx
 
 fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
     cx: &'a LateContext<'tcx>,
-    stmts: &'tcx [Stmt<'tcx>],
-    expr: Option<&'tcx Expr<'tcx>>,
+    Block { stmts, expr, .. }: &'tcx Block<'tcx>,
     loop_counters: &'c [Start<'tcx>],
 ) -> impl Iterator<Item = Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>> + 'c {
     stmts
@@ -1025,7 +1021,7 @@ fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
                 then { None } else { Some(e) }
             },
         })
-        .chain(expr.into_iter())
+        .chain((*expr).into_iter())
         .map(get_assignment)
 }
 
@@ -1184,7 +1180,7 @@ fn detect_manual_memcpy<'tcx>(
                 if let Some(loop_counters) = get_loop_counters(cx, block, expr) {
                     starts.extend(loop_counters);
                 }
-                iter_a = Some(get_assignments(cx, block.stmts, block.expr, &starts));
+                iter_a = Some(get_assignments(cx, block, &starts));
             } else {
                 iter_b = Some(get_assignment(body));
             }

From 1402d8ae4f0c07ac48bd8297ec07901346c32d32 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Fri, 2 Oct 2020 14:18:37 +1300
Subject: [PATCH 31/35] fix a FN where incr exprs with no semicolon at ends

---
 clippy_lints/src/loops.rs                      | 18 ++++++++++++------
 tests/ui/manual_memcpy/with_loop_counters.rs   |  7 +++++++
 .../ui/manual_memcpy/with_loop_counters.stderr | 11 ++++++++++-
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 2ae5a9acb75c..ea40c2af4f71 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -1014,14 +1014,20 @@ fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
         .iter()
         .filter_map(move |stmt| match stmt.kind {
             StmtKind::Local(..) | StmtKind::Item(..) => None,
-            StmtKind::Expr(e) | StmtKind::Semi(e) => if_chain! {
-                if let ExprKind::AssignOp(_, var, _) = e.kind;
-                // skip StartKind::Range
-                if loop_counters.iter().skip(1).any(|counter| Some(counter.id) == var_def_id(cx, var));
-                then { None } else { Some(e) }
-            },
+            StmtKind::Expr(e) | StmtKind::Semi(e) => Some(e),
         })
         .chain((*expr).into_iter())
+        .filter(move |e| {
+            if let ExprKind::AssignOp(_, place, _) = e.kind {
+                !loop_counters
+                    .iter()
+                    // skip StartKind::Range
+                    .skip(1)
+                    .any(|counter| same_var(cx, place, counter.id))
+            } else {
+                true
+            }
+        })
         .map(get_assignment)
 }
 
diff --git a/tests/ui/manual_memcpy/with_loop_counters.rs b/tests/ui/manual_memcpy/with_loop_counters.rs
index a49ba9eb10af..70873c9e9944 100644
--- a/tests/ui/manual_memcpy/with_loop_counters.rs
+++ b/tests/ui/manual_memcpy/with_loop_counters.rs
@@ -59,6 +59,13 @@ pub fn manual_copy_with_counters(src: &[i32], dst: &mut [i32], dst2: &mut [i32])
         dst[count] = src[i + 2];
         count += 1;
     }
+
+    // make sure incrementing expressions without semicolons at the end of loops are handled correctly.
+    let mut count = 0;
+    for i in 3..src.len() {
+        dst[i] = src[count];
+        count += 1
+    }
 }
 
 fn main() {}
diff --git a/tests/ui/manual_memcpy/with_loop_counters.stderr b/tests/ui/manual_memcpy/with_loop_counters.stderr
index 24393ad9b4d4..598c881b4d6c 100644
--- a/tests/ui/manual_memcpy/with_loop_counters.stderr
+++ b/tests/ui/manual_memcpy/with_loop_counters.stderr
@@ -89,5 +89,14 @@ LL | |         count += 1;
 LL | |     }
    | |_____^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)]);`
 
-error: aborting due to 9 previous errors
+error: it looks like you're manually copying between slices
+  --> $DIR/with_loop_counters.rs:65:5
+   |
+LL | /     for i in 3..src.len() {
+LL | |         dst[i] = src[count];
+LL | |         count += 1
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)]);`
+
+error: aborting due to 10 previous errors
 

From 41a0ccbc57902e488e62ed8ca9a1ebb565129c09 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Fri, 2 Oct 2020 21:19:14 +1300
Subject: [PATCH 32/35] add comments around `loop_counters`

---
 clippy_lints/src/loops.rs | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index ea40c2af4f71..4f279cc5ef7b 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -1005,6 +1005,10 @@ fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx
     }
 }
 
+/// Get assignments from the given block.
+/// The returned iterator yields `None` if no assignment expressions are there,
+/// filtering out the increments of the given whitelisted loop counters;
+/// because its job is to make sure there's nothing other than assignments and the increments.
 fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
     cx: &'a LateContext<'tcx>,
     Block { stmts, expr, .. }: &'tcx Block<'tcx>,
@@ -1021,7 +1025,8 @@ fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
             if let ExprKind::AssignOp(_, place, _) = e.kind {
                 !loop_counters
                     .iter()
-                    // skip StartKind::Range
+                    // skip the first item which should be `StartKind::Range`
+                    // this makes it possible to use the slice with `StartKind::Range` in the same iterator loop.
                     .skip(1)
                     .any(|counter| same_var(cx, place, counter.id))
             } else {
@@ -1191,11 +1196,11 @@ fn detect_manual_memcpy<'tcx>(
                 iter_b = Some(get_assignment(body));
             }
 
-            // The only statements in the for loops can be indexed assignments from
-            // indexed retrievals.
             let assignments = iter_a.into_iter().flatten().chain(iter_b.into_iter());
 
             let big_sugg = assignments
+                // The only statements in the for loops can be indexed assignments from
+                // indexed retrievals (except increments of loop counters).
                 .map(|o| {
                     o.and_then(|(lhs, rhs)| {
                         let rhs = fetch_cloned_expr(rhs);

From 2a0e45b8dbfa9c8494371983b128b933082a9c13 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Fri, 2 Oct 2020 21:30:21 +1300
Subject: [PATCH 33/35] supress `clippy::filter_map`

---
 clippy_lints/src/loops.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 4f279cc5ef7b..d3a1683cc0cb 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -1014,6 +1014,9 @@ fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
     Block { stmts, expr, .. }: &'tcx Block<'tcx>,
     loop_counters: &'c [Start<'tcx>],
 ) -> impl Iterator<Item = Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>> + 'c {
+    // As the `filter` and `map` below do different things, I think putting together
+    // just increases complexity. (cc #3188 and #4193)
+    #[allow(clippy::filter_map)]
     stmts
         .iter()
         .filter_map(move |stmt| match stmt.kind {

From 7820cb14421f751c05d6d2d5925236c3429cd93f Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Fri, 2 Oct 2020 23:21:24 +1300
Subject: [PATCH 34/35] Add tests for

* `dst.len()` as the end of the range with loop counters
* the increment of the loop counter at the top of the loop
---
 tests/ui/manual_memcpy/with_loop_counters.rs    | 17 +++++++++++++++++
 .../ui/manual_memcpy/with_loop_counters.stderr  | 17 +++++++++++++----
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/tests/ui/manual_memcpy/with_loop_counters.rs b/tests/ui/manual_memcpy/with_loop_counters.rs
index 70873c9e9944..ba388a05a285 100644
--- a/tests/ui/manual_memcpy/with_loop_counters.rs
+++ b/tests/ui/manual_memcpy/with_loop_counters.rs
@@ -37,6 +37,12 @@ pub fn manual_copy_with_counters(src: &[i32], dst: &mut [i32], dst2: &mut [i32])
         count += 1;
     }
 
+    let mut count = 2;
+    for i in 0..dst.len() {
+        dst[i] = src[count];
+        count += 1;
+    }
+
     let mut count = 5;
     for i in 3..10 {
         dst[i] = src[count];
@@ -66,6 +72,17 @@ pub fn manual_copy_with_counters(src: &[i32], dst: &mut [i32], dst2: &mut [i32])
         dst[i] = src[count];
         count += 1
     }
+
+    // make sure ones where the increment is not at the end of the loop.
+    // As a possible enhancement, one could adjust the offset in the suggestion according to
+    // the position. For example, if the increment is at the top of the loop;
+    // treating the loop counter as if it were initialized 1 greater than the original value.
+    let mut count = 0;
+    #[allow(clippy::needless_range_loop)]
+    for i in 0..src.len() {
+        count += 1;
+        dst[i] = src[count];
+    }
 }
 
 fn main() {}
diff --git a/tests/ui/manual_memcpy/with_loop_counters.stderr b/tests/ui/manual_memcpy/with_loop_counters.stderr
index 598c881b4d6c..2547b19f5d1d 100644
--- a/tests/ui/manual_memcpy/with_loop_counters.stderr
+++ b/tests/ui/manual_memcpy/with_loop_counters.stderr
@@ -57,6 +57,15 @@ LL | |     }
 error: it looks like you're manually copying between slices
   --> $DIR/with_loop_counters.rs:41:5
    |
+LL | /     for i in 0..dst.len() {
+LL | |         dst[i] = src[count];
+LL | |         count += 1;
+LL | |     }
+   | |_____^ help: try replacing the loop by: `dst.clone_from_slice(&src[2..(dst.len() + 2)]);`
+
+error: it looks like you're manually copying between slices
+  --> $DIR/with_loop_counters.rs:47:5
+   |
 LL | /     for i in 3..10 {
 LL | |         dst[i] = src[count];
 LL | |         count += 1;
@@ -64,7 +73,7 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/with_loop_counters.rs:48:5
+  --> $DIR/with_loop_counters.rs:54:5
    |
 LL | /     for i in 0..src.len() {
 LL | |         dst[count] = src[i];
@@ -81,7 +90,7 @@ LL |     dst2[30..(src.len() + 30)].clone_from_slice(&src[..]);
    |
 
 error: it looks like you're manually copying between slices
-  --> $DIR/with_loop_counters.rs:58:5
+  --> $DIR/with_loop_counters.rs:64:5
    |
 LL | /     for i in 0..1 << 1 {
 LL | |         dst[count] = src[i + 2];
@@ -90,7 +99,7 @@ LL | |     }
    | |_____^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)]);`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/with_loop_counters.rs:65:5
+  --> $DIR/with_loop_counters.rs:71:5
    |
 LL | /     for i in 3..src.len() {
 LL | |         dst[i] = src[count];
@@ -98,5 +107,5 @@ LL | |         count += 1
 LL | |     }
    | |_____^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)]);`
 
-error: aborting due to 10 previous errors
+error: aborting due to 11 previous errors
 

From b54188429409a6da5f931fa4be5df1f40b377015 Mon Sep 17 00:00:00 2001
From: rail <12975677+rail-rain@users.noreply.github.com>
Date: Fri, 2 Oct 2020 23:38:10 +1300
Subject: [PATCH 35/35] remove the explicit return value of `print_limit`

---
 clippy_lints/src/loops.rs | 41 +++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index d3a1683cc0cb..3e9265c12153 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -1082,30 +1082,29 @@ fn build_manual_memcpy_suggestion<'tcx>(
         }
     }
 
-    let print_limit =
-        |end: &Expr<'_>, end_str: &str, base: &Expr<'_>, sugg: MinifyingSugg<'static>| -> MinifyingSugg<'static> {
-            if_chain! {
-                if let ExprKind::MethodCall(method, _, len_args, _) = end.kind;
-                if method.ident.name == sym!(len);
-                if len_args.len() == 1;
-                if let Some(arg) = len_args.get(0);
-                if var_def_id(cx, arg) == var_def_id(cx, base);
-                then {
-                    if sugg.as_str() == end_str {
-                        sugg::EMPTY.into()
-                    } else {
-                        sugg
-                    }
+    let print_limit = |end: &Expr<'_>, end_str: &str, base: &Expr<'_>, sugg: MinifyingSugg<'static>| {
+        if_chain! {
+            if let ExprKind::MethodCall(method, _, len_args, _) = end.kind;
+            if method.ident.name == sym!(len);
+            if len_args.len() == 1;
+            if let Some(arg) = len_args.get(0);
+            if var_def_id(cx, arg) == var_def_id(cx, base);
+            then {
+                if sugg.as_str() == end_str {
+                    sugg::EMPTY.into()
                 } else {
-                    match limits {
-                        ast::RangeLimits::Closed => {
-                            sugg + &sugg::ONE.into()
-                        },
-                        ast::RangeLimits::HalfOpen => sugg,
-                    }
+                    sugg
+                }
+            } else {
+                match limits {
+                    ast::RangeLimits::Closed => {
+                        sugg + &sugg::ONE.into()
+                    },
+                    ast::RangeLimits::HalfOpen => sugg,
                 }
             }
-        };
+        }
+    };
 
     let start_str = Sugg::hir(cx, start, "").into();
     let end_str: MinifyingSugg<'_> = Sugg::hir(cx, end, "").into();