Auto merge of #51425 - QuietMisdreavus:thats-def-a-namespace-there, r=petrochenkov

refactor: create multiple HIR items for imports

When lowering `use` statements into HIR, they get a `Def` of the thing they're pointing at. This is great for things that need to know what was just pulled into scope. However, this is a bit misleading, because a `use` statement can pull things from multiple namespaces if their names collide. This is a problem for rustdoc, because if there are a module and a function with the same name (for example) then it will only document the module import, because that's that the lowered `use` statement points to.

The current version of this PR does the following:

* Whenever the resolver comes across a `use` statement, it loads the definitions into a new `import_map` instead of the existing `def_map`. This keeps the resolutions per-namespace so that all the target definitions are available.
* When lowering `use` statements, it looks up the resolutions in the `import_map` and creates multiple `Item`s if there is more than one resolution.
* To ensure the `NodeId`s are properly tracked in the lowered module, they need to be created in the AST, and pulled out as needed if multiple resolutions are available.

Fixes #34843

Author: bors

src/librustc/hir/def.rs

@@ -16,6 +16,8 @@ use syntax_pos::Span;
 use hir;
 use ty;
 
+use self::Namespace::*;
+
 #[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
 pub enum CtorKind {
     /// Constructor function automatically created by a tuple struct/variant.
@@ -116,13 +118,83 @@ impl PathResolution {
     }
 }
 
+/// Different kinds of symbols don't influence each other.
+///
+/// Therefore, they have a separate universe (namespace).
+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
+pub enum Namespace {
+    TypeNS,
+    ValueNS,
+    MacroNS,
+}
+
+/// Just a helper ‒ separate structure for each namespace.
+#[derive(Copy, Clone, Default, Debug)]
+pub struct PerNS<T> {
+    pub value_ns: T,
+    pub type_ns: T,
+    pub macro_ns: T,
+}
+
+impl<T> PerNS<T> {
+    pub fn map<U, F: FnMut(T) -> U>(self, mut f: F) -> PerNS<U> {
+        PerNS {
+            value_ns: f(self.value_ns),
+            type_ns: f(self.type_ns),
+            macro_ns: f(self.macro_ns),
+        }
+    }
+}
+
+impl<T> ::std::ops::Index<Namespace> for PerNS<T> {
+    type Output = T;
+    fn index(&self, ns: Namespace) -> &T {
+        match ns {
+            ValueNS => &self.value_ns,
+            TypeNS => &self.type_ns,
+            MacroNS => &self.macro_ns,
+        }
+    }
+}
+
+impl<T> ::std::ops::IndexMut<Namespace> for PerNS<T> {
+    fn index_mut(&mut self, ns: Namespace) -> &mut T {
+        match ns {
+            ValueNS => &mut self.value_ns,
+            TypeNS => &mut self.type_ns,
+            MacroNS => &mut self.macro_ns,
+        }
+    }
+}
+
+impl<T> PerNS<Option<T>> {
+    /// Returns whether all the items in this collection are `None`.
+    pub fn is_empty(&self) -> bool {
+        self.type_ns.is_none() && self.value_ns.is_none() && self.macro_ns.is_none()
+    }
+
+    /// Returns an iterator over the items which are `Some`.
+    pub fn present_items(self) -> impl Iterator<Item=T> {
+        use std::iter::once;
+
+        once(self.type_ns)
+            .chain(once(self.value_ns))
+            .chain(once(self.macro_ns))
+            .filter_map(|it| it)
+    }
+}
+
 /// Definition mapping
 pub type DefMap = NodeMap<PathResolution>;
 
 /// This is the replacement export map. It maps a module to all of the exports
 /// within.
 pub type ExportMap = DefIdMap<Vec<Export>>;
 
