diff --git a/Cargo.toml b/Cargo.toml index ca8bf9fac91a..137e41f52db1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ path = "src/driver.rs" [dependencies] clippy_lints = { path = "clippy_lints" } +clippy_utils = { path = "clippy_utils" } rustc_tools_util = "0.3.0" tempfile = { version = "3.2", optional = true } termize = "0.1" diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 7787fc348daa..9caed4a8b1db 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,5 +1,6 @@ +use crate::{BorrowckContext, BorrowckLintPass}; 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::mir::{expr_local, local_assignments, used_exactly_once, PossibleBorrowerMap}; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::sugg::has_enclosing_paren; @@ -8,22 +9,23 @@ use clippy_utils::{ fn_def_id, get_parent_expr, get_parent_expr_for_hir, is_lint_allowed, path_to_local, walk_to_expr_usage, }; +use rustc_ast as ast; use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX}; +use rustc_borrowck::consumers::BodyWithBorrowckFacts; use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::graph::iterate::{CycleDetector, TriColorDepthFirstSearch}; use rustc_errors::Applicability; +use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::{walk_ty, Visitor}; use rustc_hir::{ - self as hir, - def_id::{DefId, LocalDefId}, - BindingAnnotation, Body, BodyId, BorrowKind, Closure, Expr, ExprKind, FnRetTy, GenericArg, HirId, ImplItem, + self as hir, BindingAnnotation, Body, 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_lint::{late_lint_methods, LateContext, LateLintPass, LintPass}; +use rustc_middle::mir::{PlaceElem, Rvalue, StatementKind}; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability}; use rustc_middle::ty::{ self, Binder, BoundVariableKind, Clause, EarlyBinder, FnSig, GenericArgKind, List, ParamEnv, ParamTy, @@ -34,6 +36,7 @@ use rustc_span::{symbol::sym, Span, Symbol}; use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::traits::{query::evaluate_obligation::InferCtxtExt as _, Obligation, ObligationCause}; use std::collections::VecDeque; +use std::rc::Rc; declare_clippy_lint! { /// ### What it does @@ -148,7 +151,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, @@ -156,7 +159,7 @@ impl_lint_pass!(Dereferencing<'_> => [ ]); #[derive(Default)] -pub struct Dereferencing<'tcx> { +pub struct Dereferencing<'b, 'tcx> { state: Option<(State, StateData)>, // While parsing a `deref` method call in ufcs form, the path to the function is itself an @@ -167,7 +170,7 @@ pub struct Dereferencing<'tcx> { /// The body the first local was found in. Used to emit lints when the traversal of the body has /// been finished. Note we can't lint at the end of every body as they can be nested within each /// other. - current_body: Option, + current_body: Option<&'b Rc>>, /// The list of locals currently being checked by the lint. /// If the value is `None`, then the binding has been seen as a ref pattern, but is not linted. @@ -177,16 +180,15 @@ pub struct Dereferencing<'tcx> { /// 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>)>, + /// Used by `needless_borrow_impl_arg_position` to determine when a borrowed expression can + /// instead be moved. + possible_borrowers: Option>, // `IntoIterator` for arrays requires Rust 1.53. msrv: Msrv, } -impl<'tcx> Dereferencing<'tcx> { +impl<'b, 'tcx> Dereferencing<'b, 'tcx> { #[must_use] pub fn new(msrv: Msrv) -> Self { Self { @@ -256,7 +258,7 @@ struct RefPat { hir_id: HirId, } -impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { +impl<'b, 'tcx> LateLintPass<'tcx> for Dereferencing<'b, '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)` @@ -288,7 +290,13 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { match (self.state.take(), kind) { (None, kind) => { let expr_ty = typeck.expr_ty(expr); - let (position, adjustments) = walk_parents(cx, &mut self.possible_borrowers, expr, &self.msrv); + let (position, adjustments) = walk_parents( + cx, + self.current_body.as_ref().unwrap(), + &mut self.possible_borrowers, + expr, + &self.msrv, + ); match kind { RefOp::Deref => { let sub_ty = typeck.expr_ty(sub_expr); @@ -548,7 +556,6 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { then { let mut app = Applicability::MachineApplicable; let snip = snippet_with_context(cx, name.span, pat.span.ctxt(), "..", &mut app).0; - self.current_body = self.current_body.or(cx.enclosing_body); self.ref_locals.insert( id, Some(RefPat { @@ -564,40 +571,66 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { } } - 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(); - } + fn check_body_post(&mut self, cx: &LateContext<'tcx>, _body: &'tcx Body<'_>) { + self.possible_borrowers = None; - if Some(body.id()) == self.current_body { - for pat in self.ref_locals.drain(..).filter_map(|(_, x)| x) { - let replacements = pat.replacements; - let app = pat.app; - let lint = if pat.always_deref { - NEEDLESS_BORROW - } else { - REF_BINDING_TO_REFERENCE - }; - span_lint_hir_and_then( - cx, - lint, - pat.hir_id, - pat.spans, - "this pattern creates a reference to a reference", - |diag| { - diag.multipart_suggestion("try this", replacements, app); - }, - ); - } - self.current_body = None; + for pat in self.ref_locals.drain(..).filter_map(|(_, x)| x) { + let replacements = pat.replacements; + let app = pat.app; + let lint = if pat.always_deref { + NEEDLESS_BORROW + } else { + REF_BINDING_TO_REFERENCE + }; + span_lint_hir_and_then( + cx, + lint, + pat.hir_id, + pat.spans, + "this pattern creates a reference to a reference", + |diag| { + diag.multipart_suggestion("try this", replacements, app); + }, + ); } } extract_msrv_attr!(LateContext); } +#[allow(rustc::lint_pass_impl_without_macro)] +impl<'b, 'tcx> LintPass for &mut Dereferencing<'b, 'tcx> { + fn name(&self) -> &'static str { + panic!() + } +} + +macro_rules! impl_late_lint_pass { + ([], [$($(#[$attr:meta])* fn $f:ident($($param:ident: $arg:ty),*);)*]) => { + impl<'b, 'tcx> LateLintPass<'tcx> for &mut Dereferencing<'b, 'tcx> { + $(fn $f(&mut self, context: &LateContext<'tcx>, $($param: $arg),*) { + (*self).$f(context, $($param),*); + })* + } + }; +} + +late_lint_methods!(impl_late_lint_pass, []); + +impl<'b, 'tcx> BorrowckLintPass<'b, 'tcx> for Dereferencing<'b, 'tcx> { + fn check_body_with_facts( + &mut self, + cx: &BorrowckContext<'tcx>, + body_with_facts: &'b Rc>, + ) { + self.current_body = Some(body_with_facts); + + let this = cx.visit_hir_body(self); + + this.current_body = None; + } +} + fn try_parse_ref_op<'tcx>( tcx: TyCtxt<'tcx>, typeck: &'tcx TypeckResults<'_>, @@ -701,9 +734,10 @@ impl Position { /// is, and which adjustments will be applied to it. Note this will not consider auto-borrow /// locations as those follow different rules. #[expect(clippy::too_many_lines)] -fn walk_parents<'tcx>( +fn walk_parents<'b, 'tcx>( cx: &LateContext<'tcx>, - possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, + body_with_facts: &'b Rc>, + possible_borrowers: &mut Option>, e: &'tcx Expr<'_>, msrv: &Msrv, ) -> (Position, &'tcx [Adjustment<'tcx>]) { @@ -841,6 +875,7 @@ fn walk_parents<'tcx>( { needless_borrow_impl_arg_position( cx, + body_with_facts, possible_borrowers, parent, i, @@ -912,6 +947,7 @@ fn walk_parents<'tcx>( { needless_borrow_impl_arg_position( cx, + body_with_facts, possible_borrowers, parent, i + 1, @@ -1108,9 +1144,10 @@ fn call_is_qualified(expr: &Expr<'_>) -> bool { // 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, clippy::too_many_lines)] -fn needless_borrow_impl_arg_position<'tcx>( +fn needless_borrow_impl_arg_position<'b, 'tcx>( cx: &LateContext<'tcx>, - possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, + body_with_facts: &'b Rc>, + possible_borrowers: &mut Option>, parent: &Expr<'tcx>, arg_index: usize, param_ty: ParamTy, @@ -1188,7 +1225,7 @@ fn needless_borrow_impl_arg_position<'tcx>( if !is_copy(cx, referent_ty) && (referent_ty.has_significant_drop(cx.tcx, cx.param_env) - || !referent_used_exactly_once(cx, possible_borrowers, reference)) + || !referent_used_exactly_once(cx, body_with_facts, possible_borrowers, reference)) { return false; } @@ -1289,32 +1326,29 @@ fn is_mixed_projection_predicate<'tcx>( } } -fn referent_used_exactly_once<'tcx>( +fn referent_used_exactly_once<'b, 'tcx>( cx: &LateContext<'tcx>, - possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, + body_with_facts: &'b Rc>, + possible_borrowers: &mut Option>, reference: &Expr<'tcx>, ) -> bool { - let mir = enclosing_mir(cx.tcx, reference.hir_id); - if let Some(local) = expr_local(cx.tcx, reference) + let mir = &body_with_facts.body; + if let Some(local) = expr_local(mir, reference) && let [location] = *local_assignments(mir, local).as_slice() && let Some(statement) = mir.basic_blocks[location.block].statements.get(location.statement_index) && let StatementKind::Assign(box (_, Rvalue::Ref(_, _, place))) = statement.kind - && !place.has_deref() + && !place.projection.contains(&PlaceElem::Deref) // Ensure not in a loop (https://github.com/rust-lang/rust-clippy/issues/9710) && TriColorDepthFirstSearch::new(&mir.basic_blocks).run_from(location.block, &mut CycleDetector).is_none() { - 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))); + if possible_borrowers.is_none() { + *possible_borrowers = Some(PossibleBorrowerMap::new(cx.tcx, body_with_facts)); } - 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) + let possible_borrower = possible_borrowers.as_mut().unwrap(); + // If `place.local` were not included 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.at_most_borrowers(cx, &[local, place.local], place.local, location) && used_exactly_once(mir, place.local).unwrap_or(false) } else { false @@ -1625,7 +1659,7 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data } } -impl<'tcx> Dereferencing<'tcx> { +impl<'b, 'tcx> Dereferencing<'b, '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 { diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 243d1fd8606a..6670e0fd285e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -24,6 +24,7 @@ extern crate rustc_arena; extern crate rustc_ast; extern crate rustc_ast_pretty; +extern crate rustc_borrowck; extern crate rustc_data_structures; extern crate rustc_driver; extern crate rustc_errors; @@ -48,12 +49,17 @@ extern crate clippy_utils; #[macro_use] extern crate declare_clippy_lint; -use std::io; -use std::path::PathBuf; +use std::cell::RefCell; +use std::rc::Rc; +use std::sync::OnceLock; use clippy_utils::msrvs::Msrv; +use rustc_borrowck::consumers::BodyWithBorrowckFacts; use rustc_data_structures::fx::FxHashSet; -use rustc_lint::{Lint, LintId}; +use rustc_hir::intravisit::Visitor; +use rustc_hir::BodyId; +use rustc_lint::{LateContext, LateContextAndPass, LateLintPass, Lint, LintId, LintPass}; +use rustc_middle::ty::TyCtxt; use rustc_session::Session; #[cfg(feature = "internal")] @@ -340,9 +346,9 @@ mod zero_div_zero; mod zero_sized_map_values; // end lints modules, do not remove this comment, it’s used in `update_lints` -pub use crate::utils::conf::{lookup_conf_file, Conf}; +pub use crate::utils::conf::Conf; use crate::utils::{ - conf::{metadata::get_configuration_metadata, TryConf}, + conf::{metadata::get_configuration_metadata, lookup_conf_file, TryConf}, FindAll, }; @@ -363,7 +369,16 @@ pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, sess: &Se } #[doc(hidden)] -pub fn read_conf(sess: &Session, path: &io::Result<(Option, Vec)>) -> Conf { +pub fn read_conf(sess: &Session) -> &'static Conf { + static CONF: OnceLock = OnceLock::new(); + + CONF.get_or_init(|| read_conf_inner(sess)) +} + +fn read_conf_inner(sess: &Session) -> Conf { + let conf_path = lookup_conf_file(); + let path = &conf_path; + if let Ok((_, warnings)) = path { for warning in warnings { sess.warn(warning.clone()); @@ -781,7 +796,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: }); store.register_late_pass(|_| Box::new(non_copy_const::NonCopyConst)); store.register_late_pass(|_| Box::new(ptr_offset_with_cast::PtrOffsetWithCast)); - store.register_late_pass(|_| Box::new(redundant_clone::RedundantClone)); store.register_late_pass(|_| Box::new(slow_vector_initialization::SlowVectorInit)); store.register_late_pass(move |_| Box::new(unnecessary_wraps::UnnecessaryWraps::new(avoid_breaking_exported_api))); store.register_late_pass(|_| Box::new(assertions_on_constants::AssertionsOnConstants)); @@ -866,7 +880,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move |_| Box::new(wildcard_imports::WildcardImports::new(warn_on_all_wildcard_imports))); store.register_late_pass(|_| Box::::default()); store.register_late_pass(|_| Box::new(unnamed_address::UnnamedAddress)); - store.register_late_pass(move |_| Box::new(dereference::Dereferencing::new(msrv()))); store.register_late_pass(|_| Box::new(option_if_let_else::OptionIfLetElse)); store.register_late_pass(|_| Box::new(future_not_send::FutureNotSend)); let future_size_threshold = conf.future_size_threshold; @@ -1048,6 +1061,66 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: // add lints here, do not remove this comment, it's used in `new_lint` } +pub struct BorrowckContext<'tcx> { + cell: RefCell>>>, +} + +impl<'tcx> BorrowckContext<'tcx> { + pub fn new(tcx: TyCtxt<'tcx>, body_id: BodyId) -> Self { + let cx = LateContext::new(tcx, Some(body_id)); + + Self { + cell: RefCell::new(Some(Rc::new(cx))), + } + } + + /// Returns a reference to the internal `LateContext` as an `Rc`. The `Rc` must be dropped + /// before `visit_hir_body` is called (if ever). + fn as_late_context(&self) -> Rc> { + self.cell.borrow().as_ref().unwrap().clone() + } + + /// Visits the current HIR body with `pass`. Panics if references to the internal `LateContext` + /// returned by `as_late_context` have not yet been dropped. + fn visit_hir_body>(&self, pass: T) -> T { + let rc = self.cell.take().unwrap(); + + let context = Rc::into_inner(rc).unwrap(); + + let body = context.tcx.hir().body(context.enclosing_body.unwrap()); + + let mut context_and_pass = LateContextAndPass { context, pass }; + + context_and_pass.visit_body(body); + + let LateContextAndPass { context, pass } = context_and_pass; + + self.cell.replace(Some(Rc::new(context))); + + pass + } +} + +pub trait BorrowckLintPass<'b, 'tcx>: LintPass { + fn check_body_with_facts( + &mut self, + cx: &BorrowckContext<'tcx>, + body_with_facts: &'b Rc>, + ); +} + +pub fn borrowck_lint_passes<'b, 'tcx>(msrv: &Msrv) -> Vec + 'b>> +where + 'tcx: 'b, +{ + let msrv = msrv.clone(); + + vec![ + Box::new(redundant_clone::RedundantClone), + Box::new(dereference::Dereferencing::new(msrv.clone())), + ] +} + #[rustfmt::skip] fn register_removed_non_tool_lints(store: &mut rustc_lint::LintStore) { store.register_removed( diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index 685d738cbb7c..df394c32b6de 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -1,19 +1,20 @@ +use crate::{BorrowckContext, BorrowckLintPass}; 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, is_type_lang_item, walk_ptrs_ty_depth}; use clippy_utils::{fn_has_unsatisfiable_preds, match_def_path, paths}; use if_chain::if_chain; +use rustc_borrowck::consumers::BodyWithBorrowckFacts; use rustc_errors::Applicability; -use rustc_hir::intravisit::FnKind; -use rustc_hir::{def_id, Body, FnDecl, LangItem}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_hir::{def_id, LangItem}; +use rustc_lint::LateContext; use rustc_middle::mir; use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::def_id::LocalDefId; -use rustc_span::source_map::{BytePos, Span}; +use rustc_span::source_map::BytePos; use rustc_span::sym; +use std::rc::Rc; macro_rules! unwrap_or_continue { ($x:expr) => { @@ -63,25 +64,26 @@ declare_clippy_lint! { declare_lint_pass!(RedundantClone => [REDUNDANT_CLONE]); -impl<'tcx> LateLintPass<'tcx> for RedundantClone { +impl<'b, 'tcx> BorrowckLintPass<'b, 'tcx> for RedundantClone { #[expect(clippy::too_many_lines)] - fn check_fn( + fn check_body_with_facts( &mut self, - cx: &LateContext<'tcx>, - _: FnKind<'tcx>, - _: &'tcx FnDecl<'_>, - _: &'tcx Body<'_>, - _: Span, - def_id: LocalDefId, + cx: &BorrowckContext<'tcx>, + body_with_facts: &'b Rc>, ) { + let rc = cx.as_late_context(); + let cx: &LateContext<'tcx> = &rc; + + let def_id = body_with_facts.body.source.def_id(); + // Building MIR for `fn`s with unsatisfiable preds results in ICE. - if fn_has_unsatisfiable_preds(cx, def_id.to_def_id()) { + if fn_has_unsatisfiable_preds(cx, def_id) { return; } - let mir = cx.tcx.optimized_mir(def_id.to_def_id()); + let mir = &body_with_facts.body; - let mut possible_borrower = PossibleBorrowerMap::new(cx, mir); + let mut possible_borrower = PossibleBorrowerMap::new(cx.tcx, body_with_facts); for (bb, bbdata) in mir.basic_blocks.iter_enumerated() { let terminator = bbdata.terminator(); @@ -130,7 +132,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone { // `res = clone(arg)` can be turned into `res = move arg;` // if `arg` is the only borrow of `cloned` at this point. - if cannot_move_out || !possible_borrower.only_borrowers(&[arg], cloned, loc) { + if cannot_move_out || !possible_borrower.at_most_borrowers(cx, &[arg], cloned, loc) { continue; } @@ -177,7 +179,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone { // StorageDead(pred_arg); // res = to_path_buf(cloned); // ``` - if cannot_move_out || !possible_borrower.only_borrowers(&[arg, cloned], local, loc) { + if cannot_move_out || !possible_borrower.at_most_borrowers(cx, &[arg, cloned], local, loc) { continue; } @@ -257,10 +259,10 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone { } /// If `kind` is `y = func(x: &T)` where `T: !Copy`, returns `(DefId of func, x, T, y)`. -fn is_call_with_ref_arg<'tcx>( +fn is_call_with_ref_arg<'b, 'tcx>( cx: &LateContext<'tcx>, - mir: &'tcx mir::Body<'tcx>, - kind: &'tcx mir::TerminatorKind<'tcx>, + mir: &'b mir::Body<'tcx>, + kind: &'b mir::TerminatorKind<'tcx>, ) -> Option<(def_id::DefId, mir::Local, Ty<'tcx>, mir::Local)> { if_chain! { if let mir::TerminatorKind::Call { func, args, destination, .. } = kind; @@ -268,7 +270,7 @@ fn is_call_with_ref_arg<'tcx>( if let mir::Operand::Move(mir::Place { local, .. }) = &args[0]; if let ty::FnDef(def_id, _) = *func.ty(mir, cx.tcx).kind(); if let (inner_ty, 1) = walk_ptrs_ty_depth(args[0].ty(mir, cx.tcx)); - if !is_copy(cx, inner_ty); + if !is_copy(cx, cx.tcx.erase_regions(inner_ty)); then { Some((def_id, *local, inner_ty, destination.as_local()?)) } else { diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index f6c7c1fa065a..4b8d1bdd055f 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -545,7 +545,7 @@ define_Conf! { /// # Errors /// /// Returns any unexpected filesystem error encountered when searching for the config file -pub fn lookup_conf_file() -> io::Result<(Option, Vec)> { +pub(crate) fn lookup_conf_file() -> io::Result<(Option, Vec)> { /// Possible filename to search for. const CONFIG_FILE_NAMES: [&str; 2] = [".clippy.toml", "clippy.toml"]; diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 311f6dbc696d..593e4e45432e 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -20,6 +20,7 @@ extern crate rustc_ast; extern crate rustc_ast_pretty; extern crate rustc_attr; +extern crate rustc_borrowck; extern crate rustc_const_eval; extern crate rustc_data_structures; // The `rustc_driver` crate seems to be required in order to use the `rust_ast` crate. diff --git a/clippy_utils/src/mir/mod.rs b/clippy_utils/src/mir/mod.rs index 26c0015e87e0..bf504e02148d 100644 --- a/clippy_utils/src/mir/mod.rs +++ b/clippy_utils/src/mir/mod.rs @@ -1,17 +1,12 @@ -use rustc_hir::{Expr, HirId}; +use rustc_hir::Expr; 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 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. @@ -99,17 +94,9 @@ pub fn used_exactly_once(mir: &rustc_middle::mir::Body<'_>, local: rustc_middle: }) } -/// 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); +pub fn expr_local(mir: &Body<'_>, expr: &Expr<'_>) -> Option { mir.local_decls.iter_enumerated().find_map(|(local, local_decl)| { if local_decl.source_info.span == expr.span { Some(local) diff --git a/clippy_utils/src/mir/possible_borrower.rs b/clippy_utils/src/mir/possible_borrower.rs index 920ce8e655be..fc40e8cdf9b4 100644 --- a/clippy_utils/src/mir/possible_borrower.rs +++ b/clippy_utils/src/mir/possible_borrower.rs @@ -1,235 +1,112 @@ -use super::{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_borrowck::consumers::BodyWithBorrowckFacts; +use rustc_index::bit_set::BitSet; use rustc_lint::LateContext; -use rustc_middle::mir::{self, visit::Visitor as _, Mutability}; -use rustc_middle::ty::{self, visit::TypeVisitor, TyCtxt}; +use rustc_middle::mir; +use rustc_middle::ty::{self, visit::TypeVisitor, RegionKind, RegionVid, Ty, TyCtxt}; use rustc_mir_dataflow::{impls::MaybeStorageLive, Analysis, ResultsCursor}; use std::borrow::Cow; use std::ops::ControlFlow; +use std::rc::Rc; -/// Collects the possible borrowers of each local. -/// For example, `b = &a; c = &a;` will make `b` and (transitively) `c` -/// possible borrowers of `a`. +/// Result of `PossibleBorrowerAnalysis`. #[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>, +pub struct PossibleBorrowerMap<'b, 'tcx> { + body_with_facts: Rc>, + maybe_live: ResultsCursor<'b, 'tcx, MaybeStorageLive<'b>>, } -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, - } - } +impl<'b, 'tcx> PossibleBorrowerMap<'b, 'tcx> { + pub fn new(tcx: TyCtxt<'tcx>, body_with_facts: &'b Rc>) -> Self { + let body = &body_with_facts.body; - fn into_map( - self, - cx: &'a LateContext<'tcx>, - maybe_live: ResultsCursor<'b, 'tcx, MaybeStorageLive<'tcx>>, - ) -> 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 maybe_live = MaybeStorageLive::new(Cow::Owned(BitSet::new_empty(body.local_decls.len()))) + .into_engine(tcx, body) + .pass_name("possible_borrower") + .iterate_to_fixpoint() + .into_results_cursor(body); - let bs = BitSet::new_empty(self.body.local_decls.len()); PossibleBorrowerMap { - map, + body_with_facts: body_with_facts.clone(), 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<'tcx>>, - // 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::new(Cow::Owned(BitSet::new_empty(mir.local_decls.len()))) - .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( + /// Returns true if the set of borrowers of `borrowed` living at `at` includes no more than + /// `borrowers`. + /// Notes: + /// 1. It would be nice if `PossibleBorrowerMap` could store `cx` so that `at_most_borrowers` + /// would not require it to be passed in. But a `PossibleBorrowerMap` is stored in `LintPass` + /// `Dereferencing`, which outlives any `LateContext`. + /// 2. In all current uses of `at_most_borrowers`, `borrowers` is a slice of at most two + /// elements. Thus, `borrowers.contains(...)` is effectively a constant-time operation. If + /// `at_most_borrowers`'s uses were to expand beyond this, its implementation might have to be + /// adjusted. + pub fn at_most_borrowers( &mut self, - below: &[mir::Local], - above: &[mir::Local], + cx: &LateContext<'tcx>, + 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; + let mir = &self.body_with_facts.body; + if is_copy(cx, cx.tcx.erase_regions(mir.local_decls[borrowed].ty)) { + return true; } - self.bitset.1.clear(); - for b in below { - self.bitset.1.insert(*b); - } + self.maybe_live.seek_before_primary_effect(at); + + let maybe_live = self.maybe_live.get(); + + // For each borrow of `borrowed`, ask the following question: + // + // - Is there any local, live at location `at`, and with an associated region that is outlived by + // the borrow region, but that is not also outlived by any of the regions of `borrowers`? + // + // If the answer to any of these question is "yes," then there are potential additional borrowers of + // `borrowed`. + // + // Note that the `any` closure has no side effects. So the result is the same regardless of the + // order in which `index`es are visited. + #[allow(rustc::potential_query_instability)] + !self.body_with_facts.borrow_set.local_map[&borrowed] + .iter() + .any(|index| { + let root_vid = self.body_with_facts.borrow_set.location_map[index.as_usize()].region; + + maybe_live.iter().any(|local| { + let local_regions = collect_regions(self.body_with_facts.body.local_decls[local].ty); + + local_regions.iter().any(|&local_vid| { + if !self + .body_with_facts + .region_inference_context + .eval_outlives(root_vid, local_vid) + { + return false; + } - if !self.bitset.0.superset(&self.bitset.1) { - return false; - } + !borrowers + .iter() + .filter_map(|&borrower| self.borrower_vid(borrower)) + .any(|borrower_vid| { + self.body_with_facts + .region_inference_context + .eval_outlives(borrower_vid, local_vid) + }) + }) + }) + }) + } - for b in above { - self.bitset.0.remove(*b); + fn borrower_vid(&self, borrower: mir::Local) -> Option { + if let ty::Ref(region, _, _) = self.body_with_facts.body.local_decls[borrower].ty.kind() + && let ty::RegionKind::ReVar(borrower_vid) = region.kind() + { + Some(borrower_vid) + } else { + None } - - self.bitset.0.is_empty() } pub fn local_is_alive_at(&mut self, local: mir::Local, at: mir::Location) -> bool { @@ -237,3 +114,20 @@ impl<'a, 'b, 'tcx> PossibleBorrowerMap<'b, 'tcx> { self.maybe_live.contains(local) } } + +fn collect_regions(ty: Ty<'_>) -> Vec { + let mut rc = RegionCollector(Vec::new()); + rc.visit_ty(ty); + rc.0 +} + +struct RegionCollector(Vec); + +impl<'tcx> TypeVisitor> for RegionCollector { + fn visit_region(&mut self, region: ty::Region<'tcx>) -> ControlFlow { + if let RegionKind::ReVar(vid) = region.kind() { + self.0.push(vid); + } + ControlFlow::Continue(()) + } +} diff --git a/clippy_utils/src/mir/possible_origin.rs b/clippy_utils/src/mir/possible_origin.rs deleted file mode 100644 index 8e7513d740ab..000000000000 --- a/clippy_utils/src/mir/possible_origin.rs +++ /dev/null @@ -1,59 +0,0 @@ -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 deleted file mode 100644 index 7fe2960739fa..000000000000 --- a/clippy_utils/src/mir/transitive_relation.rs +++ /dev/null @@ -1,29 +0,0 @@ -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/src/driver.rs b/src/driver.rs index 3c5b6e12b968..b5a588e11635 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -10,12 +10,17 @@ // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) +extern crate rustc_borrowck; extern crate rustc_driver; extern crate rustc_interface; +extern crate rustc_lint; extern crate rustc_session; extern crate rustc_span; +use clippy_lints::{BorrowckContext, BorrowckLintPass}; +use rustc_borrowck::consumers::{do_mir_borrowck, BodyWithBorrowckFacts, ConsumerOptions}; use rustc_interface::interface; +use rustc_lint::LintPass; use rustc_session::parse::ParseSess; use rustc_span::symbol::Symbol; @@ -23,6 +28,7 @@ use std::env; use std::ops::Deref; use std::path::Path; use std::process::exit; +use std::rc::Rc; /// If a command-line option matches `find_arg`, then apply the predicate `pred` on its value. If /// true, then return it. The parameter is assumed to be either `--arg=value` or `--arg value`. @@ -118,17 +124,44 @@ struct ClippyCallbacks { clippy_args_var: Option, } +type BorrowckLintPassObject<'b, 'tcx> = Box + 'b>; + +/// Like [`rustc_lint::late::RuntimeCombinedLateLintPass`], but with [`BorrowckPass`]es instead of +/// [`rustc_lint::LateLintPass`]es, and the passes are owned instead of borrowed. +pub struct RuntimeCombinedBorrowckLintPass<'b, 'tcx> { + passes: Vec>, +} + +#[allow(rustc::lint_pass_impl_without_macro)] +impl LintPass for RuntimeCombinedBorrowckLintPass<'_, '_> { + fn name(&self) -> &'static str { + panic!() + } +} + +impl<'b, 'tcx> BorrowckLintPass<'b, 'tcx> for RuntimeCombinedBorrowckLintPass<'b, 'tcx> { + fn check_body_with_facts( + &mut self, + cx: &BorrowckContext<'tcx>, + body_with_facts: &'b Rc>, + ) { + for pass in &mut self.passes { + pass.check_body_with_facts(cx, body_with_facts); + } + } +} + impl rustc_driver::Callbacks for ClippyCallbacks { // JUSTIFICATION: necessary in clippy driver to set `mir_opt_level` #[allow(rustc::bad_opt_access)] fn config(&mut self, config: &mut interface::Config) { - let conf_path = clippy_lints::lookup_conf_file(); let previous = config.register_lints.take(); let clippy_args_var = self.clippy_args_var.take(); config.parse_sess_created = Some(Box::new(move |parse_sess| { track_clippy_args(parse_sess, &clippy_args_var); track_files(parse_sess); })); + config.register_lints = Some(Box::new(move |sess, lint_store| { // technically we're ~guaranteed that this is none but might as well call anything that // is there already. Certainly it can't hurt. @@ -136,12 +169,40 @@ impl rustc_driver::Callbacks for ClippyCallbacks { (previous)(sess, lint_store); } - let conf = clippy_lints::read_conf(sess, &conf_path); - clippy_lints::register_plugins(lint_store, sess, &conf); - clippy_lints::register_pre_expansion_lints(lint_store, sess, &conf); + let conf = clippy_lints::read_conf(sess); + clippy_lints::register_plugins(lint_store, sess, conf); + clippy_lints::register_pre_expansion_lints(lint_store, sess, conf); clippy_lints::register_renamed(lint_store); })); + config.override_queries = Some(|_sess, providers, _external_providers| { + providers.mir_borrowck = |tcx, local_def_id| { + let (result, body_with_facts) = + do_mir_borrowck(tcx, local_def_id, ConsumerOptions::RegionInferenceContext); + + let body_with_facts = Rc::new(*body_with_facts.unwrap()); + + let def_id = body_with_facts.body.source.def_id(); + let body_id = tcx.hir().body_owned_by(def_id.as_local().unwrap()); + + let cx = BorrowckContext::new(tcx, body_id); + + // FIXME: How to compute this just once? + let mut pass = { + let conf = clippy_lints::read_conf(tcx.sess); + let msrv = clippy_utils::msrvs::Msrv::read(&conf.msrv, tcx.sess); + + let passes = clippy_lints::borrowck_lint_passes(msrv); + + RuntimeCombinedBorrowckLintPass { passes } + }; + + pass.check_body_with_facts(&cx, &body_with_facts); + + tcx.arena.alloc(result) + }; + }); + // FIXME: #4825; This is required, because Clippy lints that are based on MIR have to be // run on the unoptimized MIR. On the other hand this results in some false negatives. If // MIR passes can be enabled / disabled separately, we should figure out, what passes to diff --git a/tests/ui/crashes/possible_borrower.rs b/tests/ui/crashes/possible_borrower.rs new file mode 100644 index 000000000000..d7d54a4bd14a --- /dev/null +++ b/tests/ui/crashes/possible_borrower.rs @@ -0,0 +1,24 @@ +// https://github.com/rust-lang/rust-clippy/issues/10134 + +fn meow(_s: impl AsRef) {} + +macro_rules! quad { + ($x:stmt) => { + $x + $x + $x + $x + }; +} + +fn main() { + let i = 0; + quad!(quad!(quad!(quad!(quad!(meow(format!("abc{i}"))))))); +} + +// https://github.com/rust-lang/rust-clippy/issues/10134#issuecomment-1374480660 +fn second_testcase() { + quad!(quad!(quad!(for i in 0..4 { + quad!(quad!(meow(format!("abc{i}")))); + }))); +} diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index 80cdb4e472d4..4112dccff816 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -492,3 +492,14 @@ mod issue_9782_method_variant { S.foo::<&[u8; 100]>(&a); } } + +extern crate rustc_lint; +extern crate rustc_span; + +#[allow(dead_code)] +mod span_lint { + use rustc_lint::{LateContext, Lint, LintContext}; + fn foo(cx: &LateContext<'_>, lint: &'static Lint) { + cx.struct_span_lint(lint, rustc_span::Span::default(), "", |diag| diag.note(String::new())); + } +} diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index 99f735127eb8..13b2fd473579 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -492,3 +492,14 @@ mod issue_9782_method_variant { S.foo::<&[u8; 100]>(&a); } } + +extern crate rustc_lint; +extern crate rustc_span; + +#[allow(dead_code)] +mod span_lint { + use rustc_lint::{LateContext, Lint, LintContext}; + fn foo(cx: &LateContext<'_>, lint: &'static Lint) { + cx.struct_span_lint(lint, rustc_span::Span::default(), "", |diag| diag.note(&String::new())); + } +} diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index f85b4fb46a65..b6ad4ce9b1a5 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -216,5 +216,11 @@ error: the borrowed expression implements the required traits LL | foo(&a); | ^^ help: change this to: `a` -error: aborting due to 36 previous errors +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:502:85 + | +LL | cx.struct_span_lint(lint, rustc_span::Span::default(), "", |diag| diag.note(&String::new())); + | ^^^^^^^^^^^^^^ help: change this to: `String::new()` + +error: aborting due to 37 previous errors diff --git a/tests/ui/redundant_clone.fixed b/tests/ui/redundant_clone.fixed index 5037c08ebd5f..1a3c5a577bb7 100644 --- a/tests/ui/redundant_clone.fixed +++ b/tests/ui/redundant_clone.fixed @@ -237,11 +237,24 @@ fn hashmap_neg() { } fn false_negative_5707() { - fn foo(_x: &Alpha, _y: &mut Alpha) {} + use std::marker::PhantomData; - let x = Alpha; - let mut y = Alpha; + #[derive(Clone, Debug)] + struct Alpha<'a> { + _phantom: PhantomData<&'a ()>, + } + + fn foo<'a>(_x: &'a Alpha<'_>, _y: &mut Alpha<'a>) {} + + let x = Alpha { _phantom: PhantomData }; + let mut y = Alpha { _phantom: PhantomData }; foo(&x, &mut y); let _z = x.clone(); // pr 7346 can't lint on `x` drop(y); } + +#[allow(unused, clippy::manual_retain)] +fn possible_borrower_improvements() { + let mut s = String::from("foobar"); + s = s.chars().filter(|&c| c != 'o').collect(); +} diff --git a/tests/ui/redundant_clone.rs b/tests/ui/redundant_clone.rs index 501898bf113c..59c0fa9d18b8 100644 --- a/tests/ui/redundant_clone.rs +++ b/tests/ui/redundant_clone.rs @@ -237,11 +237,24 @@ fn hashmap_neg() { } fn false_negative_5707() { - fn foo(_x: &Alpha, _y: &mut Alpha) {} + use std::marker::PhantomData; - let x = Alpha; - let mut y = Alpha; + #[derive(Clone, Debug)] + struct Alpha<'a> { + _phantom: PhantomData<&'a ()>, + } + + fn foo<'a>(_x: &'a Alpha<'_>, _y: &mut Alpha<'a>) {} + + let x = Alpha { _phantom: PhantomData }; + let mut y = Alpha { _phantom: PhantomData }; foo(&x, &mut y); let _z = x.clone(); // pr 7346 can't lint on `x` drop(y); } + +#[allow(unused, clippy::manual_retain)] +fn possible_borrower_improvements() { + let mut s = String::from("foobar"); + s = s.chars().filter(|&c| c != 'o').to_owned().collect(); +} diff --git a/tests/ui/redundant_clone.stderr b/tests/ui/redundant_clone.stderr index 8660c0e1f6a0..fd4cb5ccc410 100644 --- a/tests/ui/redundant_clone.stderr +++ b/tests/ui/redundant_clone.stderr @@ -173,11 +173,23 @@ error: redundant clone LL | foo(&x.clone(), move || { | ^^^^^^^^ help: remove this | -note: this value is dropped without further use +note: cloned value is neither consumed nor mutated --> $DIR/redundant_clone.rs:210:10 | LL | foo(&x.clone(), move || { - | ^ + | ^^^^^^^^^ + +error: redundant clone + --> $DIR/redundant_clone.rs:259:40 + | +LL | s = s.chars().filter(|&c| c != 'o').to_owned().collect(); + | ^^^^^^^^^^^ help: remove this + | +note: this value is dropped without further use + --> $DIR/redundant_clone.rs:259:9 + | +LL | s = s.chars().filter(|&c| c != 'o').to_owned().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 15 previous errors +error: aborting due to 16 previous errors