Skip to content

Commit

Permalink
resolve: Move some code around
Browse files Browse the repository at this point in the history 
Avoid using dummy spans for some external items with available spans
  • Loading branch information
@petrochenkov
petrochenkov committed Aug 9, 2019
1 parent 3ee614b commit 2cb9207
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 102 deletions.
60 changes: 30 additions & 30 deletions src/librustc_resolve/build_reduced_graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,7 @@ impl<'a> Resolver<'a> {
// This is only a guess, two equivalent idents may incorrectly get different gensyms here.
let ident = ident.gensym_if_underscore();
let expansion = ExpnId::root(); // FIXME(jseyfried) intercrate hygiene
// Record primary definitions.
match res {
Res::Def(kind @ DefKind::Mod, def_id)
| Res::Def(kind @ DefKind::Enum, def_id)
Expand All @@ -651,53 +652,52 @@ impl<'a> Resolver<'a> {
def_id,
expansion,
span);
self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion));
self.define(parent, ident, TypeNS, (module, vis, span, expansion));
}
Res::Def(DefKind::Variant, _)
Res::Def(DefKind::Struct, _)
| Res::Def(DefKind::Union, _)
| Res::Def(DefKind::Variant, _)
| Res::Def(DefKind::TyAlias, _)
| Res::Def(DefKind::ForeignTy, _)
| Res::Def(DefKind::OpaqueTy, _)
| Res::Def(DefKind::TraitAlias, _)
| Res::Def(DefKind::AssocTy, _)
| Res::Def(DefKind::AssocExistential, _)
| Res::Def(DefKind::AssocOpaqueTy, _)
| Res::PrimTy(..)
| Res::ToolMod => {
self.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion));
}
| Res::ToolMod =>
self.define(parent, ident, TypeNS, (res, vis, span, expansion)),
Res::Def(DefKind::Fn, _)
| Res::Def(DefKind::Method, _)
| Res::Def(DefKind::Static, _)
| Res::Def(DefKind::Const, _)
| Res::Def(DefKind::AssocConst, _)
| Res::Def(DefKind::Ctor(CtorOf::Variant, ..), _) => {
self.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion));
}
Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => {
self.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion));

if let Some(struct_def_id) =
self.cstore.def_key(def_id).parent
.map(|index| DefId { krate: def_id.krate, index: index }) {
self.struct_constructors.insert(struct_def_id, (res, vis));
}
| Res::Def(DefKind::Ctor(..), _) =>
self.define(parent, ident, ValueNS, (res, vis, span, expansion)),
Res::Def(DefKind::Macro(..), _)
| Res::NonMacroAttr(..) =>
self.define(parent, ident, MacroNS, (res, vis, span, expansion)),
Res::Def(DefKind::TyParam, _) | Res::Def(DefKind::ConstParam, _)
| Res::Local(..) | Res::SelfTy(..) | Res::SelfCtor(..) | Res::Err =>
bug!("unexpected resolution: {:?}", res)
}
// Record some extra data for better diagnostics.
match res {
Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => {
let field_names = self.cstore.struct_field_names_untracked(def_id);
self.insert_field_names(def_id, field_names);
}
Res::Def(DefKind::Method, def_id) => {
self.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion));

if self.cstore.associated_item_cloned_untracked(def_id).method_has_self_argument {
self.has_self.insert(def_id);
}
}
Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => {
self.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion));

// Record field names for error reporting.
let field_names = self.cstore.struct_field_names_untracked(def_id);
self.insert_field_names(def_id, field_names);
}
Res::Def(DefKind::Macro(..), _) | Res::NonMacroAttr(..) => {
self.define(parent, ident, MacroNS, (res, vis, DUMMY_SP, expansion));
Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => {
let parent = self.cstore.def_key(def_id).parent;
if let Some(struct_def_id) = parent.map(|index| DefId { index, ..def_id }) {
self.struct_constructors.insert(struct_def_id, (res, vis));
}
}
_ => bug!("unexpected resolution: {:?}", res)
_ => {}
}
}

