X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fneedless_pass_by_value.rs;h=986cd94cfb3cb794f819004ad84003f1effa099b;hb=6d1225981177587fbb68d9c4902c770c3dbaafb0;hp=424de64b5c7457aa1e96f44d731b1b6ef5a3727d;hpb=b0ec33f661a53737bc87972cbbab2b3218e66476;p=rust.git diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index 424de64b5c7..986cd94cfb3 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -1,25 +1,25 @@ use crate::utils::ptr::get_spans; use crate::utils::{ - get_trait_def_id, implements_trait, in_macro_or_desugar, is_copy, is_self, match_type, multispan_sugg, paths, + get_trait_def_id, implements_trait, is_copy, is_self, is_type_diagnostic_item, match_type, multispan_sugg, paths, snippet, snippet_opt, span_lint_and_then, }; use if_chain::if_chain; use matches::matches; +use rustc::declare_lint_pass; use rustc::hir::intravisit::FnKind; use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; -use rustc::middle::expr_use_visitor as euv; -use rustc::middle::mem_categorization as mc; use rustc::traits; use rustc::ty::{self, RegionKind, TypeFoldable}; -use rustc::{declare_lint_pass, declare_tool_lint}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::Applicability; +use rustc_session::declare_tool_lint; use rustc_target::spec::abi::Abi; +use rustc_typeck::expr_use_visitor as euv; use std::borrow::Cow; use syntax::ast::Attribute; use syntax::errors::DiagnosticBuilder; -use syntax_pos::Span; +use syntax_pos::{Span, Symbol}; declare_clippy_lint! { /// **What it does:** Checks for functions taking arguments by value, but not @@ -40,6 +40,9 @@ /// fn foo(v: Vec) { /// assert_eq!(v.len(), 42); /// } + /// ``` + /// + /// ```rust /// // should be /// fn foo(v: &[i32]) { /// assert_eq!(v.len(), 42); @@ -69,11 +72,11 @@ fn check_fn( cx: &LateContext<'a, 'tcx>, kind: FnKind<'tcx>, decl: &'tcx FnDecl, - body: &'tcx Body, + body: &'tcx Body<'_>, span: Span, hir_id: HirId, ) { - if in_macro_or_desugar(span) { + if span.from_expansion() { return; } @@ -88,12 +91,8 @@ fn check_fn( } // Exclude non-inherent impls - if let Some(Node::Item(item)) = cx - .tcx - .hir() - .find_by_hir_id(cx.tcx.hir().get_parent_node_by_hir_id(hir_id)) - { - if matches!(item.node, ItemKind::Impl(_, _, _, _, Some(_), _, _) | + if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { + if matches!(item.kind, ItemKind::Impl(_, _, _, _, Some(_), _, _) | ItemKind::Trait(..)) { return; @@ -111,7 +110,7 @@ fn check_fn( let sized_trait = need!(cx.tcx.lang_items().sized_trait()); - let fn_def_id = cx.tcx.hir().local_def_id_from_hir_id(hir_id); + let fn_def_id = cx.tcx.hir().local_def_id(hir_id); let preds = traits::elaborate_predicates(cx.tcx, cx.param_env.caller_bounds.to_vec()) .filter(|p| !p.is_global()) @@ -135,25 +134,17 @@ fn check_fn( spans_need_deref, .. } = { - let mut ctx = MovedVariablesCtxt::new(cx); - let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id); - euv::ExprUseVisitor::new( - &mut ctx, - cx.tcx, - fn_def_id, - cx.param_env, - region_scope_tree, - cx.tables, - None, - ) - .consume_body(body); + let mut ctx = MovedVariablesCtxt::default(); + cx.tcx.infer_ctxt().enter(|infcx| { + euv::ExprUseVisitor::new(&mut ctx, &infcx, fn_def_id, cx.param_env, cx.tables).consume_body(body); + }); ctx }; let fn_sig = cx.tcx.fn_sig(fn_def_id); let fn_sig = cx.tcx.erase_late_bound_regions(&fn_sig); - for (idx, ((input, &ty), arg)) in decl.inputs.iter().zip(fn_sig.inputs()).zip(&body.arguments).enumerate() { + for (idx, ((input, &ty), arg)) in decl.inputs.iter().zip(fn_sig.inputs()).zip(body.params).enumerate() { // All spans generated from a proc-macro invocation are the same... if span == input.span { return; @@ -161,7 +152,7 @@ fn check_fn( // Ignore `self`s. if idx == 0 { - if let PatKind::Binding(.., ident, _) = arg.pat.node { + if let PatKind::Binding(.., ident, _) = arg.pat.kind { if ident.as_str() == "self" { continue; } @@ -197,13 +188,13 @@ fn check_fn( if_chain! { if !is_self(arg); - if !ty.is_mutable_pointer(); + if !ty.is_mutable_ptr(); if !is_copy(cx, ty); if !whitelisted_traits.iter().any(|&t| implements_trait(cx, ty, t, &[])); if !implements_borrow_trait; if !all_borrowable_trait; - if let PatKind::Binding(mode, canonical_id, ..) = arg.pat.node; + if let PatKind::Binding(mode, canonical_id, ..) = arg.pat.kind; if !moved_vars.contains(&canonical_id); then { if mode == BindingAnnotation::Mutable || mode == BindingAnnotation::RefMut { @@ -212,7 +203,7 @@ fn check_fn( // Dereference suggestion let sugg = |db: &mut DiagnosticBuilder<'_>| { - if let ty::Adt(def, ..) = ty.sty { + if let ty::Adt(def, ..) = ty.kind { if let Some(span) = cx.tcx.hir().span_if_local(def.did) { if cx.param_env.can_type_implement_copy(cx.tcx, ty).is_ok() { db.span_help(span, "consider marking this type as Copy"); @@ -222,10 +213,10 @@ fn check_fn( let deref_span = spans_need_deref.get(&canonical_id); if_chain! { - if match_type(cx, ty, &paths::VEC); + if is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type")); if let Some(clone_spans) = get_spans(cx, Some(body.id()), idx, &[("clone", ".to_owned()")]); - if let TyKind::Path(QPath::Resolved(_, ref path)) = input.node; + if let TyKind::Path(QPath::Resolved(_, ref path)) = input.kind; if let Some(elem_ty) = path.segments.iter() .find(|seg| seg.ident.name == sym!(Vec)) .and_then(|ps| ps.args.as_ref()) @@ -326,124 +317,30 @@ fn requires_exact_signature(attrs: &[Attribute]) -> bool { }) } -struct MovedVariablesCtxt<'a, 'tcx: 'a> { - cx: &'a LateContext<'a, 'tcx>, +#[derive(Default)] +struct MovedVariablesCtxt { moved_vars: FxHashSet, /// Spans which need to be prefixed with `*` for dereferencing the /// suggested additional reference. spans_need_deref: FxHashMap>, } -impl<'a, 'tcx> MovedVariablesCtxt<'a, 'tcx> { - fn new(cx: &'a LateContext<'a, 'tcx>) -> Self { - Self { - cx, - moved_vars: FxHashSet::default(), - spans_need_deref: FxHashMap::default(), - } - } - - fn move_common(&mut self, _consume_id: HirId, _span: Span, cmt: &mc::cmt_<'tcx>) { - let cmt = unwrap_downcast_or_interior(cmt); - - if let mc::Categorization::Local(vid) = cmt.cat { +impl MovedVariablesCtxt { + fn move_common(&mut self, cmt: &euv::Place<'_>) { + if let euv::PlaceBase::Local(vid) = cmt.base { self.moved_vars.insert(vid); } } - - fn non_moving_pat(&mut self, matched_pat: &Pat, cmt: &mc::cmt_<'tcx>) { - let cmt = unwrap_downcast_or_interior(cmt); - - if let mc::Categorization::Local(vid) = cmt.cat { - let mut id = matched_pat.hir_id; - loop { - let parent = self.cx.tcx.hir().get_parent_node_by_hir_id(id); - if id == parent { - // no parent - return; - } - id = parent; - - if let Some(node) = self.cx.tcx.hir().find_by_hir_id(id) { - match node { - Node::Expr(e) => { - // `match` and `if let` - if let ExprKind::Match(ref c, ..) = e.node { - self.spans_need_deref - .entry(vid) - .or_insert_with(FxHashSet::default) - .insert(c.span); - } - }, - - Node::Stmt(s) => { - // `let = x;` - if_chain! { - if let StmtKind::Local(ref local) = s.node; - then { - self.spans_need_deref - .entry(vid) - .or_insert_with(FxHashSet::default) - .insert(local.init - .as_ref() - .map(|e| e.span) - .expect("`let` stmt without init aren't caught by match_pat")); - } - } - }, - - _ => {}, - } - } - } - } - } } -impl<'a, 'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt<'a, 'tcx> { - fn consume(&mut self, consume_id: HirId, consume_span: Span, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) { - if let euv::ConsumeMode::Move(_) = mode { - self.move_common(consume_id, consume_span, cmt); - } - } - - fn matched_pat(&mut self, matched_pat: &Pat, cmt: &mc::cmt_<'tcx>, mode: euv::MatchMode) { - if let euv::MatchMode::MovingMatch = mode { - self.move_common(matched_pat.hir_id, matched_pat.span, cmt); - } else { - self.non_moving_pat(matched_pat, cmt); - } - } - - fn consume_pat(&mut self, consume_pat: &Pat, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) { - if let euv::ConsumeMode::Move(_) = mode { - self.move_common(consume_pat.hir_id, consume_pat.span, cmt); +impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt { + fn consume(&mut self, cmt: &euv::Place<'tcx>, mode: euv::ConsumeMode) { + if let euv::ConsumeMode::Move = mode { + self.move_common(cmt); } } - fn borrow( - &mut self, - _: HirId, - _: Span, - _: &mc::cmt_<'tcx>, - _: ty::Region<'_>, - _: ty::BorrowKind, - _: euv::LoanCause, - ) { - } + fn borrow(&mut self, _: &euv::Place<'tcx>, _: ty::BorrowKind) {} - fn mutate(&mut self, _: HirId, _: Span, _: &mc::cmt_<'tcx>, _: euv::MutateMode) {} - - fn decl_without_init(&mut self, _: HirId, _: Span) {} -} - -fn unwrap_downcast_or_interior<'a, 'tcx>(mut cmt: &'a mc::cmt_<'tcx>) -> mc::cmt_<'tcx> { - loop { - match cmt.cat { - mc::Categorization::Downcast(ref c, _) | mc::Categorization::Interior(ref c, _) => { - cmt = c; - }, - _ => return (*cmt).clone(), - } - } + fn mutate(&mut self, _: &euv::Place<'tcx>) {} }