From 8df3e4e4c98a77484d8704131f0f48fcf768ea8c Mon Sep 17 00:00:00 2001 From: Urgau Date: Tue, 27 Feb 2024 22:16:28 +0100 Subject: [PATCH] Move completely away from cx.tcx.parent and only use the HIR --- compiler/rustc_lint/src/non_local_def.rs | 91 +++++++++++++----------- 1 file changed, 51 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_lint/src/non_local_def.rs b/compiler/rustc_lint/src/non_local_def.rs index 0c15c78c95cdf..f81a93147e011 100644 --- a/compiler/rustc_lint/src/non_local_def.rs +++ b/compiler/rustc_lint/src/non_local_def.rs @@ -1,5 +1,6 @@ -use rustc_hir::{Body, Item, ItemKind, OwnerNode, Path, QPath, TyKind}; -use rustc_span::def_id::{DefId, LOCAL_CRATE}; +use rustc_hir::def_id::LocalDefId; +use rustc_hir::{Body, Item, ItemKind, OwnerId, OwnerNode, Path, QPath, TyKind}; +use rustc_span::def_id::LOCAL_CRATE; use rustc_span::{sym, symbol::kw, symbol::Ident, ExpnKind, MacroKind}; use crate::lints::{NonLocalDefinitionsCargoUpdateNote, NonLocalDefinitionsDiag}; @@ -65,11 +66,13 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { return; } - let mut parent_node = { - let mut parent_node_cache = None; + let mut parent_owner = { + let mut parent_owner_cache = None; move || { - *parent_node_cache.get_or_insert_with(|| { - cx.tcx.hir().parent_owner_iter(item.hir_id()).next().unwrap().1 + *parent_owner_cache.get_or_insert_with(|| { + // Unwrap safety: can only panic when reaching the crate root + // but we made sure above that we are not at crate root. + cx.tcx.hir().parent_owner_iter(item.hir_id()).next().unwrap() }) } }; @@ -107,12 +110,12 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { // If that's the case this means that this impl block declaration // is using local items and so we don't lint on it. - let mut parent_node_is_anon_const = { - let mut parent_node_is_anon_const = None; + let mut parent_owner_is_anon_const = { + let mut parent_owner_is_anon_const = None; move || { - *parent_node_is_anon_const.get_or_insert_with(|| { + *parent_owner_is_anon_const.get_or_insert_with(|| { matches!( - parent_node(), + parent_owner().1, OwnerNode::Item(Item { ident: Ident { name: kw::Underscore, .. }, kind: ItemKind::Const(..), @@ -122,18 +125,20 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { }) } }; - let mut local_parent = { - let mut local_parent_cache = None; - move || { - *local_parent_cache - .get_or_insert_with(|| cx.tcx.parent(item.owner_id.to_def_id())) - } - }; let mut extra_local_parent = { let mut extra_parent_cache = None; - move |did| { + move || { *extra_parent_cache.get_or_insert_with(|| { - parent_node_is_anon_const().then(|| cx.tcx.parent(did)) + parent_owner_is_anon_const() + .then(|| { + cx.tcx + .hir() + .parent_owner_iter(item.hir_id()) + .skip(1) + .next() + .map(|(owner_id, _owner_node)| owner_id.def_id) + }) + .flatten() }) } }; @@ -142,14 +147,14 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { TyKind::Path(QPath::Resolved(_, ty_path)) => path_has_local_parent( ty_path, cx, - &mut local_parent, + &mut parent_owner, &mut extra_local_parent, ), TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => { path_has_local_parent( principle_poly_trait_ref.trait_ref.path, cx, - &mut local_parent, + &mut parent_owner, &mut extra_local_parent, ) } @@ -176,7 +181,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { path_has_local_parent( of_trait.path, cx, - &mut local_parent, + &mut parent_owner, &mut extra_local_parent, ) }) @@ -186,12 +191,12 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { // this impl definition is a non-local definition and so we lint on it. if !(self_ty_has_local_parent || of_trait_has_local_parent) { // Per RFC we (currently) ignore anon-const (`const _: Ty = ...`) in top-level module. - if parent_node_is_anon_const() && self.body_depth == 1 { + if parent_owner_is_anon_const() && self.body_depth == 1 { return; } let const_anon = if self.body_depth == 1 - && let OwnerNode::Item(item) = parent_node() + && let OwnerNode::Item(item) = parent_owner().1 && let ItemKind::Const(ty, _, _) = item.kind && let TyKind::Tup(&[]) = ty.kind { @@ -200,13 +205,15 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { None }; + let parent_owner = parent_owner().1; + cx.emit_span_lint( NON_LOCAL_DEFINITIONS, item.span, NonLocalDefinitionsDiag::Impl { depth: self.body_depth, body_kind_descr: "?" /* FIXME: cx.tcx.def_kind_descr(parent_def_kind, parent) */, - body_name: parent_node() + body_name: parent_owner .ident() .map(|s| s.name.to_ident_string()) .unwrap_or_else(|| "".to_string()), @@ -219,13 +226,15 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { ItemKind::Macro(_macro, MacroKind::Bang) if cx.tcx.has_attr(item.owner_id.def_id, sym::macro_export) => { + let parent_owner = parent_owner().1; + cx.emit_span_lint( NON_LOCAL_DEFINITIONS, item.span, NonLocalDefinitionsDiag::MacroRules { depth: self.body_depth, body_kind_descr: "?" /* FIXME: cx.tcx.def_kind_descr(parent_def_kind, parent) */, - body_name: parent_node() + body_name: parent_owner .ident() .map(|s| s.name.to_ident_string()) .unwrap_or_else(|| "".to_string()), @@ -247,20 +256,22 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { /// std::convert::PartialEq> /// ^^^^^^^^^^^^^^^^^^^^^^^ /// ``` -fn path_has_local_parent( +fn path_has_local_parent<'tcx>( path: &Path<'_>, - cx: &LateContext<'_>, - local_parent: &mut impl FnMut() -> DefId, - extra_local_parent: &mut impl FnMut(DefId) -> Option, + cx: &LateContext<'tcx>, + local_parent: &mut impl FnMut() -> (OwnerId, OwnerNode<'tcx>), + extra_local_parent: &mut impl FnMut() -> Option, ) -> bool { - if let Some(did) = path.res.opt_def_id() { - if !did.is_local() { - false - } else { - let res_parent = cx.tcx.parent(did); - res_parent == local_parent() || Some(res_parent) == extra_local_parent(local_parent()) - } - } else { - true - } + let Some(res_did) = path.res.opt_def_id() else { + return true; + }; + let Some(did) = res_did.as_local() else { + return false; + }; + let Some(hir_id) = cx.tcx.opt_local_def_id_to_hir_id(did) else { + return true; + }; + let owner_id = cx.tcx.hir().get_parent_item(hir_id); + let res_parent = owner_id.def_id; + res_parent == local_parent().0.def_id || Some(res_parent) == extra_local_parent() }