Expand Down Expand Up @@ -837,7 +837,7 @@ impl<'a> Resolver<'a> {
if let Some(span) = import_all {
let directive = macro_use_directive(span);
self.potentially_unused_imports.push(directive);
module.for_each_child(self, |this, ident, ns, binding| if ns == MacroNS {
self.for_each_child(module, |this, ident, ns, binding| if ns == MacroNS {
let imported_binding = this.import(binding, directive);
this.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing);
});
Expand Down
44 changes: 22 additions & 22 deletions src/librustc_resolve/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,23 +65,23 @@ fn add_typo_suggestion(
false
}

fn add_module_candidates<'a>(
resolver: &mut Resolver<'a>,
module: Module<'a>,
names: &mut Vec<TypoSuggestion>,
filter_fn: &impl Fn(Res) -> bool,
) {
for (&(ident, _), resolution) in resolver.resolutions(module).borrow().iter() {
if let Some(binding) = resolution.borrow().binding {
let res = binding.res();
if filter_fn(res) {
names.push(TypoSuggestion::from_res(ident.name, res));
impl<'a> Resolver<'a> {
fn add_module_candidates(
&mut self,
module: Module<'a>,
names: &mut Vec<TypoSuggestion>,
filter_fn: &impl Fn(Res) -> bool,
) {
for (&(ident, _), resolution) in self.resolutions(module).borrow().iter() {
if let Some(binding) = resolution.borrow().binding {
let res = binding.res();
if filter_fn(res) {
names.push(TypoSuggestion::from_res(ident.name, res));
}
}
}
}
}

impl<'a> Resolver<'a> {
/// Handles error reporting for `smart_resolve_path_fragment` function.
/// Creates base error and amends it with one short label and possibly some longer helps/notes.
pub(crate) fn smart_resolve_report_errors(
Expand Down Expand Up @@ -596,10 +596,10 @@ impl<'a> Resolver<'a> {
Scope::CrateRoot => {
let root_ident = Ident::new(kw::PathRoot, ident.span);
let root_module = this.resolve_crate_root(root_ident);
add_module_candidates(this, root_module, &mut suggestions, filter_fn);
this.add_module_candidates(root_module, &mut suggestions, filter_fn);
}
Scope::Module(module) => {
add_module_candidates(this, module, &mut suggestions, filter_fn);
this.add_module_candidates(module, &mut suggestions, filter_fn);
}
Scope::MacroUsePrelude => {
suggestions.extend(this.macro_use_prelude.iter().filter_map(|(name, binding)| {
Expand Down Expand Up @@ -647,7 +647,7 @@ impl<'a> Resolver<'a> {
Scope::StdLibPrelude => {
if let Some(prelude) = this.prelude {
let mut tmp_suggestions = Vec::new();
add_module_candidates(this, prelude, &mut tmp_suggestions, filter_fn);
this.add_module_candidates(prelude, &mut tmp_suggestions, filter_fn);
suggestions.extend(tmp_suggestions.into_iter().filter(|s| {
use_prelude || this.is_builtin_macro(s.res.opt_def_id())
}));
Expand Down Expand Up @@ -709,7 +709,7 @@ impl<'a> Resolver<'a> {
// Items in scope
if let RibKind::ModuleRibKind(module) = rib.kind {
// Items from this module
add_module_candidates(self, module, &mut names, &filter_fn);
self.add_module_candidates(module, &mut names, &filter_fn);

if let ModuleKind::Block(..) = module.kind {
// We can see through blocks
Expand Down Expand Up @@ -737,7 +737,7 @@ impl<'a> Resolver<'a> {
}));

if let Some(prelude) = self.prelude {
add_module_candidates(self, prelude, &mut names, &filter_fn);
self.add_module_candidates(prelude, &mut names, &filter_fn);
}
}
break;
Expand All @@ -759,7 +759,7 @@ impl<'a> Resolver<'a> {
mod_path, Some(TypeNS), false, span, CrateLint::No
) {
if let ModuleOrUniformRoot::Module(module) = module {
add_module_candidates(self, module, &mut names, &filter_fn);
self.add_module_candidates(module, &mut names, &filter_fn);
}
}
}
Expand Down Expand Up @@ -799,7 +799,7 @@ impl<'a> Resolver<'a> {
in_module_is_extern)) = worklist.pop() {
// We have to visit module children in deterministic order to avoid
// instabilities in reported imports (#43552).
in_module.for_each_child_stable(self, |this, ident, ns, name_binding| {
self.for_each_child_stable(in_module, |this, ident, ns, name_binding| {
// avoid imports entirely
if name_binding.is_import() && !name_binding.is_extern_crate() { return; }
// avoid non-importable candidates as well
Expand Down Expand Up @@ -911,7 +911,7 @@ impl<'a> Resolver<'a> {
// abort if the module is already found
if result.is_some() { break; }

in_module.for_each_child_stable(self, |_, ident, _, name_binding| {
self.for_each_child_stable(in_module, |_, ident, _, name_binding| {
// abort if the module is already found or if name_binding is private external
if result.is_some() || !name_binding.vis.is_visible_locally() {
return
Expand Down Expand Up @@ -943,7 +943,7 @@ impl<'a> Resolver<'a> {
fn collect_enum_variants(&mut self, def_id: DefId) -> Option<Vec<Path>> {
self.find_module(def_id).map(|(enum_module, enum_import_suggestion)| {
let mut variants = Vec::new();
enum_module.for_each_child_stable(self, |_, ident, _, name_binding| {
self.for_each_child_stable(enum_module, |_, ident, _, name_binding| {
if let Res::Def(DefKind::Variant, _) = name_binding.res() {
let mut segms = enum_import_suggestion.path.segments.clone();
segms.push(ast::PathSegment::from_ident(ident));
Expand Down
60 changes: 38 additions & 22 deletions src/librustc_resolve/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1221,25 +1221,6 @@ impl<'a> ModuleData<'a> {
}
}

fn for_each_child<F>(&'a self, resolver: &mut Resolver<'a>, mut f: F)
where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>)
{
for (&(ident, ns), name_resolution) in resolver.resolutions(self).borrow().iter() {
name_resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding));
}
}

fn for_each_child_stable<F>(&'a self, resolver: &mut Resolver<'a>, mut f: F)
where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>)
{
let resolutions = resolver.resolutions(self).borrow();
let mut resolutions = resolutions.iter().collect::<Vec<_>>();
resolutions.sort_by_cached_key(|&(&(ident, ns), _)| (ident.as_str(), ns));
for &(&(ident, ns), &resolution) in resolutions.iter() {
resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding));
}
}

fn res(&self) -> Option<Res> {
match self.kind {
ModuleKind::Def(kind, def_id, _) => Some(Res::Def(kind, def_id)),
Expand Down Expand Up @@ -1904,9 +1885,7 @@ impl<'a> Resolver<'a> {
seg.id = self.session.next_node_id();
seg
}
}

impl<'a> Resolver<'a> {
pub fn new(session: &'a Session,
cstore: &'a CStore,
krate: &Crate,
Expand Down Expand Up @@ -2116,6 +2095,43 @@ impl<'a> Resolver<'a> {
self.arenas.alloc_module(module)
}

fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> {
if module.populate_on_access.get() {
module.populate_on_access.set(false);
let def_id = module.def_id().expect("unpopulated module without a def-id");
for child in self.cstore.item_children_untracked(def_id, self.session) {
let child = child.map_id(|_| panic!("unexpected id"));
self.build_reduced_graph_for_external_crate_res(module, child);
}
}
&module.lazy_resolutions
}

fn resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace)
-> &'a RefCell<NameResolution<'a>> {
*self.resolutions(module).borrow_mut().entry((ident.modern(), ns))
.or_insert_with(|| self.arenas.alloc_name_resolution())
}

fn for_each_child<F>(&mut self, module: Module<'a>, mut f: F)
where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>)
{
for (&(ident, ns), name_resolution) in self.resolutions(module).borrow().iter() {
name_resolution.borrow().binding.map(|binding| f(self, ident, ns, binding));
}
}

fn for_each_child_stable<F>(&mut self, module: Module<'a>, mut f: F)
where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>)
{
let resolutions = self.resolutions(module).borrow();
let mut resolutions = resolutions.iter().collect::<Vec<_>>();
resolutions.sort_by_cached_key(|&(&(ident, ns), _)| (ident.as_str(), ns));
for &(&(ident, ns), &resolution) in resolutions.iter() {
resolution.borrow().binding.map(|binding| f(self, ident, ns, binding));
}
}

fn record_use(&mut self, ident: Ident, ns: Namespace,
used_binding: &'a NameBinding<'a>, is_lexical_scope: bool) {
if let Some((b2, kind)) = used_binding.ambiguity {
Expand Down Expand Up @@ -4531,7 +4547,7 @@ impl<'a> Resolver<'a> {
let mut traits = module.traits.borrow_mut();
if traits.is_none() {
let mut collected_traits = Vec::new();
module.for_each_child(self, |_, name, ns, binding| {
self.for_each_child(module, |_, name, ns, binding| {
if ns != TypeNS { return }
match binding.res() {
Res::Def(DefKind::Trait, _) |
Expand Down
22 changes: 2 additions & 20 deletions src/librustc_resolve/resolve_imports.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{CrateLint, Module, ModuleOrUniformRoot, PerNS, ScopeSet, ParentScope
use crate::Determinacy::{self, *};
use crate::Namespace::{self, TypeNS, MacroNS};
use crate::{NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError};
use crate::{Resolutions, Resolver, Segment};
use crate::{Resolver, Segment};
use crate::{names_to_string, module_to_string};
use crate::{resolve_error, ResolutionError};
use crate::ModuleKind;
Expand Down Expand Up @@ -36,7 +36,7 @@ use syntax_pos::{MultiSpan, Span};

use log::*;

use std::cell::{Cell, RefCell};
use std::cell::Cell;
use std::{mem, ptr};

type Res = def::Res<NodeId>;
Expand Down Expand Up @@ -156,24 +156,6 @@ impl<'a> NameResolution<'a> {
}

impl<'a> Resolver<'a> {
crate fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> {
if module.populate_on_access.get() {
module.populate_on_access.set(false);
let def_id = module.def_id().expect("unpopulated module without a def-id");
for child in self.cstore.item_children_untracked(def_id, self.session) {
let child = child.map_id(|_| panic!("unexpected id"));
self.build_reduced_graph_for_external_crate_res(module, child);
}
}
&module.lazy_resolutions
}

fn resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace)
-> &'a RefCell<NameResolution<'a>> {
*self.resolutions(module).borrow_mut().entry((ident.modern(), ns))
.or_insert_with(|| self.arenas.alloc_name_resolution())
}

crate fn resolve_ident_in_module_unadjusted(
&mut self,
module: ModuleOrUniformRoot<'a>,
Expand Down
8 changes: 6 additions & 2 deletions src/test/ui/imports/extern-prelude-extern-crate-restricted-shadowing.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,19 @@ error[E0659]: `Vec` is ambiguous (macro-expanded name vs less macro-expanded nam
LL | Vec::panic!();
| ^^^ ambiguous name
|
= note: `Vec` could refer to a struct from prelude
note: `Vec` could also refer to the extern crate imported here
note: `Vec` could refer to the extern crate imported here
--> $DIR/extern-prelude-extern-crate-restricted-shadowing.rs:5:9
|
LL | extern crate std as Vec;
| ^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | define_vec!();
| -------------- in this macro invocation
note: `Vec` could also refer to the struct defined here
--> $SRC_DIR/libstd/prelude/v1.rs:LL:COL
|
LL | pub use crate::vec::Vec;
| ^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

Expand Down
Loading

0 comments on commit 2cb9207

Please sign in to comment.