Skip to content

Commit a087df6

Browse files
petrochenkovpetrochenkov
committed
resolve: Move some code around
Avoid using dummy spans for some external items with available spans
1 parent aee4313 commit a087df6

8 files changed

+109
-101
lines changed

‎src/librustc_resolve/build_reduced_graph.rs

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -865,6 +865,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
865865
// This is only a guess, two equivalent idents may incorrectly get different gensyms here.
866866
let ident = ident.gensym_if_underscore();
867867
let expansion = ExpnId::root(); // FIXME(jseyfried) intercrate hygiene
868+
// Record primary definitions.
868869
match res {
869870
Res::Def(kind @ DefKind::Mod, def_id)
870871
| Res::Def(kind @ DefKind::Enum, def_id)
@@ -874,53 +875,52 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
874875
def_id,
875876
expansion,
876877
span);
877-
self.r.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion));
878+
self.r.define(parent, ident, TypeNS, (module, vis, span, expansion));
878879
}
879-
Res::Def(DefKind::Variant, _)
880+
Res::Def(DefKind::Struct, _)
881+
| Res::Def(DefKind::Union, _)
882+
| Res::Def(DefKind::Variant, _)
880883
| Res::Def(DefKind::TyAlias, _)
881884
| Res::Def(DefKind::ForeignTy, _)
882885
| Res::Def(DefKind::OpaqueTy, _)
883886
| Res::Def(DefKind::TraitAlias, _)
884887
| Res::Def(DefKind::AssocTy, _)
885888
| Res::Def(DefKind::AssocOpaqueTy, _)
886889
| Res::PrimTy(..)
887-
| Res::ToolMod => {
888-
self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion));
889-
}
890+
| Res::ToolMod =>
891+
self.r.define(parent, ident, TypeNS, (res, vis, span, expansion)),
890892
Res::Def(DefKind::Fn, _)
893+
| Res::Def(DefKind::Method, _)
891894
| Res::Def(DefKind::Static, _)
892895
| Res::Def(DefKind::Const, _)
893896
| Res::Def(DefKind::AssocConst, _)
894-
| Res::Def(DefKind::Ctor(CtorOf::Variant, ..), _) => {
895-
self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion));
896-
}
897-
Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => {
898-
self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion));
899-
900-
if let Some(struct_def_id) =
901-
self.r.cstore.def_key(def_id).parent
902-
.map(|index| DefId { krate: def_id.krate, index: index }) {
903-
self.r.struct_constructors.insert(struct_def_id, (res, vis));
904-
}
897+
| Res::Def(DefKind::Ctor(..), _) =>
898+
self.r.define(parent, ident, ValueNS, (res, vis, span, expansion)),
899+
Res::Def(DefKind::Macro(..), _)
900+
| Res::NonMacroAttr(..) =>
901+
self.r.define(parent, ident, MacroNS, (res, vis, span, expansion)),
902+
Res::Def(DefKind::TyParam, _) | Res::Def(DefKind::ConstParam, _)
903+
| Res::Local(..) | Res::SelfTy(..) | Res::SelfCtor(..) | Res::Err =>
904+
bug!("unexpected resolution: {:?}", res)
905+
}
906+
// Record some extra data for better diagnostics.
907+
match res {
908+
Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => {
909+
let field_names = self.r.cstore.struct_field_names_untracked(def_id);
910+
self.insert_field_names(def_id, field_names);
905911
}
906912
Res::Def(DefKind::Method, def_id) => {
907-
self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion));
908-
909913
if self.r.cstore.associated_item_cloned_untracked(def_id).method_has_self_argument {
910914
self.r.has_self.insert(def_id);
911915
}
912916
}
913-
Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => {
914-
self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion));
915-
916-
// Record field names for error reporting.
917-
let field_names = self.r.cstore.struct_field_names_untracked(def_id);
918-
self.insert_field_names(def_id, field_names);
919-
}
920-
Res::Def(DefKind::Macro(..), _) | Res::NonMacroAttr(..) => {
921-
self.r.define(parent, ident, MacroNS, (res, vis, DUMMY_SP, expansion));
917+
Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => {
918+
let parent = self.r.cstore.def_key(def_id).parent;
919+
if let Some(struct_def_id) = parent.map(|index| DefId { index, ..def_id }) {
920+
self.r.struct_constructors.insert(struct_def_id, (res, vis));
921+
}
922922
}
923-
_ => bug!("unexpected resolution: {:?}", res)
923+
_ => {}
924924
}
925925
}
926926

