From 4838c78ba4ef784379ae6ec5617479de2a32d3f6 Mon Sep 17 00:00:00 2001
From: Jason Newcomb <jsnewcomb@pm.me>
Date: Sun, 1 Aug 2021 18:39:56 -0400
Subject: [PATCH 1/6] Improve `manual_map` and `map_entry` Locals which can be
partially moved created within the to-be-created closure shouldn't block the
use of a closure
---
clippy_lints/src/entry.rs | 31 ++++++++++++++------
clippy_utils/src/lib.rs | 44 +++++++++++++++++++++++------
tests/ui/entry.fixed | 15 +++++-----
tests/ui/entry.rs | 9 +++---
tests/ui/entry.stderr | 16 ++++++-----
tests/ui/entry_btree.fixed | 18 ++++++++++++
tests/ui/entry_btree.rs | 18 ++++++++++++
tests/ui/entry_btree.stderr | 20 +++++++++++++
tests/ui/manual_map_option_2.fixed | 10 +++++++
tests/ui/manual_map_option_2.rs | 13 +++++++++
tests/ui/manual_map_option_2.stderr | 24 ++++++++++++++++
11 files changed, 181 insertions(+), 37 deletions(-)
create mode 100644 tests/ui/entry_btree.fixed
create mode 100644 tests/ui/entry_btree.rs
create mode 100644 tests/ui/entry_btree.stderr
create mode 100644 tests/ui/manual_map_option_2.fixed
create mode 100644 tests/ui/manual_map_option_2.rs
create mode 100644 tests/ui/manual_map_option_2.stderr
diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs
index e1d0d65edb1..7fb8e427660 100644
--- a/clippy_lints/src/entry.rs
+++ b/clippy_lints/src/entry.rs
@@ -7,8 +7,9 @@ use clippy_utils::{
};
use rustc_errors::Applicability;
use rustc_hir::{
+ hir_id::HirIdSet,
intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor},
- Block, Expr, ExprKind, Guard, HirId, Local, Stmt, StmtKind, UnOp,
+ Block, Expr, ExprKind, Guard, HirId, Pat, Stmt, StmtKind, UnOp,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -336,6 +337,8 @@ struct InsertSearcher<'cx, 'tcx> {
edits: Vec<Edit<'tcx>>,
/// A stack of loops the visitor is currently in.
loops: Vec<HirId>,
+ /// Local variables created in the expression. These don't need to be captured.
+ locals: HirIdSet,
}
impl<'tcx> InsertSearcher<'_, 'tcx> {
/// Visit the expression as a branch in control flow. Multiple insert calls can be used, but
@@ -383,13 +386,16 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> {
}
},
StmtKind::Expr(e) => self.visit_expr(e),
- StmtKind::Local(Local { init: Some(e), .. }) => {
- self.allow_insert_closure &= !self.in_tail_pos;
- self.in_tail_pos = false;
- self.is_single_insert = false;
- self.visit_expr(e);
+ StmtKind::Local(l) => {
+ self.visit_pat(l.pat);
+ if let Some(e) = l.init {
+ self.allow_insert_closure &= !self.in_tail_pos;
+ self.in_tail_pos = false;
+ self.is_single_insert = false;
+ self.visit_expr(e);
+ }
},
- _ => {
+ StmtKind::Item(_) => {
self.allow_insert_closure &= !self.in_tail_pos;
self.is_single_insert = false;
},
@@ -471,6 +477,7 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> {
// Each branch may contain it's own insert expression.
let mut is_map_used = self.is_map_used;
for arm in arms {
+ self.visit_pat(arm.pat);
if let Some(Guard::If(guard) | Guard::IfLet(_, guard)) = arm.guard {
self.visit_non_tail_expr(guard);
}
@@ -496,7 +503,8 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> {
},
_ => {
self.allow_insert_closure &= !self.in_tail_pos;
- self.allow_insert_closure &= can_move_expr_to_closure_no_visit(self.cx, expr, &self.loops);
+ self.allow_insert_closure &=
+ can_move_expr_to_closure_no_visit(self.cx, expr, &self.loops, &self.locals);
// Sub expressions are no longer in the tail position.
self.is_single_insert = false;
self.in_tail_pos = false;
@@ -505,6 +513,12 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> {
},
}
}
+
+ fn visit_pat(&mut self, p: &'tcx Pat<'tcx>) {
+ p.each_binding_or_first(&mut |_, id, _, _| {
+ self.locals.insert(id);
+ });
+ }
}
struct InsertSearchResults<'tcx> {
@@ -630,6 +644,7 @@ fn find_insert_calls(
in_tail_pos: true,
is_single_insert: true,
loops: Vec::new(),
+ locals: HirIdSet::default(),
};
s.visit_expr(expr);
let allow_insert_closure = s.allow_insert_closure;
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 1d59d6bfea1..aee9f791b03 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -67,6 +67,7 @@ use rustc_data_structures::unhash::UnhashMap;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
+use rustc_hir::hir_id::HirIdSet;
use rustc_hir::intravisit::{self, walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visitor};
use rustc_hir::LangItem::{ResultErr, ResultOk};
use rustc_hir::{
@@ -626,7 +627,12 @@ 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.
-pub fn can_move_expr_to_closure_no_visit(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, jump_targets: &[HirId]) -> bool {
+pub fn can_move_expr_to_closure_no_visit(
+ cx: &LateContext<'tcx>,
+ expr: &'tcx Expr<'_>,
+ jump_targets: &[HirId],
+ ignore_locals: &HirIdSet,
+) -> bool {
match expr.kind {
ExprKind::Break(Destination { target_id: Ok(id), .. }, _)
| ExprKind::Continue(Destination { target_id: Ok(id), .. })
@@ -642,15 +648,24 @@ pub fn can_move_expr_to_closure_no_visit(cx: &LateContext<'tcx>, expr: &'tcx Exp
| ExprKind::LlvmInlineAsm(_) => false,
// Accessing a field of a local value can only be done if the type isn't
// partially moved.
- ExprKind::Field(base_expr, _)
- if matches!(
- base_expr.kind,
- ExprKind::Path(QPath::Resolved(_, Path { res: Res::Local(_), .. }))
- ) && can_partially_move_ty(cx, cx.typeck_results().expr_ty(base_expr)) =>
- {
+ ExprKind::Field(
+ &Expr {
+ hir_id,
+ kind:
+ ExprKind::Path(QPath::Resolved(
+ _,
+ Path {
+ res: Res::Local(local_id),
+ ..
+ },
+ )),
+ ..
+ },
+ _,
+ ) if !ignore_locals.contains(local_id) && can_partially_move_ty(cx, cx.typeck_results().node_type(hir_id)) => {
// TODO: check if the local has been partially moved. Assume it has for now.
false
- }
+ },
_ => true,
}
}
@@ -659,7 +674,11 @@ pub fn can_move_expr_to_closure_no_visit(cx: &LateContext<'tcx>, expr: &'tcx Exp
pub fn can_move_expr_to_closure(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
struct V<'cx, 'tcx> {
cx: &'cx LateContext<'tcx>,
+ // Stack of potential break targets contained in the expression.
loops: Vec<HirId>,
+ /// Local variables created in the expression. These don't need to be captured.
+ locals: HirIdSet,
+ /// Whether this expression can be turned into a closure.
allow_closure: bool,
}
impl Visitor<'tcx> for V<'_, 'tcx> {
@@ -677,16 +696,23 @@ pub fn can_move_expr_to_closure(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) ->
self.visit_block(b);
self.loops.pop();
} else {
- self.allow_closure &= can_move_expr_to_closure_no_visit(self.cx, e, &self.loops);
+ self.allow_closure &= can_move_expr_to_closure_no_visit(self.cx, e, &self.loops, &self.locals);
walk_expr(self, e);
}
}
+
+ fn visit_pat(&mut self, p: &'tcx Pat<'tcx>) {
+ p.each_binding_or_first(&mut |_, id, _, _| {
+ self.locals.insert(id);
+ });
+ }
}
let mut v = V {
cx,
allow_closure: true,
loops: Vec::new(),
+ locals: HirIdSet::default(),
};
v.visit_expr(expr);
v.allow_closure
diff --git a/tests/ui/entry.fixed b/tests/ui/entry.fixed
index cfad3090ba3..8a36ec833d7 100644
--- a/tests/ui/entry.fixed
+++ b/tests/ui/entry.fixed
@@ -4,7 +4,7 @@
#![warn(clippy::map_entry)]
#![feature(asm)]
-use std::collections::{BTreeMap, HashMap};
+use std::collections::HashMap;
use std::hash::Hash;
macro_rules! m {
@@ -142,14 +142,13 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, m2: &mut HashMa
if !m.contains_key(&k) {
insert!(m, k, v);
}
-}
-fn btree_map<K: Eq + Ord + Copy, V: Copy>(m: &mut BTreeMap<K, V>, k: K, v: V, v2: V) {
- // insert then do something, use if let
- if let std::collections::btree_map::Entry::Vacant(e) = m.entry(k) {
- e.insert(v);
- foo();
- }
+ // or_insert_with. Partial move of a local declared in the closure is ok.
+ m.entry(k).or_insert_with(|| {
+ let x = (String::new(), String::new());
+ let _ = x.0;
+ v
+ });
}
fn main() {}
diff --git a/tests/ui/entry.rs b/tests/ui/entry.rs
index fa9280b58de..d972a201ad7 100644
--- a/tests/ui/entry.rs
+++ b/tests/ui/entry.rs
@@ -4,7 +4,7 @@
#![warn(clippy::map_entry)]
#![feature(asm)]
-use std::collections::{BTreeMap, HashMap};
+use std::collections::HashMap;
use std::hash::Hash;
macro_rules! m {
@@ -146,13 +146,12 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, m2: &mut HashMa
if !m.contains_key(&k) {
insert!(m, k, v);
}
-}
-fn btree_map<K: Eq + Ord + Copy, V: Copy>(m: &mut BTreeMap<K, V>, k: K, v: V, v2: V) {
- // insert then do something, use if let
+ // or_insert_with. Partial move of a local declared in the closure is ok.
if !m.contains_key(&k) {
+ let x = (String::new(), String::new());
+ let _ = x.0;
m.insert(k, v);
- foo();
}
}
diff --git a/tests/ui/entry.stderr b/tests/ui/entry.stderr
index 8f2e383d675..1076500498d 100644
--- a/tests/ui/entry.stderr
+++ b/tests/ui/entry.stderr
@@ -165,21 +165,23 @@ LL | | m.insert(m!(k), m!(v));
LL | | }
| |_____^ help: try this: `m.entry(m!(k)).or_insert_with(|| m!(v));`
-error: usage of `contains_key` followed by `insert` on a `BTreeMap`
- --> $DIR/entry.rs:153:5
+error: usage of `contains_key` followed by `insert` on a `HashMap`
+ --> $DIR/entry.rs:151:5
|
LL | / if !m.contains_key(&k) {
+LL | | let x = (String::new(), String::new());
+LL | | let _ = x.0;
LL | | m.insert(k, v);
-LL | | foo();
LL | | }
| |_____^
|
help: try this
|
-LL ~ if let std::collections::btree_map::Entry::Vacant(e) = m.entry(k) {
-LL + e.insert(v);
-LL + foo();
-LL + }
+LL ~ m.entry(k).or_insert_with(|| {
+LL + let x = (String::new(), String::new());
+LL + let _ = x.0;
+LL + v
+LL + });
|
error: aborting due to 10 previous errors
diff --git a/tests/ui/entry_btree.fixed b/tests/ui/entry_btree.fixed
new file mode 100644
index 00000000000..94979104556
--- /dev/null
+++ b/tests/ui/entry_btree.fixed
@@ -0,0 +1,18 @@
+// run-rustfix
+
+#![warn(clippy::map_entry)]
+#![allow(dead_code)]
+
+use std::collections::BTreeMap;
+
+fn foo() {}
+
+fn btree_map<K: Eq + Ord + Copy, V: Copy>(m: &mut BTreeMap<K, V>, k: K, v: V) {
+ // insert then do something, use if let
+ if let std::collections::btree_map::Entry::Vacant(e) = m.entry(k) {
+ e.insert(v);
+ foo();
+ }
+}
+
+fn main() {}
diff --git a/tests/ui/entry_btree.rs b/tests/ui/entry_btree.rs
new file mode 100644
index 00000000000..080c1d959e8
--- /dev/null
+++ b/tests/ui/entry_btree.rs
@@ -0,0 +1,18 @@
+// run-rustfix
+
+#![warn(clippy::map_entry)]
+#![allow(dead_code)]
+
+use std::collections::BTreeMap;
+
+fn foo() {}
+
+fn btree_map<K: Eq + Ord + Copy, V: Copy>(m: &mut BTreeMap<K, V>, k: K, v: V) {
+ // insert then do something, use if let
+ if !m.contains_key(&k) {
+ m.insert(k, v);
+ foo();
+ }
+}
+
+fn main() {}
diff --git a/tests/ui/entry_btree.stderr b/tests/ui/entry_btree.stderr
new file mode 100644
index 00000000000..5c6fcdf1a28
--- /dev/null
+++ b/tests/ui/entry_btree.stderr
@@ -0,0 +1,20 @@
+error: usage of `contains_key` followed by `insert` on a `BTreeMap`
+ --> $DIR/entry_btree.rs:12:5
+ |
+LL | / if !m.contains_key(&k) {
+LL | | m.insert(k, v);
+LL | | foo();
+LL | | }
+ | |_____^
+ |
+ = note: `-D clippy::map-entry` implied by `-D warnings`
+help: try this
+ |
+LL ~ if let std::collections::btree_map::Entry::Vacant(e) = m.entry(k) {
+LL + e.insert(v);
+LL + foo();
+LL + }
+ |
+
+error: aborting due to previous error
+
diff --git a/tests/ui/manual_map_option_2.fixed b/tests/ui/manual_map_option_2.fixed
new file mode 100644
index 00000000000..147a0c49e6a
--- /dev/null
+++ b/tests/ui/manual_map_option_2.fixed
@@ -0,0 +1,10 @@
+// run-rustfix
+
+#![warn(clippy::manual_map)]
+
+fn main() {
+ let _ = Some(0).map(|x| {
+ let y = (String::new(), String::new());
+ (x, y.0)
+ });
+}
diff --git a/tests/ui/manual_map_option_2.rs b/tests/ui/manual_map_option_2.rs
new file mode 100644
index 00000000000..cc612dcc601
--- /dev/null
+++ b/tests/ui/manual_map_option_2.rs
@@ -0,0 +1,13 @@
+// run-rustfix
+
+#![warn(clippy::manual_map)]
+
+fn main() {
+ let _ = match Some(0) {
+ Some(x) => Some({
+ let y = (String::new(), String::new());
+ (x, y.0)
+ }),
+ None => None,
+ };
+}
diff --git a/tests/ui/manual_map_option_2.stderr b/tests/ui/manual_map_option_2.stderr
new file mode 100644
index 00000000000..745737079f3
--- /dev/null
+++ b/tests/ui/manual_map_option_2.stderr
@@ -0,0 +1,24 @@
+error: manual implementation of `Option::map`
+ --> $DIR/manual_map_option_2.rs:6:13
+ |
+LL | let _ = match Some(0) {
+ | _____________^
+LL | | Some(x) => Some({
+LL | | let y = (String::new(), String::new());
+LL | | (x, y.0)
+LL | | }),
+LL | | None => None,
+LL | | };
+ | |_____^
+ |
+ = note: `-D clippy::manual-map` implied by `-D warnings`
+help: try this
+ |
+LL | let _ = Some(0).map(|x| {
+LL | let y = (String::new(), String::new());
+LL | (x, y.0)
+LL | });
+ |
+
+error: aborting due to previous error
+
From 251dd30d77c98cbebd1c68840fce029affe9b6a8 Mon Sep 17 00:00:00 2001
From: Jason Newcomb <jsnewcomb@pm.me>
Date: Wed, 4 Aug 2021 13:48:45 -0400
Subject: [PATCH 2/6] Improve `manual_map` In some cases check if a borrow made
in the scrutinee expression would prevent creating the closure used by `map`
---
clippy_lints/src/manual_map.rs | 36 +++++++--
clippy_utils/src/lib.rs | 117 +++++++++++++++++++++++++----
tests/ui/manual_map_option_2.fixed | 6 ++
tests/ui/manual_map_option_2.rs | 6 ++
4 files changed, 145 insertions(+), 20 deletions(-)
diff --git a/clippy_lints/src/manual_map.rs b/clippy_lints/src/manual_map.rs
index 7dec1595e0d..3ea88f52a1c 100644
--- a/clippy_lints/src/manual_map.rs
+++ b/clippy_lints/src/manual_map.rs
@@ -4,12 +4,14 @@ use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable};
use clippy_utils::{
can_move_expr_to_closure, in_constant, is_else_clause, is_lang_ctor, is_lint_allowed, path_to_local_id,
- peel_hir_expr_refs,
+ peel_hir_expr_refs, peel_hir_expr_while, CaptureKind,
};
use rustc_ast::util::parser::PREC_POSTFIX;
use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, OptionSome};
-use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, HirId, MatchSource, Mutability, Pat, PatKind};
+use rustc_hir::{
+ def::Res, Arm, BindingAnnotation, Block, Expr, ExprKind, HirId, MatchSource, Mutability, Pat, PatKind, Path, QPath,
+};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -111,10 +113,6 @@ impl LateLintPass<'_> for ManualMap {
return;
}
- if !can_move_expr_to_closure(cx, some_expr) {
- return;
- }
-
// Determine which binding mode to use.
let explicit_ref = some_pat.contains_explicit_ref_binding();
let binding_ref = explicit_ref.or_else(|| (ty_ref_count != pat_ref_count).then(|| ty_mutability));
@@ -125,6 +123,32 @@ impl LateLintPass<'_> for ManualMap {
None => "",
};
+ match can_move_expr_to_closure(cx, some_expr) {
+ Some(captures) => {
+ // Check if captures the closure will need conflict with borrows made in the scrutinee.
+ // TODO: check all the references made in the scrutinee expression. This will require interacting
+ // with the borrow checker. Currently only `<local>[.<field>]*` is checked for.
+ if let Some(binding_ref_mutability) = binding_ref {
+ let e = peel_hir_expr_while(scrutinee, |e| match e.kind {
+ ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) => Some(e),
+ _ => None,
+ });
+ if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(l), .. })) = e.kind {
+ match captures.get(l) {
+ Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return,
+ Some(CaptureKind::Ref(Mutability::Not))
+ if binding_ref_mutability == Mutability::Mut =>
+ {
+ return;
+ }
+ Some(CaptureKind::Ref(Mutability::Not)) | None => (),
+ }
+ }
+ }
+ },
+ None => return,
+ };
+
let mut app = Applicability::MachineApplicable;
// Remove address-of expressions from the scrutinee. Either `as_ref` will be called, or
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index aee9f791b03..2655ba6a8e9 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -67,19 +67,20 @@ use rustc_data_structures::unhash::UnhashMap;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
-use rustc_hir::hir_id::HirIdSet;
+use rustc_hir::hir_id::{HirIdMap, HirIdSet};
use rustc_hir::intravisit::{self, walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visitor};
use rustc_hir::LangItem::{ResultErr, ResultOk};
use rustc_hir::{
def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, GenericArgs, HirId, Impl,
- ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Node, Param, Pat, PatKind, Path,
- PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, UnOp,
+ ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node, Param, Pat,
+ PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, UnOp,
};
use rustc_lint::{LateContext, Level, Lint, LintContext};
use rustc_middle::hir::exports::Export;
use rustc_middle::hir::map::Map;
use rustc_middle::ty as rustc_ty;
-use rustc_middle::ty::{layout::IntegerExt, DefIdTree, Ty, TyCtxt, TypeFoldable};
+use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
+use rustc_middle::ty::{layout::IntegerExt, DefIdTree, Ty, TyCtxt, TypeAndMut, TypeFoldable};
use rustc_semver::RustcVersion;
use rustc_session::Session;
use rustc_span::hygiene::{ExpnKind, MacroKind};
@@ -670,8 +671,82 @@ pub fn can_move_expr_to_closure_no_visit(
}
}
-/// Checks if the expression can be moved into a closure as is.
-pub fn can_move_expr_to_closure(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
+/// How a local is captured by a closure
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum CaptureKind {
+ Value,
+ Ref(Mutability),
+}
+impl std::ops::BitOr for CaptureKind {
+ type Output = Self;
+ fn bitor(self, rhs: Self) -> Self::Output {
+ match (self, rhs) {
+ (CaptureKind::Value, _) | (_, CaptureKind::Value) => CaptureKind::Value,
+ (CaptureKind::Ref(Mutability::Mut), CaptureKind::Ref(_))
+ | (CaptureKind::Ref(_), CaptureKind::Ref(Mutability::Mut)) => CaptureKind::Ref(Mutability::Mut),
+ (CaptureKind::Ref(Mutability::Not), CaptureKind::Ref(Mutability::Not)) => CaptureKind::Ref(Mutability::Not),
+ }
+ }
+}
+impl std::ops::BitOrAssign for CaptureKind {
+ fn bitor_assign(&mut self, rhs: Self) {
+ *self = *self | rhs;
+ }
+}
+
+/// Given an expression referencing a local, determines how it would be captured in a closure.
+/// Note as this will walk up to parent expressions until the capture can be determined it should
+/// only be used while making a closure somewhere a value is consumed. e.g. a block, match arm, or
+/// function argument (other than a receiver).
+pub fn capture_local_usage(cx: &LateContext<'tcx>, e: &Expr<'_>) -> CaptureKind {
+ debug_assert!(matches!(
+ e.kind,
+ ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(_), .. }))
+ ));
+
+ let map = cx.tcx.hir();
+ let mut child_id = e.hir_id;
+ let mut capture = CaptureKind::Value;
+
+ for (parent_id, parent) in map.parent_iter(e.hir_id) {
+ if let [Adjustment {
+ kind: Adjust::Deref(_) | Adjust::Borrow(AutoBorrow::Ref(..)),
+ target,
+ }, ref adjust @ ..] = *cx
+ .typeck_results()
+ .adjustments()
+ .get(child_id)
+ .map_or(&[][..], |x| &**x)
+ {
+ if let rustc_ty::RawPtr(TypeAndMut { mutbl: mutability, .. }) | rustc_ty::Ref(_, _, mutability) =
+ *adjust.last().map_or(target, |a| a.target).kind()
+ {
+ return CaptureKind::Ref(mutability);
+ }
+ }
+
+ if let Node::Expr(e) = parent {
+ match e.kind {
+ ExprKind::AddrOf(_, mutability, _) => return CaptureKind::Ref(mutability),
+ ExprKind::Index(..) | ExprKind::Unary(UnOp::Deref, _) => capture = CaptureKind::Ref(Mutability::Not),
+ ExprKind::Assign(lhs, ..) | ExprKind::Assign(_, lhs, _) if lhs.hir_id == child_id => {
+ return CaptureKind::Ref(Mutability::Mut);
+ },
+ _ => break,
+ }
+ } else {
+ break;
+ }
+
+ child_id = parent_id;
+ }
+
+ capture
+}
+
+/// Checks if the expression can be moved into a closure as is. This will return a list of captures
+/// if so, otherwise, `None`.
+pub fn can_move_expr_to_closure(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<HirIdMap<CaptureKind>> {
struct V<'cx, 'tcx> {
cx: &'cx LateContext<'tcx>,
// Stack of potential break targets contained in the expression.
@@ -680,6 +755,9 @@ pub fn can_move_expr_to_closure(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) ->
locals: HirIdSet,
/// Whether this expression can be turned into a closure.
allow_closure: bool,
+ /// Locals which need to be captured, and whether they need to be by value, reference, or
+ /// mutable reference.
+ captures: HirIdMap<CaptureKind>,
}
impl Visitor<'tcx> for V<'_, 'tcx> {
type Map = ErasedMap<'tcx>;
@@ -691,13 +769,23 @@ pub fn can_move_expr_to_closure(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) ->
if !self.allow_closure {
return;
}
- if let ExprKind::Loop(b, ..) = e.kind {
- self.loops.push(e.hir_id);
- self.visit_block(b);
- self.loops.pop();
- } else {
- self.allow_closure &= can_move_expr_to_closure_no_visit(self.cx, e, &self.loops, &self.locals);
- walk_expr(self, e);
+
+ match e.kind {
+ ExprKind::Path(QPath::Resolved(None, &Path { res: Res::Local(l), .. })) => {
+ if !self.locals.contains(&l) {
+ let cap = capture_local_usage(self.cx, e);
+ self.captures.entry(l).and_modify(|e| *e |= cap).or_insert(cap);
+ }
+ },
+ ExprKind::Loop(b, ..) => {
+ self.loops.push(e.hir_id);
+ self.visit_block(b);
+ self.loops.pop();
+ },
+ _ => {
+ self.allow_closure &= can_move_expr_to_closure_no_visit(self.cx, e, &self.loops, &self.locals);
+ walk_expr(self, e);
+ },
}
}
@@ -713,9 +801,10 @@ pub fn can_move_expr_to_closure(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) ->
allow_closure: true,
loops: Vec::new(),
locals: HirIdSet::default(),
+ captures: HirIdMap::default(),
};
v.visit_expr(expr);
- v.allow_closure
+ v.allow_closure.then(|| v.captures)
}
/// Returns the method names and argument list of nested method call expressions that make up
diff --git a/tests/ui/manual_map_option_2.fixed b/tests/ui/manual_map_option_2.fixed
index 147a0c49e6a..637f0327954 100644
--- a/tests/ui/manual_map_option_2.fixed
+++ b/tests/ui/manual_map_option_2.fixed
@@ -7,4 +7,10 @@ fn main() {
let y = (String::new(), String::new());
(x, y.0)
});
+
+ let s = Some(String::new());
+ let _ = match &s {
+ Some(x) => Some((x.clone(), s)),
+ None => None,
+ };
}
diff --git a/tests/ui/manual_map_option_2.rs b/tests/ui/manual_map_option_2.rs
index cc612dcc601..98e00604a1b 100644
--- a/tests/ui/manual_map_option_2.rs
+++ b/tests/ui/manual_map_option_2.rs
@@ -10,4 +10,10 @@ fn main() {
}),
None => None,
};
+
+ let s = Some(String::new());
+ let _ = match &s {
+ Some(x) => Some((x.clone(), s)),
+ None => None,
+ };
}
From 9500974bdb5f7cd9a8ba056b41bd5af5f373e0d3 Mon Sep 17 00:00:00 2001
From: Jason Newcomb <jsnewcomb@pm.me>
Date: Mon, 9 Aug 2021 14:18:53 -0400
Subject: [PATCH 3/6] Fix tracking of which locals would need to be captured in
a closure. * Captures by sub closures are now considered * Copy types are
correctly borrowed by reference when their value is used * Fields are no
longer automatically borrowed by value * Bindings in `match` and `let`
patterns are now checked to determine how a local is captured
---
clippy_utils/src/lib.rs | 95 +++++++++++++++++++++++++----
tests/ui/manual_map_option_2.fixed | 34 +++++++++++
tests/ui/manual_map_option_2.rs | 37 +++++++++++
tests/ui/manual_map_option_2.stderr | 23 ++++++-
4 files changed, 176 insertions(+), 13 deletions(-)
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 2655ba6a8e9..eb9ad04527f 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -62,7 +62,7 @@ use std::collections::hash_map::Entry;
use std::hash::BuildHasherDefault;
use if_chain::if_chain;
-use rustc_ast::ast::{self, Attribute, BorrowKind, LitKind};
+use rustc_ast::ast::{self, Attribute, LitKind};
use rustc_data_structures::unhash::UnhashMap;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
@@ -78,9 +78,11 @@ use rustc_hir::{
use rustc_lint::{LateContext, Level, Lint, LintContext};
use rustc_middle::hir::exports::Export;
use rustc_middle::hir::map::Map;
+use rustc_middle::hir::place::PlaceBase;
use rustc_middle::ty as rustc_ty;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
-use rustc_middle::ty::{layout::IntegerExt, DefIdTree, Ty, TyCtxt, TypeAndMut, TypeFoldable};
+use rustc_middle::ty::binding::BindingMode;
+use rustc_middle::ty::{layout::IntegerExt, BorrowKind, DefIdTree, Ty, TyCtxt, TypeAndMut, TypeFoldable, UpvarCapture};
use rustc_semver::RustcVersion;
use rustc_session::Session;
use rustc_span::hygiene::{ExpnKind, MacroKind};
@@ -91,7 +93,7 @@ use rustc_span::{Span, DUMMY_SP};
use rustc_target::abi::Integer;
use crate::consts::{constant, Constant};
-use crate::ty::{can_partially_move_ty, is_recursively_primitive_type};
+use crate::ty::{can_partially_move_ty, is_copy, is_recursively_primitive_type};
pub fn parse_msrv(msrv: &str, sess: Option<&Session>, span: Option<Span>) -> Option<RustcVersion> {
if let Ok(version) = RustcVersion::parse(msrv) {
@@ -628,6 +630,11 @@ 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
+/// * Yield/Return statments.
+/// * Inline assembly
+/// * Usages of a field of a local where the type of the local can be partially moved.
pub fn can_move_expr_to_closure_no_visit(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
@@ -699,6 +706,22 @@ impl std::ops::BitOrAssign for CaptureKind {
/// only be used while making a closure somewhere a value is consumed. e.g. a block, match arm, or
/// function argument (other than a receiver).
pub fn capture_local_usage(cx: &LateContext<'tcx>, e: &Expr<'_>) -> CaptureKind {
+ fn pat_capture_kind(cx: &LateContext<'_>, pat: &Pat<'_>) -> CaptureKind {
+ let mut capture = CaptureKind::Ref(Mutability::Not);
+ pat.each_binding(|_, id, span, _| {
+ match cx.typeck_results().extract_binding_mode(cx.sess(), id, span).unwrap() {
+ BindingMode::BindByValue(_) if !is_copy(cx, cx.typeck_results().node_type(id)) => {
+ capture = CaptureKind::Value;
+ },
+ BindingMode::BindByReference(Mutability::Mut) if capture != CaptureKind::Value => {
+ capture = CaptureKind::Ref(Mutability::Mut);
+ },
+ _ => (),
+ }
+ });
+ capture
+ }
+
debug_assert!(matches!(
e.kind,
ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(_), .. }))
@@ -707,6 +730,7 @@ pub fn capture_local_usage(cx: &LateContext<'tcx>, e: &Expr<'_>) -> CaptureKind
let map = cx.tcx.hir();
let mut child_id = e.hir_id;
let mut capture = CaptureKind::Value;
+ let mut capture_expr_ty = e;
for (parent_id, parent) in map.parent_iter(e.hir_id) {
if let [Adjustment {
@@ -725,23 +749,47 @@ pub fn capture_local_usage(cx: &LateContext<'tcx>, e: &Expr<'_>) -> CaptureKind
}
}
- if let Node::Expr(e) = parent {
- match e.kind {
+ match parent {
+ Node::Expr(e) => match e.kind {
ExprKind::AddrOf(_, mutability, _) => return CaptureKind::Ref(mutability),
ExprKind::Index(..) | ExprKind::Unary(UnOp::Deref, _) => capture = CaptureKind::Ref(Mutability::Not),
ExprKind::Assign(lhs, ..) | ExprKind::Assign(_, lhs, _) if lhs.hir_id == child_id => {
return CaptureKind::Ref(Mutability::Mut);
},
+ ExprKind::Field(..) => {
+ if capture == CaptureKind::Value {
+ capture_expr_ty = e;
+ }
+ },
+ ExprKind::Match(_, arms, _) => {
+ let mut mutability = Mutability::Not;
+ for capture in arms.iter().map(|arm| pat_capture_kind(cx, arm.pat)) {
+ match capture {
+ CaptureKind::Value => break,
+ CaptureKind::Ref(Mutability::Mut) => mutability = Mutability::Mut,
+ CaptureKind::Ref(Mutability::Not) => (),
+ }
+ }
+ return CaptureKind::Ref(mutability);
+ },
_ => break,
- }
- } else {
- break;
+ },
+ Node::Local(l) => match pat_capture_kind(cx, l.pat) {
+ CaptureKind::Value => break,
+ capture @ CaptureKind::Ref(_) => return capture,
+ },
+ _ => break,
}
child_id = parent_id;
}
- capture
+ if capture == CaptureKind::Value && is_copy(cx, cx.typeck_results().expr_ty(capture_expr_ty)) {
+ // Copy types are never automatically captured by value.
+ CaptureKind::Ref(Mutability::Not)
+ } else {
+ capture
+ }
}
/// Checks if the expression can be moved into a closure as is. This will return a list of captures
@@ -777,6 +825,31 @@ pub fn can_move_expr_to_closure(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) ->
self.captures.entry(l).and_modify(|e| *e |= cap).or_insert(cap);
}
},
+ ExprKind::Closure(..) => {
+ let closure_id = self.cx.tcx.hir().local_def_id(e.hir_id).to_def_id();
+ for capture in self.cx.typeck_results().closure_min_captures_flattened(closure_id) {
+ let local_id = match capture.place.base {
+ PlaceBase::Local(id) => id,
+ PlaceBase::Upvar(var) => var.var_path.hir_id,
+ _ => continue,
+ };
+ if !self.locals.contains(&local_id) {
+ let capture = match capture.info.capture_kind {
+ UpvarCapture::ByValue(_) => CaptureKind::Value,
+ UpvarCapture::ByRef(borrow) => match borrow.kind {
+ BorrowKind::ImmBorrow => CaptureKind::Ref(Mutability::Not),
+ BorrowKind::UniqueImmBorrow | BorrowKind::MutBorrow => {
+ CaptureKind::Ref(Mutability::Mut)
+ },
+ },
+ };
+ self.captures
+ .entry(local_id)
+ .and_modify(|e| *e |= capture)
+ .or_insert(capture);
+ }
+ }
+ },
ExprKind::Loop(b, ..) => {
self.loops.push(e.hir_id);
self.visit_block(b);
@@ -1830,7 +1903,7 @@ pub fn peel_hir_expr_while<'tcx>(
pub fn peel_n_hir_expr_refs(expr: &'a Expr<'a>, count: usize) -> (&'a Expr<'a>, usize) {
let mut remaining = count;
let e = peel_hir_expr_while(expr, |e| match e.kind {
- ExprKind::AddrOf(BorrowKind::Ref, _, e) if remaining != 0 => {
+ ExprKind::AddrOf(ast::BorrowKind::Ref, _, e) if remaining != 0 => {
remaining -= 1;
Some(e)
},
@@ -1844,7 +1917,7 @@ pub fn peel_n_hir_expr_refs(expr: &'a Expr<'a>, count: usize) -> (&'a Expr<'a>,
pub fn peel_hir_expr_refs(expr: &'a Expr<'a>) -> (&'a Expr<'a>, usize) {
let mut count = 0;
let e = peel_hir_expr_while(expr, |e| match e.kind {
- ExprKind::AddrOf(BorrowKind::Ref, _, e) => {
+ ExprKind::AddrOf(ast::BorrowKind::Ref, _, e) => {
count += 1;
Some(e)
},
diff --git a/tests/ui/manual_map_option_2.fixed b/tests/ui/manual_map_option_2.fixed
index 637f0327954..8cc12149403 100644
--- a/tests/ui/manual_map_option_2.fixed
+++ b/tests/ui/manual_map_option_2.fixed
@@ -1,16 +1,50 @@
// run-rustfix
#![warn(clippy::manual_map)]
+#![allow(clippy::toplevel_ref_arg)]
fn main() {
+ // Lint. `y` is declared within the arm, so it isn't captured by the map closure
let _ = Some(0).map(|x| {
let y = (String::new(), String::new());
(x, y.0)
});
+ // Don't lint. `s` is borrowed until partway through the arm, but needs to be captured by the map
+ // closure
let s = Some(String::new());
let _ = match &s {
Some(x) => Some((x.clone(), s)),
None => None,
};
+
+ // Don't lint. `s` is borrowed until partway through the arm, but needs to be captured by the map
+ // closure
+ let s = Some(String::new());
+ let _ = match &s {
+ Some(x) => Some({
+ let clone = x.clone();
+ let s = || s;
+ (clone, s())
+ }),
+ None => None,
+ };
+
+ // Don't lint. `s` is borrowed until partway through the arm, but needs to be captured as a mutable
+ // reference by the map closure
+ let mut s = Some(String::new());
+ let _ = match &s {
+ Some(x) => Some({
+ let clone = x.clone();
+ let ref mut s = s;
+ (clone, s)
+ }),
+ None => None,
+ };
+
+ // Lint. `s` is captured by reference, so no lifetime issues.
+ let s = Some(String::new());
+ let _ = s.as_ref().map(|x| {
+ if let Some(ref s) = s { (x.clone(), s) } else { panic!() }
+ });
}
diff --git a/tests/ui/manual_map_option_2.rs b/tests/ui/manual_map_option_2.rs
index 98e00604a1b..0862b201ead 100644
--- a/tests/ui/manual_map_option_2.rs
+++ b/tests/ui/manual_map_option_2.rs
@@ -1,8 +1,10 @@
// run-rustfix
#![warn(clippy::manual_map)]
+#![allow(clippy::toplevel_ref_arg)]
fn main() {
+ // Lint. `y` is declared within the arm, so it isn't captured by the map closure
let _ = match Some(0) {
Some(x) => Some({
let y = (String::new(), String::new());
@@ -11,9 +13,44 @@ fn main() {
None => None,
};
+ // Don't lint. `s` is borrowed until partway through the arm, but needs to be captured by the map
+ // closure
let s = Some(String::new());
let _ = match &s {
Some(x) => Some((x.clone(), s)),
None => None,
};
+
+ // Don't lint. `s` is borrowed until partway through the arm, but needs to be captured by the map
+ // closure
+ let s = Some(String::new());
+ let _ = match &s {
+ Some(x) => Some({
+ let clone = x.clone();
+ let s = || s;
+ (clone, s())
+ }),
+ None => None,
+ };
+
+ // Don't lint. `s` is borrowed until partway through the arm, but needs to be captured as a mutable
+ // reference by the map closure
+ let mut s = Some(String::new());
+ let _ = match &s {
+ Some(x) => Some({
+ let clone = x.clone();
+ let ref mut s = s;
+ (clone, s)
+ }),
+ None => None,
+ };
+
+ // Lint. `s` is captured by reference, so no lifetime issues.
+ let s = Some(String::new());
+ let _ = match &s {
+ Some(x) => Some({
+ if let Some(ref s) = s { (x.clone(), s) } else { panic!() }
+ }),
+ None => None,
+ };
}
diff --git a/tests/ui/manual_map_option_2.stderr b/tests/ui/manual_map_option_2.stderr
index 745737079f3..9cfd83f445b 100644
--- a/tests/ui/manual_map_option_2.stderr
+++ b/tests/ui/manual_map_option_2.stderr
@@ -1,5 +1,5 @@
error: manual implementation of `Option::map`
- --> $DIR/manual_map_option_2.rs:6:13
+ --> $DIR/manual_map_option_2.rs:8:13
|
LL | let _ = match Some(0) {
| _____________^
@@ -20,5 +20,24 @@ LL | (x, y.0)
LL | });
|
-error: aborting due to previous error
+error: manual implementation of `Option::map`
+ --> $DIR/manual_map_option_2.rs:50:13
+ |
+LL | let _ = match &s {
+ | _____________^
+LL | | Some(x) => Some({
+LL | | if let Some(ref s) = s { (x.clone(), s) } else { panic!() }
+LL | | }),
+LL | | None => None,
+LL | | };
+ | |_____^
+ |
+help: try this
+ |
+LL | let _ = s.as_ref().map(|x| {
+LL | if let Some(ref s) = s { (x.clone(), s) } else { panic!() }
+LL | });
+ |
+
+error: aborting due to 2 previous errors
From 5e4d8b44f9524f28fd1ebca18f48ecad204cf184 Mon Sep 17 00:00:00 2001
From: Jason Newcomb <jsnewcomb@pm.me>
Date: Mon, 9 Aug 2021 16:26:27 -0400
Subject: [PATCH 4/6] Improve doc for `can_move_expr_to_closure_no_visit`
---
clippy_utils/src/lib.rs | 33 +++++++++++++++++++++++++++++----
1 file 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
},
From 10c0460a471d03a633cd15367bf0a85c007e0219 Mon Sep 17 00:00:00 2001
From: Jason Newcomb <jsnewcomb@pm.me>
Date: Sat, 14 Aug 2021 19:52:59 -0400
Subject: [PATCH 5/6] update stderr messages
---
tests/ui/manual_map_option_2.stderr | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tests/ui/manual_map_option_2.stderr b/tests/ui/manual_map_option_2.stderr
index 9cfd83f445b..711ff6c4a4b 100644
--- a/tests/ui/manual_map_option_2.stderr
+++ b/tests/ui/manual_map_option_2.stderr
@@ -14,10 +14,10 @@ LL | | };
= note: `-D clippy::manual-map` implied by `-D warnings`
help: try this
|
-LL | let _ = Some(0).map(|x| {
-LL | let y = (String::new(), String::new());
-LL | (x, y.0)
-LL | });
+LL ~ let _ = Some(0).map(|x| {
+LL + let y = (String::new(), String::new());
+LL + (x, y.0)
+LL ~ });
|
error: manual implementation of `Option::map`
@@ -34,9 +34,9 @@ LL | | };
|
help: try this
|
-LL | let _ = s.as_ref().map(|x| {
-LL | if let Some(ref s) = s { (x.clone(), s) } else { panic!() }
-LL | });
+LL ~ let _ = s.as_ref().map(|x| {
+LL + if let Some(ref s) = s { (x.clone(), s) } else { panic!() }
+LL ~ });
|
error: aborting due to 2 previous errors
From f0444d73def161f7f0fda7b3f5a13e9908e9c550 Mon Sep 17 00:00:00 2001
From: Jason Newcomb <jsnewcomb@pm.me>
Date: Sun, 15 Aug 2021 20:25:10 -0400
Subject: [PATCH 6/6] Use `each_binding_or_first` in `capture_local_usage`
---
clippy_utils/src/lib.rs | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index b40b42fa6d3..bd229402f41 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -733,16 +733,18 @@ impl std::ops::BitOrAssign for CaptureKind {
pub fn capture_local_usage(cx: &LateContext<'tcx>, e: &Expr<'_>) -> CaptureKind {
fn pat_capture_kind(cx: &LateContext<'_>, pat: &Pat<'_>) -> CaptureKind {
let mut capture = CaptureKind::Ref(Mutability::Not);
- pat.each_binding(|_, id, span, _| {
- match cx.typeck_results().extract_binding_mode(cx.sess(), id, span).unwrap() {
- BindingMode::BindByValue(_) if !is_copy(cx, cx.typeck_results().node_type(id)) => {
- capture = CaptureKind::Value;
- },
- BindingMode::BindByReference(Mutability::Mut) if capture != CaptureKind::Value => {
- capture = CaptureKind::Ref(Mutability::Mut);
- },
- _ => (),
- }
+ pat.each_binding_or_first(&mut |_, id, span, _| match cx
+ .typeck_results()
+ .extract_binding_mode(cx.sess(), id, span)
+ .unwrap()
+ {
+ BindingMode::BindByValue(_) if !is_copy(cx, cx.typeck_results().node_type(id)) => {
+ capture = CaptureKind::Value;
+ },
+ BindingMode::BindByReference(Mutability::Mut) if capture != CaptureKind::Value => {
+ capture = CaptureKind::Ref(Mutability::Mut);
+ },
+ _ => (),
});
capture
}