diff --git a/clippy_dev/src/serve.rs b/clippy_dev/src/serve.rs index 2e0794f12fa19..535c25e69f1bb 100644 --- a/clippy_dev/src/serve.rs +++ b/clippy_dev/src/serve.rs @@ -49,7 +49,7 @@ fn mtime(path: impl AsRef) -> SystemTime { .into_iter() .flatten() .flatten() - .map(|entry| mtime(&entry.path())) + .map(|entry| mtime(entry.path())) .max() .unwrap_or(SystemTime::UNIX_EPOCH) } else { diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index 0eb443167ecf3..8d1bfacd1dc30 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -128,7 +128,7 @@ fn generate_lint_files( for (lint_group, lints) in Lint::by_lint_group(usable_lints.into_iter().chain(internal_lints)) { let content = gen_lint_group_list(&lint_group, lints.iter()); process_file( - &format!("clippy_lints/src/lib.register_{lint_group}.rs"), + format!("clippy_lints/src/lib.register_{lint_group}.rs"), update_mode, &content, ); diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 3cd8f236e7a5f..45ee2fce4e470 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then}; +use clippy_utils::mir::{enclosing_mir, expr_local, local_assignments, used_exactly_once, PossibleBorrowerMap}; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::sugg::has_enclosing_paren; use clippy_utils::ty::{expr_sig, is_copy, peel_mid_ty_refs, ty_sig, variant_of_res}; @@ -11,13 +12,16 @@ use rustc_data_structures::fx::FxIndexMap; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_ty, Visitor}; use rustc_hir::{ - self as hir, def_id::DefId, BindingAnnotation, Body, BodyId, BorrowKind, Closure, Expr, ExprKind, FnRetTy, - GenericArg, HirId, ImplItem, ImplItemKind, Item, ItemKind, Local, MatchSource, Mutability, Node, Pat, PatKind, - Path, QPath, TraitItem, TraitItemKind, TyKind, UnOp, + self as hir, + def_id::{DefId, LocalDefId}, + BindingAnnotation, Body, BodyId, BorrowKind, Closure, Expr, ExprKind, FnRetTy, GenericArg, HirId, ImplItem, + ImplItemKind, Item, ItemKind, Local, MatchSource, Mutability, Node, Pat, PatKind, Path, QPath, TraitItem, + TraitItemKind, TyKind, UnOp, }; use rustc_index::bit_set::BitSet; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::mir::{Rvalue, StatementKind}; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability}; use rustc_middle::ty::{ self, Binder, BoundVariableKind, EarlyBinder, FnSig, GenericArgKind, List, ParamTy, PredicateKind, @@ -141,7 +145,7 @@ declare_clippy_lint! { "dereferencing when the compiler would automatically dereference" } -impl_lint_pass!(Dereferencing => [ +impl_lint_pass!(Dereferencing<'_> => [ EXPLICIT_DEREF_METHODS, NEEDLESS_BORROW, REF_BINDING_TO_REFERENCE, @@ -149,7 +153,7 @@ impl_lint_pass!(Dereferencing => [ ]); #[derive(Default)] -pub struct Dereferencing { +pub struct Dereferencing<'tcx> { state: Option<(State, StateData)>, // While parsing a `deref` method call in ufcs form, the path to the function is itself an @@ -170,11 +174,16 @@ pub struct Dereferencing { /// e.g. `m!(x) | Foo::Bar(ref x)` ref_locals: FxIndexMap>, + /// Stack of (body owner, `PossibleBorrowerMap`) pairs. Used by + /// `needless_borrow_impl_arg_position` to determine when a borrowed expression can instead + /// be moved. + possible_borrowers: Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, + // `IntoIterator` for arrays requires Rust 1.53. msrv: Option, } -impl Dereferencing { +impl<'tcx> Dereferencing<'tcx> { #[must_use] pub fn new(msrv: Option) -> Self { Self { @@ -244,7 +253,7 @@ struct RefPat { hir_id: HirId, } -impl<'tcx> LateLintPass<'tcx> for Dereferencing { +impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { #[expect(clippy::too_many_lines)] fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // Skip path expressions from deref calls. e.g. `Deref::deref(e)` @@ -278,7 +287,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { match (self.state.take(), kind) { (None, kind) => { let expr_ty = typeck.expr_ty(expr); - let (position, adjustments) = walk_parents(cx, expr, self.msrv); + let (position, adjustments) = walk_parents(cx, &mut self.possible_borrowers, expr, self.msrv); match kind { RefOp::Deref => { if let Position::FieldAccess { @@ -550,6 +559,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { } fn check_body_post(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) { + if self.possible_borrowers.last().map_or(false, |&(local_def_id, _)| { + local_def_id == cx.tcx.hir().body_owner_def_id(body.id()) + }) { + self.possible_borrowers.pop(); + } + if Some(body.id()) == self.current_body { for pat in self.ref_locals.drain(..).filter_map(|(_, x)| x) { let replacements = pat.replacements; @@ -682,6 +697,7 @@ impl Position { #[expect(clippy::too_many_lines)] fn walk_parents<'tcx>( cx: &LateContext<'tcx>, + possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, e: &'tcx Expr<'_>, msrv: Option, ) -> (Position, &'tcx [Adjustment<'tcx>]) { @@ -796,7 +812,16 @@ fn walk_parents<'tcx>( Some(hir_ty) => binding_ty_auto_deref_stability(cx, hir_ty, precedence, ty.bound_vars()), None => { if let ty::Param(param_ty) = ty.skip_binder().kind() { - needless_borrow_impl_arg_position(cx, parent, i, *param_ty, e, precedence, msrv) + needless_borrow_impl_arg_position( + cx, + possible_borrowers, + parent, + i, + *param_ty, + e, + precedence, + msrv, + ) } else { ty_auto_deref_stability(cx, cx.tcx.erase_late_bound_regions(ty), precedence) .position_for_arg() @@ -844,7 +869,16 @@ fn walk_parents<'tcx>( args.iter().position(|arg| arg.hir_id == child_id).map(|i| { let ty = cx.tcx.fn_sig(id).skip_binder().inputs()[i + 1]; if let ty::Param(param_ty) = ty.kind() { - needless_borrow_impl_arg_position(cx, parent, i + 1, *param_ty, e, precedence, msrv) + needless_borrow_impl_arg_position( + cx, + possible_borrowers, + parent, + i + 1, + *param_ty, + e, + precedence, + msrv, + ) } else { ty_auto_deref_stability( cx, @@ -1018,8 +1052,10 @@ fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool { // If the conditions are met, returns `Some(Position::ImplArg(..))`; otherwise, returns `None`. // The "is copyable" condition is to avoid the case where removing the `&` means `e` would have to // be moved, but it cannot be. +#[expect(clippy::too_many_arguments)] fn needless_borrow_impl_arg_position<'tcx>( cx: &LateContext<'tcx>, + possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, parent: &Expr<'tcx>, arg_index: usize, param_ty: ParamTy, @@ -1082,10 +1118,13 @@ fn needless_borrow_impl_arg_position<'tcx>( // elements are modified each time `check_referent` is called. let mut substs_with_referent_ty = substs_with_expr_ty.to_vec(); - let mut check_referent = |referent| { + let mut check_reference_and_referent = |reference, referent| { let referent_ty = cx.typeck_results().expr_ty(referent); - if !is_copy(cx, referent_ty) { + if !is_copy(cx, referent_ty) + && (referent_ty.has_significant_drop(cx.tcx, cx.param_env) + || !referent_used_exactly_once(cx, possible_borrowers, reference)) + { return false; } @@ -1127,7 +1166,7 @@ fn needless_borrow_impl_arg_position<'tcx>( let mut needless_borrow = false; while let ExprKind::AddrOf(_, _, referent) = expr.kind { - if !check_referent(referent) { + if !check_reference_and_referent(expr, referent) { break; } expr = referent; @@ -1155,6 +1194,36 @@ fn has_ref_mut_self_method(cx: &LateContext<'_>, trait_def_id: DefId) -> bool { }) } +fn referent_used_exactly_once<'a, 'tcx>( + cx: &'a LateContext<'tcx>, + possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, + reference: &Expr<'tcx>, +) -> bool { + let mir = enclosing_mir(cx.tcx, reference.hir_id); + if let Some(local) = expr_local(cx.tcx, reference) + && let [location] = *local_assignments(mir, local).as_slice() + && let StatementKind::Assign(box (_, Rvalue::Ref(_, _, place))) = + mir.basic_blocks[location.block].statements[location.statement_index].kind + && !place.has_deref() + { + let body_owner_local_def_id = cx.tcx.hir().enclosing_body_owner(reference.hir_id); + if possible_borrowers + .last() + .map_or(true, |&(local_def_id, _)| local_def_id != body_owner_local_def_id) + { + possible_borrowers.push((body_owner_local_def_id, PossibleBorrowerMap::new(cx, mir))); + } + let possible_borrower = &mut possible_borrowers.last_mut().unwrap().1; + // If `only_borrowers` were used here, the `copyable_iterator::warn` test would fail. The reason is + // that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible borrower of + // itself. See the comment in that method for an explanation as to why. + possible_borrower.bounded_borrowers(&[local], &[local, place.local], place.local, location) + && used_exactly_once(mir, place.local).unwrap_or(false) + } else { + false + } +} + // Iteratively replaces `param_ty` with `new_ty` in `substs`, and similarly for each resulting // projected type that is a type parameter. Returns `false` if replacing the types would have an // effect on the function signature beyond substituting `new_ty` for `param_ty`. @@ -1439,8 +1508,8 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data } } -impl Dereferencing { - fn check_local_usage<'tcx>(&mut self, cx: &LateContext<'tcx>, e: &Expr<'tcx>, local: HirId) { +impl<'tcx> Dereferencing<'tcx> { + fn check_local_usage(&mut self, cx: &LateContext<'tcx>, e: &Expr<'tcx>, local: HirId) { if let Some(outer_pat) = self.ref_locals.get_mut(&local) { if let Some(pat) = outer_pat { // Check for auto-deref diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2dcefd78763b7..ebb0f14fef528 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -38,7 +38,6 @@ extern crate rustc_infer; extern crate rustc_lexer; extern crate rustc_lint; extern crate rustc_middle; -extern crate rustc_mir_dataflow; extern crate rustc_parse; extern crate rustc_session; extern crate rustc_span; @@ -418,7 +417,7 @@ pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, sess: &Se let msrv = conf.msrv.as_ref().and_then(|s| { parse_msrv(s, None, None).or_else(|| { - sess.err(&format!( + sess.err(format!( "error reading Clippy's configuration file. `{s}` is not a valid Rust version" )); None @@ -434,7 +433,7 @@ fn read_msrv(conf: &Conf, sess: &Session) -> Option { .and_then(|v| parse_msrv(&v, None, None)); let clippy_msrv = conf.msrv.as_ref().and_then(|s| { parse_msrv(s, None, None).or_else(|| { - sess.err(&format!( + sess.err(format!( "error reading Clippy's configuration file. `{s}` is not a valid Rust version" )); None @@ -445,7 +444,7 @@ fn read_msrv(conf: &Conf, sess: &Session) -> Option { if let Some(clippy_msrv) = clippy_msrv { // if both files have an msrv, let's compare them and emit a warning if they differ if clippy_msrv != cargo_msrv { - sess.warn(&format!( + sess.warn(format!( "the MSRV in `clippy.toml` and `Cargo.toml` differ; using `{clippy_msrv}` from `clippy.toml`" )); } @@ -474,7 +473,7 @@ pub fn read_conf(sess: &Session) -> Conf { let TryConf { conf, errors, warnings } = utils::conf::read(&file_name); // all conf errors are non-fatal, we just use the default conf in case of error for error in errors { - sess.err(&format!( + sess.err(format!( "error reading Clippy's configuration file `{}`: {}", file_name.display(), format_error(error) diff --git a/clippy_lints/src/nonstandard_macro_braces.rs b/clippy_lints/src/nonstandard_macro_braces.rs index 0ca0befc13515..6c909e5ed73ea 100644 --- a/clippy_lints/src/nonstandard_macro_braces.rs +++ b/clippy_lints/src/nonstandard_macro_braces.rs @@ -266,7 +266,7 @@ impl<'de> Deserialize<'de> for MacroMatcher { .iter() .find(|b| b.0 == brace) .map(|(o, c)| ((*o).to_owned(), (*c).to_owned())) - .ok_or_else(|| de::Error::custom(&format!("expected one of `(`, `{{`, `[` found `{brace}`")))?, + .ok_or_else(|| de::Error::custom(format!("expected one of `(`, `{{`, `[` found `{brace}`")))?, }) } } diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index 9fd86331ec755..aedbe08e3e46e 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -1,25 +1,18 @@ use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then}; +use clippy_utils::mir::{visit_local_usage, LocalUsage, PossibleBorrowerMap}; use clippy_utils::source::snippet_opt; use clippy_utils::ty::{has_drop, is_copy, is_type_diagnostic_item, walk_ptrs_ty_depth}; use clippy_utils::{fn_has_unsatisfiable_preds, match_def_path, paths}; use if_chain::if_chain; -use rustc_data_structures::fx::FxHashMap; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; use rustc_hir::{def_id, Body, FnDecl, HirId}; -use rustc_index::bit_set::{BitSet, HybridBitSet}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::mir::{ - self, traversal, - visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor as _}, - Mutability, -}; -use rustc_middle::ty::{self, visit::TypeVisitor, Ty}; -use rustc_mir_dataflow::{Analysis, AnalysisDomain, CallReturnPlaces, GenKill, GenKillAnalysis, ResultsCursor}; +use rustc_middle::mir; +use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::{BytePos, Span}; use rustc_span::sym; -use std::ops::ControlFlow; macro_rules! unwrap_or_continue { ($x:expr) => { @@ -89,21 +82,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone { let mir = cx.tcx.optimized_mir(def_id.to_def_id()); - let possible_origin = { - let mut vis = PossibleOriginVisitor::new(mir); - vis.visit_body(mir); - vis.into_map(cx) - }; - let maybe_storage_live_result = MaybeStorageLive - .into_engine(cx.tcx, mir) - .pass_name("redundant_clone") - .iterate_to_fixpoint() - .into_results_cursor(mir); - let mut possible_borrower = { - let mut vis = PossibleBorrowerVisitor::new(cx, mir, possible_origin); - vis.visit_body(mir); - vis.into_map(cx, maybe_storage_live_result) - }; + let mut possible_borrower = PossibleBorrowerMap::new(cx, mir); for (bb, bbdata) in mir.basic_blocks.iter_enumerated() { let terminator = bbdata.terminator(); @@ -374,403 +353,40 @@ struct CloneUsage { /// Whether the clone value is mutated. clone_consumed_or_mutated: bool, } -fn visit_clone_usage(cloned: mir::Local, clone: mir::Local, mir: &mir::Body<'_>, bb: mir::BasicBlock) -> CloneUsage { - struct V { - cloned: mir::Local, - clone: mir::Local, - result: CloneUsage, - } - impl<'tcx> mir::visit::Visitor<'tcx> for V { - fn visit_basic_block_data(&mut self, block: mir::BasicBlock, data: &mir::BasicBlockData<'tcx>) { - let statements = &data.statements; - for (statement_index, statement) in statements.iter().enumerate() { - self.visit_statement(statement, mir::Location { block, statement_index }); - } - - self.visit_terminator( - data.terminator(), - mir::Location { - block, - statement_index: statements.len(), - }, - ); - } - - fn visit_place(&mut self, place: &mir::Place<'tcx>, ctx: PlaceContext, loc: mir::Location) { - let local = place.local; - - if local == self.cloned - && !matches!( - ctx, - PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_) - ) - { - self.result.cloned_used = true; - self.result.cloned_consume_or_mutate_loc = self.result.cloned_consume_or_mutate_loc.or_else(|| { - matches!( - ctx, - PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) - | PlaceContext::MutatingUse(MutatingUseContext::Borrow) - ) - .then(|| loc) - }); - } else if local == self.clone { - match ctx { - PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) - | PlaceContext::MutatingUse(MutatingUseContext::Borrow) => { - self.result.clone_consumed_or_mutated = true; - }, - _ => {}, - } - } - } - } - - let init = CloneUsage { - cloned_used: false, - cloned_consume_or_mutate_loc: None, - // Consider non-temporary clones consumed. - // TODO: Actually check for mutation of non-temporaries. - clone_consumed_or_mutated: mir.local_kind(clone) != mir::LocalKind::Temp, - }; - traversal::ReversePostorder::new(mir, bb) - .skip(1) - .fold(init, |usage, (tbb, tdata)| { - // Short-circuit - if (usage.cloned_used && usage.clone_consumed_or_mutated) || - // Give up on loops - tdata.terminator().successors().any(|s| s == bb) - { - return CloneUsage { - cloned_used: true, - clone_consumed_or_mutated: true, - ..usage - }; - } - - let mut v = V { - cloned, - clone, - result: usage, - }; - v.visit_basic_block_data(tbb, tdata); - v.result - }) -} - -/// Determines liveness of each local purely based on `StorageLive`/`Dead`. -#[derive(Copy, Clone)] -struct MaybeStorageLive; - -impl<'tcx> AnalysisDomain<'tcx> for MaybeStorageLive { - type Domain = BitSet; - const NAME: &'static str = "maybe_storage_live"; - - fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { - // bottom = dead - BitSet::new_empty(body.local_decls.len()) - } - - fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut Self::Domain) { - for arg in body.args_iter() { - state.insert(arg); - } - } -} - -impl<'tcx> GenKillAnalysis<'tcx> for MaybeStorageLive { - type Idx = mir::Local; - - fn statement_effect(&self, trans: &mut impl GenKill, stmt: &mir::Statement<'tcx>, _: mir::Location) { - match stmt.kind { - mir::StatementKind::StorageLive(l) => trans.gen(l), - mir::StatementKind::StorageDead(l) => trans.kill(l), - _ => (), - } - } - - fn terminator_effect( - &self, - _trans: &mut impl GenKill, - _terminator: &mir::Terminator<'tcx>, - _loc: mir::Location, - ) { - } - - fn call_return_effect( - &self, - _trans: &mut impl GenKill, - _block: mir::BasicBlock, - _return_places: CallReturnPlaces<'_, 'tcx>, - ) { - // Nothing to do when a call returns successfully - } -} - -/// Collects the possible borrowers of each local. -/// For example, `b = &a; c = &a;` will make `b` and (transitively) `c` -/// possible borrowers of `a`. -struct PossibleBorrowerVisitor<'a, 'tcx> { - possible_borrower: TransitiveRelation, - body: &'a mir::Body<'tcx>, - cx: &'a LateContext<'tcx>, - possible_origin: FxHashMap>, -} - -impl<'a, 'tcx> PossibleBorrowerVisitor<'a, 'tcx> { - fn new( - cx: &'a LateContext<'tcx>, - body: &'a mir::Body<'tcx>, - possible_origin: FxHashMap>, - ) -> Self { - Self { - possible_borrower: TransitiveRelation::default(), - cx, - body, - possible_origin, - } - } - - fn into_map( - self, - cx: &LateContext<'tcx>, - maybe_live: ResultsCursor<'tcx, 'tcx, MaybeStorageLive>, - ) -> PossibleBorrowerMap<'a, 'tcx> { - let mut map = FxHashMap::default(); - for row in (1..self.body.local_decls.len()).map(mir::Local::from_usize) { - if is_copy(cx, self.body.local_decls[row].ty) { - continue; - } - - let mut borrowers = self.possible_borrower.reachable_from(row, self.body.local_decls.len()); - borrowers.remove(mir::Local::from_usize(0)); - if !borrowers.is_empty() { - map.insert(row, borrowers); - } - } - - let bs = BitSet::new_empty(self.body.local_decls.len()); - PossibleBorrowerMap { - map, - maybe_live, - bitset: (bs.clone(), bs), - } - } -} - -impl<'a, 'tcx> mir::visit::Visitor<'tcx> for PossibleBorrowerVisitor<'a, 'tcx> { - fn visit_assign(&mut self, place: &mir::Place<'tcx>, rvalue: &mir::Rvalue<'_>, _location: mir::Location) { - let lhs = place.local; - match rvalue { - mir::Rvalue::Ref(_, _, borrowed) => { - self.possible_borrower.add(borrowed.local, lhs); - }, - other => { - if ContainsRegion - .visit_ty(place.ty(&self.body.local_decls, self.cx.tcx).ty) - .is_continue() - { - return; - } - rvalue_locals(other, |rhs| { - if lhs != rhs { - self.possible_borrower.add(rhs, lhs); - } - }); - }, - } - } - - fn visit_terminator(&mut self, terminator: &mir::Terminator<'_>, _loc: mir::Location) { - if let mir::TerminatorKind::Call { - args, - destination: mir::Place { local: dest, .. }, - .. - } = &terminator.kind - { - // TODO add doc - // If the call returns something with lifetimes, - // let's conservatively assume the returned value contains lifetime of all the arguments. - // For example, given `let y: Foo<'a> = foo(x)`, `y` is considered to be a possible borrower of `x`. - - let mut immutable_borrowers = vec![]; - let mut mutable_borrowers = vec![]; - - for op in args { - match op { - mir::Operand::Copy(p) | mir::Operand::Move(p) => { - if let ty::Ref(_, _, Mutability::Mut) = self.body.local_decls[p.local].ty.kind() { - mutable_borrowers.push(p.local); - } else { - immutable_borrowers.push(p.local); - } - }, - mir::Operand::Constant(..) => (), - } - } - - let mut mutable_variables: Vec = mutable_borrowers - .iter() - .filter_map(|r| self.possible_origin.get(r)) - .flat_map(HybridBitSet::iter) - .collect(); - - if ContainsRegion.visit_ty(self.body.local_decls[*dest].ty).is_break() { - mutable_variables.push(*dest); - } - - for y in mutable_variables { - for x in &immutable_borrowers { - self.possible_borrower.add(*x, y); - } - for x in &mutable_borrowers { - self.possible_borrower.add(*x, y); - } - } - } - } -} - -/// Collect possible borrowed for every `&mut` local. -/// For example, `_1 = &mut _2` generate _1: {_2,...} -/// Known Problems: not sure all borrowed are tracked -struct PossibleOriginVisitor<'a, 'tcx> { - possible_origin: TransitiveRelation, - body: &'a mir::Body<'tcx>, -} - -impl<'a, 'tcx> PossibleOriginVisitor<'a, 'tcx> { - fn new(body: &'a mir::Body<'tcx>) -> Self { - Self { - possible_origin: TransitiveRelation::default(), - body, - } - } - - fn into_map(self, cx: &LateContext<'tcx>) -> FxHashMap> { - let mut map = FxHashMap::default(); - for row in (1..self.body.local_decls.len()).map(mir::Local::from_usize) { - if is_copy(cx, self.body.local_decls[row].ty) { - continue; - } - - let mut borrowers = self.possible_origin.reachable_from(row, self.body.local_decls.len()); - borrowers.remove(mir::Local::from_usize(0)); - if !borrowers.is_empty() { - map.insert(row, borrowers); - } - } - map - } -} - -impl<'a, 'tcx> mir::visit::Visitor<'tcx> for PossibleOriginVisitor<'a, 'tcx> { - fn visit_assign(&mut self, place: &mir::Place<'tcx>, rvalue: &mir::Rvalue<'_>, _location: mir::Location) { - let lhs = place.local; - match rvalue { - // Only consider `&mut`, which can modify origin place - mir::Rvalue::Ref(_, rustc_middle::mir::BorrowKind::Mut { .. }, borrowed) | - // _2: &mut _; - // _3 = move _2 - mir::Rvalue::Use(mir::Operand::Move(borrowed)) | - // _3 = move _2 as &mut _; - mir::Rvalue::Cast(_, mir::Operand::Move(borrowed), _) - => { - self.possible_origin.add(lhs, borrowed.local); - }, - _ => {}, - } - } -} - -struct ContainsRegion; - -impl TypeVisitor<'_> for ContainsRegion { - type BreakTy = (); - - fn visit_region(&mut self, _: ty::Region<'_>) -> ControlFlow { - ControlFlow::BREAK - } -} - -fn rvalue_locals(rvalue: &mir::Rvalue<'_>, mut visit: impl FnMut(mir::Local)) { - use rustc_middle::mir::Rvalue::{Aggregate, BinaryOp, Cast, CheckedBinaryOp, Repeat, UnaryOp, Use}; - - let mut visit_op = |op: &mir::Operand<'_>| match op { - mir::Operand::Copy(p) | mir::Operand::Move(p) => visit(p.local), - mir::Operand::Constant(..) => (), - }; - match rvalue { - Use(op) | Repeat(op, _) | Cast(_, op, _) | UnaryOp(_, op) => visit_op(op), - Aggregate(_, ops) => ops.iter().for_each(visit_op), - BinaryOp(_, box (lhs, rhs)) | CheckedBinaryOp(_, box (lhs, rhs)) => { - visit_op(lhs); - visit_op(rhs); +fn visit_clone_usage(cloned: mir::Local, clone: mir::Local, mir: &mir::Body<'_>, bb: mir::BasicBlock) -> CloneUsage { + if let Some(( + LocalUsage { + local_use_locs: cloned_use_locs, + local_consume_or_mutate_locs: cloned_consume_or_mutate_locs, }, - _ => (), - } -} - -/// Result of `PossibleBorrowerVisitor`. -struct PossibleBorrowerMap<'a, 'tcx> { - /// Mapping `Local -> its possible borrowers` - map: FxHashMap>, - maybe_live: ResultsCursor<'a, 'tcx, MaybeStorageLive>, - // Caches to avoid allocation of `BitSet` on every query - bitset: (BitSet, BitSet), -} - -impl PossibleBorrowerMap<'_, '_> { - /// Returns true if the set of borrowers of `borrowed` living at `at` matches with `borrowers`. - fn only_borrowers(&mut self, borrowers: &[mir::Local], borrowed: mir::Local, at: mir::Location) -> bool { - self.maybe_live.seek_after_primary_effect(at); - - self.bitset.0.clear(); - let maybe_live = &mut self.maybe_live; - if let Some(bitset) = self.map.get(&borrowed) { - for b in bitset.iter().filter(move |b| maybe_live.contains(*b)) { - self.bitset.0.insert(b); - } - } else { - return false; - } - - self.bitset.1.clear(); - for b in borrowers { - self.bitset.1.insert(*b); + LocalUsage { + local_use_locs: _, + local_consume_or_mutate_locs: clone_consume_or_mutate_locs, + }, + )) = visit_local_usage( + &[cloned, clone], + mir, + mir::Location { + block: bb, + statement_index: mir.basic_blocks[bb].statements.len(), + }, + ) + .map(|mut vec| (vec.remove(0), vec.remove(0))) + { + CloneUsage { + cloned_used: !cloned_use_locs.is_empty(), + cloned_consume_or_mutate_loc: cloned_consume_or_mutate_locs.first().copied(), + // Consider non-temporary clones consumed. + // TODO: Actually check for mutation of non-temporaries. + clone_consumed_or_mutated: mir.local_kind(clone) != mir::LocalKind::Temp + || !clone_consume_or_mutate_locs.is_empty(), } - - self.bitset.0 == self.bitset.1 - } - - fn local_is_alive_at(&mut self, local: mir::Local, at: mir::Location) -> bool { - self.maybe_live.seek_after_primary_effect(at); - self.maybe_live.contains(local) - } -} - -#[derive(Default)] -struct TransitiveRelation { - relations: FxHashMap>, -} -impl TransitiveRelation { - fn add(&mut self, a: mir::Local, b: mir::Local) { - self.relations.entry(a).or_default().push(b); - } - - fn reachable_from(&self, a: mir::Local, domain_size: usize) -> HybridBitSet { - let mut seen = HybridBitSet::new_empty(domain_size); - let mut stack = vec![a]; - while let Some(u) = stack.pop() { - if let Some(edges) = self.relations.get(&u) { - for &v in edges { - if seen.insert(v) { - stack.push(v); - } - } - } + } else { + CloneUsage { + cloned_used: true, + cloned_consume_or_mutate_loc: None, + clone_consumed_or_mutated: true, } - seen } } diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index d9b22664fd25b..cd8575c90e86c 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -136,7 +136,7 @@ pub fn get_unique_inner_attr(sess: &Session, attrs: &[ast::Attribute], name: &'s .emit(); }, ast::AttrStyle::Outer => { - sess.span_err(attr.span, &format!("`{name}` cannot be an outer attribute")); + sess.span_err(attr.span, format!("`{name}` cannot be an outer attribute")); }, } } diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index e6492d76260e2..dbe75b43cb240 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -25,10 +25,12 @@ extern crate rustc_data_structures; extern crate rustc_errors; extern crate rustc_hir; extern crate rustc_hir_analysis; +extern crate rustc_index; extern crate rustc_infer; extern crate rustc_lexer; extern crate rustc_lint; extern crate rustc_middle; +extern crate rustc_mir_dataflow; extern crate rustc_parse_format; extern crate rustc_session; extern crate rustc_span; @@ -48,6 +50,7 @@ pub mod eager_or_lazy; pub mod higher; mod hir_utils; pub mod macros; +pub mod mir; pub mod msrvs; pub mod numeric_literal; pub mod paths; @@ -122,7 +125,7 @@ pub fn parse_msrv(msrv: &str, sess: Option<&Session>, span: Option) -> Opt return Some(version); } else if let Some(sess) = sess { if let Some(span) = span { - sess.span_err(span, &format!("`{msrv}` is not a valid Rust version")); + sess.span_err(span, format!("`{msrv}` is not a valid Rust version")); } } None diff --git a/clippy_utils/src/mir/maybe_storage_live.rs b/clippy_utils/src/mir/maybe_storage_live.rs new file mode 100644 index 0000000000000..d262b335d99d3 --- /dev/null +++ b/clippy_utils/src/mir/maybe_storage_live.rs @@ -0,0 +1,52 @@ +use rustc_index::bit_set::BitSet; +use rustc_middle::mir; +use rustc_mir_dataflow::{AnalysisDomain, CallReturnPlaces, GenKill, GenKillAnalysis}; + +/// Determines liveness of each local purely based on `StorageLive`/`Dead`. +#[derive(Copy, Clone)] +pub(super) struct MaybeStorageLive; + +impl<'tcx> AnalysisDomain<'tcx> for MaybeStorageLive { + type Domain = BitSet; + const NAME: &'static str = "maybe_storage_live"; + + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { + // bottom = dead + BitSet::new_empty(body.local_decls.len()) + } + + fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut Self::Domain) { + for arg in body.args_iter() { + state.insert(arg); + } + } +} + +impl<'tcx> GenKillAnalysis<'tcx> for MaybeStorageLive { + type Idx = mir::Local; + + fn statement_effect(&self, trans: &mut impl GenKill, stmt: &mir::Statement<'tcx>, _: mir::Location) { + match stmt.kind { + mir::StatementKind::StorageLive(l) => trans.gen(l), + mir::StatementKind::StorageDead(l) => trans.kill(l), + _ => (), + } + } + + fn terminator_effect( + &self, + _trans: &mut impl GenKill, + _terminator: &mir::Terminator<'tcx>, + _loc: mir::Location, + ) { + } + + fn call_return_effect( + &self, + _trans: &mut impl GenKill, + _block: mir::BasicBlock, + _return_places: CallReturnPlaces<'_, 'tcx>, + ) { + // Nothing to do when a call returns successfully + } +} diff --git a/clippy_utils/src/mir/mod.rs b/clippy_utils/src/mir/mod.rs new file mode 100644 index 0000000000000..c8aa6f3e595df --- /dev/null +++ b/clippy_utils/src/mir/mod.rs @@ -0,0 +1,165 @@ +use rustc_hir::{Expr, HirId}; +use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor}; +use rustc_middle::mir::{ + traversal, Body, InlineAsmOperand, Local, Location, Place, StatementKind, TerminatorKind, START_BLOCK, +}; +use rustc_middle::ty::TyCtxt; + +mod maybe_storage_live; + +mod possible_borrower; +pub use possible_borrower::PossibleBorrowerMap; + +mod possible_origin; + +mod transitive_relation; + +#[derive(Clone, Debug, Default)] +pub struct LocalUsage { + /// The locations where the local is used, if any. + pub local_use_locs: Vec, + /// The locations where the local is consumed or mutated, if any. + pub local_consume_or_mutate_locs: Vec, +} + +pub fn visit_local_usage(locals: &[Local], mir: &Body<'_>, location: Location) -> Option> { + let init = vec![ + LocalUsage { + local_use_locs: Vec::new(), + local_consume_or_mutate_locs: Vec::new(), + }; + locals.len() + ]; + + traversal::ReversePostorder::new(mir, location.block).try_fold(init, |usage, (tbb, tdata)| { + // Give up on loops + if tdata.terminator().successors().any(|s| s == location.block) { + return None; + } + + let mut v = V { + locals, + location, + results: usage, + }; + v.visit_basic_block_data(tbb, tdata); + Some(v.results) + }) +} + +struct V<'a> { + locals: &'a [Local], + location: Location, + results: Vec, +} + +impl<'a, 'tcx> Visitor<'tcx> for V<'a> { + fn visit_place(&mut self, place: &Place<'tcx>, ctx: PlaceContext, loc: Location) { + if loc.block == self.location.block && loc.statement_index <= self.location.statement_index { + return; + } + + let local = place.local; + + for (i, self_local) in self.locals.iter().enumerate() { + if local == *self_local { + if !matches!( + ctx, + PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_) + ) { + self.results[i].local_use_locs.push(loc); + } + if matches!( + ctx, + PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) + | PlaceContext::MutatingUse(MutatingUseContext::Borrow) + ) { + self.results[i].local_consume_or_mutate_locs.push(loc); + } + } + } + } +} + +/// Convenience wrapper around `visit_local_usage`. +pub fn used_exactly_once(mir: &rustc_middle::mir::Body<'_>, local: rustc_middle::mir::Local) -> Option { + visit_local_usage( + &[local], + mir, + Location { + block: START_BLOCK, + statement_index: 0, + }, + ) + .map(|mut vec| { + let LocalUsage { local_use_locs, .. } = vec.remove(0); + local_use_locs + .into_iter() + .filter(|location| !is_local_assignment(mir, local, *location)) + .count() + == 1 + }) +} + +/// Returns the `mir::Body` containing the node associated with `hir_id`. +#[allow(clippy::module_name_repetitions)] +pub fn enclosing_mir(tcx: TyCtxt<'_>, hir_id: HirId) -> &Body<'_> { + let body_owner_local_def_id = tcx.hir().enclosing_body_owner(hir_id); + tcx.optimized_mir(body_owner_local_def_id.to_def_id()) +} + +/// Tries to determine the `Local` corresponding to `expr`, if any. +/// This function is expensive and should be used sparingly. +pub fn expr_local(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> Option { + let mir = enclosing_mir(tcx, expr.hir_id); + mir.local_decls.iter_enumerated().find_map(|(local, local_decl)| { + if local_decl.source_info.span == expr.span { + Some(local) + } else { + None + } + }) +} + +/// Returns a vector of `mir::Location` where `local` is assigned. Each statement referred to has +/// kind `StatementKind::Assign`. +pub fn local_assignments(mir: &Body<'_>, local: Local) -> Vec { + let mut locations = Vec::new(); + for (block, data) in mir.basic_blocks.iter_enumerated() { + for statement_index in 0..=data.statements.len() { + let location = Location { block, statement_index }; + if is_local_assignment(mir, local, location) { + locations.push(location); + } + } + } + locations +} + +// `is_local_assignment` is based on `is_place_assignment`: +// https://github.com/rust-lang/rust/blob/b7413511dc85ec01ef4b91785f86614589ac6103/compiler/rustc_middle/src/mir/visit.rs#L1350 +fn is_local_assignment(mir: &Body<'_>, local: Local, location: Location) -> bool { + let Location { block, statement_index } = location; + let basic_block = &mir.basic_blocks[block]; + if statement_index < basic_block.statements.len() { + let statement = &basic_block.statements[statement_index]; + if let StatementKind::Assign(box (place, _)) = statement.kind { + place.as_local() == Some(local) + } else { + false + } + } else { + let terminator = basic_block.terminator(); + match &terminator.kind { + TerminatorKind::Call { destination, .. } => destination.as_local() == Some(local), + TerminatorKind::InlineAsm { operands, .. } => operands.iter().any(|operand| { + if let InlineAsmOperand::Out { place: Some(place), .. } = operand { + place.as_local() == Some(local) + } else { + false + } + }), + _ => false, + } + } +} diff --git a/clippy_utils/src/mir/possible_borrower.rs b/clippy_utils/src/mir/possible_borrower.rs new file mode 100644 index 0000000000000..25717bf3d2fee --- /dev/null +++ b/clippy_utils/src/mir/possible_borrower.rs @@ -0,0 +1,241 @@ +use super::{ + maybe_storage_live::MaybeStorageLive, possible_origin::PossibleOriginVisitor, + transitive_relation::TransitiveRelation, +}; +use crate::ty::is_copy; +use rustc_data_structures::fx::FxHashMap; +use rustc_index::bit_set::{BitSet, HybridBitSet}; +use rustc_lint::LateContext; +use rustc_middle::mir::{self, visit::Visitor as _, Mutability}; +use rustc_middle::ty::{self, visit::TypeVisitor}; +use rustc_mir_dataflow::{Analysis, ResultsCursor}; +use std::ops::ControlFlow; + +/// Collects the possible borrowers of each local. +/// For example, `b = &a; c = &a;` will make `b` and (transitively) `c` +/// possible borrowers of `a`. +#[allow(clippy::module_name_repetitions)] +struct PossibleBorrowerVisitor<'a, 'b, 'tcx> { + possible_borrower: TransitiveRelation, + body: &'b mir::Body<'tcx>, + cx: &'a LateContext<'tcx>, + possible_origin: FxHashMap>, +} + +impl<'a, 'b, 'tcx> PossibleBorrowerVisitor<'a, 'b, 'tcx> { + fn new( + cx: &'a LateContext<'tcx>, + body: &'b mir::Body<'tcx>, + possible_origin: FxHashMap>, + ) -> Self { + Self { + possible_borrower: TransitiveRelation::default(), + cx, + body, + possible_origin, + } + } + + fn into_map( + self, + cx: &'a LateContext<'tcx>, + maybe_live: ResultsCursor<'b, 'tcx, MaybeStorageLive>, + ) -> PossibleBorrowerMap<'b, 'tcx> { + let mut map = FxHashMap::default(); + for row in (1..self.body.local_decls.len()).map(mir::Local::from_usize) { + if is_copy(cx, self.body.local_decls[row].ty) { + continue; + } + + let mut borrowers = self.possible_borrower.reachable_from(row, self.body.local_decls.len()); + borrowers.remove(mir::Local::from_usize(0)); + if !borrowers.is_empty() { + map.insert(row, borrowers); + } + } + + let bs = BitSet::new_empty(self.body.local_decls.len()); + PossibleBorrowerMap { + map, + maybe_live, + bitset: (bs.clone(), bs), + } + } +} + +impl<'a, 'b, 'tcx> mir::visit::Visitor<'tcx> for PossibleBorrowerVisitor<'a, 'b, 'tcx> { + fn visit_assign(&mut self, place: &mir::Place<'tcx>, rvalue: &mir::Rvalue<'_>, _location: mir::Location) { + let lhs = place.local; + match rvalue { + mir::Rvalue::Ref(_, _, borrowed) => { + self.possible_borrower.add(borrowed.local, lhs); + }, + other => { + if ContainsRegion + .visit_ty(place.ty(&self.body.local_decls, self.cx.tcx).ty) + .is_continue() + { + return; + } + rvalue_locals(other, |rhs| { + if lhs != rhs { + self.possible_borrower.add(rhs, lhs); + } + }); + }, + } + } + + fn visit_terminator(&mut self, terminator: &mir::Terminator<'_>, _loc: mir::Location) { + if let mir::TerminatorKind::Call { + args, + destination: mir::Place { local: dest, .. }, + .. + } = &terminator.kind + { + // TODO add doc + // If the call returns something with lifetimes, + // let's conservatively assume the returned value contains lifetime of all the arguments. + // For example, given `let y: Foo<'a> = foo(x)`, `y` is considered to be a possible borrower of `x`. + + let mut immutable_borrowers = vec![]; + let mut mutable_borrowers = vec![]; + + for op in args { + match op { + mir::Operand::Copy(p) | mir::Operand::Move(p) => { + if let ty::Ref(_, _, Mutability::Mut) = self.body.local_decls[p.local].ty.kind() { + mutable_borrowers.push(p.local); + } else { + immutable_borrowers.push(p.local); + } + }, + mir::Operand::Constant(..) => (), + } + } + + let mut mutable_variables: Vec = mutable_borrowers + .iter() + .filter_map(|r| self.possible_origin.get(r)) + .flat_map(HybridBitSet::iter) + .collect(); + + if ContainsRegion.visit_ty(self.body.local_decls[*dest].ty).is_break() { + mutable_variables.push(*dest); + } + + for y in mutable_variables { + for x in &immutable_borrowers { + self.possible_borrower.add(*x, y); + } + for x in &mutable_borrowers { + self.possible_borrower.add(*x, y); + } + } + } + } +} + +struct ContainsRegion; + +impl TypeVisitor<'_> for ContainsRegion { + type BreakTy = (); + + fn visit_region(&mut self, _: ty::Region<'_>) -> ControlFlow { + ControlFlow::BREAK + } +} + +fn rvalue_locals(rvalue: &mir::Rvalue<'_>, mut visit: impl FnMut(mir::Local)) { + use rustc_middle::mir::Rvalue::{Aggregate, BinaryOp, Cast, CheckedBinaryOp, Repeat, UnaryOp, Use}; + + let mut visit_op = |op: &mir::Operand<'_>| match op { + mir::Operand::Copy(p) | mir::Operand::Move(p) => visit(p.local), + mir::Operand::Constant(..) => (), + }; + + match rvalue { + Use(op) | Repeat(op, _) | Cast(_, op, _) | UnaryOp(_, op) => visit_op(op), + Aggregate(_, ops) => ops.iter().for_each(visit_op), + BinaryOp(_, box (lhs, rhs)) | CheckedBinaryOp(_, box (lhs, rhs)) => { + visit_op(lhs); + visit_op(rhs); + }, + _ => (), + } +} + +/// Result of `PossibleBorrowerVisitor`. +#[allow(clippy::module_name_repetitions)] +pub struct PossibleBorrowerMap<'b, 'tcx> { + /// Mapping `Local -> its possible borrowers` + pub map: FxHashMap>, + maybe_live: ResultsCursor<'b, 'tcx, MaybeStorageLive>, + // Caches to avoid allocation of `BitSet` on every query + pub bitset: (BitSet, BitSet), +} + +impl<'a, 'b, 'tcx> PossibleBorrowerMap<'b, 'tcx> { + pub fn new(cx: &'a LateContext<'tcx>, mir: &'b mir::Body<'tcx>) -> Self { + let possible_origin = { + let mut vis = PossibleOriginVisitor::new(mir); + vis.visit_body(mir); + vis.into_map(cx) + }; + let maybe_storage_live_result = MaybeStorageLive + .into_engine(cx.tcx, mir) + .pass_name("redundant_clone") + .iterate_to_fixpoint() + .into_results_cursor(mir); + let mut vis = PossibleBorrowerVisitor::new(cx, mir, possible_origin); + vis.visit_body(mir); + vis.into_map(cx, maybe_storage_live_result) + } + + /// Returns true if the set of borrowers of `borrowed` living at `at` matches with `borrowers`. + pub fn only_borrowers(&mut self, borrowers: &[mir::Local], borrowed: mir::Local, at: mir::Location) -> bool { + self.bounded_borrowers(borrowers, borrowers, borrowed, at) + } + + /// Returns true if the set of borrowers of `borrowed` living at `at` includes at least `below` + /// but no more than `above`. + pub fn bounded_borrowers( + &mut self, + below: &[mir::Local], + above: &[mir::Local], + borrowed: mir::Local, + at: mir::Location, + ) -> bool { + self.maybe_live.seek_after_primary_effect(at); + + self.bitset.0.clear(); + let maybe_live = &mut self.maybe_live; + if let Some(bitset) = self.map.get(&borrowed) { + for b in bitset.iter().filter(move |b| maybe_live.contains(*b)) { + self.bitset.0.insert(b); + } + } else { + return false; + } + + self.bitset.1.clear(); + for b in below { + self.bitset.1.insert(*b); + } + + if !self.bitset.0.superset(&self.bitset.1) { + return false; + } + + for b in above { + self.bitset.0.remove(*b); + } + + self.bitset.0.is_empty() + } + + pub fn local_is_alive_at(&mut self, local: mir::Local, at: mir::Location) -> bool { + self.maybe_live.seek_after_primary_effect(at); + self.maybe_live.contains(local) + } +} diff --git a/clippy_utils/src/mir/possible_origin.rs b/clippy_utils/src/mir/possible_origin.rs new file mode 100644 index 0000000000000..8e7513d740ab3 --- /dev/null +++ b/clippy_utils/src/mir/possible_origin.rs @@ -0,0 +1,59 @@ +use super::transitive_relation::TransitiveRelation; +use crate::ty::is_copy; +use rustc_data_structures::fx::FxHashMap; +use rustc_index::bit_set::HybridBitSet; +use rustc_lint::LateContext; +use rustc_middle::mir; + +/// Collect possible borrowed for every `&mut` local. +/// For example, `_1 = &mut _2` generate _1: {_2,...} +/// Known Problems: not sure all borrowed are tracked +#[allow(clippy::module_name_repetitions)] +pub(super) struct PossibleOriginVisitor<'a, 'tcx> { + possible_origin: TransitiveRelation, + body: &'a mir::Body<'tcx>, +} + +impl<'a, 'tcx> PossibleOriginVisitor<'a, 'tcx> { + pub fn new(body: &'a mir::Body<'tcx>) -> Self { + Self { + possible_origin: TransitiveRelation::default(), + body, + } + } + + pub fn into_map(self, cx: &LateContext<'tcx>) -> FxHashMap> { + let mut map = FxHashMap::default(); + for row in (1..self.body.local_decls.len()).map(mir::Local::from_usize) { + if is_copy(cx, self.body.local_decls[row].ty) { + continue; + } + + let mut borrowers = self.possible_origin.reachable_from(row, self.body.local_decls.len()); + borrowers.remove(mir::Local::from_usize(0)); + if !borrowers.is_empty() { + map.insert(row, borrowers); + } + } + map + } +} + +impl<'a, 'tcx> mir::visit::Visitor<'tcx> for PossibleOriginVisitor<'a, 'tcx> { + fn visit_assign(&mut self, place: &mir::Place<'tcx>, rvalue: &mir::Rvalue<'_>, _location: mir::Location) { + let lhs = place.local; + match rvalue { + // Only consider `&mut`, which can modify origin place + mir::Rvalue::Ref(_, rustc_middle::mir::BorrowKind::Mut { .. }, borrowed) | + // _2: &mut _; + // _3 = move _2 + mir::Rvalue::Use(mir::Operand::Move(borrowed)) | + // _3 = move _2 as &mut _; + mir::Rvalue::Cast(_, mir::Operand::Move(borrowed), _) + => { + self.possible_origin.add(lhs, borrowed.local); + }, + _ => {}, + } + } +} diff --git a/clippy_utils/src/mir/transitive_relation.rs b/clippy_utils/src/mir/transitive_relation.rs new file mode 100644 index 0000000000000..7fe2960739fa2 --- /dev/null +++ b/clippy_utils/src/mir/transitive_relation.rs @@ -0,0 +1,29 @@ +use rustc_data_structures::fx::FxHashMap; +use rustc_index::bit_set::HybridBitSet; +use rustc_middle::mir; + +#[derive(Default)] +pub(super) struct TransitiveRelation { + relations: FxHashMap>, +} + +impl TransitiveRelation { + pub fn add(&mut self, a: mir::Local, b: mir::Local) { + self.relations.entry(a).or_default().push(b); + } + + pub fn reachable_from(&self, a: mir::Local, domain_size: usize) -> HybridBitSet { + let mut seen = HybridBitSet::new_empty(domain_size); + let mut stack = vec![a]; + while let Some(u) = stack.pop() { + if let Some(edges) = self.relations.get(&u) { + for &v in edges { + if seen.insert(v) { + stack.push(v); + } + } + } + } + seen + } +} diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 13938645fc3e5..78adc45365439 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -119,17 +119,11 @@ pub const RANGE_ARGUMENT_TRAIT: [&str; 3] = ["core", "ops", "RangeBounds"]; pub const RC_PTR_EQ: [&str; 4] = ["alloc", "rc", "Rc", "ptr_eq"]; pub const REFCELL_REF: [&str; 3] = ["core", "cell", "Ref"]; pub const REFCELL_REFMUT: [&str; 3] = ["core", "cell", "RefMut"]; -#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const REGEX_BUILDER_NEW: [&str; 5] = ["regex", "re_builder", "unicode", "RegexBuilder", "new"]; -#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const REGEX_BYTES_BUILDER_NEW: [&str; 5] = ["regex", "re_builder", "bytes", "RegexBuilder", "new"]; -#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const REGEX_BYTES_NEW: [&str; 4] = ["regex", "re_bytes", "Regex", "new"]; -#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const REGEX_BYTES_SET_NEW: [&str; 5] = ["regex", "re_set", "bytes", "RegexSet", "new"]; -#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const REGEX_NEW: [&str; 4] = ["regex", "re_unicode", "Regex", "new"]; -#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const REGEX_SET_NEW: [&str; 5] = ["regex", "re_set", "unicode", "RegexSet", "new"]; /// Preferably use the diagnostic item `sym::Result` where possible pub const RESULT: [&str; 3] = ["core", "result", "Result"]; diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs index cc2b3e1acec71..95b20d7f02427 100644 --- a/lintcheck/src/main.rs +++ b/lintcheck/src/main.rs @@ -345,7 +345,7 @@ impl Crate { clippy_args.push(opt); } } else { - clippy_args.extend(&["-Wclippy::pedantic", "-Wclippy::cargo"]) + clippy_args.extend(["-Wclippy::pedantic", "-Wclippy::cargo"]) } if lint_filter.is_empty() { @@ -457,15 +457,11 @@ fn build_clippy() { /// Read a `lintcheck_crates.toml` file fn read_crates(toml_path: &Path) -> (Vec, RecursiveOptions) { let toml_content: String = - std::fs::read_to_string(&toml_path).unwrap_or_else(|_| panic!("Failed to read {}", toml_path.display())); + std::fs::read_to_string(toml_path).unwrap_or_else(|_| panic!("Failed to read {}", toml_path.display())); let crate_list: SourceList = toml::from_str(&toml_content).unwrap_or_else(|e| panic!("Failed to parse {}: \n{}", toml_path.display(), e)); // parse the hashmap of the toml file into a list of crates - let tomlcrates: Vec = crate_list - .crates - .into_iter() - .map(|(_cratename, tomlcrate)| tomlcrate) - .collect(); + let tomlcrates: Vec = crate_list.crates.into_values().collect(); // flatten TomlCrates into CrateSources (one TomlCrates may represent several versions of a crate => // multiple Cratesources) @@ -602,10 +598,10 @@ fn main() { ) { let shared_target_dir = "target/lintcheck/shared_target_dir"; // if we get an Err here, the shared target dir probably does simply not exist - if let Ok(metadata) = std::fs::metadata(&shared_target_dir) { + if let Ok(metadata) = std::fs::metadata(shared_target_dir) { if metadata.is_dir() { println!("Clippy is newer than lint check logs, clearing lintcheck shared target dir..."); - std::fs::remove_dir_all(&shared_target_dir) + std::fs::remove_dir_all(shared_target_dir) .expect("failed to remove target/lintcheck/shared_target_dir"); } } @@ -779,7 +775,7 @@ fn read_stats_from_file(file_path: &Path) -> HashMap { fn print_stats(old_stats: HashMap, new_stats: HashMap<&String, usize>, lint_filter: &Vec) { let same_in_both_hashmaps = old_stats .iter() - .filter(|(old_key, old_val)| new_stats.get::<&String>(&old_key) == Some(old_val)) + .filter(|(old_key, old_val)| new_stats.get::<&String>(old_key) == Some(old_val)) .map(|(k, v)| (k.to_string(), *v)) .collect::>(); @@ -797,7 +793,7 @@ fn print_stats(old_stats: HashMap, new_stats: HashMap<&String, us // list all new counts (key is in new stats but not in old stats) new_stats_deduped .iter() - .filter(|(new_key, _)| old_stats_deduped.get::(&new_key).is_none()) + .filter(|(new_key, _)| old_stats_deduped.get::(new_key).is_none()) .for_each(|(new_key, new_value)| { println!("{} 0 => {}", new_key, new_value); }); @@ -805,16 +801,16 @@ fn print_stats(old_stats: HashMap, new_stats: HashMap<&String, us // list all changed counts (key is in both maps but value differs) new_stats_deduped .iter() - .filter(|(new_key, _new_val)| old_stats_deduped.get::(&new_key).is_some()) + .filter(|(new_key, _new_val)| old_stats_deduped.get::(new_key).is_some()) .for_each(|(new_key, new_val)| { - let old_val = old_stats_deduped.get::(&new_key).unwrap(); + let old_val = old_stats_deduped.get::(new_key).unwrap(); println!("{} {} => {}", new_key, old_val, new_val); }); // list all gone counts (key is in old status but not in new stats) old_stats_deduped .iter() - .filter(|(old_key, _)| new_stats_deduped.get::<&String>(&old_key).is_none()) + .filter(|(old_key, _)| new_stats_deduped.get::<&String>(old_key).is_none()) .filter(|(old_key, _)| lint_filter.is_empty() || lint_filter.contains(old_key)) .for_each(|(old_key, old_value)| { println!("{} {} => 0", old_key, old_value); @@ -832,12 +828,12 @@ fn create_dirs(krate_download_dir: &Path, extract_dir: &Path) { panic!("cannot create lintcheck target dir"); } }); - std::fs::create_dir(&krate_download_dir).unwrap_or_else(|err| { + std::fs::create_dir(krate_download_dir).unwrap_or_else(|err| { if err.kind() != ErrorKind::AlreadyExists { panic!("cannot create crate download dir"); } }); - std::fs::create_dir(&extract_dir).unwrap_or_else(|err| { + std::fs::create_dir(extract_dir).unwrap_or_else(|err| { if err.kind() != ErrorKind::AlreadyExists { panic!("cannot create crate extraction dir"); } @@ -863,7 +859,7 @@ fn lintcheck_test() { "lintcheck/test_sources.toml", ]; let status = std::process::Command::new("cargo") - .args(&args) + .args(args) .current_dir("..") // repo root .status(); //.output(); diff --git a/tests/compile-test.rs b/tests/compile-test.rs index fa769222d1af3..c10ee969c0146 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -283,7 +283,7 @@ fn run_ui_cargo() { env::set_current_dir(&src_path)?; let cargo_toml_path = case.path().join("Cargo.toml"); - let cargo_content = fs::read(&cargo_toml_path)?; + let cargo_content = fs::read(cargo_toml_path)?; let cargo_parsed: toml::Value = toml::from_str( std::str::from_utf8(&cargo_content).expect("`Cargo.toml` is not a valid utf-8 file!"), ) diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index aa2687159ef47..340e89d2db1d2 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -3,7 +3,11 @@ #[warn(clippy::all, clippy::needless_borrow)] #[allow(unused_variables)] -#[allow(clippy::uninlined_format_args, clippy::unnecessary_mut_passed)] +#[allow( + clippy::uninlined_format_args, + clippy::unnecessary_mut_passed, + clippy::unnecessary_to_owned +)] fn main() { let a = 5; let ref_a = &a; @@ -134,6 +138,7 @@ fn main() { multiple_constraints([[""]]); multiple_constraints_normalizes_to_same(X, X); let _ = Some("").unwrap_or(""); + let _ = std::fs::write("x", "".to_string()); only_sized(&""); // Don't lint. `Sized` is only bound let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound @@ -276,8 +281,9 @@ mod copyable_iterator { fn dont_warn(mut x: Iter) { takes_iter(&mut x); } + #[allow(unused_mut)] fn warn(mut x: &mut Iter) { - takes_iter(&mut x) + takes_iter(x) } } @@ -327,3 +333,55 @@ fn issue9383() { ManuallyDrop::drop(&mut ocean.coral); } } + +#[allow(dead_code)] +fn closure_test() { + let env = "env".to_owned(); + let arg = "arg".to_owned(); + let f = |arg| { + let loc = "loc".to_owned(); + let _ = std::fs::write("x", &env); // Don't lint. In environment + let _ = std::fs::write("x", arg); + let _ = std::fs::write("x", loc); + }; + let _ = std::fs::write("x", &env); // Don't lint. Borrowed by `f` + f(arg); +} + +#[allow(dead_code)] +mod significant_drop { + #[derive(Debug)] + struct X; + + #[derive(Debug)] + struct Y; + + impl Drop for Y { + fn drop(&mut self) {} + } + + fn foo(x: X, y: Y) { + debug(x); + debug(&y); // Don't lint. Has significant drop + } + + fn debug(_: impl std::fmt::Debug) {} +} + +#[allow(dead_code)] +mod used_exactly_once { + fn foo(x: String) { + use_x(x); + } + fn use_x(_: impl AsRef) {} +} + +#[allow(dead_code)] +mod used_more_than_once { + fn foo(x: String) { + use_x(&x); + use_x_again(&x); + } + fn use_x(_: impl AsRef) {} + fn use_x_again(_: impl AsRef) {} +} diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index d41251e8f6aac..c93711ac8e284 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -3,7 +3,11 @@ #[warn(clippy::all, clippy::needless_borrow)] #[allow(unused_variables)] -#[allow(clippy::uninlined_format_args, clippy::unnecessary_mut_passed)] +#[allow( + clippy::uninlined_format_args, + clippy::unnecessary_mut_passed, + clippy::unnecessary_to_owned +)] fn main() { let a = 5; let ref_a = &a; @@ -134,6 +138,7 @@ fn main() { multiple_constraints(&[[""]]); multiple_constraints_normalizes_to_same(&X, X); let _ = Some("").unwrap_or(&""); + let _ = std::fs::write("x", &"".to_string()); only_sized(&""); // Don't lint. `Sized` is only bound let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound @@ -276,6 +281,7 @@ mod copyable_iterator { fn dont_warn(mut x: Iter) { takes_iter(&mut x); } + #[allow(unused_mut)] fn warn(mut x: &mut Iter) { takes_iter(&mut x) } @@ -327,3 +333,55 @@ fn issue9383() { ManuallyDrop::drop(&mut ocean.coral); } } + +#[allow(dead_code)] +fn closure_test() { + let env = "env".to_owned(); + let arg = "arg".to_owned(); + let f = |arg| { + let loc = "loc".to_owned(); + let _ = std::fs::write("x", &env); // Don't lint. In environment + let _ = std::fs::write("x", &arg); + let _ = std::fs::write("x", &loc); + }; + let _ = std::fs::write("x", &env); // Don't lint. Borrowed by `f` + f(arg); +} + +#[allow(dead_code)] +mod significant_drop { + #[derive(Debug)] + struct X; + + #[derive(Debug)] + struct Y; + + impl Drop for Y { + fn drop(&mut self) {} + } + + fn foo(x: X, y: Y) { + debug(&x); + debug(&y); // Don't lint. Has significant drop + } + + fn debug(_: impl std::fmt::Debug) {} +} + +#[allow(dead_code)] +mod used_exactly_once { + fn foo(x: String) { + use_x(&x); + } + fn use_x(_: impl AsRef) {} +} + +#[allow(dead_code)] +mod used_more_than_once { + fn foo(x: String) { + use_x(&x); + use_x_again(&x); + } + fn use_x(_: impl AsRef) {} + fn use_x_again(_: impl AsRef) {} +} diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index 5af68706d4ba5..8b593268bec21 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -1,5 +1,5 @@ error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:11:15 + --> $DIR/needless_borrow.rs:15:15 | LL | let _ = x(&&a); // warn | ^^^ help: change this to: `&a` @@ -7,172 +7,208 @@ LL | let _ = x(&&a); // warn = note: `-D clippy::needless-borrow` implied by `-D warnings` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:15:13 + --> $DIR/needless_borrow.rs:19:13 | LL | mut_ref(&mut &mut b); // warn | ^^^^^^^^^^^ help: change this to: `&mut b` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:27:13 + --> $DIR/needless_borrow.rs:31:13 | LL | &&a | ^^^ help: change this to: `&a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:29:15 + --> $DIR/needless_borrow.rs:33:15 | LL | 46 => &&a, | ^^^ help: change this to: `&a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:35:27 + --> $DIR/needless_borrow.rs:39:27 | LL | break &ref_a; | ^^^^^^ help: change this to: `ref_a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:42:15 + --> $DIR/needless_borrow.rs:46:15 | LL | let _ = x(&&&a); | ^^^^ help: change this to: `&a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:43:15 + --> $DIR/needless_borrow.rs:47:15 | LL | let _ = x(&mut &&a); | ^^^^^^^^ help: change this to: `&a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:44:15 + --> $DIR/needless_borrow.rs:48:15 | LL | let _ = x(&&&mut b); | ^^^^^^^^ help: change this to: `&mut b` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:45:15 + --> $DIR/needless_borrow.rs:49:15 | LL | let _ = x(&&ref_a); | ^^^^^^^ help: change this to: `ref_a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:48:11 + --> $DIR/needless_borrow.rs:52:11 | LL | x(&b); | ^^ help: change this to: `b` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:55:13 + --> $DIR/needless_borrow.rs:59:13 | LL | mut_ref(&mut x); | ^^^^^^ help: change this to: `x` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:56:13 + --> $DIR/needless_borrow.rs:60:13 | LL | mut_ref(&mut &mut x); | ^^^^^^^^^^^ help: change this to: `x` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:57:23 + --> $DIR/needless_borrow.rs:61:23 | LL | let y: &mut i32 = &mut x; | ^^^^^^ help: change this to: `x` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:58:23 + --> $DIR/needless_borrow.rs:62:23 | LL | let y: &mut i32 = &mut &mut x; | ^^^^^^^^^^^ help: change this to: `x` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:67:14 + --> $DIR/needless_borrow.rs:71:14 | LL | 0 => &mut x, | ^^^^^^ help: change this to: `x` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:73:14 + --> $DIR/needless_borrow.rs:77:14 | LL | 0 => &mut x, | ^^^^^^ help: change this to: `x` error: this expression borrows a value the compiler would automatically borrow - --> $DIR/needless_borrow.rs:85:13 + --> $DIR/needless_borrow.rs:89:13 | LL | let _ = (&x).0; | ^^^^ help: change this to: `x` error: this expression borrows a value the compiler would automatically borrow - --> $DIR/needless_borrow.rs:87:22 + --> $DIR/needless_borrow.rs:91:22 | LL | let _ = unsafe { (&*x).0 }; | ^^^^^ help: change this to: `(*x)` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:97:5 + --> $DIR/needless_borrow.rs:101:5 | LL | (&&()).foo(); | ^^^^^^ help: change this to: `(&())` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:106:5 + --> $DIR/needless_borrow.rs:110:5 | LL | (&&5).foo(); | ^^^^^ help: change this to: `(&5)` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:131:51 + --> $DIR/needless_borrow.rs:135:51 | LL | let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap(); | ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:132:44 + --> $DIR/needless_borrow.rs:136:44 | LL | let _ = std::path::Path::new(".").join(&&"."); | ^^^^^ help: change this to: `"."` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:133:23 + --> $DIR/needless_borrow.rs:137:23 | LL | deref_target_is_x(&X); | ^^ help: change this to: `X` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:134:26 + --> $DIR/needless_borrow.rs:138:26 | LL | multiple_constraints(&[[""]]); | ^^^^^^^ help: change this to: `[[""]]` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:135:45 + --> $DIR/needless_borrow.rs:139:45 | LL | multiple_constraints_normalizes_to_same(&X, X); | ^^ help: change this to: `X` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:136:32 + --> $DIR/needless_borrow.rs:140:32 | LL | let _ = Some("").unwrap_or(&""); | ^^^ help: change this to: `""` +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:141:33 + | +LL | let _ = std::fs::write("x", &"".to_string()); + | ^^^^^^^^^^^^^^^ help: change this to: `"".to_string()` + error: this expression borrows a value the compiler would automatically borrow - --> $DIR/needless_borrow.rs:187:13 + --> $DIR/needless_borrow.rs:192:13 | LL | (&self.f)() | ^^^^^^^^^ help: change this to: `(self.f)` error: this expression borrows a value the compiler would automatically borrow - --> $DIR/needless_borrow.rs:196:13 + --> $DIR/needless_borrow.rs:201:13 | LL | (&mut self.f)() | ^^^^^^^^^^^^^ help: change this to: `(self.f)` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:298:55 + --> $DIR/needless_borrow.rs:286:20 + | +LL | takes_iter(&mut x) + | ^^^^^^ help: change this to: `x` + +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:304:55 | LL | let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap(); | ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]` -error: aborting due to 29 previous errors +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:344:37 + | +LL | let _ = std::fs::write("x", &arg); + | ^^^^ help: change this to: `arg` + +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:345:37 + | +LL | let _ = std::fs::write("x", &loc); + | ^^^^ help: change this to: `loc` + +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:364:15 + | +LL | debug(&x); + | ^^ help: change this to: `x` + +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:374:15 + | +LL | use_x(&x); + | ^^ help: change this to: `x` + +error: aborting due to 35 previous errors diff --git a/tests/ui/unnecessary_to_owned.fixed b/tests/ui/unnecessary_to_owned.fixed index f97583aa22f9a..fe09aad06bc84 100644 --- a/tests/ui/unnecessary_to_owned.fixed +++ b/tests/ui/unnecessary_to_owned.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(clippy::ptr_arg)] +#![allow(clippy::needless_borrow, clippy::ptr_arg)] #![warn(clippy::unnecessary_to_owned)] #![feature(custom_inner_attributes)] diff --git a/tests/ui/unnecessary_to_owned.rs b/tests/ui/unnecessary_to_owned.rs index aa5394a565790..3de6d0903c0f1 100644 --- a/tests/ui/unnecessary_to_owned.rs +++ b/tests/ui/unnecessary_to_owned.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(clippy::ptr_arg)] +#![allow(clippy::needless_borrow, clippy::ptr_arg)] #![warn(clippy::unnecessary_to_owned)] #![feature(custom_inner_attributes)] diff --git a/tests/versioncheck.rs b/tests/versioncheck.rs index 9e07769a8e4fd..a6d8d0307ce53 100644 --- a/tests/versioncheck.rs +++ b/tests/versioncheck.rs @@ -48,7 +48,7 @@ fn check_that_clippy_has_the_same_major_version_as_rustc() { // `RUSTC_REAL` if Clippy is build in the Rust repo with `./x.py`. let rustc = std::env::var("RUSTC_REAL").unwrap_or_else(|_| "rustc".to_string()); let rustc_version = String::from_utf8( - std::process::Command::new(&rustc) + std::process::Command::new(rustc) .arg("--version") .output() .expect("failed to run `rustc --version`")