X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Floops%2Fwhile_let_on_iterator.rs;h=20a8294a0d1acf0132adf4222149a3baec66ddf4;hb=d1b087fdee1024e11498e2d6c3e36d31e9371d3b;hp=b390476a664d9e54f4c4490df4763204cc9c3265;hpb=23d5457e6d8d4d1d943d8368af680545fb6222b2;p=rust.git diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs index b390476a664..20a8294a0d1 100644 --- a/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/clippy_lints/src/loops/while_let_on_iterator.rs @@ -7,27 +7,28 @@ }; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor}; -use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, PatKind, QPath, UnOp}; +use rustc_hir::intravisit::{walk_expr, Visitor}; +use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, Mutability, PatKind, QPath, UnOp}; use rustc_lint::LateContext; -use rustc_span::{symbol::sym, Span, Symbol}; +use rustc_middle::ty::adjustment::Adjust; +use rustc_span::{symbol::sym, Symbol}; -pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - let (scrutinee_expr, iter_expr, some_pat, loop_expr) = if_chain! { +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + let (scrutinee_expr, iter_expr_struct, iter_expr, some_pat, loop_expr) = if_chain! { if let Some(higher::WhileLet { if_then, let_pat, let_expr }) = higher::WhileLet::hir(expr); // check for `Some(..)` pattern if let PatKind::TupleStruct(QPath::Resolved(None, pat_path), some_pat, _) = let_pat.kind; if let Res::Def(_, pat_did) = pat_path.res; if match_def_path(cx, pat_did, &paths::OPTION_SOME); // check for call to `Iterator::next` - if let ExprKind::MethodCall(method_name, _, [iter_expr], _) = let_expr.kind; + if let ExprKind::MethodCall(method_name, [iter_expr], _) = let_expr.kind; if method_name.ident.name == sym::next; if is_trait_method(cx, let_expr, sym::Iterator); if let Some(iter_expr_struct) = try_parse_iter_expr(cx, iter_expr); // get the loop containing the match expression if !uses_iter(cx, &iter_expr_struct, if_then); then { - (let_expr, iter_expr_struct, some_pat, expr) + (let_expr, iter_expr_struct, iter_expr, some_pat, expr) } else { return; } @@ -47,7 +48,11 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // If the iterator is a field or the iterator is accessed after the loop is complete it needs to be // borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used // afterwards a mutable borrow of a field isn't necessary. - let by_ref = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) { + let by_ref = if cx.typeck_results().expr_ty(iter_expr).ref_mutability() == Some(Mutability::Mut) + || !iter_expr_struct.can_move + || !iter_expr_struct.fields.is_empty() + || needs_mutable_borrow(cx, &iter_expr_struct, loop_expr) + { ".by_ref()" } else { "" @@ -67,26 +72,36 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { #[derive(Debug)] struct IterExpr { - /// The span of the whole expression, not just the path and fields stored here. - span: Span, /// The fields used, in order of child to parent. fields: Vec, /// The path being used. path: Res, + /// Whether or not the iterator can be moved. + can_move: bool, } /// Parses any expression to find out which field of which variable is used. Will return `None` if /// the expression might have side effects. fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option { - let span = e.span; let mut fields = Vec::new(); + let mut can_move = true; loop { + if cx + .typeck_results() + .expr_adjustments(e) + .iter() + .any(|a| matches!(a.kind, Adjust::Deref(Some(..)))) + { + // Custom deref impls need to borrow the whole value as it's captured by reference + can_move = false; + fields.clear(); + } match e.kind { ExprKind::Path(ref path) => { break Some(IterExpr { - span, fields, path: cx.qpath_res(path, e.hir_id), + can_move, }); }, ExprKind::Field(base, name) => { @@ -99,10 +114,12 @@ fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option { + can_move = false; fields.clear(); e = base; }, ExprKind::Unary(UnOp::Deref, base) => { + can_move = false; fields.clear(); e = base; }, @@ -174,7 +191,7 @@ fn is_expr_same_child_or_parent_field(cx: &LateContext<'_>, expr: &Expr<'_>, fie /// Strips off all field and path expressions. This will return true if a field or path has been /// skipped. Used to skip them after failing to check for equality. -fn skip_fields_and_path(expr: &'tcx Expr<'_>) -> (Option<&'tcx Expr<'tcx>>, bool) { +fn skip_fields_and_path<'tcx>(expr: &'tcx Expr<'_>) -> (Option<&'tcx Expr<'tcx>>, bool) { let mut e = expr; let e = loop { match e.kind { @@ -187,18 +204,13 @@ fn skip_fields_and_path(expr: &'tcx Expr<'_>) -> (Option<&'tcx Expr<'tcx>>, bool } /// Checks if the given expression uses the iterator. -fn uses_iter(cx: &LateContext<'tcx>, iter_expr: &IterExpr, container: &'tcx Expr<'_>) -> bool { +fn uses_iter<'tcx>(cx: &LateContext<'tcx>, iter_expr: &IterExpr, container: &'tcx Expr<'_>) -> bool { struct V<'a, 'b, 'tcx> { cx: &'a LateContext<'tcx>, iter_expr: &'b IterExpr, uses_iter: bool, } - impl Visitor<'tcx> for V<'_, '_, 'tcx> { - type Map = ErasedMap<'tcx>; - fn nested_visit_map(&mut self) -> NestedVisitorMap { - NestedVisitorMap::None - } - + impl<'tcx> Visitor<'tcx> for V<'_, '_, 'tcx> { fn visit_expr(&mut self, e: &'tcx Expr<'_>) { if self.uses_iter { // return @@ -228,7 +240,7 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) { } #[allow(clippy::too_many_lines)] -fn needs_mutable_borrow(cx: &LateContext<'tcx>, iter_expr: &IterExpr, loop_expr: &'tcx Expr<'_>) -> bool { +fn needs_mutable_borrow(cx: &LateContext<'_>, iter_expr: &IterExpr, loop_expr: &Expr<'_>) -> bool { struct AfterLoopVisitor<'a, 'b, 'tcx> { cx: &'a LateContext<'tcx>, iter_expr: &'b IterExpr, @@ -236,12 +248,7 @@ struct AfterLoopVisitor<'a, 'b, 'tcx> { after_loop: bool, used_iter: bool, } - impl Visitor<'tcx> for AfterLoopVisitor<'_, '_, 'tcx> { - type Map = ErasedMap<'tcx>; - fn nested_visit_map(&mut self) -> NestedVisitorMap { - NestedVisitorMap::None - } - + impl<'tcx> Visitor<'tcx> for AfterLoopVisitor<'_, '_, 'tcx> { fn visit_expr(&mut self, e: &'tcx Expr<'_>) { if self.used_iter { return; @@ -275,13 +282,7 @@ struct NestedLoopVisitor<'a, 'b, 'tcx> { found_local: bool, used_after: bool, } - impl Visitor<'tcx> for NestedLoopVisitor<'a, 'b, 'tcx> { - type Map = ErasedMap<'tcx>; - - fn nested_visit_map(&mut self) -> NestedVisitorMap { - NestedVisitorMap::None - } - + impl<'a, 'b, 'tcx> Visitor<'tcx> for NestedLoopVisitor<'a, 'b, 'tcx> { fn visit_local(&mut self, l: &'tcx Local<'_>) { if !self.after_loop { l.pat.each_binding_or_first(&mut |_, id, _, _| {