Skip to content

Commit

Permalink
check redundant prelude imports
Browse files Browse the repository at this point in the history
detects unnecessary imports in std::prelude that can be eliminated.

For example import:

```rust
use std::{option::{Iter, IterMut, IntoIter, Option::{self, Some}}, convert::{TryFrom, TryInto}, mem::drop};
```

delete : `Option::{self, Some}`  and `mem::drop`
  • Loading branch information
surechen committed Dec 20, 2023
1 parent f704f3b commit ea7d921
Show file tree
Hide file tree
Showing 21 changed files with 242 additions and 106 deletions.
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};
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;
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ 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::{ErrorGuaranteed, MultiSpan};
use rustc_errors::MultiSpan;
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, DefKind};
use rustc_hir::def_id::LocalModDefId;
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 @@ -363,7 +363,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 @@ -853,7 +853,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 @@ -1061,7 +1061,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 @@ -1222,7 +1222,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,
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 @@ -99,9 +100,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 @@ -196,7 +197,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 @@ -286,7 +287,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 @@ -335,7 +336,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 @@ -482,5 +483,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 @@ -31,8 +31,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 @@ -1487,7 +1487,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 @@ -14,8 +14,8 @@ use crate::late::{
ConstantHasGenerics, HasGenericParams, 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 @@ -341,7 +341,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
ident,
ns,
parent_scope,
finalize,
finalize.map(|finalize| Finalize { used: Used::Scope, ..finalize }),
ignore_binding,
);
if let Ok(binding) = item {
Expand Down Expand Up @@ -507,7 +507,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 @@ -858,7 +858,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 @@ -902,7 +902,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}

self.record_use(ident, binding, restricted_shadowing);
self.record_use(ident, binding, used);
return Ok(binding);
}

Expand Down
Loading

0 comments on commit ea7d921

Please sign in to comment.