Start importing bindings from globs as soon as the glob path is known.

src/librustc_resolve/build_reduced_graph.rs

@@ -295,14 +295,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     let module = self.new_extern_crate_module(parent_link, def, is_public, item.id);
                     self.define(parent, name, TypeNS, (module, sp));
 
-                    if is_public {
-                        let export = Export { name: name, def_id: def_id };
-                        if let Some(def_id) = parent.def_id() {
-                            let node_id = self.resolver.ast_map.as_local_node_id(def_id).unwrap();
-                            self.export_map.entry(node_id).or_insert(Vec::new()).push(export);
-                        }
-                    }
-
                     self.build_reduced_graph_for_external_crate(module);
                 }
                 parent

src/librustc_resolve/lib.rs

@@ -18,6 +18,7 @@
 #![cfg_attr(not(stage0), deny(warnings))]
 
 #![feature(associated_consts)]
+#![feature(borrow_state)]
 #![feature(rustc_diagnostic_macros)]
 #![feature(rustc_private)]
 #![feature(staged_api)]
@@ -832,6 +833,9 @@ pub struct ModuleS<'a> {
 
     shadowed_traits: RefCell<Vec<&'a NameBinding<'a>>>,
 
+    glob_importers: RefCell<Vec<(Module<'a>, &'a ImportDirective)>>,
+    resolved_globs: RefCell<(Vec<Module<'a>> /* public */, Vec<Module<'a>> /* private */)>,
+
     // The number of unresolved globs that this module exports.
     glob_count: Cell<usize>,
 
@@ -866,6 +870,8 @@ impl<'a> ModuleS<'a> {
             unresolved_imports: RefCell::new(Vec::new()),
             module_children: RefCell::new(NodeMap()),
             shadowed_traits: RefCell::new(Vec::new()),
+            glob_importers: RefCell::new(Vec::new()),
+            resolved_globs: RefCell::new((Vec::new(), Vec::new())),
             glob_count: Cell::new(0),
             pub_count: Cell::new(0),
             pub_glob_count: Cell::new(0),
@@ -874,54 +880,11 @@ impl<'a> ModuleS<'a> {
         }
     }
 
