Overhaul UsePath.

`UsePath` contains a `SmallVec<[Res; 3]>`. This holds up to three `Res`
results, one per namespace (type, value, or macro). `lower_import_res`
takes a `PerNS<Option<Res<NodeId>>>` result and lowers it into the
`SmallVec`. This is pretty weird. The input `PerNS` makes it clear which
`Res` belongs to which namespace, but the `SmallVec` throws that
information away.

And code that operates on the `SmallVec` tends to use iteration (or even
just grabbing the first entry!) without knowing which namespace the
`Res` belongs to. Even weirder! Also, `SmallVec` is an overly flexible
type to use here, because it can contain any number of elements (even
though it's optimized for 3 in this case).

This commit changes `UsePath` so it also contains a
`PerNS<Option<Res<HirId>>>`. This type preserves more information and is
more self-documenting. The commit also changes a lot of the use sites to
access the result for a particular namespace. E.g. if you're looking up
a trait, it will be in the `Res` for the type namespace if it's present;
it's silly to look in the `Res` for the value namespace or macro
namespace. Overall I find the new code much easier to understand.

However, some use sites still iterate. These now use `present_items`
because that filters out the `None` results.

Also, `redundant_pub_crate.rs` gets a bigger change. A
`UseKind:ListStem` item gets no `Res` results, which means the old `all`
call in `is_not_macro_export` would succeed (because `all` succeeds on
an empty iterator) and the `ListStem` would be ignored. This is what we
want, but was more by luck than design. The new code detects `ListStem`
explicitly. The commit generalizes the name of that function
accordingly.

Finally, the commit also removes the `use_path` arena, because
`PerNS<Option<Res>>` impls `Copy` (unlike `SmallVec`) and it can be
allocated in the arena shared by all `Copy` types.
This commit is contained in:
Nicholas Nethercote 2025-05-30 01:37:48 +10:00
parent 176c34a946
commit 8747ccbcdf
23 changed files with 112 additions and 86 deletions

View file

@ -3,10 +3,11 @@ use rustc_ast::ptr::P;
use rustc_ast::visit::AssocCtxt;
use rustc_ast::*;
use rustc_errors::ErrorGuaranteed;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def::{DefKind, PerNS, Res};
use rustc_hir::def_id::{CRATE_DEF_ID, LocalDefId};
use rustc_hir::{self as hir, HirId, LifetimeSource, PredicateOrigin};
use rustc_index::{IndexSlice, IndexVec};
use rustc_middle::span_bug;
use rustc_middle::ty::{ResolverAstLowering, TyCtxt};
use rustc_span::edit_distance::find_best_match_for_name;
use rustc_span::{DUMMY_SP, DesugaringKind, Ident, Span, Symbol, kw, sym};
@ -527,7 +528,22 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
UseTreeKind::Glob => {
let res = self.expect_full_res(id);
let res = smallvec![self.lower_res(res)];
let res = self.lower_res(res);
// Put the result in the appropriate namespace.
let res = match res {
Res::Def(DefKind::Mod | DefKind::Trait, _) => {
PerNS { type_ns: Some(res), value_ns: None, macro_ns: None }
}
Res::Def(DefKind::Enum, _) => {
PerNS { type_ns: None, value_ns: Some(res), macro_ns: None }
}
Res::Err => {
// Propagate the error to all namespaces, just to be sure.
let err = Some(Res::Err);
PerNS { type_ns: err, value_ns: err, macro_ns: err }
}
_ => span_bug!(path.span, "bad glob res {:?}", res),
};
let path = Path { segments, span: path.span, tokens: None };
let path = self.lower_use_path(res, &path, ParamMode::Explicit);
hir::ItemKind::Use(path, hir::UseKind::Glob)
@ -601,7 +617,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
} else {
// For non-empty lists we can just drop all the data, the prefix is already
// present in HIR as a part of nested imports.
self.arena.alloc(hir::UsePath { res: smallvec![], segments: &[], span })
self.arena.alloc(hir::UsePath { res: PerNS::default(), segments: &[], span })
};
hir::ItemKind::Use(path, hir::UseKind::ListStem)
}

View file

