about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJason Newcomb <jsnewcomb@pm.me>2021-08-09 16:26:27 -0400
committerJason Newcomb <jsnewcomb@pm.me>2021-08-14 19:50:01 -0400
commit5e4d8b44f9524f28fd1ebca18f48ecad204cf184 (patch)
treecf36a7943667a4cdc89a2671f715ba93bb28ee53
parent9500974bdb5f7cd9a8ba056b41bd5af5f373e0d3 (diff)
downloadrust-5e4d8b44f9524f28fd1ebca18f48ecad204cf184.tar.gz
rust-5e4d8b44f9524f28fd1ebca18f48ecad204cf184.zip
Improve doc for `can_move_expr_to_closure_no_visit`
-rw-r--r--clippy_utils/src/lib.rs33
1 files changed, 29 insertions, 4 deletions
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index eb9ad04527f..b40b42fa6d3 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -631,20 +631,45 @@ pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -
 
 /// Checks if the top level expression can be moved into a closure as is.
 /// Currently checks for:
-/// * Break/Continue outside the given jump targets
+/// * Break/Continue outside the given loop HIR ids.
 /// * Yield/Return statments.
-/// * Inline assembly
+/// * Inline assembly.
 /// * Usages of a field of a local where the type of the local can be partially moved.
+///
+/// For example, given the following function:
+///
+/// ```
+/// fn f<'a>(iter: &mut impl Iterator<Item = (usize, &'a mut String)>) {
+///     for item in iter {
+///         let s = item.1;
+///         if item.0 > 10 {
+///             continue;
+///         } else {
+///             s.clear();
+///         }
+///     }
+/// }
+/// ```
+///
+/// When called on the expression `item.0` this will return false unless the local `item` is in the
+/// `ignore_locals` set. The type `(usize, &mut String)` can have the second element moved, so it
+/// isn't always safe to move into a closure when only a single field is needed.
+///
+/// When called on the `continue` expression this will return false unless the outer loop expression
+/// is in the `loop_ids` set.
+///
+/// Note that this check is not recursive, so passing the `if` expression will always return true
+/// even though sub-expressions might return false.
 pub fn can_move_expr_to_closure_no_visit(
     cx: &LateContext<'tcx>,
     expr: &'tcx Expr<'_>,
-    jump_targets: &[HirId],
+    loop_ids: &[HirId],
     ignore_locals: &HirIdSet,
 ) -> bool {
     match expr.kind {
         ExprKind::Break(Destination { target_id: Ok(id), .. }, _)
         | ExprKind::Continue(Destination { target_id: Ok(id), .. })
-            if jump_targets.contains(&id) =>
+            if loop_ids.contains(&id) =>
         {
             true
         },