diff options
| -rw-r--r-- | clippy_lints/src/matches/redundant_pattern_match.rs | 84 | ||||
| -rw-r--r-- | clippy_utils/src/visitors.rs | 131 |
2 files changed, 131 insertions, 84 deletions
diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index 65cecd333f1..8499e050af2 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -3,16 +3,13 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use clippy_utils::sugg::Sugg; use clippy_utils::ty::needs_ordered_drop; -use clippy_utils::{higher, match_def_path}; -use clippy_utils::{is_lang_ctor, is_trait_method, paths}; +use clippy_utils::visitors::any_temporaries_need_ordered_drop; +use clippy_utils::{higher, is_lang_ctor, is_trait_method, match_def_path, paths}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::LangItem::{OptionNone, PollPending}; -use rustc_hir::{ - intravisit::{walk_expr, Visitor}, - Arm, Block, Expr, ExprKind, Node, Pat, PatKind, QPath, UnOp, -}; +use rustc_hir::{Arm, Expr, ExprKind, Node, Pat, PatKind, QPath, UnOp}; use rustc_lint::LateContext; use rustc_middle::ty::{self, subst::GenericArgKind, DefIdTree, Ty}; use rustc_span::sym; @@ -47,79 +44,6 @@ fn try_get_generic_ty(ty: Ty<'_>, index: usize) -> Option<Ty<'_>> { } } -// Checks if there are any temporaries created in the given expression for which drop order -// matters. -fn temporaries_need_ordered_drop<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { - struct V<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - res: bool, - } - impl<'a, 'tcx> Visitor<'tcx> for V<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - match expr.kind { - // Taking the reference of a value leaves a temporary - // e.g. In `&String::new()` the string is a temporary value. - // Remaining fields are temporary values - // e.g. In `(String::new(), 0).1` the string is a temporary value. - ExprKind::AddrOf(_, _, expr) | ExprKind::Field(expr, _) => { - if !matches!(expr.kind, ExprKind::Path(_)) { - if needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(expr)) { - self.res = true; - } else { - self.visit_expr(expr); - } - } - }, - // the base type is always taken by reference. - // e.g. In `(vec![0])[0]` the vector is a temporary value. - ExprKind::Index(base, index) => { - if !matches!(base.kind, ExprKind::Path(_)) { - if needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(base)) { - self.res = true; - } else { - self.visit_expr(base); - } - } - self.visit_expr(index); - }, - // Method calls can take self by reference. - // e.g. In `String::new().len()` the string is a temporary value. - ExprKind::MethodCall(_, [self_arg, args @ ..], _) => { - if !matches!(self_arg.kind, ExprKind::Path(_)) { - let self_by_ref = self - .cx - .typeck_results() - .type_dependent_def_id(expr.hir_id) - .map_or(false, |id| self.cx.tcx.fn_sig(id).skip_binder().inputs()[0].is_ref()); - if self_by_ref && needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(self_arg)) { - self.res = true; - } else { - self.visit_expr(self_arg); - } - } - args.iter().for_each(|arg| self.visit_expr(arg)); - }, - // Either explicitly drops values, or changes control flow. - ExprKind::DropTemps(_) - | ExprKind::Ret(_) - | ExprKind::Break(..) - | ExprKind::Yield(..) - | ExprKind::Block(Block { expr: None, .. }, _) - | ExprKind::Loop(..) => (), - - // Only consider the final expression. - ExprKind::Block(Block { expr: Some(expr), .. }, _) => self.visit_expr(expr), - - _ => walk_expr(self, expr), - } - } - } - - let mut v = V { cx, res: false }; - v.visit_expr(expr); - v.res -} - fn find_sugg_for_if_let<'tcx>( cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, @@ -191,7 +115,7 @@ fn find_sugg_for_if_let<'tcx>( // scrutinee would be, so they have to be considered as well. // e.g. in `if let Some(x) = foo.lock().unwrap().baz.as_ref() { .. }` the lock will be held // for the duration if body. - let needs_drop = needs_ordered_drop(cx, check_ty) || temporaries_need_ordered_drop(cx, let_expr); + let needs_drop = needs_ordered_drop(cx, check_ty) || any_temporaries_need_ordered_drop(cx, let_expr); // check that `while_let_on_iterator` lint does not trigger if_chain! { diff --git a/clippy_utils/src/visitors.rs b/clippy_utils/src/visitors.rs index b6c8f1d516e..68cfa8c1aa8 100644 --- a/clippy_utils/src/visitors.rs +++ b/clippy_utils/src/visitors.rs @@ -1,16 +1,18 @@ +use crate::ty::needs_ordered_drop; use crate::{get_enclosing_block, path_to_local_id}; use core::ops::ControlFlow; use rustc_hir as hir; -use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::intravisit::{self, walk_block, walk_expr, Visitor}; use rustc_hir::{ - Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, Stmt, UnOp, UnsafeSource, - Unsafety, + Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, Let, QPath, Stmt, UnOp, + UnsafeSource, Unsafety, }; use rustc_lint::LateContext; use rustc_middle::hir::map::Map; use rustc_middle::hir::nested_filter; -use rustc_middle::ty; +use rustc_middle::ty::adjustment::Adjust; +use rustc_middle::ty::{self, Ty, TypeckResults}; /// Convenience method for creating a `Visitor` with just `visit_expr` overridden and nested /// bodies (i.e. closures) are visited. @@ -494,3 +496,124 @@ pub fn for_each_local_use_after_expr<'tcx, B>( ControlFlow::Continue(()) } } + +// Calls the given function for every unconsumed temporary created by the expression. Note the +// function is only guaranteed to be called for types which need to be dropped, but it may be called +// for other types. +pub fn for_each_unconsumed_temporary<'tcx, B>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'tcx>, + mut f: impl FnMut(Ty<'tcx>) -> ControlFlow<B>, +) -> ControlFlow<B> { + // Todo: Handle partially consumed values. + fn helper<'tcx, B>( + typeck: &'tcx TypeckResults<'tcx>, + consume: bool, + e: &'tcx Expr<'tcx>, + f: &mut impl FnMut(Ty<'tcx>) -> ControlFlow<B>, + ) -> ControlFlow<B> { + if !consume + || matches!( + typeck.expr_adjustments(e), + [adjust, ..] if matches!(adjust.kind, Adjust::Borrow(_) | Adjust::Deref(_)) + ) + { + match e.kind { + ExprKind::Path(QPath::Resolved(None, p)) + if matches!(p.res, Res::Def(DefKind::Ctor(_, CtorKind::Const), _)) => + { + f(typeck.expr_ty(e))?; + }, + ExprKind::Path(_) + | ExprKind::Unary(UnOp::Deref, _) + | ExprKind::Index(..) + | ExprKind::Field(..) + | ExprKind::AddrOf(..) => (), + _ => f(typeck.expr_ty(e))?, + } + } + match e.kind { + ExprKind::AddrOf(_, _, e) + | ExprKind::Field(e, _) + | ExprKind::Unary(UnOp::Deref, e) + | ExprKind::Match(e, ..) + | ExprKind::Let(&Let { init: e, .. }) => { + helper(typeck, false, e, f)?; + }, + ExprKind::Block(&Block { expr: Some(e), .. }, _) + | ExprKind::Box(e) + | ExprKind::Cast(e, _) + | ExprKind::Unary(_, e) => { + helper(typeck, true, e, f)?; + }, + ExprKind::Call(callee, args) => { + helper(typeck, true, callee, f)?; + for arg in args { + helper(typeck, true, arg, f)?; + } + }, + ExprKind::MethodCall(_, args, _) | ExprKind::Tup(args) | ExprKind::Array(args) => { + for arg in args { + helper(typeck, true, arg, f)?; + } + }, + ExprKind::Index(borrowed, consumed) + | ExprKind::Assign(borrowed, consumed, _) + | ExprKind::AssignOp(_, borrowed, consumed) => { + helper(typeck, false, borrowed, f)?; + helper(typeck, true, consumed, f)?; + }, + ExprKind::Binary(_, lhs, rhs) => { + helper(typeck, true, lhs, f)?; + helper(typeck, true, rhs, f)?; + }, + ExprKind::Struct(_, fields, default) => { + for field in fields { + helper(typeck, true, field.expr, f)?; + } + if let Some(default) = default { + helper(typeck, false, default, f)?; + } + }, + ExprKind::If(cond, then, else_expr) => { + helper(typeck, true, cond, f)?; + helper(typeck, true, then, f)?; + if let Some(else_expr) = else_expr { + helper(typeck, true, else_expr, f)?; + } + }, + ExprKind::Type(e, _) => { + helper(typeck, consume, e, f)?; + }, + + // Either drops temporaries, jumps out of the current expression, or has no sub expression. + ExprKind::DropTemps(_) + | ExprKind::Ret(_) + | ExprKind::Break(..) + | ExprKind::Yield(..) + | ExprKind::Block(..) + | ExprKind::Loop(..) + | ExprKind::Repeat(..) + | ExprKind::Lit(_) + | ExprKind::ConstBlock(_) + | ExprKind::Closure { .. } + | ExprKind::Path(_) + | ExprKind::Continue(_) + | ExprKind::InlineAsm(_) + | ExprKind::Err => (), + } + ControlFlow::Continue(()) + } + helper(cx.typeck_results(), true, e, &mut f) +} + +pub fn any_temporaries_need_ordered_drop<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool { + for_each_unconsumed_temporary(cx, e, |ty| { + if needs_ordered_drop(cx, ty) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) + } + }) + .is_break() +} |