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 Nov 29, 2023
1 parent f440b5f commit e9ca0af
Show file tree
Hide file tree
Showing 97 changed files with 231 additions and 207 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_macros/src/hash_stable.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use proc_macro2::{self, Ident};
use proc_macro2::Ident;
use quote::quote;
use syn::{self, parse_quote};
use syn::parse_quote;

struct Attributes {
ignore: bool,
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::Namespace::{self, 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::{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 @@ -358,7 +358,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 @@ -852,7 +852,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 @@ -1225,7 +1225,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
22 changes: 21 additions & 1 deletion compiler/rustc_resolve/src/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,26 @@ impl<'a, 'b, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'tcx> {
}
} else {
self.check_import(id);
let import = self.r.redundant_imports.remove(&id);
if let Some(import) = import {
if let ImportKind::Single {
source,
target,
ref source_bindings,
ref target_bindings,
..
} = import.kind
&& !self.unused_imports.contains_key(&self.base_id)
{
self.r.check_for_redundant_imports(
source,
import,
source_bindings,
target_bindings,
target,
);
}
}
}

visit::walk_use_tree(self, use_tree, id);
Expand Down Expand Up @@ -286,7 +306,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
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::{
};
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 @@ -1482,7 +1482,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, Some(Used::Other));
return;
}
}
Expand Down
17 changes: 11 additions & 6 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 @@ -335,14 +335,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
ModuleKind::Block => {} // We can see through blocks
_ => break,
}

let item = self.resolve_ident_in_module_unadjusted(
ModuleOrUniformRoot::Module(module),
ident,
ns,
parent_scope,
finalize,
ignore_binding,
None,
);
if let Ok(binding) = item {
// The ident resolves to an item.
Expand Down Expand Up @@ -509,6 +509,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
!matches!(scope_set, ScopeSet::Late(..)),
finalize,
ignore_binding,
Some(Used::Scope),
);
match binding {
Ok(binding) => {
Expand Down Expand Up @@ -579,6 +580,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
parent_scope,
None,
ignore_binding,
Some(Used::Other),
) {
if use_prelude || this.is_builtin_macro(binding.res()) {
result = Ok((binding, Flags::MISC_FROM_PRELUDE));
Expand Down Expand Up @@ -754,6 +756,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
false,
finalize,
ignore_binding,
Some(Used::Other),
)
}

Expand All @@ -766,6 +769,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
parent_scope: &ParentScope<'a>,
finalize: Option<Finalize>,
ignore_binding: Option<NameBinding<'a>>,
used: Option<Used>,
) -> Result<NameBinding<'a>, Determinacy> {
self.resolve_ident_in_module_unadjusted_ext(
module,
Expand All @@ -775,6 +779,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
false,
finalize,
ignore_binding,
used,
)
.map_err(|(determinacy, _)| determinacy)
}
Expand All @@ -793,6 +798,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// This binding should be ignored during in-module resolution, so that we don't get
// "self-confirming" import resolutions during import validation and checking.
ignore_binding: Option<NameBinding<'a>>,
used: Option<Used>,
) -> Result<NameBinding<'a>, (Determinacy, Weak)> {
let module = match module {
ModuleOrUniformRoot::Module(module) => module,
Expand Down Expand Up @@ -849,15 +855,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let key = BindingKey::new(ident, ns);
let resolution =
self.resolution(module, key).try_borrow_mut().map_err(|_| (Determined, Weak::No))?; // This happens when there is a cycle of imports.

// If the primary binding is unusable, search further and return the shadowed glob
// binding if it exists. What we really want here is having two separate scopes in
// a module - one for non-globs and one for globs, but until that's done use this
// hack to avoid inconsistent resolution ICEs during import validation.
let binding = [resolution.binding, resolution.shadowed_glob]
.into_iter()
.find_map(|binding| if binding == ignore_binding { None } else { binding });

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

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

Expand All @@ -923,6 +926,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// Check if one of single imports can still define the name,
// if it can then our result is not determined and can be invalidated.
for single_import in &resolution.single_imports {
debug!("single_import:{:?}", single_import);
let Some(import_vis) = single_import.vis.get() else {
continue;
};
Expand Down Expand Up @@ -1029,6 +1033,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
adjusted_parent_scope,
None,
ignore_binding,
Some(Used::Other),
);

match result {
Expand Down
41 changes: 22 additions & 19 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use crate::errors::{
use crate::Determinacy::{self, *};
use crate::{fluent_generated as fluent, Namespace::*};
use crate::{module_to_string, names_to_string, ImportSuggestion};
use crate::{AmbiguityKind, BindingKey, ModuleKind, ResolutionError, Resolver, Segment};
use crate::{AmbiguityKind, BindingKey, ResolutionError, Resolver, Segment};
use crate::{Finalize, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet};
use crate::{NameBinding, NameBindingData, NameBindingKind, PathResult};
use crate::{NameBinding, NameBindingData, NameBindingKind, PathResult, Used};

use rustc_ast::NodeId;
use rustc_data_structures::fx::FxHashSet;
Expand Down Expand Up @@ -169,7 +169,7 @@ pub(crate) struct ImportData<'a> {
/// The resolution of `module_path`.
pub imported_module: Cell<Option<ModuleOrUniformRoot<'a>>>,
pub vis: Cell<Option<ty::Visibility>>,
pub used: Cell<bool>,
pub used: Cell<Option<Used>>,
}

/// All imports are unique and allocated on a same arena,
Expand Down Expand Up @@ -282,7 +282,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}

self.arenas.alloc_name_binding(NameBindingData {
kind: NameBindingKind::Import { binding, import, used: Cell::new(false) },
kind: NameBindingKind::Import { binding, import },
ambiguity: None,
warn_ambiguity: false,
span: import.span,
Expand Down Expand Up @@ -478,9 +478,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let key = BindingKey::new(target, ns);
let _ = this.try_define(import.parent_scope.module, key, dummy_binding, false);
});
self.record_use(target, dummy_binding, false);
self.record_use(target, dummy_binding, Some(Used::Other));
} else if import.imported_module.get().is_none() {
import.used.set(true);
import.used.set(Some(Used::Other));
if let Some(id) = import.id() {
self.used_imports.insert(id);
}
Expand Down Expand Up @@ -1040,11 +1040,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
&& initial_binding.is_extern_crate()
&& !initial_binding.is_import()
{
this.record_use(
ident,
target_binding,
import.module_path.is_empty(),
);
let used = if import.module_path.is_empty() {
Used::Scope
} else {
Used::Other
};
this.record_use(ident, target_binding, Some(used));
}
}
initial_binding.res()
Expand Down Expand Up @@ -1291,14 +1292,16 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
this.import_res_map.entry(import_id).or_default()[ns] = Some(binding.res());
}
});

