user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix issue #21546 and refactor NsDef

Browse source
This commit is contained in:
Jeffrey Seyfried 2015-11-17 09:10:41 +00:00
parent 8a6187fde1
commit 572c2f3e07
4 changed files with 62 additions and 178 deletions
Download patch file Download diff file Expand all files Collapse all files

172
src/librustc_resolve/build_reduced_graph.rs
View file

@ -27,7 +27,6 @@ use resolve_imports::Shadowable;
use {resolve_error, ResolutionError};
use self::DuplicateCheckingMode::*;
use self::NamespaceError::*;
use rustc::metadata::csearch;
use rustc::metadata::decoder::{DefLike, DlDef, DlField, DlImpl};
@ -60,29 +59,12 @@ use std::rc::Rc;
// another item exists with the same name in some namespace.
#[derive(Copy, Clone, PartialEq)]
enum DuplicateCheckingMode {
ForbidDuplicateModules,
ForbidDuplicateTypesAndModules,
ForbidDuplicateTypes,
ForbidDuplicateValues,
ForbidDuplicateTypesAndValues,
OverwriteDuplicates,
}
#[derive(Copy, Clone, PartialEq)]
enum NamespaceError {
NoError,
ModuleError,
TypeError,
ValueError,
}
fn namespace_error_to_string(ns: NamespaceError) -> &'static str {
match ns {
NoError => "",
ModuleError | TypeError => "type or module",
ValueError => "value",
}
}
struct GraphBuilder<'a, 'b: 'a, 'tcx: 'b> {
resolver: &'a mut Resolver<'b, 'tcx>,
}
@ -112,16 +94,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
visit::walk_crate(&mut visitor, krate);
}
/// Adds a new child item to the module definition of the parent node and
/// returns its corresponding name bindings as well as the current parent.
/// Or, if we're inside a block, creates (or reuses) an anonymous module
/// corresponding to the innermost block ID and returns the name bindings
/// as well as the newly-created parent.
///
/// # Panics
///
/// Panics if this node does not have a module definition and we are not inside
/// a block.
/// Adds a new child item to the module definition of the parent node,
/// or if there is already a child, does duplicate checking on the child.
/// Returns the child's corresponding name bindings.
fn add_child(&self,
name: Name,
parent: &Rc<Module>,
@ -129,10 +104,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
// For printing errors
sp: Span)
-> NameBindings {
// If this is the immediate descendant of a module, then we add the
// child name directly. Otherwise, we create or reuse an anonymous
// module and add the child to that.
self.check_for_conflicts_between_external_crates_and_items(&**parent, name, sp);
// Add or reuse the child.
@ -146,79 +117,33 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
Some(child) => {
// Enforce the duplicate checking mode:
//
// * If we're requesting duplicate module checking, check that
// there isn't a module in the module with the same name.
//
// * If we're requesting duplicate type checking, check that
// there isn't a type in the module with the same name.
// the name isn't defined in the type namespace.
//
// * If we're requesting duplicate value checking, check that
// there isn't a value in the module with the same name.
// the name isn't defined in the value namespace.
//
// * If we're requesting duplicate type checking and duplicate
// value checking, check that there isn't a duplicate type
// and a duplicate value with the same name.
// * If we're requesting duplicate type and value checking,
// check that the name isn't defined in either namespace.
//
// * If no duplicate checking was requested at all, do
// nothing.
let mut duplicate_type = NoError;
let ns = match duplicate_checking_mode {
ForbidDuplicateModules => {
if child.get_module_if_available().is_some() {
duplicate_type = ModuleError;
}
Some(TypeNS)
}
ForbidDuplicateTypesAndModules => {
if child.type_ns.defined() {
duplicate_type = TypeError;
}
Some(TypeNS)
}
ForbidDuplicateValues => {
if child.value_ns.defined() {
duplicate_type = ValueError;
}
Some(ValueNS)
}
ForbidDuplicateTypesAndValues => {
let mut n = None;
match child.type_ns.def() {
Some(DefMod(_)) | None => {}
Some(_) => {
n = Some(TypeNS);
duplicate_type = TypeError;
}
}
if child.value_ns.defined() {
duplicate_type = ValueError;
n = Some(ValueNS);
}
n
}
OverwriteDuplicates => None,
ForbidDuplicateTypes if child.type_ns.defined() => TypeNS,
ForbidDuplicateValues if child.value_ns.defined() => ValueNS,
ForbidDuplicateTypesAndValues if child.type_ns.defined() => TypeNS,
ForbidDuplicateTypesAndValues if child.value_ns.defined() => ValueNS,
_ => return child,
};
if duplicate_type != NoError {
// Return an error here by looking up the namespace that
// had the duplicate.
let ns = ns.unwrap();
resolve_error(
self,
sp,
ResolutionError::DuplicateDefinition(
namespace_error_to_string(duplicate_type),
name)
);
{
let r = child[ns].span();
if let Some(sp) = r {
self.session.span_note(sp,
&format!("first definition of {} `{}` here",
namespace_error_to_string(duplicate_type),
name));
}
}
// Record an error here by looking up the namespace that had the duplicate
let ns_str = match ns { TypeNS => "type or module", ValueNS => "value" };
resolve_error(self, sp, ResolutionError::DuplicateDefinition(ns_str, name));
if let Some(sp) = child[ns].span() {
let note = format!("first definition of {} `{}` here", ns_str, name);
self.session.span_note(sp, &note);
}
child
}
@ -409,29 +334,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
}
ItemMod(..) => {
let child = parent.children.borrow().get(&name).cloned();
if let Some(child) = child {
// check if there's struct of the same name already defined
if child.type_ns.defined() &&
child.get_module_if_available().is_none() {
self.session.span_warn(sp,
&format!("duplicate definition of {} `{}`. \
Defining a module and a struct with \
the same name will be disallowed soon.",
namespace_error_to_string(TypeError),
name));
{
let r = child.type_ns.span();
if let Some(sp) = r {
self.session.span_note(sp,
&format!("first definition of {} `{}` here",
namespace_error_to_string(TypeError),
name));
}
}
}
}
let name_bindings = self.add_child(name, parent, ForbidDuplicateModules, sp);
let name_bindings = self.add_child(name, parent, ForbidDuplicateTypes, sp);
let parent_link = self.get_parent_link(parent, name);
let def = DefMod(self.ast_map.local_def_id(item.id));
@ -469,7 +372,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
ItemTy(..) => {
let name_bindings = self.add_child(name,
parent,
ForbidDuplicateTypesAndModules,
ForbidDuplicateTypes,
sp);
let parent_link = self.get_parent_link(parent, name);
@ -481,7 +384,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
ItemEnum(ref enum_definition, _) => {
let name_bindings = self.add_child(name,
parent,
ForbidDuplicateTypesAndModules,
ForbidDuplicateTypes,
sp);
let parent_link = self.get_parent_link(parent, name);
@ -501,31 +404,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
ItemStruct(ref struct_def, _) => {
// Adding to both Type and Value namespaces or just Type?
let (forbid, ctor_id) = if struct_def.is_struct() {
(ForbidDuplicateTypesAndModules, None)
(ForbidDuplicateTypes, None)
} else {
let child = parent.children.borrow().get(&name).cloned();
if let Some(child) = child {
// check if theres a DefMod
if let Some(DefMod(_)) = child.type_ns.def() {
self.session.span_warn(sp,
&format!("duplicate definition of {} `{}`. \
Defining a module and a struct \
with the same name will be \
disallowed soon.",
namespace_error_to_string(TypeError),
name));
{
let r = child.type_ns.span();
if let Some(sp) = r {
self.session
.span_note(sp,
&format!("first definition of {} `{}` here",
namespace_error_to_string(TypeError),
name));
}
}
}
}
(ForbidDuplicateTypesAndValues, Some(struct_def.id()))
};
@ -566,7 +446,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
ItemTrait(_, _, _, ref items) => {
let name_bindings = self.add_child(name,
parent,
ForbidDuplicateTypesAndModules,
ForbidDuplicateTypes,
sp);
let def_id = self.ast_map.local_def_id(item.id);

56
src/librustc_resolve/lib.rs
View file

@ -904,17 +904,20 @@ bitflags! {
}
}
// Records a possibly-private definition.
// FIXME once #21546 is resolved, the def and module fields will never both be Some,
// so they can be refactored into something like Result<Def, Rc<Module>>.
// Records a possibly-private value, type, or module definition.
#[derive(Debug)]
struct NsDef {
modifiers: DefModifiers, // see note in ImportResolution about how to use this
def: Option<Def>,
module: Option<Rc<Module>>,
def_or_module: DefOrModule,
span: Option<Span>,
}
#[derive(Debug)]
enum DefOrModule {
Def(Def),
Module(Rc<Module>),
}
impl NsDef {
fn create_from_module(module: Rc<Module>, span: Option<Span>) -> Self {
let modifiers = if module.is_public {
@ -923,14 +926,24 @@ impl NsDef {
DefModifiers::empty()
} | DefModifiers::IMPORTABLE;
NsDef { modifiers: modifiers, def: None, module: Some(module), span: span }
NsDef { modifiers: modifiers, def_or_module: DefOrModule::Module(module), span: span }
}
fn create_from_def(def: Def, modifiers: DefModifiers, span: Option<Span>) -> Self {
NsDef { modifiers: modifiers, def_or_module: DefOrModule::Def(def), span: span }
}
fn module(&self) -> Option<Rc<Module>> {
match self.def_or_module {
DefOrModule::Module(ref module) => Some(module.clone()),
DefOrModule::Def(_) => None,
}
}
fn def(&self) -> Option<Def> {
match (self.def, &self.module) {
(def @ Some(_), _) => def,
(_, &Some(ref module)) => module.def.get(),
_ => panic!("NsDef has neither a Def nor a Module"),
match self.def_or_module {
DefOrModule::Def(def) => Some(def),
DefOrModule::Module(ref module) => module.def.get(),
}
}
}
@ -964,10 +977,10 @@ impl NameBinding {
fn borrow(&self) -> ::std::cell::Ref<Option<NsDef>> { self.0.borrow() }
// Lifted versions of the NsDef fields and method
// Lifted versions of the NsDef methods and fields
fn def(&self) -> Option<Def> { self.and_then(NsDef::def) }
fn module(&self) -> Option<Rc<Module>> { self.and_then(NsDef::module) }
fn span(&self) -> Option<Span> { self.and_then(|def| def.span) }
fn module(&self) -> Option<Rc<Module>> { self.and_then(|def| def.module.clone()) }
fn modifiers(&self) -> Option<DefModifiers> { self.and_then(|def| Some(def.modifiers)) }
fn defined(&self) -> bool { self.borrow().is_some() }
@ -1029,23 +1042,14 @@ impl NameBindings {
/// Records a type definition.
fn define_type(&self, def: Def, sp: Span, modifiers: DefModifiers) {
debug!("defining type for def {:?} with modifiers {:?}",
def,
modifiers);
// Merges the type with the existing type def or creates a new one.
self.type_ns.set(NsDef {
modifiers: modifiers, def: Some(def), module: self.type_ns.module(), span: Some(sp)
});
debug!("defining type for def {:?} with modifiers {:?}", def, modifiers);
self.type_ns.set(NsDef::create_from_def(def, modifiers, Some(sp)));
}
/// Records a value definition.
fn define_value(&self, def: Def, sp: Span, modifiers: DefModifiers) {
debug!("defining value for def {:?} with modifiers {:?}",
def,
modifiers);
self.value_ns.set(NsDef {
modifiers: modifiers, def: Some(def), module: None, span: Some(sp)
});
debug!("defining value for def {:?} with modifiers {:?}", def, modifiers);
self.value_ns.set(NsDef::create_from_def(def, modifiers, Some(sp)));
}
/// Returns the module node if applicable.
@ -1524,7 +1528,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.used_imports.insert((id, namespace));
self.record_import_use(id, name);
if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() {
self.used_crates.insert(kid);
self.used_crates.insert(kid);
}
return Success((target, false));
}

2
src/librustc_resolve/resolve_imports.rs
View file

@ -1039,7 +1039,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
match import_resolution.type_target {
Some(ref target) if target.shadowable != Shadowable::Always => {
if let Some(ref ty) = *name_bindings.type_ns.borrow() {
let (what, note) = match ty.module.clone() {
let (what, note) = match ty.module() {
Some(ref module) if module.is_normal() =>
("existing submodule", "note conflicting module here"),
Some(ref module) if module.is_trait() =>

10
src/test/compile-fail/issue-21546.rs
View file

@ -16,7 +16,7 @@ mod Foo { }
#[allow(dead_code)]
struct Foo;
//~^ WARNING duplicate definition of type or module `Foo`
//~^ ERROR duplicate definition of type or module `Foo`
#[allow(non_snake_case)]
@ -25,7 +25,7 @@ mod Bar { }
#[allow(dead_code)]
struct Bar(i32);
//~^ WARNING duplicate definition of type or module `Bar`
//~^ ERROR duplicate definition of type or module `Bar`
#[allow(dead_code)]
@ -34,7 +34,7 @@ struct Baz(i32);
#[allow(non_snake_case)]
mod Baz { }
//~^ WARNING duplicate definition of type or module `Baz`
//~^ ERROR duplicate definition of type or module `Baz`
#[allow(dead_code)]
@ -43,7 +43,7 @@ struct Qux { x: bool }
#[allow(non_snake_case)]
mod Qux { }
//~^ WARNING duplicate definition of type or module `Qux`
//~^ ERROR duplicate definition of type or module `Qux`
#[allow(dead_code)]
@ -52,7 +52,7 @@ struct Quux;
#[allow(non_snake_case)]
mod Quux { }
//~^ WARNING duplicate definition of type or module `Quux`
//~^ ERROR duplicate definition of type or module `Quux`
#[allow(dead_code)]