diff options
Diffstat (limited to 'src/tools/clippy/clippy_utils/src')
24 files changed, 1398 insertions, 559 deletions
diff --git a/src/tools/clippy/clippy_utils/src/ast_utils.rs b/src/tools/clippy/clippy_utils/src/ast_utils.rs index e84adee9d..013399756 100644 --- a/src/tools/clippy/clippy_utils/src/ast_utils.rs +++ b/src/tools/clippy/clippy_utils/src/ast_utils.rs @@ -147,7 +147,9 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool { (Array(l), Array(r)) | (Tup(l), Tup(r)) => over(l, r, |l, r| eq_expr(l, r)), (Repeat(le, ls), Repeat(re, rs)) => eq_expr(le, re) && eq_expr(&ls.value, &rs.value), (Call(lc, la), Call(rc, ra)) => eq_expr(lc, rc) && over(la, ra, |l, r| eq_expr(l, r)), - (MethodCall(lc, la, _), MethodCall(rc, ra, _)) => eq_path_seg(lc, rc) && over(la, ra, |l, r| eq_expr(l, r)), + (MethodCall(lc, ls, la, _), MethodCall(rc, rs, ra, _)) => { + eq_path_seg(lc, rc) && eq_expr(ls, rs) && over(la, ra, |l, r| eq_expr(l, r)) + }, (Binary(lo, ll, lr), Binary(ro, rl, rr)) => lo.node == ro.node && eq_expr(ll, rl) && eq_expr(lr, rr), (Unary(lo, l), Unary(ro, r)) => mem::discriminant(lo) == mem::discriminant(ro) && eq_expr(l, r), (Lit(l), Lit(r)) => l.kind == r.kind, @@ -436,14 +438,14 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool { eq_defaultness(*ld, *rd) && eq_fn_sig(lf, rf) && eq_generics(lg, rg) && both(lb, rb, |l, r| eq_block(l, r)) }, ( - TyAlias(box ast::TyAlias { + Type(box ast::TyAlias { defaultness: ld, generics: lg, bounds: lb, ty: lt, .. }), - TyAlias(box ast::TyAlias { + Type(box ast::TyAlias { defaultness: rd, generics: rg, bounds: rb, diff --git a/src/tools/clippy/clippy_utils/src/attrs.rs b/src/tools/clippy/clippy_utils/src/attrs.rs index 8ab77c881..cd8575c90 100644 --- a/src/tools/clippy/clippy_utils/src/attrs.rs +++ b/src/tools/clippy/clippy_utils/src/attrs.rs @@ -131,12 +131,12 @@ pub fn get_unique_inner_attr(sess: &Session, attrs: &[ast::Attribute], name: &'s match attr.style { ast::AttrStyle::Inner if unique_attr.is_none() => unique_attr = Some(attr.clone()), ast::AttrStyle::Inner => { - sess.struct_span_err(attr.span, &format!("`{}` is defined multiple times", name)) + sess.struct_span_err(attr.span, &format!("`{name}` is defined multiple times")) .span_note(unique_attr.as_ref().unwrap().span, "first definition found here") .emit(); }, ast::AttrStyle::Outer => { - sess.span_err(attr.span, &format!("`{}` cannot be an outer attribute", name)); + sess.span_err(attr.span, format!("`{name}` cannot be an outer attribute")); }, } } diff --git a/src/tools/clippy/clippy_utils/src/check_proc_macro.rs b/src/tools/clippy/clippy_utils/src/check_proc_macro.rs index 7a8d4e806..c6bf98b7b 100644 --- a/src/tools/clippy/clippy_utils/src/check_proc_macro.rs +++ b/src/tools/clippy/clippy_utils/src/check_proc_macro.rs @@ -220,7 +220,7 @@ fn trait_item_search_pat(item: &TraitItem<'_>) -> (Pat, Pat) { fn impl_item_search_pat(item: &ImplItem<'_>) -> (Pat, Pat) { let (start_pat, end_pat) = match &item.kind { ImplItemKind::Const(..) => (Pat::Str("const"), Pat::Str(";")), - ImplItemKind::TyAlias(..) => (Pat::Str("type"), Pat::Str(";")), + ImplItemKind::Type(..) => (Pat::Str("type"), Pat::Str(";")), ImplItemKind::Fn(sig, ..) => (fn_header_search_pat(sig.header), Pat::Str("")), }; if item.vis_span.is_empty() { diff --git a/src/tools/clippy/clippy_utils/src/consts.rs b/src/tools/clippy/clippy_utils/src/consts.rs index e053708ed..07e4ef6a2 100644 --- a/src/tools/clippy/clippy_utils/src/consts.rs +++ b/src/tools/clippy/clippy_utils/src/consts.rs @@ -9,7 +9,7 @@ use rustc_hir::{BinOp, BinOpKind, Block, Expr, ExprKind, HirId, Item, ItemKind, use rustc_lint::LateContext; use rustc_middle::mir; use rustc_middle::mir::interpret::Scalar; -use rustc_middle::ty::subst::{Subst, SubstsRef}; +use rustc_middle::ty::SubstsRef; use rustc_middle::ty::{self, EarlyBinder, FloatTy, ScalarInt, Ty, TyCtxt}; use rustc_middle::{bug, span_bug}; use rustc_span::symbol::Symbol; @@ -136,17 +136,49 @@ impl Constant { (&Self::F64(l), &Self::F64(r)) => l.partial_cmp(&r), (&Self::F32(l), &Self::F32(r)) => l.partial_cmp(&r), (&Self::Bool(ref l), &Self::Bool(ref r)) => Some(l.cmp(r)), - (&Self::Tuple(ref l), &Self::Tuple(ref r)) | (&Self::Vec(ref l), &Self::Vec(ref r)) => iter::zip(l, r) - .map(|(li, ri)| Self::partial_cmp(tcx, cmp_type, li, ri)) - .find(|r| r.map_or(true, |o| o != Ordering::Equal)) - .unwrap_or_else(|| Some(l.len().cmp(&r.len()))), + (&Self::Tuple(ref l), &Self::Tuple(ref r)) if l.len() == r.len() => match *cmp_type.kind() { + ty::Tuple(tys) if tys.len() == l.len() => l + .iter() + .zip(r) + .zip(tys) + .map(|((li, ri), cmp_type)| Self::partial_cmp(tcx, cmp_type, li, ri)) + .find(|r| r.map_or(true, |o| o != Ordering::Equal)) + .unwrap_or_else(|| Some(l.len().cmp(&r.len()))), + _ => None, + }, + (&Self::Vec(ref l), &Self::Vec(ref r)) => { + let cmp_type = match *cmp_type.kind() { + ty::Array(ty, _) | ty::Slice(ty) => ty, + _ => return None, + }; + iter::zip(l, r) + .map(|(li, ri)| Self::partial_cmp(tcx, cmp_type, li, ri)) + .find(|r| r.map_or(true, |o| o != Ordering::Equal)) + .unwrap_or_else(|| Some(l.len().cmp(&r.len()))) + }, (&Self::Repeat(ref lv, ref ls), &Self::Repeat(ref rv, ref rs)) => { - match Self::partial_cmp(tcx, cmp_type, lv, rv) { + match Self::partial_cmp( + tcx, + match *cmp_type.kind() { + ty::Array(ty, _) => ty, + _ => return None, + }, + lv, + rv, + ) { Some(Equal) => Some(ls.cmp(rs)), x => x, } }, - (&Self::Ref(ref lb), &Self::Ref(ref rb)) => Self::partial_cmp(tcx, cmp_type, lb, rb), + (&Self::Ref(ref lb), &Self::Ref(ref rb)) => Self::partial_cmp( + tcx, + match *cmp_type.kind() { + ty::Ref(_, ty, _) => ty, + _ => return None, + }, + lb, + rb, + ), // TODO: are there any useful inter-type orderings? _ => None, } @@ -424,7 +456,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { .tcx .const_eval_resolve( self.param_env, - ty::Unevaluated::new(ty::WithOptConstParam::unknown(def_id), substs), + mir::UnevaluatedConst::new(ty::WithOptConstParam::unknown(def_id), substs), None, ) .ok() @@ -501,8 +533,8 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { BinOpKind::Mul => l.checked_mul(r).map(zext), BinOpKind::Div if r != 0 => l.checked_div(r).map(zext), BinOpKind::Rem if r != 0 => l.checked_rem(r).map(zext), - BinOpKind::Shr => l.checked_shr(r.try_into().expect("invalid shift")).map(zext), - BinOpKind::Shl => l.checked_shl(r.try_into().expect("invalid shift")).map(zext), + BinOpKind::Shr => l.checked_shr(r.try_into().ok()?).map(zext), + BinOpKind::Shl => l.checked_shl(r.try_into().ok()?).map(zext), BinOpKind::BitXor => Some(zext(l ^ r)), BinOpKind::BitOr => Some(zext(l | r)), BinOpKind::BitAnd => Some(zext(l & r)), @@ -521,8 +553,8 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { BinOpKind::Mul => l.checked_mul(r).map(Constant::Int), BinOpKind::Div => l.checked_div(r).map(Constant::Int), BinOpKind::Rem => l.checked_rem(r).map(Constant::Int), - BinOpKind::Shr => l.checked_shr(r.try_into().expect("shift too large")).map(Constant::Int), - BinOpKind::Shl => l.checked_shl(r.try_into().expect("shift too large")).map(Constant::Int), + BinOpKind::Shr => l.checked_shr(r.try_into().ok()?).map(Constant::Int), + BinOpKind::Shl => l.checked_shl(r.try_into().ok()?).map(Constant::Int), BinOpKind::BitXor => Some(Constant::Int(l ^ r)), BinOpKind::BitOr => Some(Constant::Int(l | r)), BinOpKind::BitAnd => Some(Constant::Int(l & r)), diff --git a/src/tools/clippy/clippy_utils/src/diagnostics.rs b/src/tools/clippy/clippy_utils/src/diagnostics.rs index ad95369b9..78f93755b 100644 --- a/src/tools/clippy/clippy_utils/src/diagnostics.rs +++ b/src/tools/clippy/clippy_utils/src/diagnostics.rs @@ -18,12 +18,11 @@ fn docs_link(diag: &mut Diagnostic, lint: &'static Lint) { if env::var("CLIPPY_DISABLE_DOCS_LINKS").is_err() { if let Some(lint) = lint.name_lower().strip_prefix("clippy::") { diag.help(&format!( - "for further information visit https://rust-lang.github.io/rust-clippy/{}/index.html#{}", + "for further information visit https://rust-lang.github.io/rust-clippy/{}/index.html#{lint}", &option_env!("RUST_RELEASE_NUM").map_or("master".to_string(), |n| { // extract just major + minor version and ignore patch versions format!("rust-{}", n.rsplit_once('.').unwrap().1) - }), - lint + }) )); } } @@ -47,10 +46,9 @@ fn docs_link(diag: &mut Diagnostic, lint: &'static Lint) { /// | ^^^^^^^^^^^^^^^^^^^^^^^ /// ``` pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: impl Into<MultiSpan>, msg: &str) { - cx.struct_span_lint(lint, sp, |diag| { - let mut diag = diag.build(msg); - docs_link(&mut diag, lint); - diag.emit(); + cx.struct_span_lint(lint, sp, msg, |diag| { + docs_link(diag, lint); + diag }); } @@ -82,15 +80,14 @@ pub fn span_lint_and_help<'a, T: LintContext>( help_span: Option<Span>, help: &str, ) { - cx.struct_span_lint(lint, span, |diag| { - let mut diag = diag.build(msg); + cx.struct_span_lint(lint, span, msg, |diag| { if let Some(help_span) = help_span { diag.span_help(help_span, help); } else { diag.help(help); } - docs_link(&mut diag, lint); - diag.emit(); + docs_link(diag, lint); + diag }); } @@ -125,15 +122,14 @@ pub fn span_lint_and_note<'a, T: LintContext>( note_span: Option<Span>, note: &str, ) { - cx.struct_span_lint(lint, span, |diag| { - let mut diag = diag.build(msg); + cx.struct_span_lint(lint, span, msg, |diag| { if let Some(note_span) = note_span { diag.span_note(note_span, note); } else { diag.note(note); } - docs_link(&mut diag, lint); - diag.emit(); + docs_link(diag, lint); + diag }); } @@ -147,19 +143,17 @@ where S: Into<MultiSpan>, F: FnOnce(&mut Diagnostic), { - cx.struct_span_lint(lint, sp, |diag| { - let mut diag = diag.build(msg); - f(&mut diag); - docs_link(&mut diag, lint); - diag.emit(); + cx.struct_span_lint(lint, sp, msg, |diag| { + f(diag); + docs_link(diag, lint); + diag }); } pub fn span_lint_hir(cx: &LateContext<'_>, lint: &'static Lint, hir_id: HirId, sp: Span, msg: &str) { - cx.tcx.struct_span_lint_hir(lint, hir_id, sp, |diag| { - let mut diag = diag.build(msg); - docs_link(&mut diag, lint); - diag.emit(); + cx.tcx.struct_span_lint_hir(lint, hir_id, sp, msg, |diag| { + docs_link(diag, lint); + diag }); } @@ -171,11 +165,10 @@ pub fn span_lint_hir_and_then( msg: &str, f: impl FnOnce(&mut Diagnostic), ) { - cx.tcx.struct_span_lint_hir(lint, hir_id, sp, |diag| { - let mut diag = diag.build(msg); - f(&mut diag); - docs_link(&mut diag, lint); - diag.emit(); + cx.tcx.struct_span_lint_hir(lint, hir_id, sp, msg, |diag| { + f(diag); + docs_link(diag, lint); + diag }); } diff --git a/src/tools/clippy/clippy_utils/src/eager_or_lazy.rs b/src/tools/clippy/clippy_utils/src/eager_or_lazy.rs index 91c9c382c..95b3e651e 100644 --- a/src/tools/clippy/clippy_utils/src/eager_or_lazy.rs +++ b/src/tools/clippy/clippy_utils/src/eager_or_lazy.rs @@ -113,7 +113,17 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS }, args, ) => match self.cx.qpath_res(path, hir_id) { - Res::Def(DefKind::Ctor(..) | DefKind::Variant, _) | Res::SelfCtor(_) => (), + Res::Def(DefKind::Ctor(..) | DefKind::Variant, _) | Res::SelfCtor(_) => { + if self + .cx + .typeck_results() + .expr_ty(e) + .has_significant_drop(self.cx.tcx, self.cx.param_env) + { + self.eagerness = ForceNoChange; + return; + } + }, Res::Def(_, id) if self.cx.tcx.is_promotable_const_fn(id) => (), // No need to walk the arguments here, `is_const_evaluatable` already did Res::Def(..) if is_const_evaluatable(self.cx, e) => { diff --git a/src/tools/clippy/clippy_utils/src/hir_utils.rs b/src/tools/clippy/clippy_utils/src/hir_utils.rs index 7212d9cd7..cf24ec8b6 100644 --- a/src/tools/clippy/clippy_utils/src/hir_utils.rs +++ b/src/tools/clippy/clippy_utils/src/hir_utils.rs @@ -962,7 +962,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { mut_ty.mutbl.hash(&mut self.s); }, TyKind::Rptr(lifetime, ref mut_ty) => { - self.hash_lifetime(*lifetime); + self.hash_lifetime(lifetime); self.hash_ty(mut_ty.ty); mut_ty.mutbl.hash(&mut self.s); }, @@ -992,7 +992,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { in_trait.hash(&mut self.s); }, TyKind::TraitObject(_, lifetime, _) => { - self.hash_lifetime(*lifetime); + self.hash_lifetime(lifetime); }, TyKind::Typeof(anon_const) => { self.hash_body(anon_const.body); diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 62da850a1..3ebfc5e00 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -3,7 +3,7 @@ #![feature(control_flow_enum)] #![feature(let_chains)] #![feature(lint_reasons)] -#![cfg_attr(bootstrap, feature(let_else))] +#![feature(never_type)] #![feature(once_cell)] #![feature(rustc_private)] #![recursion_limit = "512"] @@ -24,16 +24,18 @@ extern crate rustc_attr; extern crate rustc_data_structures; extern crate rustc_errors; extern crate rustc_hir; +extern crate rustc_hir_typeck; +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; extern crate rustc_target; extern crate rustc_trait_selection; -extern crate rustc_typeck; #[macro_use] pub mod sym_helper; @@ -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; @@ -66,6 +69,7 @@ pub use self::hir_utils::{ both, count_eq, eq_expr_value, hash_expr, hash_stmt, over, HirEqInterExpr, SpanlessEq, SpanlessHash, }; +use core::ops::ControlFlow; use std::collections::hash_map::Entry; use std::hash::BuildHasherDefault; use std::sync::OnceLock; @@ -77,8 +81,8 @@ use rustc_ast::Attribute; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::unhash::UnhashMap; use rustc_hir as hir; -use rustc_hir::def::{DefKind, Res}; -use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, CRATE_DEF_ID}; +use rustc_hir::def::{DefKind, Namespace, Res}; +use rustc_hir::def_id::{CrateNum, DefId, LocalDefId}; use rustc_hir::hir_id::{HirIdMap, HirIdSet}; use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk}; @@ -114,14 +118,14 @@ use rustc_target::abi::Integer; use crate::consts::{constant, Constant}; use crate::ty::{can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type, ty_is_fn_once_param}; -use crate::visitors::expr_visitor_no_bodies; +use crate::visitors::for_each_expr; pub fn parse_msrv(msrv: &str, sess: Option<&Session>, span: Option<Span>) -> Option<RustcVersion> { if let Ok(version) = RustcVersion::parse(msrv) { return Some(version); } else if let Some(sess) = sess { if let Some(span) = span { - sess.span_err(span, &format!("`{}` is not a valid Rust version", msrv)); + sess.span_err(span, format!("`{msrv}` is not a valid Rust version")); } } None @@ -212,7 +216,7 @@ pub fn find_binding_init<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option< /// } /// ``` pub fn in_constant(cx: &LateContext<'_>, id: HirId) -> bool { - let parent_id = cx.tcx.hir().get_parent_item(id); + let parent_id = cx.tcx.hir().get_parent_item(id).def_id; match cx.tcx.hir().get_by_def_id(parent_id) { Node::Item(&Item { kind: ItemKind::Const(..) | ItemKind::Static(..), @@ -239,19 +243,69 @@ pub fn in_constant(cx: &LateContext<'_>, id: HirId) -> bool { } } -/// Checks if a `QPath` resolves to a constructor of a `LangItem`. +/// Checks if a `Res` refers to a constructor of a `LangItem` /// For example, use this to check whether a function call or a pattern is `Some(..)`. -pub fn is_lang_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, lang_item: LangItem) -> bool { +pub fn is_res_lang_ctor(cx: &LateContext<'_>, res: Res, lang_item: LangItem) -> bool { + if let Res::Def(DefKind::Ctor(..), id) = res + && let Ok(lang_id) = cx.tcx.lang_items().require(lang_item) + && let Some(id) = cx.tcx.opt_parent(id) + { + id == lang_id + } else { + false + } +} + +pub fn is_res_diagnostic_ctor(cx: &LateContext<'_>, res: Res, diag_item: Symbol) -> bool { + if let Res::Def(DefKind::Ctor(..), id) = res + && let Some(id) = cx.tcx.opt_parent(id) + { + cx.tcx.is_diagnostic_item(diag_item, id) + } else { + false + } +} + +/// Checks if a `QPath` resolves to a constructor of a diagnostic item. +pub fn is_diagnostic_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, diagnostic_item: Symbol) -> bool { if let QPath::Resolved(_, path) = qpath { if let Res::Def(DefKind::Ctor(..), ctor_id) = path.res { - if let Ok(item_id) = cx.tcx.lang_items().require(lang_item) { - return cx.tcx.parent(ctor_id) == item_id; - } + return cx.tcx.is_diagnostic_item(diagnostic_item, cx.tcx.parent(ctor_id)); } } false } +/// Checks if the `DefId` matches the given diagnostic item or it's constructor. +pub fn is_diagnostic_item_or_ctor(cx: &LateContext<'_>, did: DefId, item: Symbol) -> bool { + let did = match cx.tcx.def_kind(did) { + DefKind::Ctor(..) => cx.tcx.parent(did), + // Constructors for types in external crates seem to have `DefKind::Variant` + DefKind::Variant => match cx.tcx.opt_parent(did) { + Some(did) if matches!(cx.tcx.def_kind(did), DefKind::Variant) => did, + _ => did, + }, + _ => did, + }; + + cx.tcx.is_diagnostic_item(item, did) +} + +/// Checks if the `DefId` matches the given `LangItem` or it's constructor. +pub fn is_lang_item_or_ctor(cx: &LateContext<'_>, did: DefId, item: LangItem) -> bool { + let did = match cx.tcx.def_kind(did) { + DefKind::Ctor(..) => cx.tcx.parent(did), + // Constructors for types in external crates seem to have `DefKind::Variant` + DefKind::Variant => match cx.tcx.opt_parent(did) { + Some(did) if matches!(cx.tcx.def_kind(did), DefKind::Variant) => did, + _ => did, + }, + _ => did, + }; + + cx.tcx.lang_items().require(item).map_or(false, |id| id == did) +} + pub fn is_unit_expr(expr: &Expr<'_>) -> bool { matches!( expr.kind, @@ -471,15 +525,49 @@ pub fn path_def_id<'tcx>(cx: &LateContext<'_>, maybe_path: &impl MaybePath<'tcx> path_res(cx, maybe_path).opt_def_id() } -/// Resolves a def path like `std::vec::Vec`. +fn find_primitive<'tcx>(tcx: TyCtxt<'tcx>, name: &str) -> impl Iterator<Item = DefId> + 'tcx { + let single = |ty| tcx.incoherent_impls(ty).iter().copied(); + let empty = || [].iter().copied(); + match name { + "bool" => single(BoolSimplifiedType), + "char" => single(CharSimplifiedType), + "str" => single(StrSimplifiedType), + "array" => single(ArraySimplifiedType), + "slice" => single(SliceSimplifiedType), + // FIXME: rustdoc documents these two using just `pointer`. + // + // Maybe this is something we should do here too. + "const_ptr" => single(PtrSimplifiedType(Mutability::Not)), + "mut_ptr" => single(PtrSimplifiedType(Mutability::Mut)), + "isize" => single(IntSimplifiedType(IntTy::Isize)), + "i8" => single(IntSimplifiedType(IntTy::I8)), + "i16" => single(IntSimplifiedType(IntTy::I16)), + "i32" => single(IntSimplifiedType(IntTy::I32)), + "i64" => single(IntSimplifiedType(IntTy::I64)), + "i128" => single(IntSimplifiedType(IntTy::I128)), + "usize" => single(UintSimplifiedType(UintTy::Usize)), + "u8" => single(UintSimplifiedType(UintTy::U8)), + "u16" => single(UintSimplifiedType(UintTy::U16)), + "u32" => single(UintSimplifiedType(UintTy::U32)), + "u64" => single(UintSimplifiedType(UintTy::U64)), + "u128" => single(UintSimplifiedType(UintTy::U128)), + "f32" => single(FloatSimplifiedType(FloatTy::F32)), + "f64" => single(FloatSimplifiedType(FloatTy::F64)), + _ => empty(), + } +} + +/// Resolves a def path like `std::vec::Vec`. `namespace_hint` can be supplied to disambiguate +/// between `std::vec` the module and `std::vec` the macro +/// /// This function is expensive and should be used sparingly. -pub fn def_path_res(cx: &LateContext<'_>, path: &[&str]) -> Res { - fn item_child_by_name(tcx: TyCtxt<'_>, def_id: DefId, name: &str) -> Option<Res> { +pub fn def_path_res(cx: &LateContext<'_>, path: &[&str], namespace_hint: Option<Namespace>) -> Res { + fn item_child_by_name(tcx: TyCtxt<'_>, def_id: DefId, name: &str, matches_ns: impl Fn(Res) -> bool) -> Option<Res> { match tcx.def_kind(def_id) { DefKind::Mod | DefKind::Enum | DefKind::Trait => tcx .module_children(def_id) .iter() - .find(|item| item.ident.name.as_str() == name) + .find(|item| item.ident.name.as_str() == name && matches_ns(item.res.expect_non_local())) .map(|child| child.res.expect_non_local()), DefKind::Impl => tcx .associated_item_def_ids(def_id) @@ -487,40 +575,17 @@ pub fn def_path_res(cx: &LateContext<'_>, path: &[&str]) -> Res { .copied() .find(|assoc_def_id| tcx.item_name(*assoc_def_id).as_str() == name) .map(|assoc_def_id| Res::Def(tcx.def_kind(assoc_def_id), assoc_def_id)), + DefKind::Struct | DefKind::Union => tcx + .adt_def(def_id) + .non_enum_variant() + .fields + .iter() + .find(|f| f.name.as_str() == name) + .map(|f| Res::Def(DefKind::Field, f.did)), _ => None, } } - fn find_primitive<'tcx>(tcx: TyCtxt<'tcx>, name: &str) -> impl Iterator<Item = DefId> + 'tcx { - let single = |ty| tcx.incoherent_impls(ty).iter().copied(); - let empty = || [].iter().copied(); - match name { - "bool" => single(BoolSimplifiedType), - "char" => single(CharSimplifiedType), - "str" => single(StrSimplifiedType), - "array" => single(ArraySimplifiedType), - "slice" => single(SliceSimplifiedType), - // FIXME: rustdoc documents these two using just `pointer`. - // - // Maybe this is something we should do here too. - "const_ptr" => single(PtrSimplifiedType(Mutability::Not)), - "mut_ptr" => single(PtrSimplifiedType(Mutability::Mut)), - "isize" => single(IntSimplifiedType(IntTy::Isize)), - "i8" => single(IntSimplifiedType(IntTy::I8)), - "i16" => single(IntSimplifiedType(IntTy::I16)), - "i32" => single(IntSimplifiedType(IntTy::I32)), - "i64" => single(IntSimplifiedType(IntTy::I64)), - "i128" => single(IntSimplifiedType(IntTy::I128)), - "usize" => single(UintSimplifiedType(UintTy::Usize)), - "u8" => single(UintSimplifiedType(UintTy::U8)), - "u16" => single(UintSimplifiedType(UintTy::U16)), - "u32" => single(UintSimplifiedType(UintTy::U32)), - "u64" => single(UintSimplifiedType(UintTy::U64)), - "u128" => single(UintSimplifiedType(UintTy::U128)), - "f32" => single(FloatSimplifiedType(FloatTy::F32)), - "f64" => single(FloatSimplifiedType(FloatTy::F64)), - _ => empty(), - } - } + fn find_crate(tcx: TyCtxt<'_>, name: &str) -> Option<DefId> { tcx.crates(()) .iter() @@ -529,32 +594,45 @@ pub fn def_path_res(cx: &LateContext<'_>, path: &[&str]) -> Res { .map(CrateNum::as_def_id) } - let (base, first, path) = match *path { - [base, first, ref path @ ..] => (base, first, path), + let (base, path) = match *path { [primitive] => { return PrimTy::from_name(Symbol::intern(primitive)).map_or(Res::Err, Res::PrimTy); }, + [base, ref path @ ..] => (base, path), _ => return Res::Err, }; let tcx = cx.tcx; let starts = find_primitive(tcx, base) .chain(find_crate(tcx, base)) - .filter_map(|id| item_child_by_name(tcx, id, first)); + .map(|id| Res::Def(tcx.def_kind(id), id)); for first in starts { let last = path .iter() .copied() + .enumerate() // for each segment, find the child item - .try_fold(first, |res, segment| { + .try_fold(first, |res, (idx, segment)| { + let matches_ns = |res: Res| { + // If at the last segment in the path, respect the namespace hint + if idx == path.len() - 1 { + match namespace_hint { + Some(ns) => res.matches_ns(ns), + None => true, + } + } else { + res.matches_ns(Namespace::TypeNS) + } + }; + let def_id = res.def_id(); - if let Some(item) = item_child_by_name(tcx, def_id, segment) { + if let Some(item) = item_child_by_name(tcx, def_id, segment, matches_ns) { Some(item) } else if matches!(res, Res::Def(DefKind::Enum | DefKind::Struct, _)) { // it is not a child item so check inherent impl items tcx.inherent_impls(def_id) .iter() - .find_map(|&impl_def_id| item_child_by_name(tcx, impl_def_id, segment)) + .find_map(|&impl_def_id| item_child_by_name(tcx, impl_def_id, segment, matches_ns)) } else { None } @@ -570,8 +648,10 @@ pub fn def_path_res(cx: &LateContext<'_>, path: &[&str]) -> Res { /// Convenience function to get the `DefId` of a trait by path. /// It could be a trait or trait alias. +/// +/// This function is expensive and should be used sparingly. pub fn get_trait_def_id(cx: &LateContext<'_>, path: &[&str]) -> Option<DefId> { - match def_path_res(cx, path) { + match def_path_res(cx, path, Some(Namespace::TypeNS)) { Res::Def(DefKind::Trait | DefKind::TraitAlias, trait_id) => Some(trait_id), _ => None, } @@ -597,8 +677,8 @@ pub fn trait_ref_of_method<'tcx>(cx: &LateContext<'tcx>, def_id: LocalDefId) -> let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id); let parent_impl = cx.tcx.hir().get_parent_item(hir_id); if_chain! { - if parent_impl != CRATE_DEF_ID; - if let hir::Node::Item(item) = cx.tcx.hir().get_by_def_id(parent_impl); + if parent_impl != hir::CRATE_OWNER_ID; + if let hir::Node::Item(item) = cx.tcx.hir().get_by_def_id(parent_impl.def_id); if let hir::ItemKind::Impl(impl_) = &item.kind; then { return impl_.of_trait.as_ref(); @@ -738,13 +818,37 @@ pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { false } }, - ExprKind::Call(repl_func, _) => is_default_equivalent_call(cx, repl_func), - ExprKind::Path(qpath) => is_lang_ctor(cx, qpath, OptionNone), + ExprKind::Call(repl_func, []) => is_default_equivalent_call(cx, repl_func), + ExprKind::Call(from_func, [ref arg]) => is_default_equivalent_from(cx, from_func, arg), + ExprKind::Path(qpath) => is_res_lang_ctor(cx, cx.qpath_res(qpath, e.hir_id), OptionNone), ExprKind::AddrOf(rustc_hir::BorrowKind::Ref, _, expr) => matches!(expr.kind, ExprKind::Array([])), _ => false, } } +fn is_default_equivalent_from(cx: &LateContext<'_>, from_func: &Expr<'_>, arg: &Expr<'_>) -> bool { + if let ExprKind::Path(QPath::TypeRelative(ty, seg)) = from_func.kind && + seg.ident.name == sym::from + { + match arg.kind { + ExprKind::Lit(hir::Lit { + node: LitKind::Str(ref sym, _), + .. + }) => return sym.is_empty() && is_path_diagnostic_item(cx, ty, sym::String), + ExprKind::Array([]) => return is_path_diagnostic_item(cx, ty, sym::Vec), + ExprKind::Repeat(_, ArrayLen::Body(len)) => { + if let ExprKind::Lit(ref const_lit) = cx.tcx.hir().body(len.body).value.kind && + let LitKind::Int(v, _) = const_lit.node + { + return v == 0 && is_path_diagnostic_item(cx, ty, sym::Vec); + } + } + _ => (), + } + } + false +} + /// Checks if the top level expression can be moved into a closure as is. /// Currently checks for: /// * Break/Continue outside the given loop HIR ids. @@ -1104,7 +1208,7 @@ pub fn is_in_panic_handler(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { /// Gets the name of the item the expression is in, if available. pub fn get_item_name(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<Symbol> { - let parent_id = cx.tcx.hir().get_parent_item(expr.hir_id); + let parent_id = cx.tcx.hir().get_parent_item(expr.hir_id).def_id; match cx.tcx.hir().find_by_def_id(parent_id) { Some( Node::Item(Item { ident, .. }) @@ -1137,17 +1241,14 @@ pub fn contains_name(name: Symbol, expr: &Expr<'_>) -> bool { /// Returns `true` if `expr` contains a return expression pub fn contains_return(expr: &hir::Expr<'_>) -> bool { - let mut found = false; - expr_visitor_no_bodies(|expr| { - if !found { - if let hir::ExprKind::Ret(..) = &expr.kind { - found = true; - } + for_each_expr(expr, |e| { + if matches!(e.kind, hir::ExprKind::Ret(..)) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) } - !found }) - .visit_expr(expr); - found + .is_some() } /// Extends the span to the beginning of the spans line, incl. whitespaces. @@ -1387,8 +1488,8 @@ pub fn is_integer_literal(expr: &Expr<'_>, value: u128) -> bool { /// Examples of coercions can be found in the Nomicon at /// <https://doc.rust-lang.org/nomicon/coercions.html>. /// -/// See `rustc_middle::ty::adjustment::Adjustment` and `rustc_typeck::check::coercion` for more -/// information on adjustments and coercions. +/// See `rustc_middle::ty::adjustment::Adjustment` and `rustc_hir_analysis::check::coercion` for +/// more information on adjustments and coercions. pub fn is_adjusted(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { cx.typeck_results().adjustments().get(e.hir_id).is_some() } @@ -1536,7 +1637,7 @@ pub fn is_self(slf: &Param<'_>) -> bool { pub fn is_self_ty(slf: &hir::Ty<'_>) -> bool { if let TyKind::Path(QPath::Resolved(None, path)) = slf.kind { - if let Res::SelfTy { .. } = path.res { + if let Res::SelfTyParam { .. } | Res::SelfTyAlias { .. } = path.res { return true; } } @@ -1554,7 +1655,7 @@ pub fn is_try<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<&'tc if_chain! { if let PatKind::TupleStruct(ref path, pat, ddpos) = arm.pat.kind; if ddpos.as_opt_usize().is_none(); - if is_lang_ctor(cx, path, ResultOk); + if is_res_lang_ctor(cx, cx.qpath_res(path, arm.pat.hir_id), ResultOk); if let PatKind::Binding(_, hir_id, _, None) = pat[0].kind; if path_to_local_id(arm.body, hir_id); then { @@ -1566,7 +1667,7 @@ pub fn is_try<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<&'tc fn is_err(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { if let PatKind::TupleStruct(ref path, _, _) = arm.pat.kind { - is_lang_ctor(cx, path, ResultErr) + is_res_lang_ctor(cx, cx.qpath_res(path, arm.pat.hir_id), ResultErr) } else { false } @@ -1648,7 +1749,7 @@ pub fn any_parent_has_attr(tcx: TyCtxt<'_>, node: HirId, symbol: Symbol) -> bool return true; } prev_enclosing_node = Some(enclosing_node); - enclosing_node = map.local_def_id_to_hir_id(map.get_parent_item(enclosing_node)); + enclosing_node = map.get_parent_item(enclosing_node).into(); } false @@ -1665,6 +1766,7 @@ pub fn any_parent_is_automatically_derived(tcx: TyCtxt<'_>, node: HirId) -> bool /// ```rust,ignore /// if let Some(args) = match_function_call(cx, cmp_max_call, &paths::CMP_MAX); /// ``` +/// This function is deprecated. Use [`match_function_call_with_def_id`]. pub fn match_function_call<'tcx>( cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, @@ -1682,6 +1784,22 @@ pub fn match_function_call<'tcx>( None } +pub fn match_function_call_with_def_id<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + fun_def_id: DefId, +) -> Option<&'tcx [Expr<'tcx>]> { + if_chain! { + if let ExprKind::Call(fun, args) = expr.kind; + if let ExprKind::Path(ref qpath) = fun.kind; + if cx.qpath_res(qpath, fun.hir_id).opt_def_id() == Some(fun_def_id); + then { + return Some(args); + } + }; + None +} + /// Checks if the given `DefId` matches any of the paths. Returns the index of matching path, if /// any. /// @@ -1990,9 +2108,9 @@ pub fn fn_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<DefId> { } } -/// Returns Option<String> where String is a textual representation of the type encapsulated in the -/// slice iff the given expression is a slice of primitives (as defined in the -/// `is_recursively_primitive_type` function) and None otherwise. +/// Returns `Option<String>` where String is a textual representation of the type encapsulated in +/// the slice iff the given expression is a slice of primitives (as defined in the +/// `is_recursively_primitive_type` function) and `None` otherwise. pub fn is_slice_of_primitives(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> { let expr_type = cx.typeck_results().expr_ty_adjusted(expr); let expr_kind = expr_type.kind(); @@ -2163,7 +2281,7 @@ fn with_test_item_names(tcx: TyCtxt<'_>, module: LocalDefId, f: impl Fn(&[Symbol Entry::Vacant(entry) => { let mut names = Vec::new(); for id in tcx.hir().module_items(module) { - if matches!(tcx.def_kind(id.def_id), DefKind::Const) + if matches!(tcx.def_kind(id.owner_id), DefKind::Const) && let item = tcx.hir().item(id) && let ItemKind::Const(ty, _body) = item.kind { if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind { @@ -2296,6 +2414,29 @@ pub fn span_contains_comment(sm: &SourceMap, span: Span) -> bool { }); } +/// Return all the comments a given span contains +/// Comments are returned wrapped with their relevant delimiters +pub fn span_extract_comment(sm: &SourceMap, span: Span) -> String { + let snippet = sm.span_to_snippet(span).unwrap_or_default(); + let mut comments_buf: Vec<String> = Vec::new(); + let mut index: usize = 0; + + for token in tokenize(&snippet) { + let token_range = index..(index + token.len as usize); + index += token.len as usize; + match token.kind { + TokenKind::BlockComment { .. } | TokenKind::LineComment { .. } => { + if let Some(comment) = snippet.get(token_range) { + comments_buf.push(comment.to_string()); + } + }, + _ => (), + } + } + + comments_buf.join("\n") +} + macro_rules! op_utils { ($($name:ident $assign:ident)*) => { /// Binary operation traits like `LangItem::Add` diff --git a/src/tools/clippy/clippy_utils/src/macros.rs b/src/tools/clippy/clippy_utils/src/macros.rs index bd89ff977..9a682fbe6 100644 --- a/src/tools/clippy/clippy_utils/src/macros.rs +++ b/src/tools/clippy/clippy_utils/src/macros.rs @@ -2,13 +2,13 @@ use crate::is_path_diagnostic_item; use crate::source::snippet_opt; -use crate::visitors::expr_visitor_no_bodies; +use crate::visitors::{for_each_expr, Descend}; use arrayvec::ArrayVec; use itertools::{izip, Either, Itertools}; use rustc_ast::ast::LitKind; -use rustc_hir::intravisit::Visitor; -use rustc_hir::{self as hir, Expr, ExprKind, HirId, Node, QPath}; +use rustc_hir::intravisit::{walk_expr, Visitor}; +use rustc_hir::{self as hir, Expr, ExprField, ExprKind, HirId, Node, QPath}; use rustc_lexer::unescape::unescape_literal; use rustc_lexer::{tokenize, unescape, LiteralKind, TokenKind}; use rustc_lint::LateContext; @@ -16,6 +16,7 @@ use rustc_parse_format::{self as rpf, Alignment}; use rustc_span::def_id::DefId; use rustc_span::hygiene::{self, MacroKind, SyntaxContext}; use rustc_span::{sym, BytePos, ExpnData, ExpnId, ExpnKind, Pos, Span, SpanData, Symbol}; +use std::iter::{once, zip}; use std::ops::ControlFlow; const FORMAT_MACRO_DIAG_ITEMS: &[Symbol] = &[ @@ -270,20 +271,19 @@ fn find_assert_args_inner<'a, const N: usize>( }; let mut args = ArrayVec::new(); let mut panic_expn = None; - expr_visitor_no_bodies(|e| { + let _: Option<!> = for_each_expr(expr, |e| { if args.is_full() { if panic_expn.is_none() && e.span.ctxt() != expr.span.ctxt() { panic_expn = PanicExpn::parse(cx, e); } - panic_expn.is_none() + ControlFlow::Continue(Descend::from(panic_expn.is_none())) } else if is_assert_arg(cx, e, expn) { args.push(e); - false + ControlFlow::Continue(Descend::No) } else { - true + ControlFlow::Continue(Descend::Yes) } - }) - .visit_expr(expr); + }); let args = args.into_inner().ok()?; // if no `panic!(..)` is found, use `PanicExpn::Empty` // to indicate that the default assertion message is used @@ -297,22 +297,19 @@ fn find_assert_within_debug_assert<'a>( expn: ExpnId, assert_name: Symbol, ) -> Option<(&'a Expr<'a>, ExpnId)> { - let mut found = None; - expr_visitor_no_bodies(|e| { - if found.is_some() || !e.span.from_expansion() { - return false; + for_each_expr(expr, |e| { + if !e.span.from_expansion() { + return ControlFlow::Continue(Descend::No); } let e_expn = e.span.ctxt().outer_expn(); if e_expn == expn { - return true; - } - if e_expn.expn_data().macro_def_id.map(|id| cx.tcx.item_name(id)) == Some(assert_name) { - found = Some((e, e_expn)); + ControlFlow::Continue(Descend::Yes) + } else if e_expn.expn_data().macro_def_id.map(|id| cx.tcx.item_name(id)) == Some(assert_name) { + ControlFlow::Break((e, e_expn)) + } else { + ControlFlow::Continue(Descend::No) } - false }) - .visit_expr(expr); - found } fn is_assert_arg(cx: &LateContext<'_>, expr: &Expr<'_>, assert_expn: ExpnId) -> bool { @@ -392,20 +389,18 @@ impl FormatString { unescape_literal(inner, mode, &mut |_, ch| match ch { Ok(ch) => unescaped.push(ch), Err(e) if !e.is_fatal() => (), - Err(e) => panic!("{:?}", e), + Err(e) => panic!("{e:?}"), }); let mut parts = Vec::new(); - expr_visitor_no_bodies(|expr| { - if let ExprKind::Lit(lit) = &expr.kind { - if let LitKind::Str(symbol, _) = lit.node { - parts.push(symbol); - } + let _: Option<!> = for_each_expr(pieces, |expr| { + if let ExprKind::Lit(lit) = &expr.kind + && let LitKind::Str(symbol, _) = lit.node + { + parts.push(symbol); } - - true - }) - .visit_expr(pieces); + ControlFlow::Continue(()) + }); Some(Self { span, @@ -418,7 +413,8 @@ impl FormatString { } struct FormatArgsValues<'tcx> { - /// See `FormatArgsExpn::value_args` + /// Values passed after the format string and implicit captures. `[1, z + 2, x]` for + /// `format!("{x} {} {}", 1, z + 2)`. value_args: Vec<&'tcx Expr<'tcx>>, /// Maps an `rt::v1::Argument::position` or an `rt::v1::Count::Param` to its index in /// `value_args` @@ -431,7 +427,7 @@ impl<'tcx> FormatArgsValues<'tcx> { fn new(args: &'tcx Expr<'tcx>, format_string_span: SpanData) -> Self { let mut pos_to_value_index = Vec::new(); let mut value_args = Vec::new(); - expr_visitor_no_bodies(|expr| { + let _: Option<!> = for_each_expr(args, |expr| { if expr.span.ctxt() == args.span.ctxt() { // ArgumentV1::new_<format_trait>(<val>) // ArgumentV1::from_usize(<val>) @@ -453,16 +449,13 @@ impl<'tcx> FormatArgsValues<'tcx> { pos_to_value_index.push(val_idx); } - - true + ControlFlow::Continue(Descend::Yes) } else { // assume that any expr with a differing span is a value value_args.push(expr); - - false + ControlFlow::Continue(Descend::No) } - }) - .visit_expr(args); + }); Self { value_args, @@ -485,64 +478,49 @@ struct ParamPosition { precision: Option<usize>, } -/// Parses the `fmt` arg of `Arguments::new_v1_formatted(pieces, args, fmt, _)` -fn parse_rt_fmt<'tcx>(fmt_arg: &'tcx Expr<'tcx>) -> Option<impl Iterator<Item = ParamPosition> + 'tcx> { - fn parse_count(expr: &Expr<'_>) -> Option<usize> { - // ::core::fmt::rt::v1::Count::Param(1usize), - if let ExprKind::Call(ctor, [val]) = expr.kind - && let ExprKind::Path(QPath::Resolved(_, path)) = ctor.kind - && path.segments.last()?.ident.name == sym::Param - && let ExprKind::Lit(lit) = &val.kind - && let LitKind::Int(pos, _) = lit.node - { - Some(pos as usize) - } else { - None +impl<'tcx> Visitor<'tcx> for ParamPosition { + fn visit_expr_field(&mut self, field: &'tcx ExprField<'tcx>) { + fn parse_count(expr: &Expr<'_>) -> Option<usize> { + // ::core::fmt::rt::v1::Count::Param(1usize), + if let ExprKind::Call(ctor, [val]) = expr.kind + && let ExprKind::Path(QPath::Resolved(_, path)) = ctor.kind + && path.segments.last()?.ident.name == sym::Param + && let ExprKind::Lit(lit) = &val.kind + && let LitKind::Int(pos, _) = lit.node + { + Some(pos as usize) + } else { + None + } + } + + match field.ident.name { + sym::position => { + if let ExprKind::Lit(lit) = &field.expr.kind + && let LitKind::Int(pos, _) = lit.node + { + self.value = pos as usize; + } + }, + sym::precision => { + self.precision = parse_count(field.expr); + }, + sym::width => { + self.width = parse_count(field.expr); + }, + _ => walk_expr(self, field.expr), } } +} +/// Parses the `fmt` arg of `Arguments::new_v1_formatted(pieces, args, fmt, _)` +fn parse_rt_fmt<'tcx>(fmt_arg: &'tcx Expr<'tcx>) -> Option<impl Iterator<Item = ParamPosition> + 'tcx> { if let ExprKind::AddrOf(.., array) = fmt_arg.kind && let ExprKind::Array(specs) = array.kind { Some(specs.iter().map(|spec| { let mut position = ParamPosition::default(); - - // ::core::fmt::rt::v1::Argument { - // position: 0usize, - // format: ::core::fmt::rt::v1::FormatSpec { - // .. - // precision: ::core::fmt::rt::v1::Count::Implied, - // width: ::core::fmt::rt::v1::Count::Implied, - // }, - // } - - // TODO: this can be made much nicer next sync with `Visitor::visit_expr_field` - if let ExprKind::Struct(_, fields, _) = spec.kind { - for field in fields { - match (field.ident.name, &field.expr.kind) { - (sym::position, ExprKind::Lit(lit)) => { - if let LitKind::Int(pos, _) = lit.node { - position.value = pos as usize; - } - }, - (sym::format, &ExprKind::Struct(_, spec_fields, _)) => { - for spec_field in spec_fields { - match spec_field.ident.name { - sym::precision => { - position.precision = parse_count(spec_field.expr); - }, - sym::width => { - position.width = parse_count(spec_field.expr); - }, - _ => {}, - } - } - }, - _ => {}, - } - } - } - + position.visit_expr(spec); position })) } else { @@ -560,19 +538,32 @@ fn span_from_inner(base: SpanData, inner: rpf::InnerSpan) -> Span { ) } +/// How a format parameter is used in the format string #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum FormatParamKind { /// An implicit parameter , such as `{}` or `{:?}`. Implicit, - /// A parameter with an explicit number, or an asterisk precision. e.g. `{1}`, `{0:?}`, - /// `{:.0$}` or `{:.*}`. + /// A parameter with an explicit number, e.g. `{1}`, `{0:?}`, or `{:.0$}` Numbered, + /// A parameter with an asterisk precision. e.g. `{:.*}`. + Starred, /// A named parameter with a named `value_arg`, such as the `x` in `format!("{x}", x = 1)`. Named(Symbol), /// An implicit named parameter, such as the `y` in `format!("{y}")`. NamedInline(Symbol), } +/// Where a format parameter is being used in the format string +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum FormatParamUsage { + /// Appears as an argument, e.g. `format!("{}", foo)` + Argument, + /// Appears as a width, e.g. `format!("{:width$}", foo, width = 1)` + Width, + /// Appears as a precision, e.g. `format!("{:.precision$}", foo, precision = 1)` + Precision, +} + /// A `FormatParam` is any place in a `FormatArgument` that refers to a supplied value, e.g. /// /// ``` @@ -588,6 +579,8 @@ pub struct FormatParam<'tcx> { pub value: &'tcx Expr<'tcx>, /// How this parameter refers to its `value`. pub kind: FormatParamKind, + /// Where this format param is being used - argument/width/precision + pub usage: FormatParamUsage, /// Span of the parameter, may be zero width. Includes the whitespace of implicit parameters. /// /// ```text @@ -600,6 +593,7 @@ pub struct FormatParam<'tcx> { impl<'tcx> FormatParam<'tcx> { fn new( mut kind: FormatParamKind, + usage: FormatParamUsage, position: usize, inner: rpf::InnerSpan, values: &FormatArgsValues<'tcx>, @@ -614,7 +608,12 @@ impl<'tcx> FormatParam<'tcx> { kind = FormatParamKind::NamedInline(name); } - Some(Self { value, kind, span }) + Some(Self { + value, + kind, + usage, + span, + }) } } @@ -628,33 +627,48 @@ pub enum Count<'tcx> { /// `FormatParamKind::Numbered`. Param(FormatParam<'tcx>), /// Not specified. - Implied, + Implied(Option<Span>), } impl<'tcx> Count<'tcx> { fn new( + usage: FormatParamUsage, count: rpf::Count<'_>, position: Option<usize>, inner: Option<rpf::InnerSpan>, values: &FormatArgsValues<'tcx>, ) -> Option<Self> { + let span = inner.map(|inner| span_from_inner(values.format_string_span, inner)); + Some(match count { - rpf::Count::CountIs(val) => Self::Is(val, span_from_inner(values.format_string_span, inner?)), - rpf::Count::CountIsName(name, span) => Self::Param(FormatParam::new( + rpf::Count::CountIs(val) => Self::Is(val, span?), + rpf::Count::CountIsName(name, _) => Self::Param(FormatParam::new( FormatParamKind::Named(Symbol::intern(name)), + usage, position?, - span, + inner?, values, )?), - rpf::Count::CountIsParam(_) | rpf::Count::CountIsStar(_) => { - Self::Param(FormatParam::new(FormatParamKind::Numbered, position?, inner?, values)?) - }, - rpf::Count::CountImplied => Self::Implied, + rpf::Count::CountIsParam(_) => Self::Param(FormatParam::new( + FormatParamKind::Numbered, + usage, + position?, + inner?, + values, + )?), + rpf::Count::CountIsStar(_) => Self::Param(FormatParam::new( + FormatParamKind::Starred, + usage, + position?, + inner?, + values, + )?), + rpf::Count::CountImplied => Self::Implied(span), }) } pub fn is_implied(self) -> bool { - matches!(self, Count::Implied) + matches!(self, Count::Implied(_)) } pub fn param(self) -> Option<FormatParam<'tcx>> { @@ -663,6 +677,14 @@ impl<'tcx> Count<'tcx> { _ => None, } } + + pub fn span(self) -> Option<Span> { + match self { + Count::Is(_, span) => Some(span), + Count::Param(param) => Some(param.span), + Count::Implied(span) => span, + } + } } /// Specification for the formatting of an argument in the format string. See @@ -691,8 +713,20 @@ impl<'tcx> FormatSpec<'tcx> { fill: spec.fill, align: spec.align, flags: spec.flags, - precision: Count::new(spec.precision, positions.precision, spec.precision_span, values)?, - width: Count::new(spec.width, positions.width, spec.width_span, values)?, + precision: Count::new( + FormatParamUsage::Precision, + spec.precision, + positions.precision, + spec.precision_span, + values, + )?, + width: Count::new( + FormatParamUsage::Width, + spec.width, + positions.width, + spec.width_span, + values, + )?, r#trait: match spec.ty { "" => sym::Display, "?" => sym::Debug, @@ -711,9 +745,19 @@ impl<'tcx> FormatSpec<'tcx> { }) } - /// Returns true if this format spec would change the contents of a string when formatted - pub fn has_string_formatting(&self) -> bool { - self.r#trait != sym::Display || !self.width.is_implied() || !self.precision.is_implied() + /// Returns true if this format spec is unchanged from the default. e.g. returns true for `{}`, + /// `{foo}` and `{2}`, but false for `{:?}`, `{foo:5}` and `{3:.5}` + pub fn is_default(&self) -> bool { + self.r#trait == sym::Display && self.is_default_for_trait() + } + + /// Has no other formatting specifiers than setting the format trait. returns true for `{}`, + /// `{foo}`, `{:?}`, but false for `{foo:5}`, `{3:.5?}` + pub fn is_default_for_trait(&self) -> bool { + self.width.is_implied() + && self.precision.is_implied() + && self.align == Alignment::AlignUnknown + && self.flags == 0 } } @@ -728,22 +772,108 @@ pub struct FormatArg<'tcx> { pub span: Span, } +impl<'tcx> FormatArg<'tcx> { + /// Span of the `:` and format specifiers + /// + /// ```ignore + /// format!("{:.}"), format!("{foo:.}") + /// ^^ ^^ + /// ``` + pub fn format_span(&self) -> Span { + let base = self.span.data(); + + // `base.hi` is `{...}|`, subtract 1 byte (the length of '}') so that it points before the closing + // brace `{...|}` + Span::new(self.param.span.hi(), base.hi - BytePos(1), base.ctxt, base.parent) + } +} + /// A parsed `format_args!` expansion. #[derive(Debug)] pub struct FormatArgsExpn<'tcx> { /// The format string literal. pub format_string: FormatString, - // The format arguments, such as `{:?}`. + /// The format arguments, such as `{:?}`. pub args: Vec<FormatArg<'tcx>>, /// Has an added newline due to `println!()`/`writeln!()`/etc. The last format string part will /// include this added newline. pub newline: bool, - /// Values passed after the format string and implicit captures. `[1, z + 2, x]` for + /// Spans of the commas between the format string and explicit values, excluding any trailing + /// comma + /// + /// ```ignore + /// format!("..", 1, 2, 3,) + /// // ^ ^ ^ + /// ``` + comma_spans: Vec<Span>, + /// Explicit values passed after the format string, ignoring implicit captures. `[1, z + 2]` for /// `format!("{x} {} {y}", 1, z + 2)`. - value_args: Vec<&'tcx Expr<'tcx>>, + explicit_values: Vec<&'tcx Expr<'tcx>>, } impl<'tcx> FormatArgsExpn<'tcx> { + /// Gets the spans of the commas inbetween the format string and explicit args, not including + /// any trailing comma + /// + /// ```ignore + /// format!("{} {}", a, b) + /// // ^ ^ + /// ``` + /// + /// Ensures that the format string and values aren't coming from a proc macro that sets the + /// output span to that of its input + fn comma_spans(cx: &LateContext<'_>, explicit_values: &[&Expr<'_>], fmt_span: Span) -> Option<Vec<Span>> { + // `format!("{} {} {c}", "one", "two", c = "three")` + // ^^^^^ ^^^^^ ^^^^^^^ + let value_spans = explicit_values + .iter() + .map(|val| hygiene::walk_chain(val.span, fmt_span.ctxt())); + + // `format!("{} {} {c}", "one", "two", c = "three")` + // ^^ ^^ ^^^^^^ + let between_spans = once(fmt_span) + .chain(value_spans) + .tuple_windows() + .map(|(start, end)| start.between(end)); + + let mut comma_spans = Vec::new(); + for between_span in between_spans { + let mut offset = 0; + let mut seen_comma = false; + + for token in tokenize(&snippet_opt(cx, between_span)?) { + match token.kind { + TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } | TokenKind::Whitespace => {}, + TokenKind::Comma if !seen_comma => { + seen_comma = true; + + let base = between_span.data(); + comma_spans.push(Span::new( + base.lo + BytePos(offset), + base.lo + BytePos(offset + 1), + base.ctxt, + base.parent, + )); + }, + // named arguments, `start_val, name = end_val` + // ^^^^^^^^^ between_span + TokenKind::Ident | TokenKind::Eq if seen_comma => {}, + // An unexpected token usually indicates the format string or a value came from a proc macro output + // that sets the span of its output to an input, e.g. `println!(some_proc_macro!("input"), ..)` that + // emits a string literal with the span set to that of `"input"` + _ => return None, + } + offset += token.len; + } + + if !seen_comma { + return None; + } + } + + Some(comma_spans) + } + pub fn parse(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<Self> { let macro_name = macro_backtrace(expr.span) .map(|macro_call| cx.tcx.item_name(macro_call.def_id)) @@ -807,6 +937,7 @@ impl<'tcx> FormatArgsExpn<'tcx> { // NamedInline is handled by `FormatParam::new()` rpf::Position::ArgumentNamed(name) => FormatParamKind::Named(Symbol::intern(name)), }, + FormatParamUsage::Argument, position.value, parsed_arg.position_span, &values, @@ -817,11 +948,22 @@ impl<'tcx> FormatArgsExpn<'tcx> { }) .collect::<Option<Vec<_>>>()?; + let mut explicit_values = values.value_args; + // remove values generated for implicitly captured vars + let len = explicit_values + .iter() + .take_while(|val| !format_string.span.contains(val.span)) + .count(); + explicit_values.truncate(len); + + let comma_spans = Self::comma_spans(cx, &explicit_values, format_string.span)?; + Some(Self { format_string, args, - value_args: values.value_args, newline, + comma_spans, + explicit_values, }) } else { None @@ -829,27 +971,25 @@ impl<'tcx> FormatArgsExpn<'tcx> { } pub fn find_nested(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, expn_id: ExpnId) -> Option<Self> { - let mut format_args = None; - expr_visitor_no_bodies(|e| { - if format_args.is_some() { - return false; - } + for_each_expr(expr, |e| { let e_ctxt = e.span.ctxt(); if e_ctxt == expr.span.ctxt() { - return true; - } - if e_ctxt.outer_expn().is_descendant_of(expn_id) { - format_args = FormatArgsExpn::parse(cx, e); + ControlFlow::Continue(Descend::Yes) + } else if e_ctxt.outer_expn().is_descendant_of(expn_id) { + if let Some(args) = FormatArgsExpn::parse(cx, e) { + ControlFlow::Break(args) + } else { + ControlFlow::Continue(Descend::No) + } + } else { + ControlFlow::Continue(Descend::No) } - false }) - .visit_expr(expr); - format_args } /// Source callsite span of all inputs pub fn inputs_span(&self) -> Span { - match *self.value_args { + match *self.explicit_values { [] => self.format_string.span, [.., last] => self .format_string @@ -858,6 +998,22 @@ impl<'tcx> FormatArgsExpn<'tcx> { } } + /// Get the span of a value expanded to the previous comma, e.g. for the value `10` + /// + /// ```ignore + /// format("{}.{}", 10, 11) + /// // ^^^^ + /// ``` + pub fn value_with_prev_comma_span(&self, value_id: HirId) -> Option<Span> { + for (comma_span, value) in zip(&self.comma_spans, &self.explicit_values) { + if value.hir_id == value_id { + return Some(comma_span.to(hygiene::walk_chain(value.span, comma_span.ctxt()))); + } + } + + None + } + /// Iterator of all format params, both values and those referenced by `width`/`precision`s. pub fn params(&'tcx self) -> impl Iterator<Item = FormatParam<'tcx>> { self.args diff --git a/src/tools/clippy/clippy_utils/src/mir/maybe_storage_live.rs b/src/tools/clippy/clippy_utils/src/mir/maybe_storage_live.rs new file mode 100644 index 000000000..d262b335d --- /dev/null +++ b/src/tools/clippy/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<mir::Local>; + 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<Self::Idx>, 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<Self::Idx>, + _terminator: &mir::Terminator<'tcx>, + _loc: mir::Location, + ) { + } + + fn call_return_effect( + &self, + _trans: &mut impl GenKill<Self::Idx>, + _block: mir::BasicBlock, + _return_places: CallReturnPlaces<'_, 'tcx>, + ) { + // Nothing to do when a call returns successfully + } +} diff --git a/src/tools/clippy/clippy_utils/src/mir/mod.rs b/src/tools/clippy/clippy_utils/src/mir/mod.rs new file mode 100644 index 000000000..818e603f6 --- /dev/null +++ b/src/tools/clippy/clippy_utils/src/mir/mod.rs @@ -0,0 +1,164 @@ +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<Location>, + /// The locations where the local is consumed or mutated, if any. + pub local_consume_or_mutate_locs: Vec<Location>, +} + +pub fn visit_local_usage(locals: &[Local], mir: &Body<'_>, location: Location) -> Option<Vec<LocalUsage>> { + 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<LocalUsage>, +} + +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<bool> { + 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<Local> { + 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. +pub fn local_assignments(mir: &Body<'_>, local: Local) -> Vec<Location> { + 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/src/tools/clippy/clippy_utils/src/mir/possible_borrower.rs b/src/tools/clippy/clippy_utils/src/mir/possible_borrower.rs new file mode 100644 index 000000000..25717bf3d --- /dev/null +++ b/src/tools/clippy/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<mir::Local, HybridBitSet<mir::Local>>, +} + +impl<'a, 'b, 'tcx> PossibleBorrowerVisitor<'a, 'b, 'tcx> { + fn new( + cx: &'a LateContext<'tcx>, + body: &'b mir::Body<'tcx>, + possible_origin: FxHashMap<mir::Local, HybridBitSet<mir::Local>>, + ) -> 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<mir::Local> = 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<Self::BreakTy> { + 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<mir::Local, HybridBitSet<mir::Local>>, + maybe_live: ResultsCursor<'b, 'tcx, MaybeStorageLive>, + // Caches to avoid allocation of `BitSet` on every query + pub bitset: (BitSet<mir::Local>, BitSet<mir::Local>), +} + +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/src/tools/clippy/clippy_utils/src/mir/possible_origin.rs b/src/tools/clippy/clippy_utils/src/mir/possible_origin.rs new file mode 100644 index 000000000..8e7513d74 --- /dev/null +++ b/src/tools/clippy/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<mir::Local, HybridBitSet<mir::Local>> { + 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/src/tools/clippy/clippy_utils/src/mir/transitive_relation.rs b/src/tools/clippy/clippy_utils/src/mir/transitive_relation.rs new file mode 100644 index 000000000..7fe296073 --- /dev/null +++ b/src/tools/clippy/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<mir::Local, Vec<mir::Local>>, +} + +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<mir::Local> { + 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/tools/clippy/clippy_utils/src/msrvs.rs b/src/tools/clippy/clippy_utils/src/msrvs.rs index 62020e21c..8b843732a 100644 --- a/src/tools/clippy/clippy_utils/src/msrvs.rs +++ b/src/tools/clippy/clippy_utils/src/msrvs.rs @@ -13,10 +13,11 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { 1,62,0 { BOOL_THEN_SOME } + 1,58,0 { FORMAT_ARGS_CAPTURE } 1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR } 1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST } 1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS } - 1,50,0 { BOOL_THEN } + 1,50,0 { BOOL_THEN, CLAMP } 1,47,0 { TAU } 1,46,0 { CONST_IF_MATCH } 1,45,0 { STR_STRIP_PREFIX } diff --git a/src/tools/clippy/clippy_utils/src/numeric_literal.rs b/src/tools/clippy/clippy_utils/src/numeric_literal.rs index 80098d976..c5dcd7b31 100644 --- a/src/tools/clippy/clippy_utils/src/numeric_literal.rs +++ b/src/tools/clippy/clippy_utils/src/numeric_literal.rs @@ -69,12 +69,13 @@ impl<'a> NumericLiteral<'a> { #[must_use] pub fn new(lit: &'a str, suffix: Option<&'a str>, float: bool) -> Self { + let unsigned_lit = lit.trim_start_matches('-'); // Determine delimiter for radix prefix, if present, and radix. - let radix = if lit.starts_with("0x") { + let radix = if unsigned_lit.starts_with("0x") { Radix::Hexadecimal - } else if lit.starts_with("0b") { + } else if unsigned_lit.starts_with("0b") { Radix::Binary - } else if lit.starts_with("0o") { + } else if unsigned_lit.starts_with("0o") { Radix::Octal } else { Radix::Decimal diff --git a/src/tools/clippy/clippy_utils/src/paths.rs b/src/tools/clippy/clippy_utils/src/paths.rs index 07170e2df..bc8514734 100644 --- a/src/tools/clippy/clippy_utils/src/paths.rs +++ b/src/tools/clippy/clippy_utils/src/paths.rs @@ -16,26 +16,17 @@ pub const APPLICABILITY_VALUES: [[&str; 3]; 4] = [ #[cfg(feature = "internal")] pub const DIAGNOSTIC_BUILDER: [&str; 3] = ["rustc_errors", "diagnostic_builder", "DiagnosticBuilder"]; pub const ARC_PTR_EQ: [&str; 4] = ["alloc", "sync", "Arc", "ptr_eq"]; -pub const ASMUT_TRAIT: [&str; 3] = ["core", "convert", "AsMut"]; -pub const ASREF_TRAIT: [&str; 3] = ["core", "convert", "AsRef"]; pub const BTREEMAP_CONTAINS_KEY: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "contains_key"]; -pub const BTREEMAP_ENTRY: [&str; 6] = ["alloc", "collections", "btree", "map", "entry", "Entry"]; pub const BTREEMAP_INSERT: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "insert"]; pub const BTREESET_ITER: [&str; 6] = ["alloc", "collections", "btree", "set", "BTreeSet", "iter"]; pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"]; -pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"]; pub const CORE_ITER_COLLECT: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "collect"]; pub const CORE_ITER_CLONED: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "cloned"]; pub const CORE_ITER_COPIED: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "copied"]; pub const CORE_ITER_FILTER: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "filter"]; -pub const CORE_ITER_INTO_ITER: [&str; 6] = ["core", "iter", "traits", "collect", "IntoIterator", "into_iter"]; pub const CSTRING_AS_C_STR: [&str; 5] = ["alloc", "ffi", "c_str", "CString", "as_c_str"]; pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"]; pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"]; -/// Preferably use the diagnostic item `sym::deref_method` where possible -pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"]; -pub const DIR_BUILDER: [&str; 3] = ["std", "fs", "DirBuilder"]; -pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"]; #[cfg(feature = "internal")] pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"]; #[cfg(feature = "internal")] @@ -43,32 +34,22 @@ pub const EARLY_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "EarlyLintPass"] pub const EXIT: [&str; 3] = ["std", "process", "exit"]; pub const F32_EPSILON: [&str; 4] = ["core", "f32", "<impl f32>", "EPSILON"]; pub const F64_EPSILON: [&str; 4] = ["core", "f64", "<impl f64>", "EPSILON"]; -pub const FILE: [&str; 3] = ["std", "fs", "File"]; -pub const FILE_TYPE: [&str; 3] = ["std", "fs", "FileType"]; -pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"]; pub const FROM_ITERATOR_METHOD: [&str; 6] = ["core", "iter", "traits", "collect", "FromIterator", "from_iter"]; pub const FROM_STR_METHOD: [&str; 5] = ["core", "str", "traits", "FromStr", "from_str"]; -pub const FUTURE_FROM_GENERATOR: [&str; 3] = ["core", "future", "from_generator"]; #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const FUTURES_IO_ASYNCREADEXT: [&str; 3] = ["futures_util", "io", "AsyncReadExt"]; #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const FUTURES_IO_ASYNCWRITEEXT: [&str; 3] = ["futures_util", "io", "AsyncWriteExt"]; pub const HASHMAP_CONTAINS_KEY: [&str; 6] = ["std", "collections", "hash", "map", "HashMap", "contains_key"]; -pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"]; pub const HASHMAP_INSERT: [&str; 6] = ["std", "collections", "hash", "map", "HashMap", "insert"]; pub const HASHSET_ITER: [&str; 6] = ["std", "collections", "hash", "set", "HashSet", "iter"]; #[cfg(feature = "internal")] pub const IDENT: [&str; 3] = ["rustc_span", "symbol", "Ident"]; #[cfg(feature = "internal")] pub const IDENT_AS_STR: [&str; 4] = ["rustc_span", "symbol", "Ident", "as_str"]; -pub const INDEX: [&str; 3] = ["core", "ops", "Index"]; -pub const INDEX_MUT: [&str; 3] = ["core", "ops", "IndexMut"]; pub const INSERT_STR: [&str; 4] = ["alloc", "string", "String", "insert_str"]; -pub const IO_READ: [&str; 3] = ["std", "io", "Read"]; -pub const IO_WRITE: [&str; 3] = ["std", "io", "Write"]; pub const ITER_COUNT: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "count"]; pub const ITER_EMPTY: [&str; 5] = ["core", "iter", "sources", "empty", "Empty"]; -pub const ITER_REPEAT: [&str; 5] = ["core", "iter", "sources", "repeat", "repeat"]; pub const ITERTOOLS_NEXT_TUPLE: [&str; 3] = ["itertools", "Itertools", "next_tuple"]; #[cfg(feature = "internal")] pub const KW_MODULE: [&str; 3] = ["rustc_span", "symbol", "kw"]; @@ -79,13 +60,7 @@ pub const LATE_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "LateLintPass"]; #[cfg(feature = "internal")] pub const LINT: [&str; 2] = ["rustc_lint_defs", "Lint"]; pub const MEM_SWAP: [&str; 3] = ["core", "mem", "swap"]; -pub const MUTEX_GUARD: [&str; 4] = ["std", "sync", "mutex", "MutexGuard"]; pub const OPEN_OPTIONS: [&str; 3] = ["std", "fs", "OpenOptions"]; -/// Preferably use the diagnostic item `sym::Option` where possible -pub const OPTION: [&str; 3] = ["core", "option", "Option"]; -pub const OPTION_NONE: [&str; 4] = ["core", "option", "Option", "None"]; -pub const OPTION_SOME: [&str; 4] = ["core", "option", "Option", "Some"]; -pub const ORD: [&str; 3] = ["core", "cmp", "Ord"]; pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString", "as_os_str"]; pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"]; pub const PARKING_LOT_MUTEX_GUARD: [&str; 3] = ["lock_api", "mutex", "MutexGuard"]; @@ -98,8 +73,6 @@ pub const PERMISSIONS: [&str; 3] = ["std", "fs", "Permissions"]; #[cfg_attr(not(unix), allow(clippy::invalid_paths))] pub const PERMISSIONS_FROM_MODE: [&str; 6] = ["std", "os", "unix", "fs", "PermissionsExt", "from_mode"]; pub const POLL: [&str; 4] = ["core", "task", "poll", "Poll"]; -pub const POLL_PENDING: [&str; 5] = ["core", "task", "poll", "Poll", "Pending"]; -pub const POLL_READY: [&str; 5] = ["core", "task", "poll", "Poll", "Ready"]; pub const PTR_COPY: [&str; 3] = ["core", "intrinsics", "copy"]; pub const PTR_COPY_NONOVERLAPPING: [&str; 3] = ["core", "intrinsics", "copy_nonoverlapping"]; pub const PTR_EQ: [&str; 3] = ["core", "ptr", "eq"]; @@ -122,26 +95,14 @@ 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"]; -pub const RESULT_ERR: [&str; 4] = ["core", "result", "Result", "Err"]; -pub const RESULT_OK: [&str; 4] = ["core", "result", "Result", "Ok"]; #[cfg(feature = "internal")] pub const RUSTC_VERSION: [&str; 2] = ["rustc_semver", "RustcVersion"]; -pub const RWLOCK_READ_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockReadGuard"]; -pub const RWLOCK_WRITE_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockWriteGuard"]; pub const SERDE_DESERIALIZE: [&str; 3] = ["serde", "de", "Deserialize"]; pub const SERDE_DE_VISITOR: [&str; 3] = ["serde", "de", "Visitor"]; pub const SLICE_FROM_RAW_PARTS: [&str; 4] = ["core", "slice", "raw", "from_raw_parts"]; diff --git a/src/tools/clippy/clippy_utils/src/ptr.rs b/src/tools/clippy/clippy_utils/src/ptr.rs index 0226f7490..88837d8a1 100644 --- a/src/tools/clippy/clippy_utils/src/ptr.rs +++ b/src/tools/clippy/clippy_utils/src/ptr.rs @@ -1,7 +1,7 @@ use crate::source::snippet; -use crate::visitors::expr_visitor_no_bodies; +use crate::visitors::{for_each_expr, Descend}; use crate::{path_to_local_id, strip_pat_refs}; -use rustc_hir::intravisit::Visitor; +use core::ops::ControlFlow; use rustc_hir::{Body, BodyId, ExprKind, HirId, PatKind}; use rustc_lint::LateContext; use rustc_span::Span; @@ -30,28 +30,23 @@ fn extract_clone_suggestions<'tcx>( replace: &[(&'static str, &'static str)], body: &'tcx Body<'_>, ) -> Option<Vec<(Span, Cow<'static, str>)>> { - let mut abort = false; let mut spans = Vec::new(); - expr_visitor_no_bodies(|expr| { - if abort { - return false; - } - if let ExprKind::MethodCall(seg, recv, [], _) = expr.kind { - if path_to_local_id(recv, id) { - if seg.ident.name.as_str() == "capacity" { - abort = true; - return false; - } - for &(fn_name, suffix) in replace { - if seg.ident.name.as_str() == fn_name { - spans.push((expr.span, snippet(cx, recv.span, "_") + suffix)); - return false; - } + for_each_expr(body, |e| { + if let ExprKind::MethodCall(seg, recv, [], _) = e.kind + && path_to_local_id(recv, id) + { + if seg.ident.as_str() == "capacity" { + return ControlFlow::Break(()); + } + for &(fn_name, suffix) in replace { + if seg.ident.as_str() == fn_name { + spans.push((e.span, snippet(cx, recv.span, "_") + suffix)); + return ControlFlow::Continue(Descend::No); } } } - !abort + ControlFlow::Continue(Descend::Yes) }) - .visit_body(body); - if abort { None } else { Some(spans) } + .is_none() + .then_some(spans) } diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 8835b9329..45b63a4aa 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -33,10 +33,10 @@ pub fn is_min_const_fn<'a, 'tcx>(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, msrv: | ty::PredicateKind::ConstEquate(..) | ty::PredicateKind::Trait(..) | ty::PredicateKind::TypeWellFormedFromEnv(..) => continue, - ty::PredicateKind::ObjectSafe(_) => panic!("object safe predicate on function: {:#?}", predicate), - ty::PredicateKind::ClosureKind(..) => panic!("closure kind predicate on function: {:#?}", predicate), - ty::PredicateKind::Subtype(_) => panic!("subtype predicate on function: {:#?}", predicate), - ty::PredicateKind::Coerce(_) => panic!("coerce predicate on function: {:#?}", predicate), + ty::PredicateKind::ObjectSafe(_) => panic!("object safe predicate on function: {predicate:#?}"), + ty::PredicateKind::ClosureKind(..) => panic!("closure kind predicate on function: {predicate:#?}"), + ty::PredicateKind::Subtype(_) => panic!("subtype predicate on function: {predicate:#?}"), + ty::PredicateKind::Coerce(_) => panic!("coerce predicate on function: {predicate:#?}"), } } match predicates.parent { @@ -129,7 +129,12 @@ fn check_rvalue<'tcx>( | Rvalue::Use(operand) | Rvalue::Cast( CastKind::PointerFromExposedAddress - | CastKind::Misc + | CastKind::IntToInt + | CastKind::FloatToInt + | CastKind::IntToFloat + | CastKind::FloatToFloat + | CastKind::FnPtrToPtr + | CastKind::PtrToPtr | CastKind::Pointer(PointerCast::MutToConstPointer | PointerCast::ArrayToPointer), operand, _, @@ -260,6 +265,7 @@ fn check_place<'tcx>(tcx: TyCtxt<'tcx>, place: Place<'tcx>, span: Span, body: &B } }, ProjectionElem::ConstantIndex { .. } + | ProjectionElem::OpaqueCast(..) | ProjectionElem::Downcast(..) | ProjectionElem::Subslice { .. } | ProjectionElem::Deref @@ -318,8 +324,7 @@ fn check_terminator<'a, 'tcx>( span, format!( "can only call other `const fn` within a `const fn`, \ - but `{:?}` is not stable as `const fn`", - func, + but `{func:?}` is not stable as `const fn`", ) .into(), )); @@ -366,10 +371,23 @@ fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: Option<RustcVersion>) -> bo // Checking MSRV is manually necessary because `rustc` has no such concept. This entire // function could be removed if `rustc` provided a MSRV-aware version of `is_const_fn`. // as a part of an unimplemented MSRV check https://github.com/rust-lang/rust/issues/65262. + + // HACK(nilstrieb): CURRENT_RUSTC_VERSION can return versions like 1.66.0-dev. `rustc-semver` + // doesn't accept the `-dev` version number so we have to strip it + // off. + let short_version = since + .as_str() + .split('-') + .next() + .expect("rustc_attr::StabilityLevel::Stable::since` is empty"); + + let since = rustc_span::Symbol::intern(short_version); + crate::meets_msrv( msrv, - RustcVersion::parse(since.as_str()) - .expect("`rustc_attr::StabilityLevel::Stable::since` is ill-formatted"), + RustcVersion::parse(since.as_str()).unwrap_or_else(|err| { + panic!("`rustc_attr::StabilityLevel::Stable::since` is ill-formatted: `{since}`, {err:?}") + }), ) } else { // Unstable const fn with the feature enabled. diff --git a/src/tools/clippy/clippy_utils/src/source.rs b/src/tools/clippy/clippy_utils/src/source.rs index d85f591fb..d28bd92d7 100644 --- a/src/tools/clippy/clippy_utils/src/source.rs +++ b/src/tools/clippy/clippy_utils/src/source.rs @@ -25,11 +25,11 @@ pub fn expr_block<'a, T: LintContext>( if expr.span.from_expansion() { Cow::Owned(format!("{{ {} }}", snippet_with_macro_callsite(cx, expr.span, default))) } else if let ExprKind::Block(_, _) = expr.kind { - Cow::Owned(format!("{}{}", code, string)) + Cow::Owned(format!("{code}{string}")) } else if string.is_empty() { - Cow::Owned(format!("{{ {} }}", code)) + Cow::Owned(format!("{{ {code} }}")) } else { - Cow::Owned(format!("{{\n{};\n{}\n}}", code, string)) + Cow::Owned(format!("{{\n{code};\n{string}\n}}")) } } @@ -392,6 +392,16 @@ pub fn trim_span(sm: &SourceMap, span: Span) -> Span { .span() } +/// Expand a span to include a preceding comma +/// ```rust,ignore +/// writeln!(o, "") -> writeln!(o, "") +/// ^^ ^^^^ +/// ``` +pub fn expand_past_previous_comma(cx: &LateContext<'_>, span: Span) -> Span { + let extended = cx.sess().source_map().span_extend_to_prev_char(span, ',', true); + extended.with_lo(extended.lo() - BytePos(1)) +} + #[cfg(test)] mod test { use super::{reindent_multiline, without_block_comments}; @@ -466,7 +476,7 @@ mod test { #[test] fn test_without_block_comments_lines_without_block_comments() { let result = without_block_comments(vec!["/*", "", "*/"]); - println!("result: {:?}", result); + println!("result: {result:?}"); assert!(result.is_empty()); let result = without_block_comments(vec!["", "/*", "", "*/", "#[crate_type = \"lib\"]", "/*", "", "*/", ""]); diff --git a/src/tools/clippy/clippy_utils/src/sugg.rs b/src/tools/clippy/clippy_utils/src/sugg.rs index cca71bbf7..aad7da61a 100644 --- a/src/tools/clippy/clippy_utils/src/sugg.rs +++ b/src/tools/clippy/clippy_utils/src/sugg.rs @@ -1,7 +1,9 @@ //! Contains utility functions to generate suggestions. #![deny(clippy::missing_docs_in_private_items)] -use crate::source::{snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite}; +use crate::source::{ + snippet, snippet_opt, snippet_with_applicability, snippet_with_context, snippet_with_macro_callsite, +}; use crate::ty::expr_sig; use crate::{get_parent_expr_for_hir, higher}; use rustc_ast::util::parser::AssocOp; @@ -10,19 +12,19 @@ use rustc_ast_pretty::pprust::token_kind_to_string; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::{Closure, ExprKind, HirId, MutTy, TyKind}; +use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{EarlyContext, LateContext, LintContext}; use rustc_middle::hir::place::ProjectionKind; use rustc_middle::mir::{FakeReadCause, Mutability}; use rustc_middle::ty; use rustc_span::source_map::{BytePos, CharPos, Pos, Span, SyntaxContext}; -use rustc_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; use std::borrow::Cow; use std::fmt::{Display, Write as _}; use std::ops::{Add, Neg, Not, Sub}; /// A helper type to build suggestion correctly handling parentheses. -#[derive(Clone, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub enum Sugg<'a> { /// An expression that never needs parentheses such as `1337` or `[0; 42]`. NonParen(Cow<'a, str>), @@ -110,7 +112,7 @@ impl<'a> Sugg<'a> { if expr.span.ctxt() == ctxt { Self::hir_from_snippet(expr, |span| snippet(cx, span, default)) } else { - let snip = snippet_with_applicability(cx, expr.span, default, applicability); + let (snip, _) = snippet_with_context(cx, expr.span, ctxt, default, applicability); Sugg::NonParen(snip) } } @@ -155,8 +157,8 @@ impl<'a> Sugg<'a> { | hir::ExprKind::Ret(..) | hir::ExprKind::Struct(..) | hir::ExprKind::Tup(..) - | hir::ExprKind::DropTemps(_) | hir::ExprKind::Err => Sugg::NonParen(get_snippet(expr.span)), + hir::ExprKind::DropTemps(inner) => Self::hir_from_snippet(inner, get_snippet), hir::ExprKind::Assign(lhs, rhs, _) => { Sugg::BinOp(AssocOp::Assign, get_snippet(lhs.span), get_snippet(rhs.span)) }, @@ -177,11 +179,11 @@ impl<'a> Sugg<'a> { pub fn ast(cx: &EarlyContext<'_>, expr: &ast::Expr, default: &'a str) -> Self { use rustc_ast::ast::RangeLimits; - let get_whole_snippet = || { - if expr.span.from_expansion() { - snippet_with_macro_callsite(cx, expr.span, default) + let snippet_without_expansion = |cx, span: Span, default| { + if span.from_expansion() { + snippet_with_macro_callsite(cx, span, default) } else { - snippet(cx, expr.span, default) + snippet(cx, span, default) } }; @@ -192,7 +194,7 @@ impl<'a> Sugg<'a> { | ast::ExprKind::If(..) | ast::ExprKind::Let(..) | ast::ExprKind::Unary(..) - | ast::ExprKind::Match(..) => Sugg::MaybeParen(get_whole_snippet()), + | ast::ExprKind::Match(..) => Sugg::MaybeParen(snippet_without_expansion(cx, expr.span, default)), ast::ExprKind::Async(..) | ast::ExprKind::Block(..) | ast::ExprKind::Break(..) @@ -221,41 +223,45 @@ impl<'a> Sugg<'a> { | ast::ExprKind::Array(..) | ast::ExprKind::While(..) | ast::ExprKind::Await(..) - | ast::ExprKind::Err => Sugg::NonParen(get_whole_snippet()), + | ast::ExprKind::Err => Sugg::NonParen(snippet_without_expansion(cx, expr.span, default)), ast::ExprKind::Range(ref lhs, ref rhs, RangeLimits::HalfOpen) => Sugg::BinOp( AssocOp::DotDot, - lhs.as_ref().map_or("".into(), |lhs| snippet(cx, lhs.span, default)), - rhs.as_ref().map_or("".into(), |rhs| snippet(cx, rhs.span, default)), + lhs.as_ref() + .map_or("".into(), |lhs| snippet_without_expansion(cx, lhs.span, default)), + rhs.as_ref() + .map_or("".into(), |rhs| snippet_without_expansion(cx, rhs.span, default)), ), ast::ExprKind::Range(ref lhs, ref rhs, RangeLimits::Closed) => Sugg::BinOp( AssocOp::DotDotEq, - lhs.as_ref().map_or("".into(), |lhs| snippet(cx, lhs.span, default)), - rhs.as_ref().map_or("".into(), |rhs| snippet(cx, rhs.span, default)), + lhs.as_ref() + .map_or("".into(), |lhs| snippet_without_expansion(cx, lhs.span, default)), + rhs.as_ref() + .map_or("".into(), |rhs| snippet_without_expansion(cx, rhs.span, default)), ), ast::ExprKind::Assign(ref lhs, ref rhs, _) => Sugg::BinOp( AssocOp::Assign, - snippet(cx, lhs.span, default), - snippet(cx, rhs.span, default), + snippet_without_expansion(cx, lhs.span, default), + snippet_without_expansion(cx, rhs.span, default), ), ast::ExprKind::AssignOp(op, ref lhs, ref rhs) => Sugg::BinOp( astbinop2assignop(op), - snippet(cx, lhs.span, default), - snippet(cx, rhs.span, default), + snippet_without_expansion(cx, lhs.span, default), + snippet_without_expansion(cx, rhs.span, default), ), ast::ExprKind::Binary(op, ref lhs, ref rhs) => Sugg::BinOp( AssocOp::from_ast_binop(op.node), - snippet(cx, lhs.span, default), - snippet(cx, rhs.span, default), + snippet_without_expansion(cx, lhs.span, default), + snippet_without_expansion(cx, rhs.span, default), ), ast::ExprKind::Cast(ref lhs, ref ty) => Sugg::BinOp( AssocOp::As, - snippet(cx, lhs.span, default), - snippet(cx, ty.span, default), + snippet_without_expansion(cx, lhs.span, default), + snippet_without_expansion(cx, ty.span, default), ), ast::ExprKind::Type(ref lhs, ref ty) => Sugg::BinOp( AssocOp::Colon, - snippet(cx, lhs.span, default), - snippet(cx, ty.span, default), + snippet_without_expansion(cx, lhs.span, default), + snippet_without_expansion(cx, ty.span, default), ), } } @@ -306,19 +312,19 @@ impl<'a> Sugg<'a> { /// Convenience method to transform suggestion into a return call pub fn make_return(self) -> Sugg<'static> { - Sugg::NonParen(Cow::Owned(format!("return {}", self))) + Sugg::NonParen(Cow::Owned(format!("return {self}"))) } /// Convenience method to transform suggestion into a block /// where the suggestion is a trailing expression pub fn blockify(self) -> Sugg<'static> { - Sugg::NonParen(Cow::Owned(format!("{{ {} }}", self))) + Sugg::NonParen(Cow::Owned(format!("{{ {self} }}"))) } /// Convenience method to prefix the expression with the `async` keyword. /// Can be used after `blockify` to create an async block. pub fn asyncify(self) -> Sugg<'static> { - Sugg::NonParen(Cow::Owned(format!("async {}", self))) + Sugg::NonParen(Cow::Owned(format!("async {self}"))) } /// Convenience method to create the `<lhs>..<rhs>` or `<lhs>...<rhs>` @@ -342,12 +348,12 @@ impl<'a> Sugg<'a> { if has_enclosing_paren(&sugg) { Sugg::MaybeParen(sugg) } else { - Sugg::NonParen(format!("({})", sugg).into()) + Sugg::NonParen(format!("({sugg})").into()) } }, Sugg::BinOp(op, lhs, rhs) => { let sugg = binop_to_string(op, &lhs, &rhs); - Sugg::NonParen(format!("({})", sugg).into()) + Sugg::NonParen(format!("({sugg})").into()) }, } } @@ -375,20 +381,18 @@ fn binop_to_string(op: AssocOp, lhs: &str, rhs: &str) -> String { | AssocOp::Greater | AssocOp::GreaterEqual => { format!( - "{} {} {}", - lhs, - op.to_ast_binop().expect("Those are AST ops").to_string(), - rhs + "{lhs} {} {rhs}", + op.to_ast_binop().expect("Those are AST ops").to_string() ) }, - AssocOp::Assign => format!("{} = {}", lhs, rhs), + AssocOp::Assign => format!("{lhs} = {rhs}"), AssocOp::AssignOp(op) => { - format!("{} {}= {}", lhs, token_kind_to_string(&token::BinOp(op)), rhs) + format!("{lhs} {}= {rhs}", token_kind_to_string(&token::BinOp(op))) }, - AssocOp::As => format!("{} as {}", lhs, rhs), - AssocOp::DotDot => format!("{}..{}", lhs, rhs), - AssocOp::DotDotEq => format!("{}..={}", lhs, rhs), - AssocOp::Colon => format!("{}: {}", lhs, rhs), + AssocOp::As => format!("{lhs} as {rhs}"), + AssocOp::DotDot => format!("{lhs}..{rhs}"), + AssocOp::DotDotEq => format!("{lhs}..={rhs}"), + AssocOp::Colon => format!("{lhs}: {rhs}"), } } @@ -519,7 +523,7 @@ impl<T: Display> Display for ParenHelper<T> { /// operators have the same /// precedence. pub fn make_unop(op: &str, expr: Sugg<'_>) -> Sugg<'static> { - Sugg::MaybeParen(format!("{}{}", op, expr.maybe_par()).into()) + Sugg::MaybeParen(format!("{op}{}", expr.maybe_par()).into()) } /// Builds the string for `<lhs> <op> <rhs>` adding parenthesis when necessary. @@ -740,7 +744,7 @@ impl<T: LintContext> DiagnosticExt<T> for rustc_errors::Diagnostic { if let Some(indent) = indentation(cx, item) { let span = item.with_hi(item.lo()); - self.span_suggestion(span, msg, format!("{}\n{}", attr, indent), applicability); + self.span_suggestion(span, msg, format!("{attr}\n{indent}"), applicability); } } @@ -754,21 +758,20 @@ impl<T: LintContext> DiagnosticExt<T> for rustc_errors::Diagnostic { .map(|l| { if first { first = false; - format!("{}\n", l) + format!("{l}\n") } else { - format!("{}{}\n", indent, l) + format!("{indent}{l}\n") } }) .collect::<String>(); - self.span_suggestion(span, msg, format!("{}\n{}", new_item, indent), applicability); + self.span_suggestion(span, msg, format!("{new_item}\n{indent}"), applicability); } } fn suggest_remove_item(&mut self, cx: &T, item: Span, msg: &str, applicability: Applicability) { let mut remove_span = item; - let hi = cx.sess().source_map().next_point(remove_span).hi(); - let fmpos = cx.sess().source_map().lookup_byte_offset(hi); + let fmpos = cx.sess().source_map().lookup_byte_offset(remove_span.hi()); if let Some(ref src) = fmpos.sf.src { let non_whitespace_offset = src[fmpos.pos.to_usize()..].find(|c| c != ' ' && c != '\t' && c != '\n'); @@ -819,10 +822,9 @@ pub fn deref_closure_args<'tcx>(cx: &LateContext<'_>, closure: &'tcx hir::Expr<' }; let fn_def_id = cx.tcx.hir().local_def_id(closure.hir_id); - cx.tcx.infer_ctxt().enter(|infcx| { - ExprUseVisitor::new(&mut visitor, &infcx, fn_def_id, cx.param_env, cx.typeck_results()) - .consume_body(closure_body); - }); + let infcx = cx.tcx.infer_ctxt().build(); + ExprUseVisitor::new(&mut visitor, &infcx, fn_def_id, cx.param_env, cx.typeck_results()) + .consume_body(closure_body); if !visitor.suggestion_start.is_empty() { return Some(DerefClosure { @@ -859,7 +861,7 @@ impl<'tcx> DerefDelegate<'_, 'tcx> { pub fn finish(&mut self) -> String { let end_span = Span::new(self.next_pos, self.closure_span.hi(), self.closure_span.ctxt(), None); let end_snip = snippet_with_applicability(self.cx, end_span, "..", &mut self.applicability); - let sugg = format!("{}{}", self.suggestion_start, end_snip); + let sugg = format!("{}{end_snip}", self.suggestion_start); if self.closure_arg_is_type_annotated_double_ref { sugg.replacen('&', "", 1) } else { @@ -921,7 +923,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> { if cmt.place.projections.is_empty() { // handle item without any projection, that needs an explicit borrowing // i.e.: suggest `&x` instead of `x` - let _ = write!(self.suggestion_start, "{}&{}", start_snip, ident_str); + let _ = write!(self.suggestion_start, "{start_snip}&{ident_str}"); } else { // cases where a parent `Call` or `MethodCall` is using the item // i.e.: suggest `.contains(&x)` for `.find(|x| [1, 2, 3].contains(x)).is_none()` @@ -936,7 +938,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> { // given expression is the self argument and will be handled completely by the compiler // i.e.: `|x| x.is_something()` ExprKind::MethodCall(_, self_expr, ..) if self_expr.hir_id == cmt.hir_id => { - let _ = write!(self.suggestion_start, "{}{}", start_snip, ident_str_with_proj); + let _ = write!(self.suggestion_start, "{start_snip}{ident_str_with_proj}"); self.next_pos = span.hi(); return; }, @@ -969,9 +971,9 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> { } else { ident_str }; - format!("{}{}", start_snip, ident) + format!("{start_snip}{ident}") } else { - format!("{}&{}", start_snip, ident_str) + format!("{start_snip}&{ident_str}") }; self.suggestion_start.push_str(&ident_sugg); self.next_pos = span.hi(); @@ -1038,13 +1040,13 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> { for item in projections { if item.kind == ProjectionKind::Deref { - replacement_str = format!("*{}", replacement_str); + replacement_str = format!("*{replacement_str}"); } } } } - let _ = write!(self.suggestion_start, "{}{}", start_snip, replacement_str); + let _ = write!(self.suggestion_start, "{start_snip}{replacement_str}"); } self.next_pos = span.hi(); } @@ -1052,7 +1054,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> { fn mutate(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {} - fn fake_read(&mut self, _: &rustc_typeck::expr_use_visitor::PlaceWithHirId<'tcx>, _: FakeReadCause, _: HirId) {} + fn fake_read(&mut self, _: &PlaceWithHirId<'tcx>, _: FakeReadCause, _: HirId) {} } #[cfg(test)] diff --git a/src/tools/clippy/clippy_utils/src/ty.rs b/src/tools/clippy/clippy_utils/src/ty.rs index a8ad6cf4f..4e024ce40 100644 --- a/src/tools/clippy/clippy_utils/src/ty.rs +++ b/src/tools/clippy/clippy_utils/src/ty.rs @@ -12,13 +12,13 @@ use rustc_hir::{Expr, FnDecl, LangItem, TyKind, Unsafety}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::LateContext; use rustc_middle::mir::interpret::{ConstValue, Scalar}; -use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst}; use rustc_middle::ty::{ self, AdtDef, Binder, BoundRegion, DefIdTree, FnSig, IntTy, ParamEnv, Predicate, PredicateKind, ProjectionTy, Region, RegionKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor, UintTy, VariantDef, VariantDiscr, }; +use rustc_middle::ty::{GenericArg, GenericArgKind}; use rustc_span::symbol::Ident; -use rustc_span::{sym, Span, Symbol, DUMMY_SP}; +use rustc_span::{sym, Span, Symbol}; use rustc_target::abi::{Size, VariantIdx}; use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::traits::query::normalize::AtExt; @@ -28,7 +28,14 @@ use crate::{match_def_path, path_res, paths}; // Checks if the given type implements copy. pub fn is_copy<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { - ty.is_copy_modulo_regions(cx.tcx.at(DUMMY_SP), cx.param_env) + ty.is_copy_modulo_regions(cx.tcx, cx.param_env) +} + +/// This checks whether a given type is known to implement Debug. +pub fn has_debug_impl<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + cx.tcx + .get_diagnostic_item(sym::Debug) + .map_or(false, |debug| implements_trait(cx, ty, debug, &[])) } /// Checks whether a type can be partially moved. @@ -165,11 +172,10 @@ pub fn implements_trait_with_env<'tcx>( return false; } let ty_params = tcx.mk_substs(ty_params.iter()); - tcx.infer_ctxt().enter(|infcx| { - infcx - .type_implements_trait(trait_id, ty, ty_params, param_env) - .must_apply_modulo_regions() - }) + let infcx = tcx.infer_ctxt().build(); + infcx + .type_implements_trait(trait_id, ty, ty_params, param_env) + .must_apply_modulo_regions() } /// Checks whether this type implements `Drop`. @@ -235,27 +241,26 @@ fn is_normalizable_helper<'tcx>( } // prevent recursive loops, false-negative is better than endless loop leading to stack overflow cache.insert(ty, false); - let result = cx.tcx.infer_ctxt().enter(|infcx| { - let cause = rustc_middle::traits::ObligationCause::dummy(); - if infcx.at(&cause, param_env).normalize(ty).is_ok() { - match ty.kind() { - ty::Adt(def, substs) => def.variants().iter().all(|variant| { - variant - .fields - .iter() - .all(|field| is_normalizable_helper(cx, param_env, field.ty(cx.tcx, substs), cache)) - }), - _ => ty.walk().all(|generic_arg| match generic_arg.unpack() { - GenericArgKind::Type(inner_ty) if inner_ty != ty => { - is_normalizable_helper(cx, param_env, inner_ty, cache) - }, - _ => true, // if inner_ty == ty, we've already checked it - }), - } - } else { - false + let infcx = cx.tcx.infer_ctxt().build(); + let cause = rustc_middle::traits::ObligationCause::dummy(); + let result = if infcx.at(&cause, param_env).normalize(ty).is_ok() { + match ty.kind() { + ty::Adt(def, substs) => def.variants().iter().all(|variant| { + variant + .fields + .iter() + .all(|field| is_normalizable_helper(cx, param_env, field.ty(cx.tcx, substs), cache)) + }), + _ => ty.walk().all(|generic_arg| match generic_arg.unpack() { + GenericArgKind::Type(inner_ty) if inner_ty != ty => { + is_normalizable_helper(cx, param_env, inner_ty, cache) + }, + _ => true, // if inner_ty == ty, we've already checked it + }), } - }); + } else { + false + }; cache.insert(ty, result); result } @@ -652,21 +657,18 @@ fn sig_for_projection<'tcx>(cx: &LateContext<'tcx>, ty: ProjectionTy<'tcx>) -> O let mut output = None; let lang_items = cx.tcx.lang_items(); - for pred in cx + for (pred, _) in cx .tcx .bound_explicit_item_bounds(ty.item_def_id) - .transpose_iter() - .map(|x| x.map_bound(|(p, _)| p)) + .subst_iter_copied(cx.tcx, ty.substs) { - match pred.0.kind().skip_binder() { + match pred.kind().skip_binder() { PredicateKind::Trait(p) if (lang_items.fn_trait() == Some(p.def_id()) || lang_items.fn_mut_trait() == Some(p.def_id()) || lang_items.fn_once_trait() == Some(p.def_id())) => { - let i = pred - .map_bound(|pred| pred.kind().rebind(p.trait_ref.substs.type_at(1))) - .subst(cx.tcx, ty.substs); + let i = pred.kind().rebind(p.trait_ref.substs.type_at(1)); if inputs.map_or(false, |inputs| inputs != i) { // Multiple different fn trait impls. Is this even allowed? @@ -679,10 +681,7 @@ fn sig_for_projection<'tcx>(cx: &LateContext<'tcx>, ty: ProjectionTy<'tcx>) -> O // Multiple different fn trait impls. Is this even allowed? return None; } - output = Some( - pred.map_bound(|pred| pred.kind().rebind(p.term.ty().unwrap())) - .subst(cx.tcx, ty.substs), - ); + output = pred.kind().rebind(p.term.ty()).transpose(); }, _ => (), } diff --git a/src/tools/clippy/clippy_utils/src/usage.rs b/src/tools/clippy/clippy_utils/src/usage.rs index 3af5dfb62..000fb51c0 100644 --- a/src/tools/clippy/clippy_utils/src/usage.rs +++ b/src/tools/clippy/clippy_utils/src/usage.rs @@ -1,15 +1,16 @@ use crate as utils; -use crate::visitors::{expr_visitor, expr_visitor_no_bodies}; +use crate::visitors::{for_each_expr, for_each_expr_with_closures, Descend}; +use core::ops::ControlFlow; use rustc_hir as hir; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::HirIdSet; use rustc_hir::{Expr, ExprKind, HirId, Node}; +use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::LateContext; use rustc_middle::hir::nested_filter; use rustc_middle::mir::FakeReadCause; use rustc_middle::ty; -use rustc_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; /// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined. pub fn mutated_variables<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> Option<HirIdSet> { @@ -17,16 +18,15 @@ pub fn mutated_variables<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> used_mutably: HirIdSet::default(), skip: false, }; - cx.tcx.infer_ctxt().enter(|infcx| { - ExprUseVisitor::new( - &mut delegate, - &infcx, - expr.hir_id.owner, - cx.param_env, - cx.typeck_results(), - ) - .walk_expr(expr); - }); + let infcx = cx.tcx.infer_ctxt().build(); + ExprUseVisitor::new( + &mut delegate, + &infcx, + expr.hir_id.owner.def_id, + cx.param_env, + cx.typeck_results(), + ) + .walk_expr(expr); if delegate.skip { return None; @@ -73,7 +73,12 @@ impl<'tcx> Delegate<'tcx> for MutVarsDelegate { self.update(cmt); } - fn fake_read(&mut self, _: &rustc_typeck::expr_use_visitor::PlaceWithHirId<'tcx>, _: FakeReadCause, _: HirId) {} + fn fake_read( + &mut self, + _: &rustc_hir_typeck::expr_use_visitor::PlaceWithHirId<'tcx>, + _: FakeReadCause, + _: HirId, + ) {} } pub struct ParamBindingIdCollector { @@ -142,28 +147,17 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for BindingUsageFinder<'a, 'tcx> { } pub fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool { - let mut seen_return_break_continue = false; - expr_visitor_no_bodies(|ex| { - if seen_return_break_continue { - return false; - } - match &ex.kind { - ExprKind::Ret(..) | ExprKind::Break(..) | ExprKind::Continue(..) => { - seen_return_break_continue = true; - }, + for_each_expr(expression, |e| { + match e.kind { + ExprKind::Ret(..) | ExprKind::Break(..) | ExprKind::Continue(..) => ControlFlow::Break(()), // Something special could be done here to handle while or for loop // desugaring, as this will detect a break if there's a while loop // or a for loop inside the expression. - _ => { - if ex.span.from_expansion() { - seen_return_break_continue = true; - } - }, + _ if e.span.from_expansion() => ControlFlow::Break(()), + _ => ControlFlow::Continue(()), } - !seen_return_break_continue }) - .visit_expr(expression); - seen_return_break_continue + .is_some() } pub fn local_used_after_expr(cx: &LateContext<'_>, local_id: HirId, after: &Expr<'_>) -> bool { @@ -194,23 +188,16 @@ pub fn local_used_after_expr(cx: &LateContext<'_>, local_id: HirId, after: &Expr return true; } - let mut used_after_expr = false; let mut past_expr = false; - expr_visitor(cx, |expr| { - if used_after_expr { - return false; - } - - if expr.hir_id == after.hir_id { + for_each_expr_with_closures(cx, block, |e| { + if e.hir_id == after.hir_id { past_expr = true; - return false; - } - - if past_expr && utils::path_to_local_id(expr, local_id) { - used_after_expr = true; + ControlFlow::Continue(Descend::No) + } else if past_expr && utils::path_to_local_id(e, local_id) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(Descend::Yes) } - !used_after_expr }) - .visit_block(block); - used_after_expr + .is_some() } diff --git a/src/tools/clippy/clippy_utils/src/visitors.rs b/src/tools/clippy/clippy_utils/src/visitors.rs index 232d57190..d4294f18f 100644 --- a/src/tools/clippy/clippy_utils/src/visitors.rs +++ b/src/tools/clippy/clippy_utils/src/visitors.rs @@ -5,14 +5,13 @@ use rustc_hir as hir; use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::intravisit::{self, walk_block, walk_expr, Visitor}; use rustc_hir::{ - Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, Let, Pat, QPath, Stmt, UnOp, - UnsafeSource, Unsafety, + AnonConst, Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, Let, Pat, QPath, + Stmt, UnOp, UnsafeSource, Unsafety, }; use rustc_lint::LateContext; -use rustc_middle::hir::map::Map; use rustc_middle::hir::nested_filter; use rustc_middle::ty::adjustment::Adjust; -use rustc_middle::ty::{self, Ty, TypeckResults}; +use rustc_middle::ty::{self, Ty, TyCtxt, TypeckResults}; use rustc_span::Span; mod internal { @@ -48,6 +47,26 @@ impl Continue for Descend { } } +/// A type which can be visited. +pub trait Visitable<'tcx> { + /// Calls the corresponding `visit_*` function on the visitor. + fn visit<V: Visitor<'tcx>>(self, visitor: &mut V); +} +macro_rules! visitable_ref { + ($t:ident, $f:ident) => { + impl<'tcx> Visitable<'tcx> for &'tcx $t<'tcx> { + fn visit<V: Visitor<'tcx>>(self, visitor: &mut V) { + visitor.$f(self); + } + } + }; +} +visitable_ref!(Arm, visit_arm); +visitable_ref!(Block, visit_block); +visitable_ref!(Body, visit_body); +visitable_ref!(Expr, visit_expr); +visitable_ref!(Stmt, visit_stmt); + /// Calls the given function once for each expression contained. This does not enter any bodies or /// nested items. pub fn for_each_expr<'tcx, B, C: Continue>( @@ -82,57 +101,63 @@ pub fn for_each_expr<'tcx, B, C: Continue>( v.res } -/// Convenience method for creating a `Visitor` with just `visit_expr` overridden and nested -/// bodies (i.e. closures) are visited. -/// If the callback returns `true`, the expr just provided to the callback is walked. -#[must_use] -pub fn expr_visitor<'tcx>(cx: &LateContext<'tcx>, f: impl FnMut(&'tcx Expr<'tcx>) -> bool) -> impl Visitor<'tcx> { - struct V<'tcx, F> { - hir: Map<'tcx>, +/// Calls the given function once for each expression contained. This will enter bodies, but not +/// nested items. +pub fn for_each_expr_with_closures<'tcx, B, C: Continue>( + cx: &LateContext<'tcx>, + node: impl Visitable<'tcx>, + f: impl FnMut(&'tcx Expr<'tcx>) -> ControlFlow<B, C>, +) -> Option<B> { + struct V<'tcx, B, F> { + tcx: TyCtxt<'tcx>, f: F, + res: Option<B>, } - impl<'tcx, F: FnMut(&'tcx Expr<'tcx>) -> bool> Visitor<'tcx> for V<'tcx, F> { + impl<'tcx, B, C: Continue, F: FnMut(&'tcx Expr<'tcx>) -> ControlFlow<B, C>> Visitor<'tcx> for V<'tcx, B, F> { type NestedFilter = nested_filter::OnlyBodies; fn nested_visit_map(&mut self) -> Self::Map { - self.hir + self.tcx.hir() } - fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - if (self.f)(expr) { - walk_expr(self, expr); + fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) { + if self.res.is_some() { + return; } - } - } - V { hir: cx.tcx.hir(), f } -} - -/// Convenience method for creating a `Visitor` with just `visit_expr` overridden and nested -/// bodies (i.e. closures) are not visited. -/// If the callback returns `true`, the expr just provided to the callback is walked. -#[must_use] -pub fn expr_visitor_no_bodies<'tcx>(f: impl FnMut(&'tcx Expr<'tcx>) -> bool) -> impl Visitor<'tcx> { - struct V<F>(F); - impl<'tcx, F: FnMut(&'tcx Expr<'tcx>) -> bool> Visitor<'tcx> for V<F> { - fn visit_expr(&mut self, e: &'tcx Expr<'_>) { - if (self.0)(e) { - walk_expr(self, e); + match (self.f)(e) { + ControlFlow::Continue(c) if c.descend() => walk_expr(self, e), + ControlFlow::Break(b) => self.res = Some(b), + ControlFlow::Continue(_) => (), } } + + // Only walk closures + fn visit_anon_const(&mut self, _: &'tcx AnonConst) {} + // Avoid unnecessary `walk_*` calls. + fn visit_ty(&mut self, _: &'tcx hir::Ty<'tcx>) {} + fn visit_pat(&mut self, _: &'tcx Pat<'tcx>) {} + fn visit_qpath(&mut self, _: &'tcx QPath<'tcx>, _: HirId, _: Span) {} + // Avoid monomorphising all `visit_*` functions. + fn visit_nested_item(&mut self, _: ItemId) {} } - V(f) + let mut v = V { + tcx: cx.tcx, + f, + res: None, + }; + node.visit(&mut v); + v.res } /// returns `true` if expr contains match expr desugared from try fn contains_try(expr: &hir::Expr<'_>) -> bool { - let mut found = false; - expr_visitor_no_bodies(|e| { - if !found { - found = matches!(e.kind, hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar)); + for_each_expr(expr, |e| { + if matches!(e.kind, hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar)) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) } - !found }) - .visit_expr(expr); - found + .is_some() } pub fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_>, expr: &'hir hir::Expr<'hir>, callback: F) -> bool @@ -228,68 +253,29 @@ where } } -/// A type which can be visited. -pub trait Visitable<'tcx> { - /// Calls the corresponding `visit_*` function on the visitor. - fn visit<V: Visitor<'tcx>>(self, visitor: &mut V); -} -macro_rules! visitable_ref { - ($t:ident, $f:ident) => { - impl<'tcx> Visitable<'tcx> for &'tcx $t<'tcx> { - fn visit<V: Visitor<'tcx>>(self, visitor: &mut V) { - visitor.$f(self); - } - } - }; -} -visitable_ref!(Arm, visit_arm); -visitable_ref!(Block, visit_block); -visitable_ref!(Body, visit_body); -visitable_ref!(Expr, visit_expr); -visitable_ref!(Stmt, visit_stmt); - -// impl<'tcx, I: IntoIterator> Visitable<'tcx> for I -// where -// I::Item: Visitable<'tcx>, -// { -// fn visit<V: Visitor<'tcx>>(self, visitor: &mut V) { -// for x in self { -// x.visit(visitor); -// } -// } -// } - /// Checks if the given resolved path is used in the given body. pub fn is_res_used(cx: &LateContext<'_>, res: Res, body: BodyId) -> bool { - let mut found = false; - expr_visitor(cx, |e| { - if found { - return false; - } - + for_each_expr_with_closures(cx, cx.tcx.hir().body(body).value, |e| { if let ExprKind::Path(p) = &e.kind { if cx.qpath_res(p, e.hir_id) == res { - found = true; + return ControlFlow::Break(()); } } - !found + ControlFlow::Continue(()) }) - .visit_expr(cx.tcx.hir().body(body).value); - found + .is_some() } /// Checks if the given local is used. pub fn is_local_used<'tcx>(cx: &LateContext<'tcx>, visitable: impl Visitable<'tcx>, id: HirId) -> bool { - let mut is_used = false; - let mut visitor = expr_visitor(cx, |expr| { - if !is_used { - is_used = path_to_local_id(expr, id); + for_each_expr_with_closures(cx, visitable, |e| { + if path_to_local_id(e, id) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) } - !is_used - }); - visitable.visit(&mut visitor); - drop(visitor); - is_used + }) + .is_some() } /// Checks if the given expression is a constant. |