Skip to content

Commit ad6f843

Browse files
committed
check redundant prelude imports
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`
1 parent f440b5f commit ad6f843

File tree

98 files changed

+232
-208
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

98 files changed

+232
-208
lines changed

compiler/rustc_macros/src/hash_stable.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
use proc_macro2::{self, Ident};
1+
use proc_macro2::Ident;
22
use quote::quote;
3-
use syn::{self, parse_quote};
3+
use syn::parse_quote;
44

55
struct Attributes {
66
ignore: bool,

compiler/rustc_macros/src/lift.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use quote::quote;
2-
use syn::{self, parse_quote};
2+
use syn::parse_quote;
33

44
pub fn lift_derive(mut s: synstructure::Structure<'_>) -> proc_macro2::TokenStream {
55
s.add_bounds(synstructure::AddBounds::Generics);

compiler/rustc_resolve/src/build_reduced_graph.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::Namespace::{self, MacroNS, TypeNS, ValueNS};
1212
use crate::{errors, BindingKey, MacroData, NameBindingData};
1313
use crate::{Determinacy, ExternPreludeEntry, Finalize, Module, ModuleKind, ModuleOrUniformRoot};
1414
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PerNS, ResolutionError};
15-
use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, VisResolutionError};
15+
use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, Used, VisResolutionError};
1616

1717
use rustc_ast::visit::{self, AssocCtxt, Visitor};
1818
use rustc_ast::{self as ast, AssocItem, AssocItemKind, MetaItemKind, StmtKind};
@@ -358,7 +358,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
358358
root_span,
359359
root_id,
360360
vis: Cell::new(Some(vis)),
361-
used: Cell::new(false),
361+
used: Default::default(),
362362
});
363363

364364
self.r.indeterminate_imports.push(import);
@@ -852,7 +852,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
852852
span: item.span,
853853
module_path: Vec::new(),
854854
vis: Cell::new(Some(vis)),
855-
used: Cell::new(used),
855+
used: Cell::new(used.then_some(Used::Other)),
856856
});
857857
self.r.potentially_unused_imports.push(import);
858858
let imported_binding = self.r.import(binding, import);
@@ -1061,7 +1061,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
10611061
span,
10621062
module_path: Vec::new(),
10631063
vis: Cell::new(Some(ty::Visibility::Restricted(CRATE_DEF_ID))),
1064-
used: Cell::new(false),
1064+
used: Default::default(),
10651065
})
10661066
};
10671067

@@ -1225,7 +1225,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
12251225
span,
12261226
module_path: Vec::new(),
12271227
vis: Cell::new(Some(vis)),
1228-
used: Cell::new(true),
1228+
used: Cell::new(Some(Used::Other)),
12291229
});
12301230
let import_binding = self.r.import(binding, import);
12311231
self.r.define(self.r.graph_root, ident, MacroNS, import_binding);

compiler/rustc_resolve/src/check_unused.rs

+21-1
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,26 @@ impl<'a, 'b, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'tcx> {
182182
}
183183
} else {
184184
self.check_import(id);
185+
let import = self.r.redundant_imports.remove(&id);
186+
if let Some(import) = import {
187+
if let ImportKind::Single {
188+
source,
189+
target,
190+
ref source_bindings,
191+
ref target_bindings,
192+
..
193+
} = import.kind
194+
&& !self.unused_imports.contains_key(&self.base_id)
195+
{
196+
self.r.check_for_redundant_imports(
197+
source,
198+
import,
199+
source_bindings,
200+
target_bindings,
201+
target,
202+
);
203+
}
204+
}
185205
}
186206

187207
visit::walk_use_tree(self, use_tree, id);
@@ -286,7 +306,7 @@ impl Resolver<'_, '_> {
286306

287307
for import in self.potentially_unused_imports.iter() {
288308
match import.kind {
289-
_ if import.used.get()
309+
_ if import.used.get().is_some()
290310
|| import.expect_vis().is_public()
291311
|| import.span.is_dummy() =>
292312
{

compiler/rustc_resolve/src/diagnostics.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ use crate::errors::{
3333
};
3434
use crate::imports::{Import, ImportKind};
3535
use crate::late::{PatternSource, Rib};
36-
use crate::path_names_to_string;
3736
use crate::{errors as errs, BindingKey};
37+
use crate::{path_names_to_string, Used};
3838
use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, BindingError, Finalize};
3939
use crate::{HasGenericParams, MacroRulesScope, Module, ModuleKind, ModuleOrUniformRoot};
4040
use crate::{LexicalScopeBinding, NameBinding, NameBindingKind, PrivacyError, VisResolutionError};
@@ -1482,7 +1482,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
14821482
);
14831483
// Silence the 'unused import' warning we might get,
14841484
// since this diagnostic already covers that import.
1485-
self.record_use(ident, binding, false);
1485+
self.record_use(ident, binding, Some(Used::Other));
14861486
return;
14871487
}
14881488
}

compiler/rustc_resolve/src/ident.rs

+11-6
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ use crate::late::{
1414
ConstantHasGenerics, HasGenericParams, NoConstantGenericsReason, PathSource, Rib, RibKind,
1515
};
1616
use crate::macros::{sub_namespace_match, MacroRulesScope};
17-
use crate::BindingKey;
1817
use crate::{errors, AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, Determinacy, Finalize};
18+
use crate::{BindingKey, Used};
1919
use crate::{ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot};
2020
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PrivacyError, Res};
2121
use crate::{ResolutionError, Resolver, Scope, ScopeSet, Segment, ToNameBinding, Weak};
@@ -335,14 +335,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
335335
ModuleKind::Block => {} // We can see through blocks
336336
_ => break,
337337
}
338-
339338
let item = self.resolve_ident_in_module_unadjusted(
340339
ModuleOrUniformRoot::Module(module),
341340
ident,
342341
ns,
343342
parent_scope,
344343
finalize,
345344
ignore_binding,
345+
None,
346346
);
347347
if let Ok(binding) = item {
348348
// The ident resolves to an item.
@@ -509,6 +509,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
509509
!matches!(scope_set, ScopeSet::Late(..)),
510510
finalize,
511511
ignore_binding,
512+
Some(Used::Scope),
512513
);
513514
match binding {
514515
Ok(binding) => {
@@ -579,6 +580,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
579580
parent_scope,
580581
None,
581582
ignore_binding,
583+
Some(Used::Other),
582584
) {
583585
if use_prelude || this.is_builtin_macro(binding.res()) {
584586
result = Ok((binding, Flags::MISC_FROM_PRELUDE));
@@ -754,6 +756,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
754756
false,
755757
finalize,
756758
ignore_binding,
759+
Some(Used::Other),
757760
)
758761
}
759762

@@ -766,6 +769,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
766769
parent_scope: &ParentScope<'a>,
767770
finalize: Option<Finalize>,
768771
ignore_binding: Option<NameBinding<'a>>,
772+
used: Option<Used>,
769773
) -> Result<NameBinding<'a>, Determinacy> {
770774
self.resolve_ident_in_module_unadjusted_ext(
771775
module,
@@ -775,6 +779,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
775779
false,
776780
finalize,
777781
ignore_binding,
782+
used,
778783
)
779784
.map_err(|(determinacy, _)| determinacy)
780785
}
@@ -793,6 +798,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
793798
// This binding should be ignored during in-module resolution, so that we don't get
794799
// "self-confirming" import resolutions during import validation and checking.
795800
ignore_binding: Option<NameBinding<'a>>,
801+
used: Option<Used>,
796802
) -> Result<NameBinding<'a>, (Determinacy, Weak)> {
797803
let module = match module {
798804
ModuleOrUniformRoot::Module(module) => module,
@@ -849,15 +855,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
849855
let key = BindingKey::new(ident, ns);
850856
let resolution =
851857
self.resolution(module, key).try_borrow_mut().map_err(|_| (Determined, Weak::No))?; // This happens when there is a cycle of imports.
852-
853858
// If the primary binding is unusable, search further and return the shadowed glob
854859
// binding if it exists. What we really want here is having two separate scopes in
855860
// a module - one for non-globs and one for globs, but until that's done use this
856861
// hack to avoid inconsistent resolution ICEs during import validation.
857862
let binding = [resolution.binding, resolution.shadowed_glob]
858863
.into_iter()
859864
.find_map(|binding| if binding == ignore_binding { None } else { binding });
860-
861865
if let Some(Finalize { path_span, report_private, .. }) = finalize {
862866
let Some(binding) = binding else {
863867
return Err((Determined, Weak::No));
@@ -901,8 +905,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
901905
self.macro_expanded_macro_export_errors.insert((path_span, binding.span));
902906
}
903907
}
904-
905-
self.record_use(ident, binding, restricted_shadowing);
908+
self.record_use(ident, binding, used);
906909
return Ok(binding);
907910
}
908911

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

10341039
match result {

compiler/rustc_resolve/src/imports.rs

+22-19
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ use crate::errors::{
1010
use crate::Determinacy::{self, *};
1111
use crate::{fluent_generated as fluent, Namespace::*};
1212
use crate::{module_to_string, names_to_string, ImportSuggestion};
13-
use crate::{AmbiguityKind, BindingKey, ModuleKind, ResolutionError, Resolver, Segment};
13+
use crate::{AmbiguityKind, BindingKey, ResolutionError, Resolver, Segment};
1414
use crate::{Finalize, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet};
15-
use crate::{NameBinding, NameBindingData, NameBindingKind, PathResult};
15+
use crate::{NameBinding, NameBindingData, NameBindingKind, PathResult, Used};
1616

1717
use rustc_ast::NodeId;
1818
use rustc_data_structures::fx::FxHashSet;
@@ -169,7 +169,7 @@ pub(crate) struct ImportData<'a> {
169169
/// The resolution of `module_path`.
170170
pub imported_module: Cell<Option<ModuleOrUniformRoot<'a>>>,
171171
pub vis: Cell<Option<ty::Visibility>>,
172-
pub used: Cell<bool>,
172+
pub used: Cell<Option<Used>>,
173173
}
174174

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

284284
self.arenas.alloc_name_binding(NameBindingData {
285-
kind: NameBindingKind::Import { binding, import, used: Cell::new(false) },
285+
kind: NameBindingKind::Import { binding, import },
286286
ambiguity: None,
287287
warn_ambiguity: false,
288288
span: import.span,
@@ -478,9 +478,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
478478
let key = BindingKey::new(target, ns);
479479
let _ = this.try_define(import.parent_scope.module, key, dummy_binding, false);
480480
});
481-
self.record_use(target, dummy_binding, false);
481+
self.record_use(target, dummy_binding, Some(Used::Other));
482482
} else if import.imported_module.get().is_none() {
483-
import.used.set(true);
483+
import.used.set(Some(Used::Other));
484484
if let Some(id) = import.id() {
485485
self.used_imports.insert(id);
486486
}
@@ -1040,11 +1040,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
10401040
&& initial_binding.is_extern_crate()
10411041
&& !initial_binding.is_import()
10421042
{
1043-
this.record_use(
1044-
ident,
1045-
target_binding,
1046-
import.module_path.is_empty(),
1047-
);
1043+
let used = if import.module_path.is_empty() {
1044+
Used::Scope
1045+
} else {
1046+
Used::Other
1047+
};
1048+
this.record_use(ident, target_binding, Some(used));
10481049
}
10491050
}
10501051
initial_binding.res()
@@ -1291,14 +1292,16 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
12911292
this.import_res_map.entry(import_id).or_default()[ns] = Some(binding.res());
12921293
}
12931294
});
1294-
1295-
self.check_for_redundant_imports(ident, import, source_bindings, target_bindings, target);
1295+
if let ImportKind::Single { id, .. } = import.kind {
1296+
debug!("redundant_imports insert id:{:?} import:{:?}", id, import);
1297+
self.redundant_imports.insert(id, import);
1298+
}
12961299

12971300
debug!("(resolving single import) successfully resolved import");
12981301
None
12991302
}
13001303

1301-
fn check_for_redundant_imports(
1304+
pub(crate) fn check_for_redundant_imports(
13021305
&mut self,
13031306
ident: Ident,
13041307
import: Import<'a>,
@@ -1313,13 +1316,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13131316
if import.parent_scope.expansion != LocalExpnId::ROOT {
13141317
return;
13151318
}
1316-
13171319
// Skip if we are inside a named module (in contrast to an anonymous
13181320
// module defined by a block).
1319-
if let ModuleKind::Def(..) = import.parent_scope.module.kind {
1321+
// Skip if the import is public or was used through non scope-based resolution,
1322+
// e.g. through a module-relative path.
1323+
if self.effective_visibilities.is_exported(self.local_def_id(import.root_id))
1324+
|| import.used.get() == Some(Used::Other)
1325+
{
13201326
return;
13211327
}
1322-
13231328
let mut is_redundant = PerNS { value_ns: None, type_ns: None, macro_ns: None };
13241329

13251330
let mut redundant_span = PerNS { value_ns: None, type_ns: None, macro_ns: None };
@@ -1348,7 +1353,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13481353
}
13491354
}
13501355
});
1351-
13521356
if !is_redundant.is_empty() && is_redundant.present_items().all(|is_redundant| is_redundant)
13531357
{
13541358
let mut redundant_spans: Vec<_> = redundant_span.present_items().collect();
@@ -1372,7 +1376,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13721376
self.tcx.sess.create_err(CannotGlobImportAllCrates { span: import.span }).emit();
13731377
return;
13741378
};
1375-
13761379
if module.is_trait() {
13771380
self.tcx.sess.create_err(ItemsInTraitsAreNotImportable { span: import.span }).emit();
13781381
return;

compiler/rustc_resolve/src/late.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
//! `build_reduced_graph.rs`, `macros.rs` and `imports.rs`.
88
99
use crate::errors::ImportsCannotReferTo;
10-
use crate::BindingKey;
1110
use crate::{path_names_to_string, rustdoc, BindingError, Finalize, LexicalScopeBinding};
11+
use crate::{BindingKey, Used};
1212
use crate::{Module, ModuleOrUniformRoot, NameBinding, ParentScope, PathResult};
1313
use crate::{ResolutionError, Resolver, Segment, UseError};
1414

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

0 commit comments

Comments
 (0)