Rollup merge of rust-lang#97609 - Elliot-Roberts:unused-trait-refactor, r=cjgillot

Iterate over `maybe_unused_trait_imports` when checking dead trait imports

Closes rust-lang#96873
r? `@cjgillot`

Some questions, if you have time:

- Is there a way to shorten the `rustc_data_structures::fx::FxIndexSet` path in the query declaration? I wasn't sure where to put a `use`.
- Was returning by reference from the query the right choice here?
- How would I go about evaluating the importance of the `is_dummy()` call in `check_crate`? I don't see failing tests when I comment it out. Should I just try to determine whether dummy spans can ever be put into `maybe_unused_trait_imports`?
- Am I doing anything silly with the various ID types?
- Is that `let-else` with `unreachable!()` bad? (i.e is there a better idiom? Would `panic!("<explanation>")` be better?)
- If I want to evaluate the perf of using a `Vec` as mentioned in rust-lang#96873, is the best way to use the CI or is it feasible locally?

Thanks :)

Author: matthiaskrgr

compiler/rustc_middle/src/query/mod.rs

@@ -1728,8 +1728,8 @@ rustc_queries! {
     query upvars_mentioned(def_id: DefId) -> Option<&'tcx FxIndexMap<hir::HirId, hir::Upvar>> {
         desc { |tcx| "collecting upvars mentioned in `{}`", tcx.def_path_str(def_id) }
     }
-    query maybe_unused_trait_import(def_id: LocalDefId) -> bool {
-        desc { |tcx| "maybe_unused_trait_import for `{}`", tcx.def_path_str(def_id.to_def_id()) }
+    query maybe_unused_trait_imports(_: ()) -> &'tcx FxIndexSet<LocalDefId> {
+        desc { "fetching potentially unused trait imports" }
     }
     query maybe_unused_extern_crates(_: ()) -> &'tcx [(LocalDefId, Span)] {
         desc { "looking up all possibly unused extern crates" }

compiler/rustc_middle/src/ty/context.rs

@@ -2893,8 +2893,8 @@ pub fn provide(providers: &mut ty::query::Providers) {
         assert_eq!(id, LOCAL_CRATE);
         tcx.crate_name
     };
