1
Fork 0

Auto merge of #35116 - jseyfried:groundwork_for_new_import_semantics, r=nrc

resolve: diagnostics improvement and groundwork for RFC 1560

Fixes #35115, fixes #35135, and lays groundwork for #32213 (cc #35120).
r? @nrc
This commit is contained in:
bors 2016-08-05 14:42:42 -07:00 committed by GitHub
commit 7bf54f90d6
8 changed files with 225 additions and 220 deletions

View file

@ -17,7 +17,6 @@
//! - not reference the erased type `Self` except for in this receiver; //! - not reference the erased type `Self` except for in this receiver;
//! - not have generic type parameters //! - not have generic type parameters
use super::supertraits;
use super::elaborate_predicates; use super::elaborate_predicates;
use hir::def_id::DefId; use hir::def_id::DefId;

View file

@ -16,7 +16,7 @@
use resolve_imports::ImportDirectiveSubclass::{self, GlobImport}; use resolve_imports::ImportDirectiveSubclass::{self, GlobImport};
use Module; use Module;
use Namespace::{self, TypeNS, ValueNS}; use Namespace::{self, TypeNS, ValueNS};
use {NameBinding, NameBindingKind}; use {NameBinding, NameBindingKind, ToNameBinding};
use ParentLink::{ModuleParentLink, BlockParentLink}; use ParentLink::{ModuleParentLink, BlockParentLink};
use Resolver; use Resolver;
use {resolve_error, resolve_struct_error, ResolutionError}; use {resolve_error, resolve_struct_error, ResolutionError};
@ -39,10 +39,6 @@ use syntax::visit::{self, Visitor};
use syntax_pos::{Span, DUMMY_SP}; use syntax_pos::{Span, DUMMY_SP};
trait ToNameBinding<'a> {
fn to_name_binding(self) -> NameBinding<'a>;
}
impl<'a> ToNameBinding<'a> for (Module<'a>, Span, ty::Visibility) { impl<'a> ToNameBinding<'a> for (Module<'a>, Span, ty::Visibility) {
fn to_name_binding(self) -> NameBinding<'a> { fn to_name_binding(self) -> NameBinding<'a> {
NameBinding { kind: NameBindingKind::Module(self.0), span: self.1, vis: self.2 } NameBinding { kind: NameBindingKind::Module(self.0), span: self.1, vis: self.2 }
@ -68,18 +64,13 @@ impl<'b> Resolver<'b> {
visit::walk_crate(&mut visitor, krate); visit::walk_crate(&mut visitor, krate);
} }
/// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined.
fn try_define<T>(&self, parent: Module<'b>, name: Name, ns: Namespace, def: T)
where T: ToNameBinding<'b>
{
let _ = parent.try_define_child(name, ns, def.to_name_binding());
}
/// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined; /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined;
/// otherwise, reports an error. /// otherwise, reports an error.
fn define<T: ToNameBinding<'b>>(&self, parent: Module<'b>, name: Name, ns: Namespace, def: T) { fn define<T>(&mut self, parent: Module<'b>, name: Name, ns: Namespace, def: T)
where T: ToNameBinding<'b>,
{
let binding = def.to_name_binding(); let binding = def.to_name_binding();
if let Err(old_binding) = parent.try_define_child(name, ns, binding.clone()) { if let Err(old_binding) = self.try_define(parent, name, ns, binding.clone()) {
self.report_conflict(parent, name, ns, old_binding, &binding); self.report_conflict(parent, name, ns, old_binding, &binding);
} }
} }
@ -399,14 +390,14 @@ impl<'b> Resolver<'b> {
name, vis); name, vis);
let parent_link = ModuleParentLink(parent, name); let parent_link = ModuleParentLink(parent, name);
let module = self.new_module(parent_link, Some(def), true); let module = self.new_module(parent_link, Some(def), true);
self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis)); let _ = self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis));
} }
Def::Variant(_, variant_id) => { Def::Variant(_, variant_id) => {
debug!("(building reduced graph for external crate) building variant {}", name); debug!("(building reduced graph for external crate) building variant {}", name);
// Variants are always treated as importable to allow them to be glob used. // Variants are always treated as importable to allow them to be glob used.
// All variants are defined in both type and value namespaces as future-proofing. // All variants are defined in both type and value namespaces as future-proofing.
self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis)); let _ = self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis));
self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis)); let _ = self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis));
if self.session.cstore.variant_kind(variant_id) == Some(VariantKind::Struct) { if self.session.cstore.variant_kind(variant_id) == Some(VariantKind::Struct) {
// Not adding fields for variants as they are not accessed with a self receiver // Not adding fields for variants as they are not accessed with a self receiver
self.structs.insert(variant_id, Vec::new()); self.structs.insert(variant_id, Vec::new());
@ -419,7 +410,7 @@ impl<'b> Resolver<'b> {
Def::Method(..) => { Def::Method(..) => {
debug!("(building reduced graph for external crate) building value (fn/static) {}", debug!("(building reduced graph for external crate) building value (fn/static) {}",
name); name);
self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis)); let _ = self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis));
} }
Def::Trait(def_id) => { Def::Trait(def_id) => {
debug!("(building reduced graph for external crate) building type {}", name); debug!("(building reduced graph for external crate) building type {}", name);
@ -441,20 +432,20 @@ impl<'b> Resolver<'b> {
let parent_link = ModuleParentLink(parent, name); let parent_link = ModuleParentLink(parent, name);
let module = self.new_module(parent_link, Some(def), true); let module = self.new_module(parent_link, Some(def), true);
self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis)); let _ = self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis));
} }
Def::TyAlias(..) | Def::AssociatedTy(..) => { Def::TyAlias(..) | Def::AssociatedTy(..) => {
debug!("(building reduced graph for external crate) building type {}", name); debug!("(building reduced graph for external crate) building type {}", name);
self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis)); let _ = self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis));
} }
Def::Struct(def_id) Def::Struct(def_id)
if self.session.cstore.tuple_struct_definition_if_ctor(def_id).is_none() => { if self.session.cstore.tuple_struct_definition_if_ctor(def_id).is_none() => {
debug!("(building reduced graph for external crate) building type and value for {}", debug!("(building reduced graph for external crate) building type and value for {}",
name); name);
self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis)); let _ = self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis));
if let Some(ctor_def_id) = self.session.cstore.struct_ctor_def_id(def_id) { if let Some(ctor_def_id) = self.session.cstore.struct_ctor_def_id(def_id) {
let def = Def::Struct(ctor_def_id); let def = Def::Struct(ctor_def_id);
self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis)); let _ = self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis));
} }
// Record the def ID and fields of this struct. // Record the def ID and fields of this struct.