@@ -981,7 +981,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
981981
if let Some(span) = import_all {
982982
let directive = macro_use_directive(self, span);
983983
self.r.potentially_unused_imports.push(directive);
984-
module.for_each_child(&mut self.r, |this, ident, ns, binding| if ns == MacroNS {
984+
self.r.for_each_child(module, |this, ident, ns, binding| if ns == MacroNS {
985985
let imported_binding = this.import(binding, directive);
986986
this.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing);
987987
});

‎src/librustc_resolve/diagnostics.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -73,23 +73,23 @@ crate fn add_typo_suggestion(
7373
false
7474
}
7575

76-
crate fn add_module_candidates<'a>(
77-
resolver: &mut Resolver<'a>,
78-
module: Module<'a>,
79-
names: &mut Vec<TypoSuggestion>,
80-
filter_fn: &impl Fn(Res) -> bool,
81-
) {
82-
for (&(ident, _), resolution) in resolver.resolutions(module).borrow().iter() {
83-
if let Some(binding) = resolution.borrow().binding {
84-
let res = binding.res();
85-
if filter_fn(res) {
86-
names.push(TypoSuggestion::from_res(ident.name, res));
76+
impl<'a> Resolver<'a> {
77+
crate fn add_module_candidates(
78+
&mut self,
79+
module: Module<'a>,
80+
names: &mut Vec<TypoSuggestion>,
81+
filter_fn: &impl Fn(Res) -> bool,
82+
) {
83+
for (&(ident, _), resolution) in self.resolutions(module).borrow().iter() {
84+
if let Some(binding) = resolution.borrow().binding {
85+
let res = binding.res();
86+
if filter_fn(res) {
87+
names.push(TypoSuggestion::from_res(ident.name, res));
88+
}
8789
}
8890
}
8991
}
90-
}
9192

92-
impl<'a> Resolver<'a> {
9393
/// Combines an error with provided span and emits it.
9494
///
9595
/// This takes the error provided, combines it with the span and any additional spans inside the
@@ -394,10 +394,10 @@ impl<'a> Resolver<'a> {
394394
Scope::CrateRoot => {
395395
let root_ident = Ident::new(kw::PathRoot, ident.span);
396396
let root_module = this.resolve_crate_root(root_ident);
397-
add_module_candidates(this, root_module, &mut suggestions, filter_fn);
397+
this.add_module_candidates(root_module, &mut suggestions, filter_fn);
398398
}
399399
Scope::Module(module) => {
400-
add_module_candidates(this, module, &mut suggestions, filter_fn);
400+
this.add_module_candidates(module, &mut suggestions, filter_fn);
401401
}
402402
Scope::MacroUsePrelude => {
403403
suggestions.extend(this.macro_use_prelude.iter().filter_map(|(name, binding)| {
@@ -445,7 +445,7 @@ impl<'a> Resolver<'a> {
445445
Scope::StdLibPrelude => {
446446
if let Some(prelude) = this.prelude {
447447
let mut tmp_suggestions = Vec::new();
448-
add_module_candidates(this, prelude, &mut tmp_suggestions, filter_fn);
448+
this.add_module_candidates(prelude, &mut tmp_suggestions, filter_fn);
449449
suggestions.extend(tmp_suggestions.into_iter().filter(|s| {
450450
use_prelude || this.is_builtin_macro(s.res.opt_def_id())
451451
}));
@@ -503,7 +503,7 @@ impl<'a> Resolver<'a> {
503503
in_module_is_extern)) = worklist.pop() {
504504
// We have to visit module children in deterministic order to avoid
505505
// instabilities in reported imports (#43552).
506-
in_module.for_each_child_stable(self, |this, ident, ns, name_binding| {
506+
self.for_each_child_stable(in_module, |this, ident, ns, name_binding| {
507507
// avoid imports entirely
508508
if name_binding.is_import() && !name_binding.is_extern_crate() { return; }
509509
// avoid non-importable candidates as well

‎src/librustc_resolve/late.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1934,7 +1934,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
19341934
let mut traits = module.traits.borrow_mut();
19351935
if traits.is_none() {
19361936
let mut collected_traits = Vec::new();
1937-
module.for_each_child(&mut self.r, |_, name, ns, binding| {
1937+
self.r.for_each_child(module, |_, name, ns, binding| {
19381938
if ns != TypeNS { return }
19391939
match binding.res() {
19401940
Res::Def(DefKind::Trait, _) |

‎src/librustc_resolve/late/diagnostics.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use crate::{CrateLint, Module, ModuleKind, ModuleOrUniformRoot};
22
use crate::{PathResult, PathSource, Segment};
33
use crate::path_names_to_string;
4-
use crate::diagnostics::{add_typo_suggestion, add_module_candidates};
5-
use crate::diagnostics::{ImportSuggestion, TypoSuggestion};
4+
use crate::diagnostics::{add_typo_suggestion, ImportSuggestion, TypoSuggestion};
65
use crate::late::{LateResolutionVisitor, RibKind};
76

87
use errors::{Applicability, DiagnosticBuilder, DiagnosticId};
@@ -548,7 +547,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
548547
// Items in scope
549548
if let RibKind::ModuleRibKind(module) = rib.kind {
550549
// Items from this module
551-
add_module_candidates(&mut self.r, module, &mut names, &filter_fn);
550+
self.r.add_module_candidates(module, &mut names, &filter_fn);
552551

553552
if let ModuleKind::Block(..) = module.kind {
554553
// We can see through blocks
@@ -577,7 +576,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
577576
}));
578577

579578
if let Some(prelude) = self.r.prelude {
580-
add_module_candidates(&mut self.r, prelude, &mut names, &filter_fn);
579+
self.r.add_module_candidates(prelude, &mut names, &filter_fn);
581580
}
582581
}
583582
break;
@@ -599,7 +598,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
599598
mod_path, Some(TypeNS), false, span, CrateLint::No
600599
) {
601600
if let ModuleOrUniformRoot::Module(module) = module {
602-
add_module_candidates(&mut self.r, module, &mut names, &filter_fn);
601+
self.r.add_module_candidates(module, &mut names, &filter_fn);
603602
}
604603
}
605604
}
@@ -717,7 +716,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
717716
// abort if the module is already found
718717
if result.is_some() { break; }
719718

720-
in_module.for_each_child_stable(&mut self.r, |_, ident, _, name_binding| {
719+
self.r.for_each_child_stable(in_module, |_, ident, _, name_binding| {
721720
// abort if the module is already found or if name_binding is private external
722721
if result.is_some() || !name_binding.vis.is_visible_locally() {
723722
return
@@ -749,7 +748,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
749748
fn collect_enum_variants(&mut self, def_id: DefId) -> Option<Vec<Path>> {
750749
self.find_module(def_id).map(|(enum_module, enum_import_suggestion)| {
751750
let mut variants = Vec::new();
752-
enum_module.for_each_child_stable(&mut self.r, |_, ident, _, name_binding| {
751+
self.r.for_each_child_stable(enum_module, |_, ident, _, name_binding| {
753752
if let Res::Def(DefKind::Variant, _) = name_binding.res() {
754753
let mut segms = enum_import_suggestion.path.segments.clone();
755754
segms.push(ast::PathSegment::from_ident(ident));

‎src/librustc_resolve/lib.rs

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ use std::collections::BTreeSet;
5050
use rustc_data_structures::ptr_key::PtrKey;
5151
use rustc_data_structures::sync::Lrc;
5252

53+
use build_reduced_graph::BuildReducedGraphVisitor;
5354
use diagnostics::{Suggestion, ImportSuggestion};
5455
use diagnostics::{find_span_of_binding_until_next_binding, extend_span_to_previous_binding};
5556
use late::{PathSource, Rib, RibKind::*};
@@ -473,25 +474,6 @@ impl<'a> ModuleData<'a> {
473474
}
474475
}
475476

476-
fn for_each_child<F>(&'a self, resolver: &mut Resolver<'a>, mut f: F)
477-
where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>)
478-
{
479-
for (&(ident, ns), name_resolution) in resolver.resolutions(self).borrow().iter() {
480-
name_resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding));
481-
}
482-
}
483-
484-
fn for_each_child_stable<F>(&'a self, resolver: &mut Resolver<'a>, mut f: F)
485-
where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>)
486-
{
487-
let resolutions = resolver.resolutions(self).borrow();
488-
let mut resolutions = resolutions.iter().collect::<Vec<_>>();
489-
resolutions.sort_by_cached_key(|&(&(ident, ns), _)| (ident.as_str(), ns));
490-
for &(&(ident, ns), &resolution) in resolutions.iter() {
491-
resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding));
492-
}
493-
}
494-
495477
fn res(&self) -> Option<Res> {
496478
match self.kind {
497479
ModuleKind::Def(kind, def_id, _) => Some(Res::Def(kind, def_id)),
@@ -1230,6 +1212,44 @@ impl<'a> Resolver<'a> {
12301212
self.arenas.alloc_module(module)
12311213
}
12321214

1215+
fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> {
1216+
if module.populate_on_access.get() {
1217+
module.populate_on_access.set(false);
1218+
let def_id = module.def_id().expect("unpopulated module without a def-id");
1219+
for child in self.cstore.item_children_untracked(def_id, self.session) {
1220+
let child = child.map_id(|_| panic!("unexpected id"));
1221+
BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self }
1222+
.build_reduced_graph_for_external_crate_res(module, child);
1223+
}
1224+
}
1225+
&module.lazy_resolutions
1226+
}
1227+
1228+
fn resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace)
1229+
-> &'a RefCell<NameResolution<'a>> {
1230+
*self.resolutions(module).borrow_mut().entry((ident.modern(), ns))
1231+
.or_insert_with(|| self.arenas.alloc_name_resolution())
1232+
}
1233+
1234+
fn for_each_child<F>(&mut self, module: Module<'a>, mut f: F)
1235+
where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>)
1236+
{
1237+
for (&(ident, ns), name_resolution) in self.resolutions(module).borrow().iter() {
1238+
name_resolution.borrow().binding.map(|binding| f(self, ident, ns, binding));
1239+
}
1240+
}
1241+
1242+
fn for_each_child_stable<F>(&mut self, module: Module<'a>, mut f: F)
1243+
where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>)
1244+
{
1245+
let resolutions = self.resolutions(module).borrow();
1246+
let mut resolutions = resolutions.iter().collect::<Vec<_>>();
1247+
resolutions.sort_by_cached_key(|&(&(ident, ns), _)| (ident.as_str(), ns));
1248+
for &(&(ident, ns), &resolution) in resolutions.iter() {
1249+
resolution.borrow().binding.map(|binding| f(self, ident, ns, binding));
1250+
}
1251+
}
1252+
12331253
fn record_use(&mut self, ident: Ident, ns: Namespace,
12341254
used_binding: &'a NameBinding<'a>, is_lexical_scope: bool) {
12351255
if let Some((b2, kind)) = used_binding.ambiguity {

‎src/librustc_resolve/resolve_imports.rs

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@ use crate::{CrateLint, Module, ModuleOrUniformRoot, PerNS, ScopeSet, ParentScope
55
use crate::Determinacy::{self, *};
66
use crate::Namespace::{self, TypeNS, MacroNS};
77
use crate::{NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError};
8-
use crate::{Resolutions, Resolver, ResolutionError, Segment};
8+
use crate::{Resolver, ResolutionError, Segment, ModuleKind};
99
use crate::{names_to_string, module_to_string};
10-
use crate::ModuleKind;
11-
use crate::build_reduced_graph::BuildReducedGraphVisitor;
1210
use crate::diagnostics::Suggestion;
1311

1412
use errors::Applicability;
@@ -36,7 +34,7 @@ use syntax_pos::{MultiSpan, Span};
3634

3735
use log::*;
3836

39-
use std::cell::{Cell, RefCell};
37+
use std::cell::Cell;
4038
use std::{mem, ptr};
4139

4240
type Res = def::Res<NodeId>;
@@ -160,25 +158,6 @@ impl<'a> NameResolution<'a> {
160158
}
161159

162160
impl<'a> Resolver<'a> {
163-
crate fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> {
164-
if module.populate_on_access.get() {
165-
module.populate_on_access.set(false);
166-
let def_id = module.def_id().expect("unpopulated module without a def-id");
167-
for child in self.cstore.item_children_untracked(def_id, self.session) {
168-
let child = child.map_id(|_| panic!("unexpected id"));
169-
BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self }
170-
.build_reduced_graph_for_external_crate_res(module, child);
171-
}
172-
}
173-
&module.lazy_resolutions
174-
}
175-
176-
crate fn resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace)
177-
-> &'a RefCell<NameResolution<'a>> {
178-
*self.resolutions(module).borrow_mut().entry((ident.modern(), ns))
179-
.or_insert_with(|| self.arenas.alloc_name_resolution())
180-
}
181-
182161
crate fn resolve_ident_in_module_unadjusted(
183162
&mut self,
184163
module: ModuleOrUniformRoot<'a>,
@@ -1037,7 +1016,8 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
10371016

10381017
return if all_ns_failed {
10391018
let resolutions = match module {
1040-
ModuleOrUniformRoot::Module(module) => Some(self.r.resolutions(module).borrow()),
1019+
ModuleOrUniformRoot::Module(module) =>
1020+
Some(self.r.resolutions(module).borrow()),
10411021
_ => None,
10421022
};
10431023
let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter());
@@ -1290,8 +1270,8 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
12901270

12911271
// Ensure that `resolutions` isn't borrowed during `try_define`,
12921272
// since it might get updated via a glob cycle.
1293-
let bindings = self.r.resolutions(module).borrow().iter().filter_map(|(&ident, resolution)| {
1294-
resolution.borrow().binding().map(|binding| (ident, binding))
1273+
let bindings = self.r.resolutions(module).borrow().iter().filter_map(|(ident, resolution)| {
1274+
resolution.borrow().binding().map(|binding| (*ident, binding))
12951275
}).collect::<Vec<_>>();
12961276
for ((mut ident, ns), binding) in bindings {
12971277
let scope = match ident.span.reverse_glob_adjust(module.expansion, directive.span) {

‎src/test/ui/imports/extern-prelude-extern-crate-restricted-shadowing.stderr

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,19 @@ error[E0659]: `Vec` is ambiguous (macro-expanded name vs less macro-expanded nam
1313
LL | Vec::panic!();
1414
| ^^^ ambiguous name
1515
|
16-
= note: `Vec` could refer to a struct from prelude
17-
note: `Vec` could also refer to the crate imported here
16+
note: `Vec` could refer to the crate imported here
1817
--> $DIR/extern-prelude-extern-crate-restricted-shadowing.rs:5:9
1918
|
2019
LL | extern crate std as Vec;
2120
| ^^^^^^^^^^^^^^^^^^^^^^^^
2221
...
2322
LL | define_vec!();
2423
| -------------- in this macro invocation
24+
note: `Vec` could also refer to the struct defined here
25+
--> $SRC_DIR/libstd/prelude/v1.rs:LL:COL
26+
|
27+
LL | pub use crate::vec::Vec;
28+
| ^^^^^^^^^^^^^^^
2529

2630
error: aborting due to 2 previous errors
2731

0 commit comments

Comments
 (0)