nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-10-31 14:31:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix a bug in which the visibility of a use declaration defining a name in one namespace (e.g. the value namespace) is overridden by a later use declaration defining the same name in the other namespace (e.g. the type namespace).

Browse Source
This commit is contained in:
Jeffrey Seyfried 2015-12-01 23:06:34 +00:00
parent 81ae8be71c
commit 27c4f9e7b1
6 changed files with 152 additions and 146 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/librustc_resolve/build_reduced_graph.rs
View File

@ -16,7 +16,7 @@
use DefModifiers; use DefModifiers;
use resolve_imports::ImportDirective; use resolve_imports::ImportDirective;
use resolve_imports::ImportDirectiveSubclass::{self, SingleImport, GlobImport}; use resolve_imports::ImportDirectiveSubclass::{self, SingleImport, GlobImport};
use resolve_imports::ImportResolution; use resolve_imports::{ImportResolution, NsImportResolution};
use Module; use Module;
use Namespace::{TypeNS, ValueNS}; use Namespace::{TypeNS, ValueNS};
use NameBindings; use NameBindings;
@ -827,9 +827,10 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
resolution.outstanding_references += 1; resolution.outstanding_references += 1;
// the source of this name is different now // the source of this name is different now
resolution.type_id = id; let ns_resolution =
resolution.value_id = id; NsImportResolution { id: id, is_public: is_public, target: None };
resolution.is_public = is_public; resolution[TypeNS] = ns_resolution.clone();
resolution[ValueNS] = ns_resolution;
return; return;
} }
None => {} None => {}

26
src/librustc_resolve/lib.rs
View File