-    fn resolve_name(&self, name: Name, ns: Namespace, allow_private_imports: bool)
-                    -> ResolveResult<&'a NameBinding<'a>> {
-        let glob_count =
-            if allow_private_imports { self.glob_count.get() } else { self.pub_glob_count.get() };
-
-        self.resolutions.borrow().get(&(name, ns)).cloned().unwrap_or_default().result(glob_count)
-            .and_then(|binding| {
-                let allowed = allow_private_imports || !binding.is_import() || binding.is_public();
-                if allowed { Success(binding) } else { Failed(None) }
-            })
-    }
-
-    // Define the name or return the existing binding if there is a collision.
-    fn try_define_child(&self, name: Name, ns: Namespace, binding: NameBinding<'a>)
-                        -> Result<(), &'a NameBinding<'a>> {
-        let binding = self.arenas.alloc_name_binding(binding);
-        let mut children = self.resolutions.borrow_mut();
-        let resolution = children.entry((name, ns)).or_insert_with(Default::default);
-
-        // FIXME #31379: We can use methods from imported traits shadowed by non-import items
-        if let Some(old_binding) = resolution.binding {
-            if !old_binding.is_import() && binding.is_import() {
-                if let Some(Def::Trait(_)) = binding.def() {
-                    self.shadowed_traits.borrow_mut().push(binding);
-                }
-            }
-        }
-
-        resolution.try_define(binding)
-    }
-
     fn add_import_directive(&self, import_directive: ImportDirective) {
         let import_directive = self.arenas.alloc_import_directive(import_directive);
         self.unresolved_imports.borrow_mut().push(import_directive);
     }
 
-    fn increment_outstanding_references_for(&self, name: Name, ns: Namespace) {
-        let mut children = self.resolutions.borrow_mut();
-        children.entry((name, ns)).or_insert_with(Default::default).outstanding_references += 1;
-    }
-
-    fn decrement_outstanding_references_for(&self, name: Name, ns: Namespace) {
-        match self.resolutions.borrow_mut().get_mut(&(name, ns)).unwrap().outstanding_references {
-            0 => panic!("No more outstanding references!"),
-            ref mut outstanding_references => { *outstanding_references -= 1; }
-        }
-    }
-
     fn for_each_child<F: FnMut(Name, Namespace, &'a NameBinding<'a>)>(&self, mut f: F) {
         for (&(name, ns), name_resolution) in self.resolutions.borrow().iter() {
             name_resolution.binding.map(|binding| f(name, ns, binding));

src/librustc_resolve/resolve_imports.rs

@@ -125,47 +125,163 @@ impl ImportDirective {
 /// Records information about the resolution of a name in a module.
 pub struct NameResolution<'a> {
     /// The number of unresolved single imports that could define the name.
-    pub outstanding_references: usize,
+    outstanding_references: usize,
     /// The least shadowable known binding for this name, or None if there are no known bindings.
     pub binding: Option<&'a NameBinding<'a>>,
+    duplicate_globs: Vec<&'a NameBinding<'a>>,
 }
 
 impl<'a> NameResolution<'a> {
-    pub fn result(&self, outstanding_globs: usize) -> ResolveResult<&'a NameBinding<'a>> {
+    fn try_define(&mut self, binding: &'a NameBinding<'a>) -> Result<(), &'a NameBinding<'a>> {
-        // If no unresolved imports (single or glob) can define the name, self.binding is final.
+        match self.binding {
-        if self.outstanding_references == 0 && outstanding_globs == 0 {
+            Some(old_binding) if !old_binding.defined_with(DefModifiers::PRELUDE) => {
-            return self.binding.map(Success).unwrap_or(Failed(None));
+                if binding.defined_with(DefModifiers::GLOB_IMPORTED) {
+                    self.duplicate_globs.push(binding);
+                } else if old_binding.defined_with(DefModifiers::GLOB_IMPORTED) {
+                    self.duplicate_globs.push(old_binding);
+                    self.binding = Some(binding);
+                } else {
+                    return Err(old_binding);
+                }
+            }
+            _ => self.binding = Some(binding),
         }
 
-        if let Some(binding) = self.binding {
+        Ok(())
-            // Single imports will never be shadowable by other single or glob imports.
+    }
-            if !binding.defined_with(DefModifiers::GLOB_IMPORTED) { return Success(binding); }
+
-            // Non-PRELUDE glob imports will never be shadowable by other glob imports.
+    // Returns the resolution of the name assuming no more globs will define it.
-            if self.outstanding_references == 0 && !binding.defined_with(DefModifiers::PRELUDE) {
+    fn result(&self) -> ResolveResult<&'a NameBinding<'a>> {
-                return Success(binding);
+        match self.binding {
+            Some(binding) if !binding.defined_with(DefModifiers::GLOB_IMPORTED) => Success(binding),
+            _ if self.outstanding_references > 0 => Indeterminate,
+            Some(binding) => Success(binding),
+            None => Failed(None),
+        }
+    }
+
+    // Returns Some(the resolution of the name), or None if the resolution depends
+    // on whether more globs can define the name.
+    fn try_result(&self) -> Option<ResolveResult<&'a NameBinding<'a>>> {
+        match self.result() {
+            Success(binding) if binding.defined_with(DefModifiers::PRELUDE) => None,
+            Failed(_) => None,
+            result @ _ => Some(result),
+        }
+    }
+
+    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() {
+            if duplicate_glob.defined_with(DefModifiers::PRELUDE) { continue }
+
+            // 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> {
+    pub fn resolve_name(&self, name: Name, ns: Namespace, allow_private_imports: bool)
+                        -> ResolveResult<&'a NameBinding<'a>> {
+        let resolutions = match self.resolutions.borrow_state() {
+            ::std::cell::BorrowState::Unused => self.resolutions.borrow(),
+            _ => return Failed(None), // This happens when there is a cycle of glob imports
+        };
+
+        let resolution = resolutions.get(&(name, ns)).cloned().unwrap_or_default();
+        if let Some(result) = resolution.try_result() {
+            // If the resolution doesn't depend on glob definability, check privacy and return.
+            return result.and_then(|binding| {
+                let allowed = allow_private_imports || !binding.is_import() || binding.is_public();
+                if allowed { Success(binding) } else { Failed(None) }
+            });
+        }
+
+        let (ref mut public_globs, ref mut private_globs) = *self.resolved_globs.borrow_mut();
+
+        // Check if the public globs are determined
+        if self.pub_glob_count.get() > 0 {
+            return Indeterminate;
+        }
+        for module in public_globs.iter() {
+            if let Indeterminate = module.resolve_name(name, ns, false) {
+                return Indeterminate;
             }
         }
 
-        Indeterminate
+        if !allow_private_imports {
+            return Failed(None);
+        }
+
+        // Check if the private globs are determined
+        if self.glob_count.get() > 0 {
+            return Indeterminate;
+        }
+        for module in private_globs.iter() {
+            if let Indeterminate = module.resolve_name(name, ns, false) {
+                return Indeterminate;
+            }
+        }
+
+        resolution.result()
     }
 
     // Define the name or return the existing binding if there is a collision.
-    pub fn try_define(&mut self, binding: &'a NameBinding<'a>) -> Result<(), &'a NameBinding<'a>> {
+    pub fn try_define_child(&self, name: Name, ns: Namespace, binding: NameBinding<'a>)
-        let is_prelude = |binding: &NameBinding| binding.defined_with(DefModifiers::PRELUDE);
+                            -> Result<(), &'a NameBinding<'a>> {
-        let old_binding = match self.binding {
+        if self.resolutions.borrow_state() != ::std::cell::BorrowState::Unused { return Ok(()); }
-            Some(_) if is_prelude(binding) => return Ok(()),
+        self.update_resolution(name, ns, |resolution| {
-            Some(old_binding) if !is_prelude(old_binding) => old_binding,
+            resolution.try_define(self.arenas.alloc_name_binding(binding))
-            _ => { self.binding = Some(binding); return Ok(()); }
+        })
-        };
+    }
 
-        // FIXME #31337: We currently allow items to shadow glob-imported re-exports.
+    pub fn increment_outstanding_references_for(&self, name: Name, ns: Namespace) {
-        if !old_binding.is_import() && binding.defined_with(DefModifiers::GLOB_IMPORTED) {
+        let mut resolutions = self.resolutions.borrow_mut();
-            if let NameBindingKind::Import { binding, .. } = binding.kind {
+        resolutions.entry((name, ns)).or_insert_with(Default::default).outstanding_references += 1;
-                if binding.is_import() { return Ok(()); }
+    }
+
+    fn decrement_outstanding_references_for(&self, name: Name, ns: Namespace) {
+        self.update_resolution(name, ns, |resolution| match resolution.outstanding_references {
+            0 => panic!("No more outstanding references!"),
+            ref mut outstanding_references => *outstanding_references -= 1,
+        })
+    }
+
+    // Use `update` to mutate the resolution for the name.
+    // If the resolution becomes a success, define it in the module's glob importers.
+    fn update_resolution<T, F>(&self, name: Name, ns: Namespace, update: F) -> T
+        where F: FnOnce(&mut NameResolution<'a>) -> T
+    {
+        let mut resolutions = self.resolutions.borrow_mut();
+        let resolution = resolutions.entry((name, ns)).or_insert_with(Default::default);
+        let was_success = resolution.try_result().and_then(ResolveResult::success).is_some();
+
+        let t = update(resolution);
+        if !was_success {
+            if let Some(Success(binding)) = resolution.try_result() {
+                self.define_in_glob_importers(name, ns, binding);
             }
         }
+        t
+    }
 
-        Err(old_binding)
+    fn define_in_glob_importers(&self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>) {
+        if !binding.defined_with(DefModifiers::PUBLIC | DefModifiers::IMPORTABLE) { return }
+        if binding.is_extern_crate() { return }
+        for &(importer, directive) in self.glob_importers.borrow_mut().iter() {
+            let _ = importer.try_define_child(name, ns, directive.import(binding, None));
+        }
     }
 }
 
@@ -206,11 +322,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
             if self.resolver.unresolved_imports == 0 {
                 debug!("(resolving imports) success");
+                self.finalize_resolutions(self.resolver.graph_root);
                 break;
             }
 
             if self.resolver.unresolved_imports == prev_unresolved_imports {
                 // resolving failed
+                self.finalize_resolutions(self.resolver.graph_root);
                 if errors.len() > 0 {
                     for e in errors {
                         self.import_resolving_error(e)
@@ -306,7 +424,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
     /// If successful, the resolved bindings are written into the module.
     fn resolve_import_for_module(&mut self,
                                  module_: Module<'b>,
-                                 import_directive: &ImportDirective)
+                                 import_directive: &'b ImportDirective)
                                  -> ResolveResult<()> {
         debug!("(resolving import for module) resolving import `{}::...` in `{}`",
                names_to_string(&import_directive.module_path),
@@ -343,7 +461,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
     fn resolve_import(&mut self,
                       module_: Module<'b>,
                       target_module: Module<'b>,
-                      directive: &ImportDirective)
+                      directive: &'b ImportDirective)
                       -> ResolveResult<()> {
         let (source, target, value_determined, type_determined) = match directive.subclass {
             SingleImport { source, target, ref value_determined, ref type_determined } =>
@@ -418,32 +536,24 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                     .emit();
             }
 
-            (_, &Success(name_binding)) if !name_binding.is_import() && directive.is_public => {
+            (_, &Success(name_binding)) if !name_binding.is_import() &&
-                if !name_binding.is_public() {
+                                           directive.is_public &&
-                    if name_binding.is_extern_crate() {
+                                           !name_binding.is_public() => {
-                        let msg = format!("extern crate `{}` is private, and cannot be reexported \
+                if name_binding.is_extern_crate() {
-                                           (error E0364), consider declaring with `pub`",
+                    let msg = format!("extern crate `{}` is private, and cannot be reexported \
-                                           source);
+                                       (error E0364), consider declaring with `pub`",
-                        self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC,
-                                                       directive.id,
-                                                       directive.span,
-                                                       msg);
-                    } else {
-                        let msg = format!("`{}` is private, and cannot be reexported", source);
-                        let note_msg =
-                            format!("consider declaring type or module `{}` with `pub`", source);
-                        struct_span_err!(self.resolver.session, directive.span, E0365, "{}", &msg)
-                            .span_note(directive.span, &note_msg)
-                            .emit();
-                    }
-                } else if name_binding.defined_with(DefModifiers::PRIVATE_VARIANT) {
-                    let msg = format!("variant `{}` is private, and cannot be reexported \
-                                       (error E0364), consider declaring its enum as `pub`",
                                        source);
                     self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC,
                                                    directive.id,
                                                    directive.span,
                                                    msg);
+                } else {
+                    let msg = format!("`{}` is private, and cannot be reexported", source);
+                    let note_msg =
+                        format!("consider declaring type or module `{}` with `pub`", source);
+                    struct_span_err!(self.resolver.session, directive.span, E0365, "{}", &msg)
+                        .span_note(directive.span, &note_msg)
+                        .emit();
                 }
             }
 
@@ -484,36 +594,29 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
     fn resolve_glob_import(&mut self,
                            module_: Module<'b>,
                            target_module: Module<'b>,
-                           directive: &ImportDirective)
+                           directive: &'b ImportDirective)
                            -> ResolveResult<()> {
-        // We must bail out if the node has unresolved imports of any kind (including globs).
-        if target_module.pub_count.get() > 0 {
-            debug!("(resolving glob import) target module has unresolved pub imports; bailing out");
-            return Indeterminate;
-        }
-
         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
             let msg = "Cannot glob-import a module into itself.".into();
             return Failed(Some((directive.span, msg)));
         }
-
-        // Add all children from the containing module.
         build_reduced_graph::populate_module_if_necessary(self.resolver, target_module);
-        target_module.for_each_child(|name, ns, binding| {
-            if !binding.defined_with(DefModifiers::IMPORTABLE | DefModifiers::PUBLIC) { return }
-            self.define(module_, name, ns, directive.import(binding, None));
 
-            if ns == TypeNS && directive.is_public &&
+        // Add to target_module's glob_importers and module_'s resolved_globs
-               binding.defined_with(DefModifiers::PRIVATE_VARIANT) {
+        target_module.glob_importers.borrow_mut().push((module_, directive));
-                let msg = format!("variant `{}` is private, and cannot be reexported (error \
+        match *module_.resolved_globs.borrow_mut() {
-                                   E0364), consider declaring its enum as `pub`", name);
+            (ref mut public_globs, _) if directive.is_public => public_globs.push(target_module),
-                self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC,
+            (_, ref mut private_globs) => private_globs.push(target_module),
-                                               directive.id,
+        }
-                                               directive.span,
+
-                                               msg);
+        for (&(name, ns), resolution) in target_module.resolutions.borrow().iter() {
+            if let Some(Success(binding)) = resolution.try_result() {
+                if binding.defined_with(DefModifiers::IMPORTABLE | DefModifiers::PUBLIC) {
+                    self.define(module_, name, ns, directive.import(binding, None));
+                }
             }
-        });
+        }
 
         // Record the destination of this import
         if let Some(did) = target_module.def_id() {
@@ -535,12 +638,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
               binding: NameBinding<'b>) {
         if let Err(old_binding) = parent.try_define_child(name, ns, binding.clone()) {
             self.report_conflict(name, ns, &binding, old_binding);
-        } else if binding.is_public() { // Add to the export map
-            if let (Some(parent_def_id), Some(def)) = (parent.def_id(), binding.def()) {
-                let parent_node_id = self.resolver.ast_map.as_local_node_id(parent_def_id).unwrap();
-                let export = Export { name: name, def_id: def.def_id() };
-                self.resolver.export_map.entry(parent_node_id).or_insert(Vec::new()).push(export);
-            }
         }
     }
 
@@ -604,6 +701,58 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             err.emit();
         }
     }
+
+    // Miscellaneous post-processing, including recording reexports, recording shadowed traits,
+    // reporting conflicts, and reporting the PRIVATE_IN_PUBLIC lint.
+    fn finalize_resolutions(&mut self, module: Module<'b>) {
+        // Since import resolution is finished, globs will not define any more names.
+        module.pub_glob_count.set(0); module.glob_count.set(0);
+        *module.resolved_globs.borrow_mut() = (Vec::new(), Vec::new());
+
+        let mut reexports = Vec::new();
+        for (&(name, ns), resolution) in module.resolutions.borrow().iter() {
+            resolution.report_conflicts(|b1, b2| self.report_conflict(name, ns, b1, b2));
+            let binding = match resolution.binding {
+                Some(binding) => binding,
+                None => continue,
+            };
+
+            if binding.is_public() && (binding.is_import() || binding.is_extern_crate()) {
+                if let Some(def) = binding.def() {
+                    reexports.push(Export { name: name, def_id: def.def_id() });
+                }
+            }
+
+            if let NameBindingKind::Import { binding: orig_binding, id, .. } = binding.kind {
+                if ns == TypeNS && binding.is_public() &&
+                   orig_binding.defined_with(DefModifiers::PRIVATE_VARIANT) {
+                    let msg = format!("variant `{}` is private, and cannot be reexported \
+                                       (error E0364), consider declaring its enum as `pub`",
+                                      name);
+                    let lint = lint::builtin::PRIVATE_IN_PUBLIC;
+                    self.resolver.session.add_lint(lint, id, binding.span.unwrap(), msg);
+                }
+            }
+
+            // FIXME #31379: We can use methods from imported traits shadowed by non-import items
+            if !binding.is_import() {
+                for glob_binding in resolution.duplicate_globs.iter() {
+                    module.shadowed_traits.borrow_mut().push(glob_binding);
+                }
+            }
+        }
+
+        if reexports.len() > 0 {
+            if let Some(def_id) = module.def_id() {
+                let node_id = self.resolver.ast_map.as_local_node_id(def_id).unwrap();
+                self.resolver.export_map.insert(node_id, reexports);
+            }
+        }
+
+        for (_, child) in module.module_children.borrow().iter() {
+            self.finalize_resolutions(child);
+        }
+    }
 }
 
 fn import_path_to_string(names: &[Name], subclass: &ImportDirectiveSubclass) -> String {