Skip to content

Commit

Permalink
Refactor away the field Module::external_module_children in resolve
Browse files Browse the repository at this point in the history
  • Loading branch information
@jseyfried
jseyfried committed Jan 31, 2016
1 parent 9a07087 commit e768fa7
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 115 deletions.
38 changes: 25 additions & 13 deletions src/librustc_resolve/build_reduced_graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,20 +107,37 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
/// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined;
/// otherwise, reports an error.
fn define<T: ToNameBinding<'b>>(&self, parent: Module<'b>, name: Name, ns: Namespace, def: T) {
let name_binding = def.to_name_binding();
let span = name_binding.span.unwrap_or(DUMMY_SP);
self.check_for_conflicts_between_external_crates_and_items(&parent, name, span);
if !parent.try_define_child(name, ns, name_binding) {
let binding = def.to_name_binding();
let old_binding = match parent.try_define_child(name, ns, binding.clone()) {
Some(old_binding) => old_binding,
None => return,
};

let span = binding.span.unwrap_or(DUMMY_SP);
if !old_binding.is_extern_crate() && !binding.is_extern_crate() {
// Record an error here by looking up the namespace that had the duplicate
let ns_str = match ns { TypeNS => "type or module", ValueNS => "value" };
let resolution_error = ResolutionError::DuplicateDefinition(ns_str, name);
let mut err = resolve_struct_error(self, span, resolution_error);

if let Some(sp) = parent.children.borrow().get(&(name, ns)).unwrap().span {
if let Some(sp) = old_binding.span {
let note = format!("first definition of {} `{}` here", ns_str, name);
err.span_note(sp, &note);
}
err.emit();
} else if old_binding.is_extern_crate() && binding.is_extern_crate() {
span_err!(self.session,
span,
E0259,
"an external crate named `{}` has already been imported into this module",
name);
} else {
span_err!(self.session,
span,
E0260,
"the name `{}` conflicts with an external crate \
that has been imported into this module",
name);
}
}

Expand Down Expand Up @@ -289,14 +306,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
self.external_exports.insert(def_id);
let parent_link = ModuleParentLink(parent, name);
let def = Def::Mod(def_id);
let external_module = self.new_module(parent_link, Some(def), false, true);

debug!("(build reduced graph for item) found extern `{}`",
module_to_string(&*external_module));
self.check_for_conflicts_for_external_crate(parent, name, sp);
parent.external_module_children
.borrow_mut()
.insert(name, external_module);
let external_module = self.new_extern_crate_module(parent_link, def);
self.define(parent, name, TypeNS, (external_module, sp));

self.build_reduced_graph_for_external_crate(&external_module);
}
parent
Expand Down
104 changes: 32 additions & 72 deletions src/librustc_resolve/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ enum SuggestionType {
}

pub enum ResolutionError<'a> {
/// error E0260: name conflicts with an extern crate
NameConflictsWithExternCrate(Name),
/// error E0401: can't use type parameters from outer function
TypeParametersFromOuterFunction,
/// error E0402: cannot use an outer type parameter in this context
Expand Down Expand Up @@ -228,14 +226,6 @@ fn resolve_struct_error<'b, 'a: 'b, 'tcx: 'a>(resolver: &'b Resolver<'a, 'tcx>,
}

