Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking import use types for more accurate redundant import checking #117772

Merged
merged 1 commit into from
Feb 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions compiler/rustc_data_structures/src/owned_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::sync::Lrc;
// Use our fake Send/Sync traits when on not parallel compiler,
// so that `OwnedSlice` only implements/requires Send/Sync
// for parallel compiler builds.
use crate::sync::{Send, Sync};
surechen marked this conversation as resolved.
Show resolved Hide resolved
use crate::sync;

/// An owned slice.
///
Expand Down Expand Up @@ -33,7 +33,7 @@ pub struct OwnedSlice {
// \/
// ⊂(´・◡・⊂ )∘˚˳° (I am the phantom remnant of #97770)
#[expect(dead_code)]
owner: Lrc<dyn Send + Sync>,
owner: Lrc<dyn sync::Send + sync::Sync>,
}

/// Makes an [`OwnedSlice`] out of an `owner` and a `slicer` function.
Expand All @@ -60,7 +60,7 @@ pub struct OwnedSlice {
/// ```
pub fn slice_owned<O, F>(owner: O, slicer: F) -> OwnedSlice
where
O: Send + Sync + 'static,
O: sync::Send + sync::Sync + 'static,
F: FnOnce(&O) -> &[u8],
{
try_slice_owned(owner, |x| Ok::<_, !>(slicer(x))).into_ok()
Expand All @@ -71,7 +71,7 @@ where
/// See [`slice_owned`] for the infallible version.
pub fn try_slice_owned<O, F, E>(owner: O, slicer: F) -> Result<OwnedSlice, E>
where
O: Send + Sync + 'static,
O: sync::Send + sync::Sync + 'static,
F: FnOnce(&O) -> Result<&[u8], E>,
{
// We wrap the owner of the bytes in, so it doesn't move.
Expand Down Expand Up @@ -139,11 +139,11 @@ impl Borrow<[u8]> for OwnedSlice {

// Safety: `OwnedSlice` is conceptually `(&'self.1 [u8], Arc<dyn Send + Sync>)`, which is `Send`
#[cfg(parallel_compiler)]
unsafe impl Send for OwnedSlice {}
unsafe impl sync::Send for OwnedSlice {}

// Safety: `OwnedSlice` is conceptually `(&'self.1 [u8], Arc<dyn Send + Sync>)`, which is `Sync`
#[cfg(parallel_compiler)]
unsafe impl Sync for OwnedSlice {}
unsafe impl sync::Sync for OwnedSlice {}

#[cfg(test)]
mod tests;
3 changes: 1 addition & 2 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ use super::compare_impl_item::check_type_bounds;
use super::compare_impl_item::{compare_impl_method, compare_impl_ty};
use super::*;
use rustc_attr as attr;
use rustc_errors::{codes::*, ErrorGuaranteed, MultiSpan};
use rustc_errors::{codes::*, MultiSpan};
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, DefKind};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::Node;
use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
use rustc_infer::traits::{Obligation, TraitEngineExt as _};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{errors, structured_errors::StructuredDiagnostic};
use rustc_errors::{codes::*, DiagnosticBuilder, ErrCode};
use rustc_errors::{codes::*, DiagnosticBuilder};
use rustc_middle::ty::{Ty, TypeVisitableExt};
use rustc_session::Session;
use rustc_span::Span;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{errors, structured_errors::StructuredDiagnostic};
use rustc_errors::{codes::*, DiagnosticBuilder, ErrCode};
use rustc_errors::{codes::*, DiagnosticBuilder};
use rustc_middle::ty::{Ty, TypeVisitableExt};
use rustc_session::Session;
use rustc_span::Span;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use crate::structured_errors::StructuredDiagnostic;
use rustc_errors::{
codes::*, pluralize, Applicability, Diagnostic, DiagnosticBuilder, ErrCode, MultiSpan,
};
use rustc_errors::{codes::*, pluralize, Applicability, Diagnostic, DiagnosticBuilder, MultiSpan};
use rustc_hir as hir;
use rustc_middle::ty::{self as ty, AssocItems, AssocKind, TyCtxt};
use rustc_session::Session;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_data_structures::unord::UnordMap;
use rustc_errors::{
codes::*, pluralize, struct_span_code_err, AddToDiagnostic, Applicability, Diagnostic,
DiagnosticBuilder, ErrCode, ErrorGuaranteed, StashKey,
DiagnosticBuilder, ErrorGuaranteed, StashKey,
};
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, DefKind, Res};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use itertools::Itertools;
use rustc_ast as ast;
use rustc_data_structures::fx::FxIndexSet;
use rustc_errors::{
codes::*, pluralize, Applicability, Diagnostic, ErrCode, ErrorGuaranteed, MultiSpan, StashKey,
codes::*, pluralize, Applicability, Diagnostic, ErrorGuaranteed, MultiSpan, StashKey,
};
use rustc_hir as hir;
use rustc_hir::def::{CtorOf, DefKind, Res};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ use crate::expectation::Expectation;
use crate::fn_ctxt::LoweredTy;
use crate::gather_locals::GatherLocalsVisitor;
use rustc_data_structures::unord::UnordSet;
use rustc_errors::{codes::*, struct_span_code_err, ErrCode, ErrorGuaranteed};
use rustc_errors::{codes::*, struct_span_code_err, ErrorGuaranteed};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::Visitor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::errors::{
use crate::infer::error_reporting::TypeErrCtxt;
use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use crate::infer::InferCtxt;
use rustc_errors::{codes::*, DiagnosticBuilder, ErrCode, IntoDiagnosticArg};
use rustc_errors::{codes::*, DiagnosticBuilder, IntoDiagnosticArg};
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::def::{CtorOf, DefKind, Namespace};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_data_structures::unhash::UnhashMap;
use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
use rustc_expand::proc_macro::{AttrProcMacro, BangProcMacro, DeriveProcMacro};
use rustc_hir::def::Res;
use rustc_hir::def_id::{DefIdMap, CRATE_DEF_INDEX, LOCAL_CRATE};
use rustc_hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE};
use rustc_hir::definitions::{DefPath, DefPathData};
use rustc_hir::diagnostic_items::DiagnosticItems;
use rustc_index::Idx;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::ty::{Region, UserTypeAnnotationIndex};
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
use rustc_data_structures::packed::Pu128;
use rustc_hir::def_id::DefId;
use rustc_hir::{self, CoroutineKind};
use rustc_hir::CoroutineKind;
use rustc_index::IndexVec;
use rustc_span::source_map::Spanned;
use rustc_target::abi::{FieldIdx, VariantIdx};
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
use crate::def_collector::collect_definitions;
use crate::imports::{ImportData, ImportKind};
use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
use crate::Namespace::{self, MacroNS, TypeNS, ValueNS};
use crate::Namespace::{MacroNS, TypeNS, ValueNS};
use crate::{errors, BindingKey, MacroData, NameBindingData};
use crate::{Determinacy, ExternPreludeEntry, Finalize, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PerNS, ResolutionError};
use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, VisResolutionError};
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, ResolutionError};
use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, Used, VisResolutionError};

