Skip to content

Commit

Permalink
Only use the HIR, never tcx.parent
Browse files Browse the repository at this point in the history
  • Loading branch information
@Urgau
Urgau committed Feb 29, 2024
1 parent 7c21492 commit 87a3a57
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 83 deletions.
102 changes: 70 additions & 32 deletions compiler/rustc_lint/src/non_local_def.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use rustc_hir::{def::DefKind, Body, Item, ItemKind, Node, Path, QPath, TyKind};
use rustc_span::def_id::{DefId, LOCAL_CRATE};
use rustc_hir::def_id::LocalDefId;
use rustc_hir::{Body, Item, ItemKind, Path, QPath, TyKind};
use rustc_hir::{OwnerId, OwnerNode};
use rustc_span::def_id::LOCAL_CRATE;
use rustc_span::symbol::Ident;
use rustc_span::{sym, symbol::kw, ExpnKind, MacroKind};

use crate::lints::{NonLocalDefinitionsCargoUpdateNote, NonLocalDefinitionsDiag};
Expand Down Expand Up @@ -72,9 +75,13 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
};
}

lazy!(parent = || cx.tcx.parent(item.owner_id.def_id.into()));
lazy!(parent_def_kind = || cx.tcx.def_kind(parent()));
lazy!(parent_opt_item_name = || cx.tcx.opt_item_name(parent()));
lazy!(
parent_owner = || {
// 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()
}
);

let cargo_update = || {
let oexpn = item.span.ctxt().outer_expn_data();
Expand Down Expand Up @@ -110,22 +117,37 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
// is using local items and so we don't lint on it.

lazy!(
parent_is_anon_const = || parent_def_kind() == DefKind::Const
&& parent_opt_item_name() == Some(kw::Underscore)
parent_owner_is_anon_const = || matches!(
parent_owner().1,
OwnerNode::Item(Item {
ident: Ident { name: kw::Underscore, .. },
kind: ItemKind::Const(..),
..
})
)
);
lazy!(
extra_local_parent = || parent_is_anon_const().then(|| cx.tcx.parent(parent()))
extra_local_parent = || 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()
);

let self_ty_has_local_parent = match impl_.self_ty.kind {
TyKind::Path(QPath::Resolved(_, ty_path)) => {
path_has_local_parent(ty_path, cx, &parent, &extra_local_parent)
path_has_local_parent(ty_path, cx, &parent_owner, &extra_local_parent)
}
TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => {
path_has_local_parent(
principle_poly_trait_ref.trait_ref.path,
cx,
&parent,
&parent_owner,
&extra_local_parent,
)
}
Expand All @@ -150,7 +172,12 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
|| impl_
.of_trait
.map(|of_trait| {
path_has_local_parent(of_trait.path, cx, &parent, &extra_local_parent)
path_has_local_parent(
of_trait.path,
cx,
&parent_owner,
&extra_local_parent,
)
})
.unwrap_or(false);

Expand All @@ -159,15 +186,13 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
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 self.body_depth == 1 && parent_is_anon_const() {
if self.body_depth == 1 && parent_owner_is_anon_const() {
return;
}

let const_anon = if self.body_depth == 1
&& parent_def_kind() == DefKind::Const
&& parent_opt_item_name() != Some(kw::Underscore)
&& let Some(parent) = parent().as_local()
&& let Node::Item(item) = cx.tcx.hir_node_by_def_id(parent)
&& let OwnerNode::Item(item) = parent_owner().1
&& item.ident.name != kw::Underscore
&& let ItemKind::Const(ty, _, _) = item.kind
&& let TyKind::Tup(&[]) = ty.kind
{
Expand All @@ -181,9 +206,10 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
item.span,
NonLocalDefinitionsDiag::Impl {
depth: self.body_depth,
body_kind_descr: cx.tcx.def_kind_descr(parent_def_kind(), parent()),
body_name: parent_opt_item_name()
.map(|s| s.to_ident_string())
body_kind_descr: "?" /*cx.tcx.def_kind_descr(parent_def_kind(), parent())*/,
body_name: parent_owner().1
.ident()
.map(|s| s.name.to_ident_string())
.unwrap_or_else(|| "<unnameable>".to_string()),
cargo_update: cargo_update(),
const_anon,
Expand All @@ -199,9 +225,11 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
item.span,
NonLocalDefinitionsDiag::MacroRules {
depth: self.body_depth,
body_kind_descr: cx.tcx.def_kind_descr(parent_def_kind(), parent()),
body_name: parent_opt_item_name()
.map(|s| s.to_ident_string())
body_kind_descr: "?", /*cx.tcx.def_kind_descr(parent_def_kind(), parent())*/
body_name: parent_owner()
.1
.ident()
.map(|s| s.name.to_ident_string())
.unwrap_or_else(|| "<unnameable>".to_string()),
cargo_update: cargo_update(),
},
Expand All @@ -221,16 +249,26 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
/// std::convert::PartialEq<Foo<Bar>>
/// ^^^^^^^^^^^^^^^^^^^^^^^
/// ```
fn path_has_local_parent(
fn path_has_local_parent<'tcx>(
path: &Path<'_>,
cx: &LateContext<'_>,
parent: impl Fn() -> DefId,
extra_local_parent: impl Fn() -> Option<DefId>,
cx: &LateContext<'tcx>,
local_parent: impl Fn() -> (OwnerId, OwnerNode<'tcx>),
extra_local_parent: impl Fn() -> Option<LocalDefId>,
) -> bool {
path.res.opt_def_id().is_some_and(|did| {
did.is_local() && {
let res_parent = cx.tcx.parent(did);
res_parent == parent() || Some(res_parent) == extra_local_parent()
}
})
let Some(res_did) = path.res.opt_def_id() else {
return true;
};
let Some(did) = res_did.as_local() else {
return false;
};

let res_parent = {
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);
owner_id.def_id
};

res_parent == local_parent().0.def_id || Some(res_parent) == extra_local_parent()
}
Loading

0 comments on commit 87a3a57

Please sign in to comment.