match resolution_error {
ResolutionError::NameConflictsWithExternCrate(name) => {
struct_span_err!(resolver.session,
span,
E0260,
"the name `{}` conflicts with an external crate \
that has been imported into this module",
name)
}
ResolutionError::TypeParametersFromOuterFunction => {
struct_span_err!(resolver.session,
span,
Expand Down Expand Up @@ -801,14 +791,11 @@ pub struct ModuleS<'a> {
parent_link: ParentLink<'a>,
def: Cell<Option<Def>>,
is_public: bool,
is_extern_crate: bool,

children: RefCell<HashMap<(Name, Namespace), NameBinding<'a>>>,
imports: RefCell<Vec<ImportDirective>>,

// The external module children of this node that were declared with
// `extern crate`.
external_module_children: RefCell<HashMap<Name, Module<'a>>>,

// The anonymous children of this node. Anonymous children are pseudo-
// modules that are implicitly created around items contained within
// blocks.
Expand Down Expand Up @@ -854,9 +841,9 @@ impl<'a> ModuleS<'a> {
parent_link: parent_link,
def: Cell::new(def),
is_public: is_public,
is_extern_crate: false,
children: RefCell::new(HashMap::new()),
imports: RefCell::new(Vec::new()),
external_module_children: RefCell::new(HashMap::new()),
anonymous_children: RefCell::new(NodeMap()),
import_resolutions: RefCell::new(HashMap::new()),
glob_count: Cell::new(0),
Expand All @@ -871,10 +858,21 @@ impl<'a> ModuleS<'a> {
self.children.borrow().get(&(name, ns)).cloned()
}

fn try_define_child(&self, name: Name, ns: Namespace, binding: NameBinding<'a>) -> bool {
// If the name is not yet defined, define the name and return None.
// Otherwise, return the existing definition.
fn try_define_child(&self, name: Name, ns: Namespace, binding: NameBinding<'a>)
-> Option<NameBinding<'a>> {
match self.children.borrow_mut().entry((name, ns)) {
hash_map::Entry::Vacant(entry) => { entry.insert(binding); true }
hash_map::Entry::Occupied(_) => false,
hash_map::Entry::Vacant(entry) => { entry.insert(binding); None }
hash_map::Entry::Occupied(entry) => { Some(entry.get().clone()) },
}
}

fn for_each_local_child<F: FnMut(Name, Namespace, &NameBinding<'a>)>(&self, mut f: F) {
for (&(name, ns), name_binding) in self.children.borrow().iter() {
if !name_binding.is_extern_crate() {
f(name, ns, name_binding)
}
}
}

Expand Down Expand Up @@ -1005,6 +1003,10 @@ impl<'a> NameBinding<'a> {
let def = self.def().unwrap();
(def, LastMod(if self.is_public() { AllPublic } else { DependsOn(def.def_id()) }))
}

fn is_extern_crate(&self) -> bool {
self.module().map(|module| module.is_extern_crate).unwrap_or(false)
}
}

/// Interns the names of the primitive types.
Expand Down Expand Up @@ -1184,6 +1186,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.arenas.modules.alloc(ModuleS::new(parent_link, def, external, is_public))
}

fn new_extern_crate_module(&self, parent_link: ParentLink<'a>, def: Def) -> Module<'a> {
let mut module = ModuleS::new(parent_link, Some(def), false, true);
module.is_extern_crate = true;
self.arenas.modules.alloc(module)
}

fn get_ribs<'b>(&'b mut self, ns: Namespace) -> &'b mut Vec<Rib<'a>> {
match ns { ValueNS => &mut self.value_ribs, TypeNS => &mut self.type_ribs }
}
Expand Down Expand Up @@ -1211,32 +1219,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}

/// Check that an external crate doesn't collide with items or other external crates.
fn check_for_conflicts_for_external_crate(&self, module: Module<'a>, name: Name, span: Span) {
if module.external_module_children.borrow().contains_key(&name) {
span_err!(self.session,
span,
E0259,
"an external crate named `{}` has already been imported into this module",
name);
}
if let Some(name_binding) = module.get_child(name, TypeNS) {
resolve_error(self,
name_binding.span.unwrap_or(codemap::DUMMY_SP),
ResolutionError::NameConflictsWithExternCrate(name));
}
}

/// Checks that the names of items don't collide with external crates.
fn check_for_conflicts_between_external_crates_and_items(&self,
module: Module<'a>,
name: Name,
span: Span) {
if module.external_module_children.borrow().contains_key(&name) {
resolve_error(self, span, ResolutionError::NameConflictsWithExternCrate(name));
}
}

/// Resolves the given module path from the given root `module_`.
fn resolve_module_path_from_root(&mut self,
module_: Module<'a>,
Expand All @@ -1245,11 +1227,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
span: Span,
lp: LastPrivate)
-> ResolveResult<(Module<'a>, LastPrivate)> {
fn search_parent_externals<'a>(needle: Name, module: Module<'a>)
-> Option<Module<'a>> {
match module.external_module_children.borrow().get(&needle) {
Some(_) => Some(module),
None => match module.parent_link {
fn search_parent_externals<'a>(needle: Name, module: Module<'a>) -> Option<Module<'a>> {
match module.get_child(needle, TypeNS) {
Some(ref binding) if binding.is_extern_crate() => Some(module),
_ => match module.parent_link {
ModuleParentLink(ref parent, _) => {
search_parent_externals(needle, parent)
}
Expand Down Expand Up @@ -1480,17 +1461,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}

// Search for external modules.
if namespace == TypeNS {
let children = module_.external_module_children.borrow();
if let Some(module) = children.get(&name) {
let name_binding = NameBinding::create_from_module(module, None);
debug!("lower name bindings succeeded");
return Success((Target::new(module_, name_binding, Shadowable::Never),
false));
}
}

// Finally, proceed up the scope chain looking for parent modules.
let mut search_module = module_;
loop {
Expand Down Expand Up @@ -1684,16 +1654,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
Some(..) | None => {} // Continue.
}

// Finally, search through external children.
if namespace == TypeNS {
let children = module_.external_module_children.borrow();
if let Some(module) = children.get(&name) {
let name_binding = NameBinding::create_from_module(module, None);
return Success((Target::new(module_, name_binding, Shadowable::Never),
false));
}
}

// We're out of luck.
debug!("(resolving name in module) failed to resolve `{}`", name);
return Failed(None);
Expand All @@ -1712,7 +1672,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// Descend into children and anonymous children.
build_reduced_graph::populate_module_if_necessary(self, &module_);

for (_, child_node) in module_.children.borrow().iter() {
module_.for_each_local_child(|_, _, child_node| {
match child_node.module() {
None => {
// Continue.
Expand All @@ -1721,7 +1681,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.report_unresolved_imports(child_module);
}
}
}
});

for (_, module_) in module_.anonymous_children.borrow().iter() {
self.report_unresolved_imports(module_);
Expand Down
46 changes: 17 additions & 29 deletions src/librustc_resolve/resolve_imports.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
self.resolver.current_module = orig_module;

build_reduced_graph::populate_module_if_necessary(self.resolver, &module_);
for (_, child_node) in module_.children.borrow().iter() {
module_.for_each_local_child(|_, _, child_node| {
match child_node.module() {
None => {
// Nothing to do.
Expand All @@ -222,7 +222,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
errors.extend(self.resolve_imports_for_module_subtree(child_module));
}
}
}
});

for (_, child_module) in module_.anonymous_children.borrow().iter() {
errors.extend(self.resolve_imports_for_module_subtree(child_module));
Expand Down Expand Up @@ -386,18 +386,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
-> (ResolveResult<(Module<'b>, NameBinding<'b>)>, bool) {
build_reduced_graph::populate_module_if_necessary(self.resolver, module);
if let Some(name_binding) = module.get_child(name, ns) {
return (Success((module, name_binding)), false);
}

if ns == TypeNS {
if let Some(extern_crate) = module.external_module_children.borrow().get(&name) {
if name_binding.is_extern_crate() {
// track the extern crate as used.
if let Some(DefId{ krate: kid, .. }) = extern_crate.def_id() {
self.resolver.used_crates.insert(kid);
if let Some(DefId { krate, .. }) = name_binding.module().unwrap().def_id() {
self.resolver.used_crates.insert(krate);
}
let name_binding = NameBinding::create_from_module(extern_crate, None);
return (Success((module, name_binding)), false);
}
return (Success((module, name_binding)), false)
}

// If there is an unresolved glob at this point in the containing module, bail out.
Expand Down Expand Up @@ -725,13 +720,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
// Add all children from the containing module.
build_reduced_graph::populate_module_if_necessary(self.resolver, &target_module);

for (&name, name_binding) in target_module.children.borrow().iter() {
target_module.for_each_local_child(|name, ns, name_binding| {
self.merge_import_resolution(module_,
target_module,
import_directive,
name,
(name, ns),
name_binding.clone());
}
});

// Record the destination of this import
if let Some(did) = target_module.def_id() {
Expand Down Expand Up @@ -881,21 +876,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
import: &ImportResolution<'b>,
import_span: Span,
(name, ns): (Name, Namespace)) {
// First, check for conflicts between imports and `extern crate`s.
if ns == TypeNS {
if module.external_module_children.borrow().contains_key(&name) {
match import.target {
Some(ref target) if target.shadowable != Shadowable::Always => {
let msg = format!("import `{0}` conflicts with imported crate \
in this module (maybe you meant `use {0}::*`?)",
name);
span_err!(self.resolver.session, import_span, E0254, "{}", &msg[..]);
}
Some(_) | None => {}
}
}
}

// Check for item conflicts.
let name_binding = match module.get_child(name, ns) {
None => {
Expand Down Expand Up @@ -924,6 +904,14 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
} else {
match import.target {
Some(ref target) if target.shadowable != Shadowable::Always => {
if name_binding.is_extern_crate() {
let msg = format!("import `{0}` conflicts with imported crate \
in this module (maybe you meant `use {0}::*`?)",
name);
span_err!(self.resolver.session, import_span, E0254, "{}", &msg[..]);
return;
}

let (what, note) = match name_binding.module() {
Some(ref module) if module.is_normal() =>
("existing submodule", "note conflicting module here"),
Expand Down
3 changes: 2 additions & 1 deletion src/test/compile-fail/resolve-conflict-item-vs-extern-crate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn std() {} //~ ERROR the name `std` conflicts with an external crate
fn std() {}
mod std {} //~ ERROR the name `std` conflicts with an external crate

fn main() {
}

0 comments on commit e768fa7

Please sign in to comment.