-    providers.maybe_unused_trait_import =
-        |tcx, id| tcx.resolutions(()).maybe_unused_trait_imports.contains(&id);
+    providers.maybe_unused_trait_imports =
+        |tcx, ()| &tcx.resolutions(()).maybe_unused_trait_imports;
     providers.maybe_unused_extern_crates =
         |tcx, ()| &tcx.resolutions(()).maybe_unused_extern_crates[..];
     providers.names_imported_by_glob_use = |tcx, id| {

compiler/rustc_middle/src/ty/mod.rs

@@ -28,7 +28,7 @@ pub use generics::*;
 use rustc_ast as ast;
 use rustc_attr as attr;
 use rustc_data_structures::fingerprint::Fingerprint;
-use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
+use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_data_structures::intern::{Interned, WithStableHash};
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::tagged_ptr::CopyTaggedPtr;
@@ -138,7 +138,7 @@ pub struct ResolverOutputs {
     pub has_pub_restricted: bool,
     pub access_levels: AccessLevels,
     pub extern_crate_map: FxHashMap<LocalDefId, CrateNum>,
-    pub maybe_unused_trait_imports: FxHashSet<LocalDefId>,
+    pub maybe_unused_trait_imports: FxIndexSet<LocalDefId>,
     pub maybe_unused_extern_crates: Vec<(LocalDefId, Span)>,
     pub reexport_map: FxHashMap<LocalDefId, Vec<ModChild>>,
     pub glob_map: FxHashMap<LocalDefId, FxHashSet<Symbol>>,

compiler/rustc_middle/src/ty/query.rs

@@ -37,7 +37,7 @@ use crate::ty::{self, AdtSizedConstraint, CrateInherentImpls, ParamEnvAnd, Ty, T
 use rustc_ast as ast;
 use rustc_ast::expand::allocator::AllocatorKind;
 use rustc_attr as attr;
-use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
+use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_data_structures::steal::Steal;
 use rustc_data_structures::svh::Svh;
 use rustc_data_structures::sync::Lrc;

compiler/rustc_resolve/src/lib.rs

@@ -28,7 +28,7 @@ use rustc_ast::node_id::NodeMap;
 use rustc_ast::{self as ast, NodeId, CRATE_NODE_ID};
 use rustc_ast::{AngleBracketedArg, Crate, Expr, ExprKind, GenericArg, GenericArgs, LitKind, Path};
 use rustc_ast_lowering::{LifetimeRes, ResolverAstLowering};
-use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
+use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_data_structures::intern::Interned;
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::{Applicability, DiagnosticBuilder, ErrorGuaranteed};
@@ -941,7 +941,7 @@ pub struct Resolver<'a> {
     visibilities: FxHashMap<LocalDefId, ty::Visibility>,
     has_pub_restricted: bool,
     used_imports: FxHashSet<NodeId>,
-    maybe_unused_trait_imports: FxHashSet<LocalDefId>,
+    maybe_unused_trait_imports: FxIndexSet<LocalDefId>,
     maybe_unused_extern_crates: Vec<(LocalDefId, Span)>,
 
     /// Privacy errors are delayed until the end in order to deduplicate them.

compiler/rustc_typeck/src/check_unused.rs

@@ -16,48 +16,32 @@ pub fn check_crate(tcx: TyCtxt<'_>) {
         used_trait_imports.extend(imports.iter());
     }
 
-    for id in tcx.hir().items() {
-        if matches!(tcx.def_kind(id.def_id), DefKind::Use) {
-            if tcx.visibility(id.def_id).is_public() {
-                continue;
-            }
-            let item = tcx.hir().item(id);
-            if item.span.is_dummy() {
-                continue;
-            }
-            if let hir::ItemKind::Use(path, _) = item.kind {
-                check_import(tcx, &mut used_trait_imports, item.item_id(), path.span);
-            }
+    for &id in tcx.maybe_unused_trait_imports(()) {
+        debug_assert_eq!(tcx.def_kind(id), DefKind::Use);
+        if tcx.visibility(id).is_public() {
+            continue;
+        }
+        if used_trait_imports.contains(&id) {
+            continue;
         }
+        let item = tcx.hir().expect_item(id);
+        if item.span.is_dummy() {
+            continue;
+        }
+        let hir::ItemKind::Use(path, _) = item.kind else { unreachable!() };
+        tcx.struct_span_lint_hir(lint::builtin::UNUSED_IMPORTS, item.hir_id(), path.span, |lint| {
+            let msg = if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(path.span) {
+                format!("unused import: `{}`", snippet)
+            } else {
+                "unused import".to_owned()
+            };
+            lint.build(&msg).emit();
+        });
     }
 
     unused_crates_lint(tcx);
 }
 
-fn check_import<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    used_trait_imports: &mut FxHashSet<LocalDefId>,
-    item_id: hir::ItemId,
-    span: Span,
-) {
-    if !tcx.maybe_unused_trait_import(item_id.def_id) {
-        return;
-    }
-
-    if used_trait_imports.contains(&item_id.def_id) {
-        return;
-    }
-
-    tcx.struct_span_lint_hir(lint::builtin::UNUSED_IMPORTS, item_id.hir_id(), span, |lint| {
-        let msg = if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(span) {
-            format!("unused import: `{}`", snippet)
-        } else {
-            "unused import".to_owned()
-        };
-        lint.build(&msg).emit();
-    });
-}
-
 fn unused_crates_lint(tcx: TyCtxt<'_>) {
     let lint = lint::builtin::UNUSED_EXTERN_CRATES;