View file

@ -158,7 +158,7 @@ enum ResolutionError<'a> {
/// error E0435: attempt to use a non-constant value in a constant /// error E0435: attempt to use a non-constant value in a constant
AttemptToUseNonConstantValueInConstant, AttemptToUseNonConstantValueInConstant,
/// error E0530: X bindings cannot shadow Ys /// error E0530: X bindings cannot shadow Ys
BindingShadowsSomethingUnacceptable(&'a str, &'a str, Name), BindingShadowsSomethingUnacceptable(&'a str, Name, &'a NameBinding<'a>),
/// error E0531: unresolved pattern path kind `name` /// error E0531: unresolved pattern path kind `name`
PatPathUnresolved(&'a str, &'a Path), PatPathUnresolved(&'a str, &'a Path),
/// error E0532: expected pattern path kind, found another pattern path kind /// error E0532: expected pattern path kind, found another pattern path kind
@ -428,17 +428,16 @@ fn resolve_struct_error<'b, 'a: 'b, 'c>(resolver: &'b Resolver<'a>,
E0435, E0435,
"attempt to use a non-constant value in a constant") "attempt to use a non-constant value in a constant")
} }
ResolutionError::BindingShadowsSomethingUnacceptable(what_binding, shadows_what, name) => { ResolutionError::BindingShadowsSomethingUnacceptable(what_binding, name, binding) => {
let shadows_what = PathResolution::new(binding.def().unwrap()).kind_name();
let mut err = struct_span_err!(resolver.session, let mut err = struct_span_err!(resolver.session,
span, span,
E0530, E0530,
"{}s cannot shadow {}s", what_binding, shadows_what); "{}s cannot shadow {}s", what_binding, shadows_what);
err.span_label(span, &format!("cannot be named the same as a {}", shadows_what)); err.span_label(span, &format!("cannot be named the same as a {}", shadows_what));
if let Success(binding) = resolver.current_module.resolve_name(name, ValueNS, true) { let participle = if binding.is_import() { "imported" } else { "defined" };
let participle = if binding.is_import() { "imported" } else { "defined" }; let msg = &format!("a {} `{}` is {} here", shadows_what, name, participle);
err.span_label(binding.span, &format!("a {} `{}` is {} here", err.span_label(binding.span, msg);
shadows_what, name, participle));
}
err err
} }
ResolutionError::PatPathUnresolved(expected_what, path) => { ResolutionError::PatPathUnresolved(expected_what, path) => {
@ -718,12 +717,16 @@ impl<'a> LexicalScopeBinding<'a> {
} }
} }
fn module(self) -> Option<Module<'a>> { fn item(self) -> Option<&'a NameBinding<'a>> {
match self { match self {
LexicalScopeBinding::Item(binding) => binding.module(), LexicalScopeBinding::Item(binding) => Some(binding),
_ => None, _ => None,
} }
} }
fn module(self) -> Option<Module<'a>> {
self.item().and_then(NameBinding::module)
}
} }
/// The link from a module up to its nearest parent node. /// The link from a module up to its nearest parent node.
@ -824,6 +827,16 @@ pub struct NameBinding<'a> {
vis: ty::Visibility, vis: ty::Visibility,
} }
pub trait ToNameBinding<'a> {
fn to_name_binding(self) -> NameBinding<'a>;
}
impl<'a> ToNameBinding<'a> for NameBinding<'a> {
fn to_name_binding(self) -> NameBinding<'a> {
self
}
}
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
enum NameBindingKind<'a> { enum NameBindingKind<'a> {
Def(Def), Def(Def),
@ -1203,34 +1216,27 @@ impl<'a> Resolver<'a> {
match ns { ValueNS => &mut self.value_ribs, TypeNS => &mut self.type_ribs } match ns { ValueNS => &mut self.value_ribs, TypeNS => &mut self.type_ribs }
} }
#[inline] fn record_use(&mut self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>) {
fn record_use(&mut self, name: Name, binding: &'a NameBinding<'a>) {
// track extern crates for unused_extern_crate lint // track extern crates for unused_extern_crate lint
if let Some(DefId { krate, .. }) = binding.module().and_then(ModuleS::def_id) { if let Some(DefId { krate, .. }) = binding.module().and_then(ModuleS::def_id) {
self.used_crates.insert(krate); self.used_crates.insert(krate);
} }
let directive = match binding.kind { if let NameBindingKind::Import { directive, .. } = binding.kind {
NameBindingKind::Import { directive, .. } => directive, self.used_imports.insert((directive.id, ns));
_ => return, self.add_to_glob_map(directive.id, name);
};
if !self.make_glob_map {
return;
} }
if self.glob_map.contains_key(&directive.id) {
self.glob_map.get_mut(&directive.id).unwrap().insert(name);
return;
}
let mut new_set = FnvHashSet();
new_set.insert(name);
self.glob_map.insert(directive.id, new_set);
} }
/// Resolves the given module path from the given root `module_`. fn add_to_glob_map(&mut self, id: NodeId, name: Name) {
if self.make_glob_map {
self.glob_map.entry(id).or_insert_with(FnvHashSet).insert(name);
}
}
/// Resolves the given module path from the given root `search_module`.
fn resolve_module_path_from_root(&mut self, fn resolve_module_path_from_root(&mut self,
module_: Module<'a>, mut search_module: Module<'a>,
module_path: &[Name], module_path: &[Name],
index: usize, index: usize,
span: Span) span: Span)
@ -1247,7 +1253,6 @@ impl<'a> Resolver<'a> {
} }
} }
let mut search_module = module_;
let mut index = index; let mut index = index;
let module_path_len = module_path.len(); let module_path_len = module_path.len();
@ -1444,10 +1449,9 @@ impl<'a> Resolver<'a> {
} }
/// Returns the nearest normal module parent of the given module. /// Returns the nearest normal module parent of the given module.
fn get_nearest_normal_module_parent(&self, module_: Module<'a>) -> Option<Module<'a>> { fn get_nearest_normal_module_parent(&self, mut module: Module<'a>) -> Option<Module<'a>> {
let mut module_ = module_;
loop { loop {
match module_.parent_link { match module.parent_link {
NoParentLink => return None, NoParentLink => return None,
ModuleParentLink(new_module, _) | ModuleParentLink(new_module, _) |
BlockParentLink(new_module, _) => { BlockParentLink(new_module, _) => {
@ -1455,7 +1459,7 @@ impl<'a> Resolver<'a> {
if new_module.is_normal() { if new_module.is_normal() {
return Some(new_module); return Some(new_module);
} }
module_ = new_module; module = new_module;
} }
} }
} }
@ -1463,12 +1467,12 @@ impl<'a> Resolver<'a> {
/// Returns the nearest normal module parent of the given module, or the /// Returns the nearest normal module parent of the given module, or the
/// module itself if it is a normal module. /// module itself if it is a normal module.
fn get_nearest_normal_module_parent_or_self(&self, module_: Module<'a>) -> Module<'a> { fn get_nearest_normal_module_parent_or_self(&self, module: Module<'a>) -> Module<'a> {
if module_.is_normal() { if module.is_normal() {
return module_; return module;
} }
match self.get_nearest_normal_module_parent(module_) { match self.get_nearest_normal_module_parent(module) {
None => module_, None => module,
Some(new_module) => new_module, Some(new_module) => new_module,
} }
} }
@ -1485,8 +1489,8 @@ impl<'a> Resolver<'a> {
"super" => 0, "super" => 0,
_ => return Success(NoPrefixFound), _ => return Success(NoPrefixFound),
}; };
let module_ = self.current_module; let mut containing_module =
let mut containing_module = self.get_nearest_normal_module_parent_or_self(module_); self.get_nearest_normal_module_parent_or_self(self.current_module);
// Now loop through all the `super`s we find. // Now loop through all the `super`s we find.
while i < module_path.len() && "super" == module_path[i].as_str() { while i < module_path.len() && "super" == module_path[i].as_str() {
@ -1525,10 +1529,7 @@ impl<'a> Resolver<'a> {
self.populate_module_if_necessary(module); self.populate_module_if_necessary(module);
module.resolve_name(name, namespace, use_lexical_scope).and_then(|binding| { module.resolve_name(name, namespace, use_lexical_scope).and_then(|binding| {
if record_used { if record_used {
if let NameBindingKind::Import { directive, .. } = binding.kind { self.record_use(name, namespace, binding);
self.used_imports.insert((directive.id, namespace));
}
self.record_use(name, binding);
} }
Success(binding) Success(binding)
}) })
@ -2308,16 +2309,17 @@ impl<'a> Resolver<'a> {
PatKind::Ident(bmode, ref ident, ref opt_pat) => { PatKind::Ident(bmode, ref ident, ref opt_pat) => {
// First try to resolve the identifier as some existing // First try to resolve the identifier as some existing
// entity, then fall back to a fresh binding. // entity, then fall back to a fresh binding.
let resolution = self.resolve_identifier(ident.node, ValueNS, true) let binding = self.resolve_ident_in_lexical_scope(ident.node, ValueNS, false)
.map(|local_def| PathResolution::new(local_def.def)) .and_then(LexicalScopeBinding::item);
.and_then(|resolution| { let resolution = binding.and_then(NameBinding::def).and_then(|def| {
let always_binding = !pat_src.is_refutable() || opt_pat.is_some() || let always_binding = !pat_src.is_refutable() || opt_pat.is_some() ||
bmode != BindingMode::ByValue(Mutability::Immutable); bmode != BindingMode::ByValue(Mutability::Immutable);
match resolution.base_def { match def {
Def::Struct(..) | Def::Variant(..) | Def::Struct(..) | Def::Variant(..) |
Def::Const(..) | Def::AssociatedConst(..) if !always_binding => { Def::Const(..) | Def::AssociatedConst(..) if !always_binding => {
// A constant, unit variant, etc pattern. // A constant, unit variant, etc pattern.
Some(resolution) self.record_use(ident.node.name, ValueNS, binding.unwrap());
Some(PathResolution::new(def))
} }
Def::Struct(..) | Def::Variant(..) | Def::Struct(..) | Def::Variant(..) |
Def::Const(..) | Def::AssociatedConst(..) | Def::Static(..) => { Def::Const(..) | Def::AssociatedConst(..) | Def::Static(..) => {
@ -2326,7 +2328,7 @@ impl<'a> Resolver<'a> {
self, self,
ident.span, ident.span,
ResolutionError::BindingShadowsSomethingUnacceptable( ResolutionError::BindingShadowsSomethingUnacceptable(
pat_src.descr(), resolution.kind_name(), ident.node.name) pat_src.descr(), ident.node.name, binding.unwrap())
); );
None None
} }
@ -3136,10 +3138,10 @@ impl<'a> Resolver<'a> {
if let NameBindingKind::Import { directive, .. } = binding.kind { if let NameBindingKind::Import { directive, .. } = binding.kind {
let id = directive.id; let id = directive.id;
this.maybe_unused_trait_imports.insert(id); this.maybe_unused_trait_imports.insert(id);
this.add_to_glob_map(id, trait_name);
import_id = Some(id); import_id = Some(id);
} }
add_trait_info(&mut found_traits, trait_def_id, import_id, name); add_trait_info(&mut found_traits, trait_def_id, import_id, name);
this.record_use(trait_name, binding);
} }
} }
}; };

View file

@ -12,7 +12,7 @@ use self::ImportDirectiveSubclass::*;
use Module; use Module;
use Namespace::{self, TypeNS, ValueNS}; use Namespace::{self, TypeNS, ValueNS};
use {NameBinding, NameBindingKind, PrivacyError}; use {NameBinding, NameBindingKind, PrivacyError, ToNameBinding};
use ResolveResult; use ResolveResult;
use ResolveResult::*; use ResolveResult::*;
use Resolver; use Resolver;
@ -21,7 +21,7 @@ use {names_to_string, module_to_string};
use {resolve_error, ResolutionError}; use {resolve_error, ResolutionError};
use rustc::ty; use rustc::ty;
use rustc::lint; use rustc::lint::builtin::PRIVATE_IN_PUBLIC;
use rustc::hir::def::*; use rustc::hir::def::*;
use syntax::ast::{NodeId, Name}; use syntax::ast::{NodeId, Name};
@ -71,19 +71,6 @@ pub struct ImportDirective<'a> {
} }
impl<'a> ImportDirective<'a> { impl<'a> ImportDirective<'a> {
// Given the binding to which this directive resolves in a particular namespace,
// this returns the binding for the name this directive defines in that namespace.
fn import(&'a self, binding: &'a NameBinding<'a>) -> NameBinding<'a> {
NameBinding {
kind: NameBindingKind::Import {
binding: binding,
directive: self,
},
span: self.span,
vis: self.vis,
}
}
pub fn is_glob(&self) -> bool { pub fn is_glob(&self) -> bool {
match self.subclass { ImportDirectiveSubclass::GlobImport { .. } => true, _ => false } match self.subclass { ImportDirectiveSubclass::GlobImport { .. } => true, _ => false }
} }
@ -137,23 +124,6 @@ impl<'a> SingleImports<'a> {
} }
impl<'a> NameResolution<'a> { impl<'a> NameResolution<'a> {
fn try_define(&mut self, binding: &'a NameBinding<'a>) -> Result<(), &'a NameBinding<'a>> {
if let Some(old_binding) = self.binding {
if binding.is_glob_import() {
self.duplicate_globs.push(binding);
} else if old_binding.is_glob_import() {
self.duplicate_globs.push(old_binding);
self.binding = Some(binding);
} else {
return Err(old_binding);
}
} else {
self.binding = Some(binding);
}
Ok(())
}
// Returns the binding for the name if it is known or None if it not known. // Returns the binding for the name if it is known or None if it not known.
fn binding(&self) -> Option<&'a NameBinding<'a>> { fn binding(&self) -> Option<&'a NameBinding<'a>> {
self.binding.and_then(|binding| match self.single_imports { self.binding.and_then(|binding| match self.single_imports {
@ -203,24 +173,6 @@ impl<'a> NameResolution<'a> {
self.binding.map(Success) self.binding.map(Success)
} }
fn report_conflicts<F: FnMut(&NameBinding, &NameBinding)>(&self, mut report: F) {
let binding = match self.binding {
Some(binding) => binding,
None => return,
};
for duplicate_glob in self.duplicate_globs.iter() {
// FIXME #31337: We currently allow items to shadow glob-imported re-exports.
if !binding.is_import() {
if let NameBindingKind::Import { binding, .. } = duplicate_glob.kind {
if binding.is_import() { continue }
}
}
report(duplicate_glob, binding);
}
}
} }
impl<'a> ::ModuleS<'a> { impl<'a> ::ModuleS<'a> {
@ -261,14 +213,6 @@ impl<'a> ::ModuleS<'a> {
Failed(None) Failed(None)
} }
// Define the name or return the existing binding if there is a collision.
pub fn try_define_child(&self, name: Name, ns: Namespace, binding: NameBinding<'a>)
-> Result<(), &'a NameBinding<'a>> {
self.update_resolution(name, ns, |resolution| {
resolution.try_define(self.arenas.alloc_name_binding(binding))
})
}
pub fn add_import_directive(&self, pub fn add_import_directive(&self,
module_path: Vec<Name>, module_path: Vec<Name>,
subclass: ImportDirectiveSubclass, subclass: ImportDirectiveSubclass,
@ -298,19 +242,59 @@ impl<'a> ::ModuleS<'a> {
GlobImport { .. } => self.globs.borrow_mut().push(directive), GlobImport { .. } => self.globs.borrow_mut().push(directive),
} }
} }
}
// Use `update` to mutate the resolution for the name. impl<'a> Resolver<'a> {
// If the resolution becomes a success, define it in the module's glob importers. // Given a binding and an import directive that resolves to it,
fn update_resolution<T, F>(&self, name: Name, ns: Namespace, update: F) -> T // return the corresponding binding defined by the import directive.
where F: FnOnce(&mut NameResolution<'a>) -> T fn import(&mut self, binding: &'a NameBinding<'a>, directive: &'a ImportDirective<'a>)
-> NameBinding<'a> {
NameBinding {
kind: NameBindingKind::Import {
binding: binding,
directive: directive,
},
span: directive.span,
vis: directive.vis,
}
}
// Define the name or return the existing binding if there is a collision.
pub fn try_define<T>(&mut self, module: Module<'a>, name: Name, ns: Namespace, binding: T)
-> Result<(), &'a NameBinding<'a>>
where T: ToNameBinding<'a>
{ {
// Ensure that `resolution` isn't borrowed during `define_in_glob_importers`, let binding = self.arenas.alloc_name_binding(binding.to_name_binding());
// where it might end up getting re-defined via a glob cycle. self.update_resolution(module, name, ns, |_, resolution| {
if let Some(old_binding) = resolution.binding {
if binding.is_glob_import() {
resolution.duplicate_globs.push(binding);
} else if old_binding.is_glob_import() {
resolution.duplicate_globs.push(old_binding);
resolution.binding = Some(binding);
} else {
return Err(old_binding);
}
} else {
resolution.binding = Some(binding);
}
Ok(())
})
}
// Use `f` to mutate the resolution of the name in the module.
// If the resolution becomes a success, define it in the module's glob importers.
fn update_resolution<T, F>(&mut self, module: Module<'a>, name: Name, ns: Namespace, f: F) -> T
where F: FnOnce(&mut Resolver<'a>, &mut NameResolution<'a>) -> T
{
// Ensure that `resolution` isn't borrowed when defining in the module's glob importers,
// during which the resolution might end up getting re-defined via a glob cycle.
let (new_binding, t) = { let (new_binding, t) = {
let mut resolution = &mut *self.resolution(name, ns).borrow_mut(); let mut resolution = &mut *module.resolution(name, ns).borrow_mut();
let was_known = resolution.binding().is_some(); let was_known = resolution.binding().is_some();
let t = update(resolution); let t = f(self, resolution);
if was_known { return t; } if was_known { return t; }
match resolution.binding() { match resolution.binding() {
@ -319,15 +303,15 @@ impl<'a> ::ModuleS<'a> {
} }
}; };
self.define_in_glob_importers(name, ns, new_binding); // Define `new_binding` in `module`s glob importers.
t if new_binding.is_importable() && new_binding.is_pseudo_public() {
} for &(importer, directive) in module.glob_importers.borrow_mut().iter() {
let imported_binding = self.import(new_binding, directive);
fn define_in_glob_importers(&self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>) { let _ = self.try_define(importer, name, ns, imported_binding);
if !binding.is_importable() || !binding.is_pseudo_public() { return } }
for &(importer, directive) in self.glob_importers.borrow_mut().iter() {
let _ = importer.try_define_child(name, ns, directive.import(binding));
} }
t
} }
} }
@ -343,6 +327,25 @@ struct ImportResolver<'a, 'b: 'a> {
resolver: &'a mut Resolver<'b>, resolver: &'a mut Resolver<'b>,
} }
impl<'a, 'b: 'a> ::std::ops::Deref for ImportResolver<'a, 'b> {
type Target = Resolver<'b>;
fn deref(&self) -> &Resolver<'b> {
self.resolver
}
}
impl<'a, 'b: 'a> ::std::ops::DerefMut for ImportResolver<'a, 'b> {
fn deref_mut(&mut self) -> &mut Resolver<'b> {
self.resolver
}
}
impl<'a, 'b: 'a> ty::NodeIdTree for ImportResolver<'a, 'b> {
fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool {
self.resolver.is_descendant_of(node, ancestor)
}
}
impl<'a, 'b:'a> ImportResolver<'a, 'b> { impl<'a, 'b:'a> ImportResolver<'a, 'b> {
// Import resolution // Import resolution
// //
@ -360,31 +363,29 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
let mut errors = Vec::new(); let mut errors = Vec::new();
loop { loop {
debug!("(resolving imports) iteration {}, {} imports left", debug!("(resolving imports) iteration {}, {} imports left", i, self.unresolved_imports);
i,
self.resolver.unresolved_imports);
// Attempt to resolve imports in all local modules. // Attempt to resolve imports in all local modules.
for module in self.resolver.arenas.local_modules().iter() { for module in self.arenas.local_modules().iter() {
self.resolver.current_module = module; self.current_module = module;
self.resolve_imports_in_current_module(&mut errors); self.resolve_imports_in_current_module(&mut errors);
} }
if self.resolver.unresolved_imports == 0 { if self.unresolved_imports == 0 {
debug!("(resolving imports) success"); debug!("(resolving imports) success");
for module in self.resolver.arenas.local_modules().iter() { for module in self.arenas.local_modules().iter() {
self.finalize_resolutions_in(module, false); self.finalize_resolutions_in(module, false);
} }
break; break;
} }
if self.resolver.unresolved_imports == prev_unresolved_imports { if self.unresolved_imports == prev_unresolved_imports {
// resolving failed // resolving failed
// Report unresolved imports only if no hard error was already reported // Report unresolved imports only if no hard error was already reported
// to avoid generating multiple errors on the same import. // to avoid generating multiple errors on the same import.
// Imports that are still indeterminate at this point are actually blocked // Imports that are still indeterminate at this point are actually blocked
// by errored imports, so there is no point reporting them. // by errored imports, so there is no point reporting them.
for module in self.resolver.arenas.local_modules().iter() { for module in self.arenas.local_modules().iter() {
self.finalize_resolutions_in(module, errors.len() == 0); self.finalize_resolutions_in(module, errors.len() == 0);
} }
for e in errors { for e in errors {
@ -394,29 +395,31 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
} }
i += 1; i += 1;
prev_unresolved_imports = self.resolver.unresolved_imports; prev_unresolved_imports = self.unresolved_imports;
} }
} }
// Define a "dummy" resolution containing a Def::Err as a placeholder for a // Define a "dummy" resolution containing a Def::Err as a placeholder for a
// failed resolution // failed resolution
fn import_dummy_binding(&self, source_module: Module<'b>, directive: &'b ImportDirective<'b>) { fn import_dummy_binding(&mut self,
source_module: Module<'b>,
directive: &'b ImportDirective<'b>) {
if let SingleImport { target, .. } = directive.subclass { if let SingleImport { target, .. } = directive.subclass {
let dummy_binding = self.resolver.arenas.alloc_name_binding(NameBinding { let dummy_binding = self.arenas.alloc_name_binding(NameBinding {
kind: NameBindingKind::Def(Def::Err), kind: NameBindingKind::Def(Def::Err),
span: DUMMY_SP, span: DUMMY_SP,
vis: ty::Visibility::Public, vis: ty::Visibility::Public,
}); });
let dummy_binding = directive.import(dummy_binding); let dummy_binding = self.import(dummy_binding, directive);
let _ = source_module.try_define_child(target, ValueNS, dummy_binding.clone()); let _ = self.try_define(source_module, target, ValueNS, dummy_binding.clone());
let _ = source_module.try_define_child(target, TypeNS, dummy_binding); let _ = self.try_define(source_module, target, TypeNS, dummy_binding);
} }
} }
/// Resolves an `ImportResolvingError` into the correct enum discriminant /// Resolves an `ImportResolvingError` into the correct enum discriminant
/// and passes that on to `resolve_error`. /// and passes that on to `resolve_error`.
fn import_resolving_error(&self, e: ImportResolvingError<'b>) { fn import_resolving_error(&mut self, e: ImportResolvingError<'b>) {
// If the error is a single failed import then create a "fake" import // If the error is a single failed import then create a "fake" import
// resolution for it so that later resolve stages won't complain. // resolution for it so that later resolve stages won't complain.
self.import_dummy_binding(e.source_module, e.import_directive); self.import_dummy_binding(e.source_module, e.import_directive);
@ -430,7 +433,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
/// Attempts to resolve imports for the given module only. /// Attempts to resolve imports for the given module only.
fn resolve_imports_in_current_module(&mut self, errors: &mut Vec<ImportResolvingError<'b>>) { fn resolve_imports_in_current_module(&mut self, errors: &mut Vec<ImportResolvingError<'b>>) {
let mut imports = Vec::new(); let mut imports = Vec::new();
let mut unresolved_imports = self.resolver.current_module.unresolved_imports.borrow_mut(); let mut unresolved_imports = self.current_module.unresolved_imports.borrow_mut();
::std::mem::swap(&mut imports, &mut unresolved_imports); ::std::mem::swap(&mut imports, &mut unresolved_imports);
for import_directive in imports { for import_directive in imports {
@ -441,7 +444,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
None => (import_directive.span, String::new()), None => (import_directive.span, String::new()),
}; };
errors.push(ImportResolvingError { errors.push(ImportResolvingError {
source_module: self.resolver.current_module, source_module: self.current_module,
import_directive: import_directive, import_directive: import_directive,
span: span, span: span,
help: help, help: help,
@ -450,8 +453,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
Indeterminate => unresolved_imports.push(import_directive), Indeterminate => unresolved_imports.push(import_directive),
Success(()) => { Success(()) => {
// Decrement the count of unresolved imports. // Decrement the count of unresolved imports.
assert!(self.resolver.unresolved_imports >= 1); assert!(self.unresolved_imports >= 1);
self.resolver.unresolved_imports -= 1; self.unresolved_imports -= 1;
} }
} }
} }
@ -465,13 +468,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResult<()> { fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> ResolveResult<()> {
debug!("(resolving import for module) resolving import `{}::...` in `{}`", debug!("(resolving import for module) resolving import `{}::...` in `{}`",
names_to_string(&directive.module_path), names_to_string(&directive.module_path),
module_to_string(self.resolver.current_module)); module_to_string(self.current_module));
let target_module = match directive.target_module.get() { let target_module = match directive.target_module.get() {
Some(module) => module, Some(module) => module,
_ => match self.resolver.resolve_module_path(&directive.module_path, _ => match self.resolve_module_path(&directive.module_path,
DontUseLexicalScope, DontUseLexicalScope,
directive.span) { directive.span) {
Success(module) => module, Success(module) => module,
Indeterminate => return Indeterminate, Indeterminate => return Indeterminate,
Failed(err) => return Failed(err), Failed(err) => return Failed(err),
@ -486,38 +489,36 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}; };
// We need to resolve both namespaces for this to succeed. // We need to resolve both namespaces for this to succeed.
let value_result = let value_result = self.resolve_name_in_module(target_module, source, ValueNS, false, true);
self.resolver.resolve_name_in_module(target_module, source, ValueNS, false, true); let type_result = self.resolve_name_in_module(target_module, source, TypeNS, false, true);
let type_result =
self.resolver.resolve_name_in_module(target_module, source, TypeNS, false, true);
let module_ = self.resolver.current_module; let module = self.current_module;
let mut privacy_error = true; let mut privacy_error = true;
for &(ns, result, determined) in &[(ValueNS, &value_result, value_determined), for &(ns, result, determined) in &[(ValueNS, &value_result, value_determined),
(TypeNS, &type_result, type_determined)] { (TypeNS, &type_result, type_determined)] {
match *result { match *result {
Failed(..) if !determined.get() => { Failed(..) if !determined.get() => {
determined.set(true); determined.set(true);
module_.update_resolution(target, ns, |resolution| { self.update_resolution(module, target, ns, |_, resolution| {
resolution.single_imports.directive_failed() resolution.single_imports.directive_failed()
}); });
} }
Success(binding) if !binding.is_importable() => { Success(binding) if !binding.is_importable() => {
let msg = format!("`{}` is not directly importable", target); let msg = format!("`{}` is not directly importable", target);
span_err!(self.resolver.session, directive.span, E0253, "{}", &msg); span_err!(self.session, directive.span, E0253, "{}", &msg);
// Do not import this illegal binding. Import a dummy binding and pretend // Do not import this illegal binding. Import a dummy binding and pretend
// everything is fine // everything is fine
self.import_dummy_binding(module_, directive); self.import_dummy_binding(module, directive);
return Success(()); return Success(());
} }
Success(binding) if !self.resolver.is_accessible(binding.vis) => {} Success(binding) if !self.is_accessible(binding.vis) => {}
Success(binding) if !determined.get() => { Success(binding) if !determined.get() => {
determined.set(true); determined.set(true);
let imported_binding = directive.import(binding); let imported_binding = self.import(binding, directive);
let conflict = module_.try_define_child(target, ns, imported_binding); let conflict = self.try_define(module, target, ns, imported_binding);
if let Err(old_binding) = conflict { if let Err(old_binding) = conflict {
let binding = &directive.import(binding); let binding = &self.import(binding, directive);
self.resolver.report_conflict(module_, target, ns, binding, old_binding); self.report_conflict(module, target, ns, binding, old_binding);
} }
privacy_error = false; privacy_error = false;
} }
@ -556,39 +557,35 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
if privacy_error { if privacy_error {
for &(ns, result) in &[(ValueNS, &value_result), (TypeNS, &type_result)] { for &(ns, result) in &[(ValueNS, &value_result), (TypeNS, &type_result)] {
let binding = match *result { Success(binding) => binding, _ => continue }; let binding = match *result { Success(binding) => binding, _ => continue };
self.resolver.privacy_errors.push(PrivacyError(directive.span, source, binding)); self.privacy_errors.push(PrivacyError(directive.span, source, binding));
let _ = module_.try_define_child(target, ns, directive.import(binding)); let imported_binding = self.import(binding, directive);
let _ = self.try_define(module, target, ns, imported_binding);
} }
} }
match (&value_result, &type_result) { match (&value_result, &type_result) {
(&Success(binding), _) if !binding.pseudo_vis() (&Success(binding), _) if !binding.pseudo_vis().is_at_least(directive.vis, self) &&
.is_at_least(directive.vis, self.resolver) && self.is_accessible(binding.vis) => {
self.resolver.is_accessible(binding.vis) => {
let msg = format!("`{}` is private, and cannot be reexported", source); let msg = format!("`{}` is private, and cannot be reexported", source);
let note_msg = format!("consider marking `{}` as `pub` in the imported module", let note_msg = format!("consider marking `{}` as `pub` in the imported module",
source); source);
struct_span_err!(self.resolver.session, directive.span, E0364, "{}", &msg) struct_span_err!(self.session, directive.span, E0364, "{}", &msg)
.span_note(directive.span, &note_msg) .span_note(directive.span, &note_msg)
.emit(); .emit();
} }
(_, &Success(binding)) if !binding.pseudo_vis() (_, &Success(binding)) if !binding.pseudo_vis().is_at_least(directive.vis, self) &&
.is_at_least(directive.vis, self.resolver) && self.is_accessible(binding.vis) => {
self.resolver.is_accessible(binding.vis) => {
if binding.is_extern_crate() { if binding.is_extern_crate() {
let msg = format!("extern crate `{}` is private, and cannot be reexported \ let msg = format!("extern crate `{}` is private, and cannot be reexported \
(error E0364), consider declaring with `pub`", (error E0364), consider declaring with `pub`",
source); source);
self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, self.session.add_lint(PRIVATE_IN_PUBLIC, directive.id, directive.span, msg);
directive.id,
directive.span,
msg);
} else { } else {
let msg = format!("`{}` is private, and cannot be reexported", source); let msg = format!("`{}` is private, and cannot be reexported", source);
let note_msg = let note_msg =
format!("consider declaring type or module `{}` with `pub`", source); format!("consider declaring type or module `{}` with `pub`", source);
struct_span_err!(self.resolver.session, directive.span, E0365, "{}", &msg) struct_span_err!(self.session, directive.span, E0365, "{}", &msg)
.span_note(directive.span, &note_msg) .span_note(directive.span, &note_msg)
.emit(); .emit();
} }
@ -605,7 +602,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
None => value_result.success().and_then(NameBinding::def).unwrap(), None => value_result.success().and_then(NameBinding::def).unwrap(),
}; };
let path_resolution = PathResolution::new(def); let path_resolution = PathResolution::new(def);
self.resolver.def_map.insert(directive.id, path_resolution); self.def_map.insert(directive.id, path_resolution);
debug!("(resolving single import) successfully resolved import"); debug!("(resolving single import) successfully resolved import");
return Success(()); return Success(());
@ -618,40 +615,41 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
fn resolve_glob_import(&mut self, target_module: Module<'b>, directive: &'b ImportDirective<'b>) fn resolve_glob_import(&mut self, target_module: Module<'b>, directive: &'b ImportDirective<'b>)
-> ResolveResult<()> { -> ResolveResult<()> {
if let Some(Def::Trait(_)) = target_module.def { if let Some(Def::Trait(_)) = target_module.def {
self.resolver.session.span_err(directive.span, "items in traits are not importable."); self.session.span_err(directive.span, "items in traits are not importable.");
} }
let module_ = self.resolver.current_module; let module = self.current_module;
if module_.def_id() == target_module.def_id() { if module.def_id() == target_module.def_id() {
// This means we are trying to glob import a module into itself, and it is a no-go // This means we are trying to glob import a module into itself, and it is a no-go
let msg = "Cannot glob-import a module into itself.".into(); let msg = "Cannot glob-import a module into itself.".into();
return Failed(Some((directive.span, msg))); return Failed(Some((directive.span, msg)));
} }
self.resolver.populate_module_if_necessary(target_module); self.populate_module_if_necessary(target_module);
if let GlobImport { is_prelude: true } = directive.subclass { if let GlobImport { is_prelude: true } = directive.subclass {
self.resolver.prelude = Some(target_module); self.prelude = Some(target_module);
return Success(()); return Success(());
} }
// Add to target_module's glob_importers // Add to target_module's glob_importers
target_module.glob_importers.borrow_mut().push((module_, directive)); target_module.glob_importers.borrow_mut().push((module, directive));
// Ensure that `resolutions` isn't borrowed during `try_define_child`, // Ensure that `resolutions` isn't borrowed during `try_define`,
// since it might get updated via a glob cycle. // since it might get updated via a glob cycle.
let bindings = target_module.resolutions.borrow().iter().filter_map(|(name, resolution)| { let bindings = target_module.resolutions.borrow().iter().filter_map(|(name, resolution)| {
resolution.borrow().binding().map(|binding| (*name, binding)) resolution.borrow().binding().map(|binding| (*name, binding))
}).collect::<Vec<_>>(); }).collect::<Vec<_>>();
for ((name, ns), binding) in bindings { for ((name, ns), binding) in bindings {
if binding.is_importable() && binding.is_pseudo_public() { if binding.is_importable() && binding.is_pseudo_public() {
let _ = module_.try_define_child(name, ns, directive.import(binding)); let imported_binding = self.import(binding, directive);
let _ = self.try_define(module, name, ns, imported_binding);
} }
} }
// Record the destination of this import // Record the destination of this import
if let Some(did) = target_module.def_id() { if let Some(did) = target_module.def_id() {
let resolution = PathResolution::new(Def::Mod(did)); let resolution = PathResolution::new(Def::Mod(did));
self.resolver.def_map.insert(directive.id, resolution); self.def_map.insert(directive.id, resolution);
} }
debug!("(resolving glob import) successfully resolved import"); debug!("(resolving glob import) successfully resolved import");
@ -667,15 +665,23 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
let mut reexports = Vec::new(); let mut reexports = Vec::new();
for (&(name, ns), resolution) in module.resolutions.borrow().iter() { for (&(name, ns), resolution) in module.resolutions.borrow().iter() {
let resolution = resolution.borrow(); let resolution = resolution.borrow();
resolution.report_conflicts(|b1, b2| {
self.resolver.report_conflict(module, name, ns, b1, b2)
});
let binding = match resolution.binding { let binding = match resolution.binding {
Some(binding) => binding, Some(binding) => binding,
None => continue, None => continue,
}; };
// Report conflicts
for duplicate_glob in resolution.duplicate_globs.iter() {
// FIXME #31337: We currently allow items to shadow glob-imported re-exports.
if !binding.is_import() {
if let NameBindingKind::Import { binding, .. } = duplicate_glob.kind {
if binding.is_import() { continue }
}
}
self.report_conflict(module, name, ns, duplicate_glob, binding);
}
if binding.vis == ty::Visibility::Public && if binding.vis == ty::Visibility::Public &&
(binding.is_import() || binding.is_extern_crate()) { (binding.is_import() || binding.is_extern_crate()) {
if let Some(def) = binding.def() { if let Some(def) = binding.def() {
@ -685,20 +691,19 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
if let NameBindingKind::Import { binding: orig_binding, directive, .. } = binding.kind { if let NameBindingKind::Import { binding: orig_binding, directive, .. } = binding.kind {
if ns == TypeNS && orig_binding.is_variant() && if ns == TypeNS && orig_binding.is_variant() &&
!orig_binding.vis.is_at_least(binding.vis, self.resolver) { !orig_binding.vis.is_at_least(binding.vis, self) {
let msg = format!("variant `{}` is private, and cannot be reexported \ let msg = format!("variant `{}` is private, and cannot be reexported \
(error E0364), consider declaring its enum as `pub`", (error E0364), consider declaring its enum as `pub`",
name); name);
let lint = lint::builtin::PRIVATE_IN_PUBLIC; self.session.add_lint(PRIVATE_IN_PUBLIC, directive.id, binding.span, msg);
self.resolver.session.add_lint(lint, directive.id, binding.span, msg);
} }
} }
} }
if reexports.len() > 0 { if reexports.len() > 0 {
if let Some(def_id) = module.def_id() { if let Some(def_id) = module.def_id() {
let node_id = self.resolver.definitions.as_local_node_id(def_id).unwrap(); let node_id = self.definitions.as_local_node_id(def_id).unwrap();
self.resolver.export_map.insert(node_id, reexports); self.export_map.insert(node_id, reexports);
} }
} }

View file

@ -18,7 +18,7 @@ use self::utils::{DIB, span_start, create_DIArray, is_node_local_to_unit};
use self::namespace::mangled_name_of_item; use self::namespace::mangled_name_of_item;
use self::type_names::compute_debuginfo_type_name; use self::type_names::compute_debuginfo_type_name;
use self::metadata::{type_metadata, diverging_type_metadata}; use self::metadata::{type_metadata, diverging_type_metadata};
use self::metadata::{file_metadata, scope_metadata, TypeMap}; use self::metadata::{file_metadata, TypeMap};
use self::source_loc::InternalDebugLocation::{self, UnknownLocation}; use self::source_loc::InternalDebugLocation::{self, UnknownLocation};
use llvm; use llvm;

View file

@ -25,4 +25,5 @@ fn main() {
//~^ NOTE cannot be named the same as a constant //~^ NOTE cannot be named the same as a constant
let d = 4; //~ ERROR let bindings cannot shadow constants let d = 4; //~ ERROR let bindings cannot shadow constants
//~^ NOTE cannot be named the same as a constant //~^ NOTE cannot be named the same as a constant
fn f() {} // Check that the `NOTE`s still work with an item here (c.f. issue #35115).
} }

View file

@ -79,6 +79,13 @@ fn g() {
} }
} }
// c.f. issue #35135
#[allow(unused_variables)]
fn h() {
use test2::foo; //~ ERROR unused import
let foo = 0;
}
fn main() { fn main() {
cal(foo::Point{x:3, y:9}); cal(foo::Point{x:3, y:9});
let mut a = 3; let mut a = 3;

View file

@ -24,7 +24,7 @@ struct Test {
const TEST_REPOS: &'static [Test] = &[Test { const TEST_REPOS: &'static [Test] = &[Test {
name: "cargo", name: "cargo",
repo: "https://github.com/rust-lang/cargo", repo: "https://github.com/rust-lang/cargo",
sha: "7d79da08238e3d47e0bc4406155bdcc45ccb8c82", sha: "fd90fd642d404d8c66505ca8db742c664ea352f2",
lock: None, lock: None,
}, },
Test { Test {