+/// Map used to track the `use` statements within a scope, matching it with all the items in every
+/// namespace.
+pub type ImportMap = NodeMap<PerNS<Option<PathResolution>>>;
+
 #[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable)]
 pub struct Export {
     /// The name of the target.

src/librustc/hir/lowering.rs

@@ -45,7 +45,7 @@ use hir;
 use hir::HirVec;
 use hir::map::{DefKey, DefPathData, Definitions};
 use hir::def_id::{DefId, DefIndex, DefIndexAddressSpace, CRATE_DEF_INDEX};
-use hir::def::{Def, PathResolution};
+use hir::def::{Def, PathResolution, PerNS};
 use lint::builtin::{self, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES};
 use middle::cstore::CrateStore;
 use rustc_data_structures::indexed_vec::IndexVec;
@@ -152,6 +152,9 @@ pub trait Resolver {
     /// Obtain the resolution for a node id
     fn get_resolution(&mut self, id: NodeId) -> Option<PathResolution>;
 
+    /// Obtain the possible resolutions for the given `use` statement.
+    fn get_import(&mut self, id: NodeId) -> PerNS<Option<PathResolution>>;
+
     /// We must keep the set of definitions up to date as we add nodes that weren't in the AST.
     /// This should only return `None` during testing.
     fn definitions(&mut self) -> &mut Definitions;
@@ -571,6 +574,15 @@ impl<'a> LoweringContext<'a> {
         })
     }
 
+    fn expect_full_def_from_use(&mut self, id: NodeId) -> impl Iterator<Item=Def> {
+        self.resolver.get_import(id).present_items().map(|pr| {
+            if pr.unresolved_segments() != 0 {
+                bug!("path not fully resolved: {:?}", pr);
+            }
+            pr.base_def()
+        })
+    }
+
     fn diagnostic(&self) -> &errors::Handler {
         self.sess.diagnostic()
     }