@ -64,7 +64,7 @@ use rustc_middle::ty::{ResolverAstLowering, TyCtxt};
use rustc_session::parse::{add_feature_diagnostics, feature_err};
use rustc_span::symbol::{Ident, Symbol, kw, sym};
use rustc_span::{DUMMY_SP, DesugaringKind, Span};
use smallvec::{SmallVec, smallvec};
use smallvec::SmallVec;
use thin_vec::ThinVec;
use tracing::{debug, instrument, trace};
@ -705,14 +705,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.resolver.get_partial_res(id).map_or(Res::Err, |pr| pr.expect_full_res())
}
fn lower_import_res(&mut self, id: NodeId, span: Span) -> SmallVec<[Res; 3]> {
let res = self.resolver.get_import_res(id).present_items();
let res: SmallVec<_> = res.map(|res| self.lower_res(res)).collect();
if res.is_empty() {
fn lower_import_res(&mut self, id: NodeId, span: Span) -> PerNS<Option<Res>> {
let per_ns = self.resolver.get_import_res(id);
let per_ns = per_ns.map(|res| res.map(|res| self.lower_res(res)));
if per_ns.is_empty() {
// Propagate the error to all namespaces, just to be sure.
self.dcx().span_delayed_bug(span, "no resolution for an import");
return smallvec![Res::Err];
let err = Some(Res::Err);
return PerNS { type_ns: err, value_ns: err, macro_ns: err };
}
res
per_ns
}
fn make_lang_item_qpath(

View file

@ -1,13 +1,13 @@
use std::sync::Arc;
use rustc_ast::{self as ast, *};
use rustc_hir::def::{DefKind, PartialRes, Res};
use rustc_hir::def::{DefKind, PartialRes, PerNS, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir, GenericArg};
use rustc_middle::{span_bug, ty};
use rustc_session::parse::add_feature_diagnostics;
use rustc_span::{BytePos, DUMMY_SP, DesugaringKind, Ident, Span, Symbol, sym};
use smallvec::{SmallVec, smallvec};
use smallvec::smallvec;
use tracing::{debug, instrument};
use super::errors::{
@ -226,11 +226,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
pub(crate) fn lower_use_path(
&mut self,
res: SmallVec<[Res; 3]>,
res: PerNS<Option<Res>>,
p: &Path,
param_mode: ParamMode,
) -> &'hir hir::UsePath<'hir> {
assert!((1..=3).contains(&res.len()));
assert!(!res.is_empty());
self.arena.alloc(hir::UsePath {
res,
segments: self.arena.alloc_from_iter(p.segments.iter().map(|segment| {

View file

@ -8,7 +8,6 @@ macro_rules! arena_types {
[] asm_template: rustc_ast::InlineAsmTemplatePiece,
[] attribute: rustc_hir::Attribute,
[] owner_info: rustc_hir::OwnerInfo<'tcx>,
[] use_path: rustc_hir::UsePath<'tcx>,
[] lit: rustc_hir::Lit,
[] macro_def: rustc_ast::MacroDef,
]);

View file

@ -584,7 +584,7 @@ impl<CTX: crate::HashStableContext> ToStableHashKey<CTX> for Namespace {
}
/// Just a helper separate structure for each namespace.
#[derive(Copy, Clone, Default, Debug)]
#[derive(Copy, Clone, Default, Debug, HashStable_Generic)]
pub struct PerNS<T> {
pub value_ns: T,
pub type_ns: T,
@ -596,10 +596,16 @@ impl<T> PerNS<T> {
PerNS { value_ns: f(self.value_ns), type_ns: f(self.type_ns), macro_ns: f(self.macro_ns) }
}
/// Note: Do you really want to use this? Often you know which namespace a
/// name will belong in, and you can consider just that namespace directly,
/// rather than iterating through all of them.
pub fn into_iter(self) -> IntoIter<T, 3> {
[self.value_ns, self.type_ns, self.macro_ns].into_iter()
}
/// Note: Do you really want to use this? Often you know which namespace a
/// name will belong in, and you can consider just that namespace directly,
/// rather than iterating through all of them.
pub fn iter(&self) -> IntoIter<&T, 3> {
[&self.value_ns, &self.type_ns, &self.macro_ns].into_iter()
}
@ -634,6 +640,10 @@ impl<T> PerNS<Option<T>> {
}
/// Returns an iterator over the items which are `Some`.
///
/// Note: Do you really want to use this? Often you know which namespace a
/// name will belong in, and you can consider just that namespace directly,
/// rather than iterating through all of them.
pub fn present_items(self) -> impl Iterator<Item = T> {
[self.type_ns, self.value_ns, self.macro_ns].into_iter().flatten()
}

View file

@ -30,7 +30,7 @@ use thin_vec::ThinVec;
use tracing::debug;
use crate::LangItem;
use crate::def::{CtorKind, DefKind, Res};
use crate::def::{CtorKind, DefKind, PerNS, Res};
use crate::def_id::{DefId, LocalDefIdMap};
pub(crate) use crate::hir_id::{HirId, ItemLocalId, ItemLocalMap, OwnerId};
use crate::intravisit::{FnKind, VisitorExt};
@ -347,7 +347,7 @@ pub struct Path<'hir, R = Res> {
}
/// Up to three resolutions for type, value and macro namespaces.
pub type UsePath<'hir> = Path<'hir, SmallVec<[Res; 3]>>;
pub type UsePath<'hir> = Path<'hir, PerNS<Option<Res>>>;
impl Path<'_> {
pub fn is_global(&self) -> bool {

View file

@ -1148,7 +1148,7 @@ pub fn walk_use<'v, V: Visitor<'v>>(
hir_id: HirId,
) -> V::Result {
let UsePath { segments, ref res, span } = *path;
for &res in res {
for res in res.present_items() {
try_visit!(visitor.visit_path(&Path { segments, res, span }, hir_id));
}
V::Result::output()

View file

@ -338,7 +338,9 @@ impl<'tcx> LateLintPass<'tcx> for TypeIr {
cx.emit_span_lint(USAGE_OF_TYPE_IR_INHERENT, seg.ident.span, TypeIrInherentUsage);
}
// Final path resolutions, like `use rustc_type_ir::inherent`
else if path.res.iter().any(|&res| is_mod_inherent(res)) {
else if let Some(type_ns) = path.res.type_ns
&& is_mod_inherent(type_ns)
{
cx.emit_span_lint(
USAGE_OF_TYPE_IR_INHERENT,
path.segments.last().unwrap().ident.span,
@ -351,7 +353,8 @@ impl<'tcx> LateLintPass<'tcx> for TypeIr {
(segment.ident.span, item.kind.ident().unwrap().span, "*")
}
[.., segment]
if path.res.iter().any(|&res| is_mod_inherent(res))
if let Some(type_ns) = path.res.type_ns
&& is_mod_inherent(type_ns)
&& let rustc_hir::UseKind::Single(ident) = kind =>
{
let (lo, snippet) =

View file

@ -1,4 +1,3 @@
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{self as hir};
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::kw;
@ -47,17 +46,15 @@ declare_lint_pass!(UnqualifiedLocalImports => [UNQUALIFIED_LOCAL_IMPORTS]);
impl<'tcx> LateLintPass<'tcx> for UnqualifiedLocalImports {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
let hir::ItemKind::Use(path, _kind) = item.kind else { return };
// `path` has three resolutions for the type, module, value namespaces.
// Check if any of them qualifies: local crate, and not a macro.
// (Macros can't be imported any other way so we don't complain about them.)
let is_local_import = |res: &Res| {
matches!(
res,
hir::def::Res::Def(def_kind, def_id)
if def_id.is_local() && !matches!(def_kind, DefKind::Macro(_)),
)
};
if !path.res.iter().any(is_local_import) {
// Check the type and value namespace resolutions for a local crate.
let is_local_import = matches!(
path.res.type_ns,
Some(hir::def::Res::Def(_, def_id)) if def_id.is_local()
) || matches!(
path.res.value_ns,
Some(hir::def::Res::Def(_, def_id)) if def_id.is_local()
);
if !is_local_import {
return;
}
// So this does refer to something local. Let's check whether it starts with `self`,

View file

@ -1047,26 +1047,21 @@ fn find_fallback_pattern_typo<'tcx>(
let hir::ItemKind::Use(path, _) = item.kind else {
continue;
};
for res in &path.res {
if let Res::Def(DefKind::Const, id) = res
&& infcx.can_eq(param_env, ty, cx.tcx.type_of(id).instantiate_identity())
{
if cx.tcx.visibility(id).is_accessible_from(parent, cx.tcx) {
// The original const is accessible, suggest using it directly.
let item_name = cx.tcx.item_name(*id);
accessible.push(item_name);
accessible_path.push(with_no_trimmed_paths!(cx.tcx.def_path_str(id)));
} else if cx
.tcx
.visibility(item.owner_id)
.is_accessible_from(parent, cx.tcx)
{
// The const is accessible only through the re-export, point at
// the `use`.
let ident = item.kind.ident().unwrap();
imported.push(ident.name);
imported_spans.push(ident.span);
}
if let Some(value_ns) = path.res.value_ns
&& let Res::Def(DefKind::Const, id) = value_ns
&& infcx.can_eq(param_env, ty, cx.tcx.type_of(id).instantiate_identity())
{
if cx.tcx.visibility(id).is_accessible_from(parent, cx.tcx) {
// The original const is accessible, suggest using it directly.
let item_name = cx.tcx.item_name(id);
accessible.push(item_name);
accessible_path.push(with_no_trimmed_paths!(cx.tcx.def_path_str(id)));
} else if cx.tcx.visibility(item.owner_id).is_accessible_from(parent, cx.tcx) {
// The const is accessible only through the re-export, point at
// the `use`.
let ident = item.kind.ident().unwrap();
imported.push(ident.name);
imported_spans.push(ident.span);
}
}
}

View file

@ -132,7 +132,7 @@ impl<'tcx> Visitor<'tcx> for ExportableItemCollector<'tcx> {
self.add_exportable(def_id);
}
hir::ItemKind::Use(path, _) => {
for res in &path.res {
for res in path.res.present_items() {
// Only local items are exportable.
if let Some(res_id) = res.opt_def_id()
&& let Some(res_id) = res_id.as_local()

View file

@ -118,9 +118,10 @@ impl<'a, 'ra, 'tcx> UnusedImportCheckVisitor<'a, 'ra, 'tcx> {
ast::UseTreeKind::Simple(Some(ident)) => {
if ident.name == kw::Underscore
&& !self.r.import_res_map.get(&id).is_some_and(|per_ns| {
per_ns.iter().filter_map(|res| res.as_ref()).any(|res| {
matches!(res, Res::Def(DefKind::Trait | DefKind::TraitAlias, _))
})
matches!(
per_ns.type_ns,
Some(Res::Def(DefKind::Trait | DefKind::TraitAlias, _))
)
})
{
self.unused_import(self.base_id).add(id);

View file

@ -1606,7 +1606,7 @@ fn first_non_private<'tcx>(
&& let hir::Node::Item(item) = cx.tcx.hir_node_by_def_id(local_use_def_id)
&& let hir::ItemKind::Use(path, hir::UseKind::Single(_)) = item.kind
{
for res in &path.res {
for res in path.res.present_items() {
if let Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) = res {
continue;
}
@ -3014,7 +3014,7 @@ fn clean_use_statement<'tcx>(
) -> Vec<Item> {
let mut items = Vec::new();
let hir::UsePath { segments, ref res, span } = *path;
for &res in res {
for res in res.present_items() {
let path = hir::Path { segments, res, span };
items.append(&mut clean_use_statement_inner(import, name, &path, kind, cx, inlined_names));
}

View file

@ -440,7 +440,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
hir::ItemKind::GlobalAsm { .. } => {}
hir::ItemKind::Use(_, hir::UseKind::ListStem) => {}
hir::ItemKind::Use(path, kind) => {
for &res in &path.res {
for res in path.res.present_items() {
// Struct and variant constructors and proc macro stubs always show up alongside
// their definitions, we've already processed them so just discard these.
if should_ignore_res(res) {

View file

@ -106,8 +106,8 @@ impl_lint_pass!(DisallowedTypes => [DISALLOWED_TYPES]);
impl<'tcx> LateLintPass<'tcx> for DisallowedTypes {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if let ItemKind::Use(path, UseKind::Single(_)) = &item.kind {
for res in &path.res {
self.check_res_emit(cx, res, item.span);
if let Some(res) = path.res.type_ns {
self.check_res_emit(cx, &res, item.span);
}
}
}

View file

@ -51,7 +51,9 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants {
// so lint on the `use` statement directly.
if let ItemKind::Use(path, kind @ (UseKind::Single(_) | UseKind::Glob)) = item.kind
&& !item.span.in_external_macro(cx.sess().source_map())
&& let Some(def_id) = path.res[0].opt_def_id()
// use `present_items` because it could be in either type_ns or value_ns
&& let Some(res) = path.res.present_items().next()
&& let Some(def_id) = res.opt_def_id()
&& self.msrv.meets(cx, msrvs::NUMERIC_ASSOCIATED_CONSTANTS)
{
let module = if is_integer_module(cx, def_id) {

View file

@ -100,10 +100,7 @@ impl LateLintPass<'_> for MacroUseImports {
&& let hir_id = item.hir_id()
&& let attrs = cx.tcx.hir_attrs(hir_id)
&& let Some(mac_attr) = attrs.iter().find(|attr| attr.has_name(sym::macro_use))
&& let Some(id) = path.res.iter().find_map(|res| match res {
Res::Def(DefKind::Mod, id) => Some(id),
_ => None,
})
&& let Some(Res::Def(DefKind::Mod, id)) = path.res.type_ns
&& !id.is_local()
{
for kid in cx.tcx.module_children(id) {

View file

@ -131,8 +131,9 @@ impl Visitor<'_> for IdentVisitor<'_, '_> {
// If however the identifier is different, this means it is an alias (`use foo::bar as baz`). In
// this case, we need to emit the warning for `baz`.
if let Some(imported_item_path) = usenode
&& let Some(Res::Def(_, imported_item_defid)) = imported_item_path.res.first()
&& cx.tcx.item_name(*imported_item_defid).as_str() == str
// use `present_items` because it could be in any of type_ns, value_ns, macro_ns
&& let Some(Res::Def(_, imported_item_defid)) = imported_item_path.res.value_ns
&& cx.tcx.item_name(imported_item_defid).as_str() == str
{
return;
}

View file

@ -72,7 +72,8 @@ impl_lint_pass!(ImportRename => [MISSING_ENFORCED_IMPORT_RENAMES]);
impl LateLintPass<'_> for ImportRename {
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
if let ItemKind::Use(path, UseKind::Single(_)) = &item.kind {
for &res in &path.res {
// use `present_items` because it could be in any of type_ns, value_ns, macro_ns
for res in path.res.present_items() {
if let Res::Def(_, id) = res
&& let Some(name) = self.renames.get(&id)
// Remove semicolon since it is not present for nested imports

View file

@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::HasSession;
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{Item, ItemKind};
use rustc_hir::{Item, ItemKind, UseKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::impl_lint_pass;
@ -49,7 +49,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantPubCrate {
if cx.tcx.visibility(item.owner_id.def_id) == ty::Visibility::Restricted(CRATE_DEF_ID.to_def_id())
&& !cx.effective_visibilities.is_exported(item.owner_id.def_id)
&& self.is_exported.last() == Some(&false)
&& !is_macro_export(item)
&& !is_ignorable_export(item)
&& !item.span.in_external_macro(cx.sess().source_map())
{
let span = item
@ -86,13 +86,12 @@ impl<'tcx> LateLintPass<'tcx> for RedundantPubCrate {
}
}
fn is_macro_export<'tcx>(item: &'tcx Item<'tcx>) -> bool {
if let ItemKind::Use(path, _) = item.kind {
if path
.res
.iter()
.all(|res| matches!(res, Res::Def(DefKind::Macro(MacroKind::Bang), _)))
{
// We ignore macro exports. And `ListStem` uses, which aren't interesting.
fn is_ignorable_export<'tcx>(item: &'tcx Item<'tcx>) -> bool {
if let ItemKind::Use(path, kind) = item.kind {
let ignore = matches!(path.res.macro_ns, Some(Res::Def(DefKind::Macro(MacroKind::Bang), _)))
|| kind == UseKind::ListStem;
if ignore {
return true;
}
} else if let ItemKind::Macro(..) = item.kind {

View file

@ -64,7 +64,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedTraitNames {
// Ignore imports that already use Underscore
&& ident.name != kw::Underscore
// Only check traits
&& let Some(Res::Def(DefKind::Trait, _)) = path.res.first()
&& let Some(Res::Def(DefKind::Trait, _)) = path.res.type_ns
&& cx.tcx.maybe_unused_trait_imports(()).contains(&item.owner_id.def_id)
// Only check this import if it is visible to its module only (no pub, pub(crate), ...)
&& let module = cx.tcx.parent_module_from_def_id(item.owner_id.def_id)

View file

@ -169,8 +169,8 @@ impl LateLintPass<'_> for WildcardImports {
format!("{import_source_snippet}::{imports_string}")
};
// Glob imports always have a single resolution.
let (lint, message) = if let Res::Def(DefKind::Enum, _) = use_path.res[0] {
// Glob imports always have a single resolution. Enums are in the value namespace.
let (lint, message) = if let Some(Res::Def(DefKind::Enum, _)) = use_path.res.value_ns {
(ENUM_GLOB_USE, "usage of wildcard import for enum variants")
} else {
(WILDCARD_IMPORTS, "usage of wildcard import")

View file

@ -306,10 +306,13 @@ fn local_item_child_by_name(tcx: TyCtxt<'_>, local_id: LocalDefId, ns: PathNS, n
let item = tcx.hir_item(item_id);
if let ItemKind::Use(path, UseKind::Single(ident)) = item.kind {
if ident.name == name {
path.res
.iter()
.find(|res| ns.matches(res.ns()))
.and_then(Res::opt_def_id)
let opt_def_id = |ns: Option<Res>| ns.and_then(|res| res.opt_def_id());
match ns {
PathNS::Type => opt_def_id(path.res.type_ns),
PathNS::Value => opt_def_id(path.res.value_ns),
PathNS::Macro => opt_def_id(path.res.macro_ns),
PathNS::Arbitrary => unreachable!(),
}
} else {
None
}