Skip to content

Commit 1b2b7e6

Browse files
committed
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.
1 parent abf5d4c commit 1b2b7e6

File tree

9 files changed

+29
-26
lines changed

9 files changed

+29
-26
lines changed

clippy_lints/src/disallowed_types.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ impl_lint_pass!(DisallowedTypes => [DISALLOWED_TYPES]);
106106
impl<'tcx> LateLintPass<'tcx> for DisallowedTypes {
107107
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
108108
if let ItemKind::Use(path, UseKind::Single(_)) = &item.kind {
109-
for res in &path.res {
110-
self.check_res_emit(cx, res, item.span);
109+
if let Some(res) = path.res.type_ns {
110+
self.check_res_emit(cx, &res, item.span);
111111
}
112112
}
113113
}

clippy_lints/src/legacy_numeric_constants.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants {
5151
// so lint on the `use` statement directly.
5252
if let ItemKind::Use(path, kind @ (UseKind::Single(_) | UseKind::Glob)) = item.kind
5353
&& !item.span.in_external_macro(cx.sess().source_map())
54-
&& let Some(def_id) = path.res[0].opt_def_id()
54+
// use `present_items` because it could be in either type_ns or value_ns
55+
&& let Some(res) = path.res.present_items().next()
56+
&& let Some(def_id) = res.opt_def_id()
5557
&& self.msrv.meets(cx, msrvs::NUMERIC_ASSOCIATED_CONSTANTS)
5658
{
5759
let module = if is_integer_module(cx, def_id) {

clippy_lints/src/macro_use.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,7 @@ impl LateLintPass<'_> for MacroUseImports {
100100
&& let hir_id = item.hir_id()
101101
&& let attrs = cx.tcx.hir_attrs(hir_id)
102102
&& let Some(mac_attr) = attrs.iter().find(|attr| attr.has_name(sym::macro_use))
103-
&& let Some(id) = path.res.iter().find_map(|res| match res {
104-
Res::Def(DefKind::Mod, id) => Some(id),
105-
_ => None,
106-
})
103+
&& let Some(Res::Def(DefKind::Mod, id)) = path.res.type_ns
107104
&& !id.is_local()
108105
{
109106
for kid in cx.tcx.module_children(id) {

clippy_lints/src/min_ident_chars.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,9 @@ impl Visitor<'_> for IdentVisitor<'_, '_> {
131131
// If however the identifier is different, this means it is an alias (`use foo::bar as baz`). In
132132
// this case, we need to emit the warning for `baz`.
133133
if let Some(imported_item_path) = usenode
134-
&& let Some(Res::Def(_, imported_item_defid)) = imported_item_path.res.first()
135-
&& cx.tcx.item_name(*imported_item_defid).as_str() == str
134+
// use `present_items` because it could be in any of type_ns, value_ns, macro_ns
135+
&& let Some(Res::Def(_, imported_item_defid)) = imported_item_path.res.value_ns
136+
&& cx.tcx.item_name(imported_item_defid).as_str() == str
136137
{
137138
return;
138139
}

clippy_lints/src/missing_enforced_import_rename.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ impl_lint_pass!(ImportRename => [MISSING_ENFORCED_IMPORT_RENAMES]);
7272
impl LateLintPass<'_> for ImportRename {
7373
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
7474
if let ItemKind::Use(path, UseKind::Single(_)) = &item.kind {
75-
for &res in &path.res {
75+
// use `present_items` because it could be in any of type_ns, value_ns, macro_ns
76+
for res in path.res.present_items() {
7677
if let Res::Def(_, id) = res
7778
&& let Some(name) = self.renames.get(&id)
7879
// Remove semicolon since it is not present for nested imports

clippy_lints/src/redundant_pub_crate.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::source::HasSession;
33
use rustc_errors::Applicability;
44
use rustc_hir::def::{DefKind, Res};
5-
use rustc_hir::{Item, ItemKind};
5+
use rustc_hir::{Item, ItemKind, UseKind};
66
use rustc_lint::{LateContext, LateLintPass};
77
use rustc_middle::ty;
88
use rustc_session::impl_lint_pass;
@@ -49,7 +49,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantPubCrate {
4949
if cx.tcx.visibility(item.owner_id.def_id) == ty::Visibility::Restricted(CRATE_DEF_ID.to_def_id())
5050
&& !cx.effective_visibilities.is_exported(item.owner_id.def_id)
5151
&& self.is_exported.last() == Some(&false)
52-
&& !is_macro_export(item)
52+
&& !is_ignorable_export(item)
5353
&& !item.span.in_external_macro(cx.sess().source_map())
5454
{
5555
let span = item
@@ -86,13 +86,12 @@ impl<'tcx> LateLintPass<'tcx> for RedundantPubCrate {
8686
}
8787
}
8888

89-
fn is_macro_export<'tcx>(item: &'tcx Item<'tcx>) -> bool {
90-
if let ItemKind::Use(path, _) = item.kind {
91-
if path
92-
.res
93-
.iter()
94-
.all(|res| matches!(res, Res::Def(DefKind::Macro(MacroKind::Bang), _)))
95-
{
89+
// We ignore macro exports. And `ListStem` uses, which aren't interesting.
90+
fn is_ignorable_export<'tcx>(item: &'tcx Item<'tcx>) -> bool {
91+
if let ItemKind::Use(path, kind) = item.kind {
92+
let ignore = matches!(path.res.macro_ns, Some(Res::Def(DefKind::Macro(MacroKind::Bang), _)))
93+
|| kind == UseKind::ListStem;
94+
if ignore {
9695
return true;
9796
}
9897
} else if let ItemKind::Macro(..) = item.kind {

clippy_lints/src/unused_trait_names.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedTraitNames {
6464
// Ignore imports that already use Underscore
6565
&& ident.name != kw::Underscore
6666
// Only check traits
67-
&& let Some(Res::Def(DefKind::Trait, _)) = path.res.first()
67+
&& let Some(Res::Def(DefKind::Trait, _)) = path.res.type_ns
6868
&& cx.tcx.maybe_unused_trait_imports(()).contains(&item.owner_id.def_id)
6969
// Only check this import if it is visible to its module only (no pub, pub(crate), ...)
7070
&& let module = cx.tcx.parent_module_from_def_id(item.owner_id.def_id)

clippy_lints/src/wildcard_imports.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,8 @@ impl LateLintPass<'_> for WildcardImports {
169169
format!("{import_source_snippet}::{imports_string}")
170170
};
171171

172-
// Glob imports always have a single resolution.
173-
let (lint, message) = if let Res::Def(DefKind::Enum, _) = use_path.res[0] {
172+
// Glob imports always have a single resolution. Enums are in the value namespace.
173+
let (lint, message) = if let Some(Res::Def(DefKind::Enum, _)) = use_path.res.value_ns {
174174
(ENUM_GLOB_USE, "usage of wildcard import for enum variants")
175175
} else {
176176
(WILDCARD_IMPORTS, "usage of wildcard import")

clippy_utils/src/paths.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -306,10 +306,13 @@ fn local_item_child_by_name(tcx: TyCtxt<'_>, local_id: LocalDefId, ns: PathNS, n
306306
let item = tcx.hir_item(item_id);
307307
if let ItemKind::Use(path, UseKind::Single(ident)) = item.kind {
308308
if ident.name == name {
309-
path.res
310-
.iter()
311-
.find(|res| ns.matches(res.ns()))
312-
.and_then(Res::opt_def_id)
309+
let opt_def_id = |ns: Option<Res>| ns.and_then(|res| res.opt_def_id());
310+
match ns {
311+
PathNS::Type => opt_def_id(path.res.type_ns),
312+
PathNS::Value => opt_def_id(path.res.value_ns),
313+
PathNS::Macro => opt_def_id(path.res.macro_ns),
314+
PathNS::Arbitrary => unreachable!(),
315+
}
313316
} else {
314317
None
315318
}

0 commit comments

Comments
 (0)