diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index 5b890f6abe08..bb6e0e5c51c2 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -78,16 +78,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoxedLocal { let fn_def_id = cx.tcx.hir().local_def_id(hir_id); let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id); - ExprUseVisitor::new( - &mut v, - cx.tcx, - fn_def_id, - cx.param_env, - region_scope_tree, - cx.tables, - None, - ) - .consume_body(body); + ExprUseVisitor::new(&mut v, cx.tcx, fn_def_id, cx.param_env, region_scope_tree, cx.tables).consume_body(body); for node in v.set { span_lint( @@ -114,86 +105,47 @@ fn is_argument(map: &hir::map::Map<'_>, id: HirId) -> bool { } impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { - fn consume(&mut self, _: HirId, _: Span, cmt: &cmt_<'tcx>, mode: ConsumeMode) { + fn consume(&mut self, cmt: &cmt_<'tcx>, mode: ConsumeMode) { if let Categorization::Local(lid) = cmt.cat { - if let Move(DirectRefMove) | Move(CaptureMove) = mode { + if let ConsumeMode::Move = mode { // moved out or in. clearly can't be localized self.set.remove(&lid); } } - } - fn matched_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: MatchMode) {} - fn consume_pat(&mut self, consume_pat: &Pat, cmt: &cmt_<'tcx>, _: ConsumeMode) { let map = &self.cx.tcx.hir(); - if is_argument(map, consume_pat.hir_id) { - // Skip closure arguments - let parent_id = map.get_parent_node(consume_pat.hir_id); - if let Some(Node::Expr(..)) = map.find(map.get_parent_node(parent_id)) { - return; - } - - if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) { - self.set.insert(consume_pat.hir_id); - } - return; - } - if let Categorization::Rvalue(..) = cmt.cat { - if let Some(Node::Stmt(st)) = map.find(map.get_parent_node(cmt.hir_id)) { - if let StmtKind::Local(ref loc) = st.kind { - if let Some(ref ex) = loc.init { - if let ExprKind::Box(..) = ex.kind { - if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) { - // let x = box (...) - self.set.insert(consume_pat.hir_id); - } - // TODO Box::new - // TODO vec![] - // TODO "foo".to_owned() and friends - } - } + if let Categorization::Local(lid) = cmt.cat { + if let Some(Node::Binding(_)) = map.find(cmt.hir_id) { + if self.set.contains(&lid) { + // let y = x where x is known + // remove x, insert y + self.set.insert(cmt.hir_id); + self.set.remove(&lid); } } } + } + + fn borrow(&mut self, cmt: &cmt_<'tcx>, _: ty::BorrowKind) { if let Categorization::Local(lid) = cmt.cat { - if self.set.contains(&lid) { - // let y = x where x is known - // remove x, insert y - self.set.insert(consume_pat.hir_id); - self.set.remove(&lid); - } + self.set.remove(&lid); } } - fn borrow( - &mut self, - _: HirId, - _: Span, - cmt: &cmt_<'tcx>, - _: ty::Region<'_>, - _: ty::BorrowKind, - loan_cause: LoanCause, - ) { - if let Categorization::Local(lid) = cmt.cat { - match loan_cause { - // `x.foo()` - // Used without autoderef-ing (i.e., `x.clone()`). - LoanCause::AutoRef | - - // `&x` - // `foo(&x)` where no extra autoref-ing is happening. - LoanCause::AddrOf | - // `match x` can move. - LoanCause::MatchDiscriminant => { - self.set.remove(&lid); - } + fn mutate(&mut self, cmt: &cmt_<'tcx>) { + let map = &self.cx.tcx.hir(); + if is_argument(map, cmt.hir_id) { + // Skip closure arguments + let parent_id = map.get_parent_node(cmt.hir_id); + if let Some(Node::Expr(..)) = map.find(map.get_parent_node(parent_id)) { + return; + } - // Do nothing for matches, etc. These can't escape. - _ => {} + if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) { + self.set.insert(cmt.hir_id); } + return; } } - fn decl_without_init(&mut self, _: HirId, _: Span) {} - fn mutate(&mut self, _: HirId, _: Span, _: &cmt_<'tcx>, _: MutateMode) {} } impl<'a, 'tcx> EscapeDelegate<'a, 'tcx> { diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 52477f68cd30..ea496a0294ab 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1547,37 +1547,31 @@ struct MutatePairDelegate { } impl<'tcx> Delegate<'tcx> for MutatePairDelegate { - fn consume(&mut self, _: HirId, _: Span, _: &cmt_<'tcx>, _: ConsumeMode) {} + fn consume(&mut self, _: &cmt_<'tcx>, _: ConsumeMode) {} - fn matched_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: MatchMode) {} - - fn consume_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: ConsumeMode) {} - - fn borrow(&mut self, _: HirId, sp: Span, cmt: &cmt_<'tcx>, _: ty::Region<'_>, bk: ty::BorrowKind, _: LoanCause) { + fn borrow(&mut self, cmt: &cmt_<'tcx>, bk: ty::BorrowKind) { if let ty::BorrowKind::MutBorrow = bk { if let Categorization::Local(id) = cmt.cat { if Some(id) == self.hir_id_low { - self.span_low = Some(sp) + self.span_low = Some(cmt.span) } if Some(id) == self.hir_id_high { - self.span_high = Some(sp) + self.span_high = Some(cmt.span) } } } } - fn mutate(&mut self, _: HirId, sp: Span, cmt: &cmt_<'tcx>, _: MutateMode) { + fn mutate(&mut self, cmt: &cmt_<'tcx>) { if let Categorization::Local(id) = cmt.cat { if Some(id) == self.hir_id_low { - self.span_low = Some(sp) + self.span_low = Some(cmt.span) } if Some(id) == self.hir_id_high { - self.span_high = Some(sp) + self.span_high = Some(cmt.span) } } } - - fn decl_without_init(&mut self, _: HirId, _: Span) {} } impl<'tcx> MutatePairDelegate { @@ -1655,7 +1649,6 @@ fn check_for_mutation( cx.param_env, region_scope_tree, cx.tables, - None, ) .walk_expr(body); delegate.mutation_span() diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index be233c6e69ad..d10d635e0e8b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -21,11 +21,11 @@ use syntax::symbol::{sym, LocalInternedString, Symbol}; use crate::utils::usage::mutated_variables; use crate::utils::{ get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, implements_trait, in_macro, is_copy, - is_ctor_function, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment, match_def_path, - match_qpath, match_trait_method, match_type, match_var, method_calls, method_chain_args, paths, remove_blocks, - return_ty, same_tys, single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite, - span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, sugg, walk_ptrs_ty, - walk_ptrs_ty_depth, SpanlessEq, + is_ctor_or_promotable_const_function, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment, + match_def_path, match_qpath, match_trait_method, match_type, match_var, method_calls, method_chain_args, paths, + remove_blocks, return_ty, same_tys, single_segment_path, snippet, snippet_with_applicability, + snippet_with_macro_callsite, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, + span_note_and_lint, sugg, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq, }; declare_clippy_lint! { @@ -1281,22 +1281,13 @@ fn lint_or_fun_call<'a, 'tcx>( fn visit_expr(&mut self, expr: &'tcx hir::Expr) { let call_found = match &expr.kind { // ignore enum and struct constructors - hir::ExprKind::Call(..) => !is_ctor_function(self.cx, expr), + hir::ExprKind::Call(..) => !is_ctor_or_promotable_const_function(self.cx, expr), hir::ExprKind::MethodCall(..) => true, _ => false, }; if call_found { - // don't lint for constant values - let owner_def = self.cx.tcx.hir().get_parent_did(expr.hir_id); - let promotable = self - .cx - .tcx - .rvalue_promotable_map(owner_def) - .contains(&expr.hir_id.local_id); - if !promotable { - self.found |= true; - } + self.found |= true; } if !self.found { diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index e2ac39f82628..80f178289e4d 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -134,18 +134,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue { spans_need_deref, .. } = { - let mut ctx = MovedVariablesCtxt::new(cx); + let mut ctx = MovedVariablesCtxt::default(); 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); + euv::ExprUseVisitor::new(&mut ctx, cx.tcx, fn_def_id, cx.param_env, region_scope_tree, cx.tables) + .consume_body(body); ctx }; @@ -325,115 +317,34 @@ fn requires_exact_signature(attrs: &[Attribute]) -> bool { }) } -struct MovedVariablesCtxt<'a, 'tcx> { - 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>) { +impl MovedVariablesCtxt { + fn move_common(&mut self, cmt: &mc::cmt_<'_>) { let cmt = unwrap_downcast_or_interior(cmt); if let mc::Categorization::Local(vid) = cmt.cat { 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(id); - if id == parent { - // no parent - return; - } - id = parent; - - if let Some(node) = self.cx.tcx.hir().find(id) { - match node { - Node::Expr(e) => { - // `match` and `if let` - if let ExprKind::Match(ref c, ..) = e.kind { - 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.kind; - 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: &mc::cmt_<'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 mutate(&mut self, _: HirId, _: Span, _: &mc::cmt_<'tcx>, _: euv::MutateMode) {} + fn borrow(&mut self, _: &mc::cmt_<'tcx>, _: ty::BorrowKind) {} - fn decl_without_init(&mut self, _: HirId, _: Span) {} + fn mutate(&mut self, _: &mc::cmt_<'tcx>) {} } fn unwrap_downcast_or_interior<'a, 'tcx>(mut cmt: &'a mc::cmt_<'tcx>) -> mc::cmt_<'tcx> { diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 27dad6322d88..ae288f460ead 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -803,13 +803,15 @@ pub fn is_copy<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool { } /// Checks if an expression is constructing a tuple-like enum variant or struct -pub fn is_ctor_function(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { +pub fn is_ctor_or_promotable_const_function(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { if let ExprKind::Call(ref fun, _) = expr.kind { if let ExprKind::Path(ref qp) = fun.kind { - return matches!( - cx.tables.qpath_res(qp, fun.hir_id), - def::Res::Def(DefKind::Variant, ..) | Res::Def(DefKind::Ctor(..), _) - ); + let res = cx.tables.qpath_res(qp, fun.hir_id); + return match res { + def::Res::Def(DefKind::Variant, ..) | Res::Def(DefKind::Ctor(..), _) => true, + def::Res::Def(_, def_id) => cx.tcx.is_promotable_const_fn(def_id), + _ => false, + }; } } false diff --git a/clippy_lints/src/utils/usage.rs b/clippy_lints/src/utils/usage.rs index 3427d88ff032..41662099fd3d 100644 --- a/clippy_lints/src/utils/usage.rs +++ b/clippy_lints/src/utils/usage.rs @@ -6,7 +6,6 @@ use rustc::middle::mem_categorization::cmt_; use rustc::middle::mem_categorization::Categorization; use rustc::ty; use rustc_data_structures::fx::FxHashSet; -use syntax::source_map::Span; /// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined. pub fn mutated_variables<'a, 'tcx>(expr: &'tcx Expr, cx: &'a LateContext<'a, 'tcx>) -> Option> { @@ -23,7 +22,6 @@ pub fn mutated_variables<'a, 'tcx>(expr: &'tcx Expr, cx: &'a LateContext<'a, 'tc cx.param_env, region_scope_tree, cx.tables, - None, ) .walk_expr(expr); @@ -66,21 +64,15 @@ impl<'tcx> MutVarsDelegate { } impl<'tcx> Delegate<'tcx> for MutVarsDelegate { - fn consume(&mut self, _: HirId, _: Span, _: &cmt_<'tcx>, _: ConsumeMode) {} + fn consume(&mut self, _: &cmt_<'tcx>, _: ConsumeMode) {} - fn matched_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: MatchMode) {} - - fn consume_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: ConsumeMode) {} - - fn borrow(&mut self, _: HirId, _: Span, cmt: &cmt_<'tcx>, _: ty::Region<'_>, bk: ty::BorrowKind, _: LoanCause) { + fn borrow(&mut self, cmt: &cmt_<'tcx>, bk: ty::BorrowKind) { if let ty::BorrowKind::MutBorrow = bk { self.update(&cmt.cat) } } - fn mutate(&mut self, _: HirId, _: Span, cmt: &cmt_<'tcx>, _: MutateMode) { + fn mutate(&mut self, cmt: &cmt_<'tcx>) { self.update(&cmt.cat) } - - fn decl_without_init(&mut self, _: HirId, _: Span) {} } diff --git a/tests/ui/escape_analysis.stderr b/tests/ui/escape_analysis.stderr index 73fa9bfe19b5..19342fe1be74 100644 --- a/tests/ui/escape_analysis.stderr +++ b/tests/ui/escape_analysis.stderr @@ -12,11 +12,5 @@ error: local variable doesn't need to be boxed here LL | pub fn new(_needs_name: Box>) -> () {} | ^^^^^^^^^^^ -error: local variable doesn't need to be boxed here - --> $DIR/escape_analysis.rs:170:23 - | -LL | fn closure_borrow(x: Box) { - | ^ - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors diff --git a/tests/ui/mut_range_bound.stderr b/tests/ui/mut_range_bound.stderr index 50e94efde532..0eeb76e0ec5f 100644 --- a/tests/ui/mut_range_bound.stderr +++ b/tests/ui/mut_range_bound.stderr @@ -2,7 +2,7 @@ error: attempt to mutate range bound within loop; note that the range of the loo --> $DIR/mut_range_bound.rs:16:9 | LL | m = 5; - | ^^^^^ + | ^ | = note: `-D clippy::mut-range-bound` implied by `-D warnings` @@ -10,19 +10,19 @@ error: attempt to mutate range bound within loop; note that the range of the loo --> $DIR/mut_range_bound.rs:23:9 | LL | m *= 2; - | ^^^^^^ + | ^ error: attempt to mutate range bound within loop; note that the range of the loop is unchanged --> $DIR/mut_range_bound.rs:31:9 | LL | m = 5; - | ^^^^^ + | ^ error: attempt to mutate range bound within loop; note that the range of the loop is unchanged --> $DIR/mut_range_bound.rs:32:9 | LL | n = 7; - | ^^^^^ + | ^ error: attempt to mutate range bound within loop; note that the range of the loop is unchanged --> $DIR/mut_range_bound.rs:46:22 diff --git a/tests/ui/needless_pass_by_value.stderr b/tests/ui/needless_pass_by_value.stderr index 5efeea0685c6..3cb64a227f1a 100644 --- a/tests/ui/needless_pass_by_value.stderr +++ b/tests/ui/needless_pass_by_value.stderr @@ -28,12 +28,7 @@ error: this argument is passed by value, but not consumed in the function body --> $DIR/needless_pass_by_value.rs:49:18 | LL | fn test_match(x: Option>, y: Option>) { - | ^^^^^^^^^^^^^^^^^^^^^^ -help: consider taking a reference instead - | -LL | fn test_match(x: &Option>, y: Option>) { -LL | match *x { - | + | ^^^^^^^^^^^^^^^^^^^^^^ help: consider taking a reference instead: `&Option>` error: this argument is passed by value, but not consumed in the function body --> $DIR/needless_pass_by_value.rs:62:24 @@ -45,14 +40,7 @@ error: this argument is passed by value, but not consumed in the function body --> $DIR/needless_pass_by_value.rs:62:36 | LL | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) { - | ^^^^^^^ -help: consider taking a reference instead - | -LL | fn test_destructure(x: Wrapper, y: &Wrapper, z: Wrapper) { -LL | let Wrapper(s) = z; // moved -LL | let Wrapper(ref t) = *y; // not moved -LL | let Wrapper(_) = *y; // still not moved - | + | ^^^^^^^ help: consider taking a reference instead: `&Wrapper` error: this argument is passed by value, but not consumed in the function body --> $DIR/needless_pass_by_value.rs:78:49 @@ -152,37 +140,25 @@ error: this argument is passed by value, but not consumed in the function body --> $DIR/needless_pass_by_value.rs:131:45 | LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) { - | ^^^^^^^^^^^ + | ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper` | help: consider marking this type as Copy --> $DIR/needless_pass_by_value.rs:123:1 | LL | struct CopyWrapper(u32); | ^^^^^^^^^^^^^^^^^^^^^^^^ -help: consider taking a reference instead - | -LL | fn test_destructure_copy(x: CopyWrapper, y: &CopyWrapper, z: CopyWrapper) { -LL | let CopyWrapper(s) = z; // moved -LL | let CopyWrapper(ref t) = *y; // not moved -LL | let CopyWrapper(_) = *y; // still not moved - | error: this argument is passed by value, but not consumed in the function body --> $DIR/needless_pass_by_value.rs:131:61 | LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) { - | ^^^^^^^^^^^ + | ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper` | help: consider marking this type as Copy --> $DIR/needless_pass_by_value.rs:123:1 | LL | struct CopyWrapper(u32); | ^^^^^^^^^^^^^^^^^^^^^^^^ -help: consider taking a reference instead - | -LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: &CopyWrapper) { -LL | let CopyWrapper(s) = *z; // moved - | error: this argument is passed by value, but not consumed in the function body --> $DIR/needless_pass_by_value.rs:143:40