use rustc_ast::visit::{self, AssocCtxt, Visitor};
use rustc_ast::{self as ast, AssocItem, AssocItemKind, MetaItemKind, StmtKind};
Expand Down Expand Up @@ -362,7 +362,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
root_span,
root_id,
vis: Cell::new(Some(vis)),
used: Cell::new(false),
used: Default::default(),
});

self.r.indeterminate_imports.push(import);
Expand Down Expand Up @@ -885,7 +885,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
span: item.span,
module_path: Vec::new(),
vis: Cell::new(Some(vis)),
used: Cell::new(used),
used: Cell::new(used.then_some(Used::Other)),
});
self.r.potentially_unused_imports.push(import);
let imported_binding = self.r.import(binding, import);
Expand Down Expand Up @@ -1090,7 +1090,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
span,
module_path: Vec::new(),
vis: Cell::new(Some(ty::Visibility::Restricted(CRATE_DEF_ID))),
used: Cell::new(false),
used: Default::default(),
})
};

Expand Down Expand Up @@ -1261,7 +1261,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
span,
module_path: Vec::new(),
vis: Cell::new(Some(vis)),
used: Cell::new(true),
used: Cell::new(Some(Used::Other)),
});
let import_binding = self.r.import(binding, import);
self.r.define(self.r.graph_root, ident, MacroNS, import_binding);
Expand Down
46 changes: 36 additions & 10 deletions compiler/rustc_resolve/src/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ use crate::imports::ImportKind;
use crate::module_to_string;
use crate::Resolver;