@@ -1532,13 +1544,13 @@ impl<'a> LoweringContext<'a> {
 
     fn lower_path_extra(
         &mut self,
-        id: NodeId,
+        def: Def,
         p: &Path,
         name: Option<Name>,
         param_mode: ParamMode,
     ) -> hir::Path {
         hir::Path {
-            def: self.expect_full_def(id),
+            def,
             segments: p.segments
                 .iter()
                 .map(|segment| {
@@ -1558,7 +1570,8 @@ impl<'a> LoweringContext<'a> {
     }
 
     fn lower_path(&mut self, id: NodeId, p: &Path, param_mode: ParamMode) -> hir::Path {
-        self.lower_path_extra(id, p, None, param_mode)
+        let def = self.expect_full_def(id);
+        self.lower_path_extra(def, p, None, param_mode)
     }
 
     fn lower_path_segment(
@@ -2363,7 +2376,7 @@ impl<'a> LoweringContext<'a> {
         let path = &tree.prefix;
 
         match tree.kind {
-            UseTreeKind::Simple(rename) => {
+            UseTreeKind::Simple(rename, id1, id2) => {
                 *name = tree.ident().name;
 
                 // First apply the prefix to the path
@@ -2387,7 +2400,58 @@ impl<'a> LoweringContext<'a> {
                     }
                 }
 
-                let path = P(self.lower_path(id, &path, ParamMode::Explicit));
+                let parent_def_index = self.current_hir_id_owner.last().unwrap().0;
+                let mut defs = self.expect_full_def_from_use(id);
+                // we want to return *something* from this function, so hang onto the first item
+                // for later
+                let mut ret_def = defs.next().unwrap_or(Def::Err);
+
+                for (def, &new_node_id) in defs.zip([id1, id2].iter()) {
+                    let vis = vis.clone();
+                    let name = name.clone();
+                    let span = path.span;
+                    self.resolver.definitions().create_def_with_parent(
+                        parent_def_index,
+                        new_node_id,
+                        DefPathData::Misc,
+                        DefIndexAddressSpace::High,
+                        Mark::root(),
+                        span);
+                    self.allocate_hir_id_counter(new_node_id, &path);
+
+                    self.with_hir_id_owner(new_node_id, |this| {
+                        let new_id = this.lower_node_id(new_node_id);
+                        let path = this.lower_path_extra(def, &path, None, ParamMode::Explicit);
+                        let item = hir::ItemUse(P(path), hir::UseKind::Single);
+                        let vis = match vis {
+                            hir::Visibility::Public => hir::Visibility::Public,
+                            hir::Visibility::Crate(sugar) => hir::Visibility::Crate(sugar),
+                            hir::Visibility::Inherited => hir::Visibility::Inherited,
+                            hir::Visibility::Restricted { ref path, id: _ } => {
+                                hir::Visibility::Restricted {
+                                    path: path.clone(),
+                                    // We are allocating a new NodeId here
+                                    id: this.next_id().node_id,
+                                }
+                            }
+                        };
+
+                        this.items.insert(
+                            new_id.node_id,
+                            hir::Item {
+                                id: new_id.node_id,
+                                hir_id: new_id.hir_id,
+                                name: name,
+                                attrs: attrs.clone(),
+                                node: item,
+                                vis,
+                                span,
+                            },
+                        );
+                    });
+                }
+
+                let path = P(self.lower_path_extra(ret_def, &path, None, ParamMode::Explicit));
                 hir::ItemUse(path, hir::UseKind::Single)
             }
             UseTreeKind::Glob => {
@@ -2654,7 +2718,7 @@ impl<'a> LoweringContext<'a> {
         match i.node {
             ItemKind::Use(ref use_tree) => {
                 let mut vec = SmallVector::one(hir::ItemId { id: i.id });
-                self.lower_item_id_use_tree(use_tree, &mut vec);
+                self.lower_item_id_use_tree(use_tree, i.id, &mut vec);
                 return vec;
             }
             ItemKind::MacroDef(..) => return SmallVector::new(),
@@ -2663,14 +2727,25 @@ impl<'a> LoweringContext<'a> {
         SmallVector::one(hir::ItemId { id: i.id })
     }
 
-    fn lower_item_id_use_tree(&self, tree: &UseTree, vec: &mut SmallVector<hir::ItemId>) {
+    fn lower_item_id_use_tree(&mut self,
+                              tree: &UseTree,
+                              base_id: NodeId,
+                              vec: &mut SmallVector<hir::ItemId>)
+    {
         match tree.kind {
             UseTreeKind::Nested(ref nested_vec) => for &(ref nested, id) in nested_vec {
                 vec.push(hir::ItemId { id });
-                self.lower_item_id_use_tree(nested, vec);
+                self.lower_item_id_use_tree(nested, id, vec);
             },
             UseTreeKind::Glob => {}
-            UseTreeKind::Simple(..) => {}
+            UseTreeKind::Simple(_, id1, id2) => {
+                for (_, &id) in self.expect_full_def_from_use(base_id)
+                                    .skip(1)
+                                    .zip([id1, id2].iter())
+                {
+                    vec.push(hir::ItemId { id });
+                }
+            },
         }
     }
 

src/librustc_lint/unused.rs

@@ -393,7 +393,7 @@ impl UnusedImportBraces {
             // Trigger the lint if the nested item is a non-self single item
             let node_ident;
             match items[0].0.kind {
-                ast::UseTreeKind::Simple(rename) => {
+                ast::UseTreeKind::Simple(rename, ..) => {
                     let orig_ident = items[0].0.prefix.segments.last().unwrap().ident;
                     if orig_ident.name == keywords::SelfValue.name() {
                         return;

src/librustc_resolve/build_reduced_graph.rs

@@ -118,7 +118,7 @@ impl<'a> Resolver<'a> {
             .collect();
 
         match use_tree.kind {
-            ast::UseTreeKind::Simple(rename) => {
+            ast::UseTreeKind::Simple(rename, ..) => {
                 let mut ident = use_tree.ident();
                 let mut source = module_path.pop().unwrap();
                 let mut type_ns_only = false;

src/librustc_resolve/lib.rs

@@ -27,7 +27,8 @@ extern crate arena;
 extern crate rustc;
 extern crate rustc_data_structures;
 
-use self::Namespace::*;
+pub use rustc::hir::def::{Namespace, PerNS};
+
 use self::TypeParameters::*;
 use self::RibKind::*;
 
@@ -37,6 +38,7 @@ use rustc::middle::cstore::{CrateStore, CrateLoader};
 use rustc::session::Session;
 use rustc::lint;
 use rustc::hir::def::*;
+use rustc::hir::def::Namespace::*;
 use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, DefId};
 use rustc::ty;
 use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap};
@@ -614,45 +616,6 @@ impl<'a> PathSource<'a> {
     }
 }
 
-/// Different kinds of symbols don't influence each other.
-///
-/// Therefore, they have a separate universe (namespace).
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
-pub enum Namespace {
-    TypeNS,
-    ValueNS,
-    MacroNS,
-}
-
-/// Just a helper ‒ separate structure for each namespace.
-#[derive(Clone, Default, Debug)]
-pub struct PerNS<T> {
-    value_ns: T,
-    type_ns: T,
-    macro_ns: T,
-}
-
-impl<T> ::std::ops::Index<Namespace> for PerNS<T> {
-    type Output = T;
-    fn index(&self, ns: Namespace) -> &T {
-        match ns {
-            ValueNS => &self.value_ns,
-            TypeNS => &self.type_ns,
-            MacroNS => &self.macro_ns,
-        }
-    }
-}
-
-impl<T> ::std::ops::IndexMut<Namespace> for PerNS<T> {
-    fn index_mut(&mut self, ns: Namespace) -> &mut T {
-        match ns {
-            ValueNS => &mut self.value_ns,
-            TypeNS => &mut self.type_ns,
-            MacroNS => &mut self.macro_ns,
-        }
-    }
-}
-
 struct UsePlacementFinder {
     target_module: NodeId,
     span: Option<Span>,
@@ -1346,6 +1309,7 @@ pub struct Resolver<'a> {
     primitive_type_table: PrimitiveTypeTable,
 
     def_map: DefMap,
+    import_map: ImportMap,
     pub freevars: FreevarMap,
     freevars_seen: NodeMap<NodeMap<usize>>,
     pub export_map: ExportMap,
@@ -1518,6 +1482,10 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
         self.def_map.get(&id).cloned()
     }
 
+    fn get_import(&mut self, id: NodeId) -> PerNS<Option<PathResolution>> {
+        self.import_map.get(&id).cloned().unwrap_or_default()
+    }
+
     fn definitions(&mut self) -> &mut Definitions {
         &mut self.definitions
     }
@@ -1665,6 +1633,7 @@ impl<'a> Resolver<'a> {
             primitive_type_table: PrimitiveTypeTable::new(),
 
             def_map: NodeMap(),
+            import_map: NodeMap(),
             freevars: NodeMap(),
             freevars_seen: NodeMap(),
             export_map: FxHashMap(),
@@ -2215,7 +2184,7 @@ impl<'a> Resolver<'a> {
                     }
                 }
             }
-            ast::UseTreeKind::Simple(_) => {},
+            ast::UseTreeKind::Simple(..) => {},
             ast::UseTreeKind::Glob => {},
         }
     }

src/librustc_resolve/resolve_imports.rs

@@ -918,7 +918,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
         // 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.
         self.per_ns(|this, ns| if let Some(binding) = result[ns].get().ok() {
-            this.def_map.entry(directive.id).or_insert(PathResolution::new(binding.def()));
+            let mut import = this.import_map.entry(directive.id).or_default();
+            import[ns] = Some(PathResolution::new(binding.def()));
         });
 
         debug!("(resolving single import) successfully resolved import");