@ -328,7 +328,7 @@ fn resolve_error<'b, 'a: 'b, 'tcx: 'a>(resolver: &'b Resolver<'a, 'tcx>,
.import_resolutions .import_resolutions
.borrow() .borrow()
.get(&name) { .get(&name) {
let item = resolver.ast_map.expect_item(directive.value_id); let item = resolver.ast_map.expect_item(directive.value_ns.id);
resolver.session.span_note(item.span, "constant imported here"); resolver.session.span_note(item.span, "constant imported here");
} }
} }
@ -1491,7 +1491,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// adjacent import statements are processed as though they mutated the // adjacent import statements are processed as though they mutated the
// current scope. // current scope.
if let Some(import_resolution) = module_.import_resolutions.borrow().get(&name) { if let Some(import_resolution) = module_.import_resolutions.borrow().get(&name) {
match (*import_resolution).target_for_namespace(namespace) { match import_resolution[namespace].target.clone() {
None => { None => {
// Not found; continue. // Not found; continue.
debug!("(resolving item in lexical scope) found import resolution, but not \ debug!("(resolving item in lexical scope) found import resolution, but not \
@ -1501,7 +1501,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
Some(target) => { Some(target) => {
debug!("(resolving item in lexical scope) using import resolution"); debug!("(resolving item in lexical scope) using import resolution");
// track used imports and extern crates as well // track used imports and extern crates as well
let id = import_resolution.id(namespace); let id = import_resolution[namespace].id;
self.used_imports.insert((id, namespace)); self.used_imports.insert((id, namespace));
self.record_import_use(id, name); self.record_import_use(id, name);
if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() { if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() {
@ -1712,13 +1712,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// Check the list of resolved imports. // Check the list of resolved imports.
match module_.import_resolutions.borrow().get(&name) { match module_.import_resolutions.borrow().get(&name) {
Some(import_resolution) if allow_private_imports || import_resolution.is_public => { Some(import_resolution) if allow_private_imports ||
import_resolution[namespace].is_public => {
if import_resolution.is_public && import_resolution.outstanding_references != 0 { if import_resolution[namespace].is_public &&
import_resolution.outstanding_references != 0 {
debug!("(resolving name in module) import unresolved; bailing out"); debug!("(resolving name in module) import unresolved; bailing out");
return Indeterminate; return Indeterminate;
} }
match import_resolution.target_for_namespace(namespace) { match import_resolution[namespace].target.clone() {
None => { None => {
debug!("(resolving name in module) name found, but not in namespace {:?}", debug!("(resolving name in module) name found, but not in namespace {:?}",
namespace); namespace);
@ -1726,7 +1728,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
Some(target) => { Some(target) => {
debug!("(resolving name in module) resolved to import"); debug!("(resolving name in module) resolved to import");
// track used imports and extern crates as well // track used imports and extern crates as well
let id = import_resolution.id(namespace); let id = import_resolution[namespace].id;
self.used_imports.insert((id, namespace)); self.used_imports.insert((id, namespace));
self.record_import_use(id, name); self.record_import_use(id, name);
if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() { if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() {
@ -3651,9 +3653,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// Look for imports. // Look for imports.
for (_, import) in search_module.import_resolutions.borrow().iter() { for (_, import) in search_module.import_resolutions.borrow().iter() {
let target = match import.target_for_namespace(TypeNS) { let target = match import.type_ns.target {
None => continue, None => continue,
Some(target) => target, Some(ref target) => target,
}; };
let did = match target.binding.def() { let did = match target.binding.def() {
Some(DefTrait(trait_def_id)) => trait_def_id, Some(DefTrait(trait_def_id)) => trait_def_id,
@ -3661,7 +3663,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}; };
if self.trait_item_map.contains_key(&(name, did)) { if self.trait_item_map.contains_key(&(name, did)) {
add_trait_info(&mut found_traits, did, name); add_trait_info(&mut found_traits, did, name);
let id = import.type_id; let id = import.type_ns.id;
self.used_imports.insert((id, TypeNS)); self.used_imports.insert((id, TypeNS));
let trait_name = self.get_trait_name(did); let trait_name = self.get_trait_name(did);
self.record_import_use(id, trait_name); self.record_import_use(id, trait_name);
@ -3734,7 +3736,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let import_resolutions = module_.import_resolutions.borrow(); let import_resolutions = module_.import_resolutions.borrow();
for (&name, import_resolution) in import_resolutions.iter() { for (&name, import_resolution) in import_resolutions.iter() {
let value_repr; let value_repr;
match import_resolution.target_for_namespace(ValueNS) { match import_resolution.value_ns.target {
None => { None => {
value_repr = "".to_string(); value_repr = "".to_string();
} }
@ -3745,7 +3747,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
} }
let type_repr; let type_repr;
match import_resolution.target_for_namespace(TypeNS) { match import_resolution.type_ns.target {
None => { None => {
type_repr = "".to_string(); type_repr = "".to_string();
} }

11
src/librustc_resolve/record_exports.rs
View File

@ -130,13 +130,14 @@ impl<'a, 'b, 'tcx> ExportRecorder<'a, 'b, 'tcx> {
fn add_exports_for_module(&mut self, exports: &mut Vec<Export>, module_: &Module) { fn add_exports_for_module(&mut self, exports: &mut Vec<Export>, module_: &Module) {
for (name, import_resolution) in module_.import_resolutions.borrow().iter() { for (name, import_resolution) in module_.import_resolutions.borrow().iter() {
if !import_resolution.is_public {
continue;
}
let xs = [TypeNS, ValueNS]; let xs = [TypeNS, ValueNS];
for &ns in &xs { for &ns in &xs {
match import_resolution.target_for_namespace(ns) { if !import_resolution[ns].is_public {
Some(target) => { continue;
}
match import_resolution[ns].target {
Some(ref target) => {
debug!("(computing exports) maybe export '{}'", name); debug!("(computing exports) maybe export '{}'", name);
self.add_export_of_namebinding(exports, *name, &target.binding) self.add_export_of_namebinding(exports, *name, &target.binding)
} }

206
src/librustc_resolve/resolve_imports.rs
View File

@ -102,80 +102,61 @@ impl Target {
} }
} }
/// An ImportResolution represents a particular `use` directive.
#[derive(Debug)] #[derive(Debug)]
/// An ImportResolution records what we know about an imported name.
/// More specifically, it records the number of unresolved `use` directives that import the name,
/// and for each namespace, it records the `use` directive importing the name in the namespace
/// and the `Target` to which the name in the namespace resolves (if applicable).
/// Different `use` directives may import the same name in different namespaces.
pub struct ImportResolution { pub struct ImportResolution {
/// Whether this resolution came from a `use` or a `pub use`. Note that this // When outstanding_references reaches zero, outside modules can count on the targets being
/// should *not* be used whenever resolution is being performed. Privacy // correct. Before then, all bets are off; future `use` directives could override the name.
/// testing occurs during a later phase of compilation. // Since shadowing is forbidden, the only way outstanding_references > 1 in a legal program
// is if the name is imported by exactly two `use` directives, one of which resolves to a
// value and the other of which resolves to a type.
pub outstanding_references: usize,
pub type_ns: NsImportResolution,
pub value_ns: NsImportResolution,
}
/// Records what we know about an imported name in a namespace (see `ImportResolution`).
#[derive(Clone,Debug)]
pub struct NsImportResolution {
/// Whether the name in the namespace was imported with a `use` or a `pub use`.
pub is_public: bool, pub is_public: bool,
// The number of outstanding references to this name. When this reaches /// Resolution of the name in the namespace
// zero, outside modules can count on the targets being correct. Before pub target: Option<Target>,
// then, all bets are off; future imports could override this name.
// Note that this is usually either 0 or 1 - shadowing is forbidden the only
// way outstanding_references is > 1 in a legal program is if the name is
// used in both namespaces.
pub outstanding_references: usize,
/// The value that this `use` directive names, if there is one. /// The source node of the `use` directive
pub value_target: Option<Target>, pub id: NodeId,
/// The source node of the `use` directive leading to the value target }
/// being non-none
pub value_id: NodeId,
/// The type that this `use` directive names, if there is one. impl ::std::ops::Index<Namespace> for ImportResolution {
pub type_target: Option<Target>, type Output = NsImportResolution;
/// The source node of the `use` directive leading to the type target fn index(&self, ns: Namespace) -> &NsImportResolution {
/// being non-none match ns { TypeNS => &self.type_ns, ValueNS => &self.value_ns }
pub type_id: NodeId, }
}
impl ::std::ops::IndexMut<Namespace> for ImportResolution {
fn index_mut(&mut self, ns: Namespace) -> &mut NsImportResolution {
match ns { TypeNS => &mut self.type_ns, ValueNS => &mut self.value_ns }
}
} }
impl ImportResolution { impl ImportResolution {
pub fn new(id: NodeId, is_public: bool) -> ImportResolution { pub fn new(id: NodeId, is_public: bool) -> ImportResolution {
let resolution = NsImportResolution { id: id, is_public: is_public, target: None };
ImportResolution { ImportResolution {
type_id: id, outstanding_references: 0, type_ns: resolution.clone(), value_ns: resolution,
value_id: id,
outstanding_references: 0,
value_target: None,
type_target: None,
is_public: is_public,
}
}
pub fn target_for_namespace(&self, namespace: Namespace) -> Option<Target> {
match namespace {
TypeNS => self.type_target.clone(),
ValueNS => self.value_target.clone(),
}
}
pub fn id(&self, namespace: Namespace) -> NodeId {
match namespace {
TypeNS => self.type_id,
ValueNS => self.value_id,
} }
} }
pub fn shadowable(&self, namespace: Namespace) -> Shadowable { pub fn shadowable(&self, namespace: Namespace) -> Shadowable {
let target = self.target_for_namespace(namespace); match self[namespace].target {
if target.is_none() { Some(ref target) => target.shadowable,
return Shadowable::Always; None => Shadowable::Always,
}
target.unwrap().shadowable
}
pub fn set_target_and_id(&mut self, namespace: Namespace, target: Option<Target>, id: NodeId) {
match namespace {
TypeNS => {
self.type_target = target;
self.type_id = id;
}
ValueNS => {
self.value_target = target;
self.value_id = id;
}
} }
} }
} }
@ -530,11 +511,11 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
// Import resolutions must be declared with "pub" // Import resolutions must be declared with "pub"
// in order to be exported. // in order to be exported.
if !import_resolution.is_public { if !import_resolution[namespace].is_public {
return UnboundResult; return UnboundResult;
} }
match import_resolution.target_for_namespace(namespace) { match import_resolution[namespace].target.clone() {
None => { None => {
return UnboundResult; return UnboundResult;
} }
@ -545,7 +526,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
}) => { }) => {
debug!("(resolving single import) found import in ns {:?}", debug!("(resolving single import) found import in ns {:?}",
namespace); namespace);
let id = import_resolution.id(namespace); let id = import_resolution[namespace].id;
// track used imports and extern crates as well // track used imports and extern crates as well
this.used_imports.insert((id, namespace)); this.used_imports.insert((id, namespace));
this.record_import_use(id, source); this.record_import_use(id, source);
@ -567,14 +548,14 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
import_resolution, import_resolution,
ValueNS, ValueNS,
source); source);
value_used_reexport = import_resolution.is_public; value_used_reexport = import_resolution.value_ns.is_public;
} }
if type_result.is_unknown() { if type_result.is_unknown() {
type_result = get_binding(self.resolver, type_result = get_binding(self.resolver,
import_resolution, import_resolution,
TypeNS, TypeNS,
source); source);
type_used_reexport = import_resolution.is_public; type_used_reexport = import_resolution.type_ns.is_public;
} }
} }
@ -663,11 +644,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
directive.span, directive.span,
target); target);
let target = Some(Target::new(target_module.clone(), import_resolution[namespace] = NsImportResolution {
name_binding.clone(), target: Some(Target::new(target_module.clone(),
directive.shadowable)); name_binding.clone(),
import_resolution.set_target_and_id(namespace, target, directive.id); directive.shadowable)),
import_resolution.is_public = directive.is_public; id: directive.id,
is_public: directive.is_public
};
*used_public = name_binding.is_public(); *used_public = name_binding.is_public();
} }
UnboundResult => { UnboundResult => {
@ -702,7 +685,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
// Record what this import resolves to for later uses in documentation, // Record what this import resolves to for later uses in documentation,
// this may resolve to either a value or a type, but for documentation // this may resolve to either a value or a type, but for documentation
// purposes it's good enough to just favor one over the other. // purposes it's good enough to just favor one over the other.
let value_def_and_priv = import_resolution.value_target.as_ref().map(|target| { let value_def_and_priv = import_resolution.value_ns.target.as_ref().map(|target| {
let def = target.binding.def().unwrap(); let def = target.binding.def().unwrap();
(def, (def,
if value_used_public { if value_used_public {
@ -711,7 +694,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
DependsOn(def.def_id()) DependsOn(def.def_id())
}) })
}); });
let type_def_and_priv = import_resolution.type_target.as_ref().map(|target| { let type_def_and_priv = import_resolution.type_ns.target.as_ref().map(|target| {
let def = target.binding.def().unwrap(); let def = target.binding.def().unwrap();
(def, (def,
if type_used_public { if type_used_public {
@ -791,54 +774,26 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
*name, *name,
module_to_string(module_)); module_to_string(module_));
if !target_import_resolution.is_public {
debug!("(resolving glob import) nevermind, just kidding");
continue;
}
// Here we merge two import resolutions. // Here we merge two import resolutions.
let mut import_resolutions = module_.import_resolutions.borrow_mut(); let mut import_resolutions = module_.import_resolutions.borrow_mut();
match import_resolutions.get_mut(name) { let mut dest_import_resolution = import_resolutions.entry(*name).or_insert_with(|| {
Some(dest_import_resolution) => { ImportResolution::new(id, is_public)
// Merge the two import resolutions at a finer-grained });
// level.
match target_import_resolution.value_target { for &ns in [TypeNS, ValueNS].iter() {
None => { match target_import_resolution[ns].target {
// Continue. Some(ref target) if target_import_resolution[ns].is_public => {
} self.check_for_conflicting_import(&dest_import_resolution,
Some(ref value_target) => { import_directive.span,
self.check_for_conflicting_import(&dest_import_resolution, *name,
import_directive.span, ns);
*name, dest_import_resolution[ns] = NsImportResolution {
ValueNS); id: id, is_public: is_public, target: Some(target.clone())
dest_import_resolution.value_target = Some(value_target.clone()); };
}
} }
match target_import_resolution.type_target { _ => {}
None => {
// Continue.
}
Some(ref type_target) => {
self.check_for_conflicting_import(&dest_import_resolution,
import_directive.span,
*name,
TypeNS);
dest_import_resolution.type_target = Some(type_target.clone());
}
}
dest_import_resolution.is_public = is_public;
continue;
} }
None => {}
} }
// Simple: just copy the old import resolution.
let mut new_import_resolution = ImportResolution::new(id, is_public);
new_import_resolution.value_target = target_import_resolution.value_target.clone();
new_import_resolution.type_target = target_import_resolution.type_target.clone();
import_resolutions.insert(*name, new_import_resolution);
} }
// Add all children from the containing module. // Add all children from the containing module.
@ -909,10 +864,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
"{}", "{}",
msg); msg);
} else { } else {
let target = Target::new(containing_module.clone(), dest_import_resolution[namespace] = NsImportResolution {
name_bindings[namespace].clone(), target: Some(Target::new(containing_module.clone(),
import_directive.shadowable); name_bindings[namespace].clone(),
dest_import_resolution.set_target_and_id(namespace, Some(target), id); import_directive.shadowable)),
id: id,
is_public: is_public
};
} }
} }
}; };
@ -920,8 +878,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
merge_child_item(TypeNS); merge_child_item(TypeNS);
} }
dest_import_resolution.is_public = is_public;
self.check_for_conflicts_between_imports_and_items(module_, self.check_for_conflicts_between_imports_and_items(module_,
dest_import_resolution, dest_import_resolution,
import_directive.span, import_directive.span,
@ -934,12 +890,12 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
import_span: Span, import_span: Span,
name: Name, name: Name,
namespace: Namespace) { namespace: Namespace) {
let target = import_resolution.target_for_namespace(namespace); let target = &import_resolution[namespace].target;
debug!("check_for_conflicting_import: {}; target exists: {}", debug!("check_for_conflicting_import: {}; target exists: {}",
name, name,
target.is_some()); target.is_some());
match target { match *target {
Some(ref target) if target.shadowable != Shadowable::Always => { Some(ref target) if target.shadowable != Shadowable::Always => {
let ns_word = match namespace { let ns_word = match namespace {
TypeNS => { TypeNS => {
@ -957,7 +913,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
"a {} named `{}` has already been imported in this module", "a {} named `{}` has already been imported in this module",
ns_word, ns_word,
name); name);
let use_id = import_resolution.id(namespace); let use_id = import_resolution[namespace].id;
let item = self.resolver.ast_map.expect_item(use_id); let item = self.resolver.ast_map.expect_item(use_id);
// item is syntax::ast::Item; // item is syntax::ast::Item;
span_note!(self.resolver.session, span_note!(self.resolver.session,
@ -990,7 +946,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
if module.external_module_children if module.external_module_children
.borrow() .borrow()
.contains_key(&name) { .contains_key(&name) {
match import_resolution.type_target { match import_resolution.type_ns.target {
Some(ref target) if target.shadowable != Shadowable::Always => { Some(ref target) if target.shadowable != Shadowable::Always => {
let msg = format!("import `{0}` conflicts with imported crate in this module \ let msg = format!("import `{0}` conflicts with imported crate in this module \
(maybe you meant `use {0}::*`?)", (maybe you meant `use {0}::*`?)",
@ -1011,7 +967,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
Some(ref name_bindings) => (*name_bindings).clone(), Some(ref name_bindings) => (*name_bindings).clone(),
}; };
match import_resolution.value_target { match import_resolution.value_ns.target {
Some(ref target) if target.shadowable != Shadowable::Always => { Some(ref target) if target.shadowable != Shadowable::Always => {
if let Some(ref value) = *name_bindings.value_ns.borrow() { if let Some(ref value) = *name_bindings.value_ns.borrow() {
span_err!(self.resolver.session, span_err!(self.resolver.session,
@ -1027,7 +983,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
Some(_) | None => {} Some(_) | None => {}
} }
match import_resolution.type_target { match import_resolution.type_ns.target {
Some(ref target) if target.shadowable != Shadowable::Always => { Some(ref target) if target.shadowable != Shadowable::Always => {
if let Some(ref ty) = *name_bindings.type_ns.borrow() { if let Some(ref ty) = *name_bindings.type_ns.borrow() {
let (what, note) = match ty.module() { let (what, note) = match ty.module() {

26
src/test/compile-fail/shadowed-use-visibility.rs Normal file
View File

@ -0,0 +1,26 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
mod foo {
pub fn f() {}
use foo as bar;
pub use self::f as bar;
}
mod bar {
use foo::bar::f as g; //~ ERROR unresolved import
use foo as f;
pub use foo::*;
}
use bar::f::f; //~ ERROR unresolved import
fn main() {}

20
src/test/run-pass/shadowed-use-visibility.rs Normal file
View File

@ -0,0 +1,20 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
mod foo {
pub fn f() {}
pub use self::f as bar;
use foo as bar;
}
fn main() {
foo::bar();
}