use crate::NameBindingKind;
use rustc_ast as ast;
use rustc_ast::visit::{self, Visitor};
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
use rustc_data_structures::unord::UnordSet;
use rustc_errors::{pluralize, MultiSpan};
use rustc_hir::def::{DefKind, Res};
Expand All @@ -38,14 +39,14 @@ use rustc_session::lint::BuiltinLintDiagnostics;
use rustc_span::symbol::{kw, Ident};
use rustc_span::{Span, DUMMY_SP};

struct UnusedImport<'a> {
use_tree: &'a ast::UseTree,
struct UnusedImport {
use_tree: ast::UseTree,
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
use_tree_id: ast::NodeId,
item_span: Span,
unused: UnordSet<ast::NodeId>,
}

impl<'a> UnusedImport<'a> {
impl UnusedImport {
fn add(&mut self, id: ast::NodeId) {
self.unused.insert(id);
}
Expand All @@ -54,7 +55,7 @@ impl<'a> UnusedImport<'a> {
struct UnusedImportCheckVisitor<'a, 'b, 'tcx> {
r: &'a mut Resolver<'b, 'tcx>,
/// All the (so far) unused imports, grouped path list
unused_imports: FxIndexMap<ast::NodeId, UnusedImport<'a>>,
unused_imports: FxIndexMap<ast::NodeId, UnusedImport>,
extern_crate_items: Vec<ExternCrateToLint>,
base_use_tree: Option<&'a ast::UseTree>,
base_id: ast::NodeId,
Expand Down Expand Up @@ -100,9 +101,9 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> {
}
}

fn unused_import(&mut self, id: ast::NodeId) -> &mut UnusedImport<'a> {
fn unused_import(&mut self, id: ast::NodeId) -> &mut UnusedImport {
let use_tree_id = self.base_id;
let use_tree = self.base_use_tree.unwrap();
let use_tree = self.base_use_tree.unwrap().clone();
let item_span = self.item_span;

self.unused_imports.entry(id).or_insert_with(|| UnusedImport {
Expand Down Expand Up @@ -197,7 +198,7 @@ enum UnusedSpanResult {
}

fn calc_unused_spans(
unused_import: &UnusedImport<'_>,
unused_import: &UnusedImport,
use_tree: &ast::UseTree,
use_tree_id: ast::NodeId,
) -> UnusedSpanResult {
Expand Down Expand Up @@ -287,7 +288,7 @@ impl Resolver<'_, '_> {

for import in self.potentially_unused_imports.iter() {
match import.kind {
_ if import.used.get()
_ if import.used.get().is_some()
|| import.expect_vis().is_public()
|| import.span.is_dummy() =>
{
Expand Down Expand Up @@ -336,7 +337,7 @@ impl Resolver<'_, '_> {

for unused in visitor.unused_imports.values() {
let mut fixes = Vec::new();
let spans = match calc_unused_spans(unused, unused.use_tree, unused.use_tree_id) {
let spans = match calc_unused_spans(unused, &unused.use_tree, unused.use_tree_id) {
UnusedSpanResult::Used => continue,
UnusedSpanResult::FlatUnused(span, remove) => {
fixes.push((remove, String::new()));
Expand Down Expand Up @@ -483,5 +484,30 @@ impl Resolver<'_, '_> {
BuiltinLintDiagnostics::ExternCrateNotIdiomatic { vis_span, ident_span },
);
}

let unused_imports = visitor.unused_imports;
let mut check_redundant_imports = FxIndexSet::default();
for module in self.arenas.local_modules().iter() {
for (_key, resolution) in self.resolutions(*module).borrow().iter() {
let resolution = resolution.borrow();

if let Some(binding) = resolution.binding
&& let NameBindingKind::Import { import, .. } = binding.kind
&& let ImportKind::Single { id, .. } = import.kind
{
if let Some(unused_import) = unused_imports.get(&import.root_id)
&& unused_import.unused.contains(&id)
{
continue;
}

check_redundant_imports.insert(import);
}
}
}

for import in check_redundant_imports {
self.check_for_redundant_imports(import);
}
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/def_collector.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{ImplTraitContext, Resolver};
use rustc_ast::visit::{self, FnKind};
use rustc_ast::visit::FnKind;
use rustc_ast::*;
use rustc_expand::expand::AstFragment;
use rustc_hir::def::{CtorKind, CtorOf, DefKind};
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ use crate::errors::{AddedMacroUse, ChangeImportBinding, ChangeImportBindingSugge
use crate::errors::{ConsiderAddingADerive, ExplicitUnsafeTraits, MaybeMissingMacroRulesName};
use crate::imports::{Import, ImportKind};
use crate::late::{PatternSource, Rib};
use crate::path_names_to_string;
use crate::{errors as errs, BindingKey};
use crate::{path_names_to_string, Used};
use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, BindingError, Finalize};
use crate::{HasGenericParams, MacroRulesScope, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{LexicalScopeBinding, NameBinding, NameBindingKind, PrivacyError, VisResolutionError};
Expand Down Expand Up @@ -1503,7 +1503,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
);
// Silence the 'unused import' warning we might get,
// since this diagnostic already covers that import.
self.record_use(ident, binding, false);
self.record_use(ident, binding, Used::Other);
return;
}
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_resolve/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use rustc_span::Span;
use crate::errors::{ParamKindInEnumDiscriminant, ParamKindInNonTrivialAnonConst};
use crate::late::{ConstantHasGenerics, NoConstantGenericsReason, PathSource, Rib, RibKind};
use crate::macros::{sub_namespace_match, MacroRulesScope};
use crate::BindingKey;
use crate::{errors, AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, Determinacy, Finalize};
use crate::{BindingKey, Used};
use crate::{ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PrivacyError, Res};
use crate::{ResolutionError, Resolver, Scope, ScopeSet, Segment, ToNameBinding, Weak};
Expand Down Expand Up @@ -339,7 +339,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
ident,
ns,
parent_scope,
finalize,
finalize.map(|finalize| Finalize { used: Used::Scope, ..finalize }),
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
ignore_binding,
);
if let Ok(binding) = item {
Expand Down Expand Up @@ -506,7 +506,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
ns,
adjusted_parent_scope,
!matches!(scope_set, ScopeSet::Late(..)),
finalize,
finalize.map(|finalize| Finalize { used: Used::Scope, ..finalize }),
ignore_binding,
);
match binding {
Expand Down Expand Up @@ -857,7 +857,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
.into_iter()
.find_map(|binding| if binding == ignore_binding { None } else { binding });

if let Some(Finalize { path_span, report_private, .. }) = finalize {
if let Some(Finalize { path_span, report_private, used, .. }) = finalize {
let Some(binding) = binding else {
return Err((Determined, Weak::No));
};
Expand Down Expand Up @@ -901,7 +901,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}

self.record_use(ident, binding, restricted_shadowing);
self.record_use(ident, binding, used);
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
return Ok(binding);
}

Expand Down