self.check_for_redundant_imports(ident, import, source_bindings, target_bindings, target);
if let ImportKind::Single { id, .. } = import.kind {
debug!("redundant_imports insert id:{:?} import:{:?}", id, import);
self.redundant_imports.insert(id, import);
}

debug!("(resolving single import) successfully resolved import");
None
}

fn check_for_redundant_imports(
pub(crate) fn check_for_redundant_imports(
&mut self,
ident: Ident,
import: Import<'a>,
Expand All @@ -1313,13 +1316,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
if import.parent_scope.expansion != LocalExpnId::ROOT {
return;
}

// Skip if we are inside a named module (in contrast to an anonymous
// module defined by a block).
if let ModuleKind::Def(..) = import.parent_scope.module.kind {
// Skip if the import is public or was used through non scope-based resolution,
// e.g. through a module-relative path.
if self.effective_visibilities.is_exported(self.local_def_id(import.root_id))
|| import.used.get() == Some(Used::Other)
{
return;
}

let mut is_redundant = PerNS { value_ns: None, type_ns: None, macro_ns: None };

let mut redundant_span = PerNS { value_ns: None, type_ns: None, macro_ns: None };
Expand Down Expand Up @@ -1348,7 +1353,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
});

if !is_redundant.is_empty() && is_redundant.present_items().all(|is_redundant| is_redundant)
{
let mut redundant_spans: Vec<_> = redundant_span.present_items().collect();
Expand All @@ -1372,7 +1376,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.tcx.sess.create_err(CannotGlobImportAllCrates { span: import.span }).emit();
return;
};

if module.is_trait() {
self.tcx.sess.create_err(ItemsInTraitsAreNotImportable { span: import.span }).emit();
return;
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
//! `build_reduced_graph.rs`, `macros.rs` and `imports.rs`.

use crate::errors::ImportsCannotReferTo;
use crate::BindingKey;
use crate::{path_names_to_string, rustdoc, BindingError, Finalize, LexicalScopeBinding};
use crate::{BindingKey, Used};
use crate::{Module, ModuleOrUniformRoot, NameBinding, ParentScope, PathResult};
use crate::{ResolutionError, Resolver, Segment, UseError};

Expand Down Expand Up @@ -3482,7 +3482,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
// whether they can be shadowed by fresh bindings or not, so force an error.
// issues/33118#issuecomment-233962221 (see below) still applies here,
// but we have to ignore it for backward compatibility.
self.r.record_use(ident, binding, false);
self.r.record_use(ident, binding, Some(Used::Other));
return None;
}
LexicalScopeBinding::Item(binding) => (binding.res(), Some(binding)),
Expand All @@ -3497,7 +3497,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
) if is_syntactic_ambiguity => {
// Disambiguate in favor of a unit struct/variant or constant pattern.
if let Some(binding) = binding {
self.r.record_use(ident, binding, false);
self.r.record_use(ident, binding, Some(Used::Other));
}
Some(res)
}
Expand Down

0 comments on commit e9ca0af

Please sign in to comment.