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 https://github.com/rust-lang/rust/issues/34843

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,6 +118,72 @@ 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>;
 
@@ -123,6 +191,10 @@ pub type DefMap = NodeMap<PathResolution>;
 /// 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");

src/librustdoc/visit_ast.rs

@@ -365,6 +365,11 @@ impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> {
                 });
                 true
             }
+            hir_map::NodeStructCtor(_) if !glob => {
+                // struct constructors always show up alongside their struct definitions, we've
+                // already processed that so just discard this
+                true
+            }
             _ => false,
         };
         self.view_item_stack.remove(&def_node_id);

src/libsyntax/ast.rs

@@ -1860,7 +1860,10 @@ pub type Variant = Spanned<Variant_>;
 #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
 pub enum UseTreeKind {
     /// `use prefix` or `use prefix as rename`
-    Simple(Option<Ident>),
+    ///
+    /// The extra `NodeId`s are for HIR lowering, when additional statements are created for each
+    /// namespace.
+    Simple(Option<Ident>, NodeId, NodeId),
     /// `use prefix::{...}`
     Nested(Vec<(UseTree, NodeId)>),
     /// `use prefix::*`
@@ -1879,8 +1882,8 @@ pub struct UseTree {
 impl UseTree {
     pub fn ident(&self) -> Ident {
         match self.kind {
-            UseTreeKind::Simple(Some(rename)) => rename,
+            UseTreeKind::Simple(Some(rename), ..) => rename,
-            UseTreeKind::Simple(None) =>
+            UseTreeKind::Simple(None, ..) =>
                 self.prefix.segments.last().expect("empty prefix in a simple import").ident,
             _ => panic!("`UseTree::ident` can only be used on a simple import"),
         }

src/libsyntax/ext/build.rs

@@ -1175,7 +1175,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
         self.item_use(sp, vis, P(ast::UseTree {
             span: sp,
             prefix: path,
-            kind: ast::UseTreeKind::Simple(rename),
+            kind: ast::UseTreeKind::Simple(rename, ast::DUMMY_NODE_ID, ast::DUMMY_NODE_ID),
         }))
     }
 
@@ -1185,7 +1185,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
             (ast::UseTree {
                 span: sp,
                 prefix: self.path(sp, vec![*id]),
-                kind: ast::UseTreeKind::Simple(None),
+                kind: ast::UseTreeKind::Simple(None, ast::DUMMY_NODE_ID, ast::DUMMY_NODE_ID),
             }, ast::DUMMY_NODE_ID)
         }).collect();
 

src/libsyntax/feature_gate.rs

@@ -1545,7 +1545,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
     }
 
     fn visit_use_tree(&mut self, use_tree: &'a ast::UseTree, id: NodeId, _nested: bool) {
-        if let ast::UseTreeKind::Simple(Some(ident)) = use_tree.kind {
+        if let ast::UseTreeKind::Simple(Some(ident), ..) = use_tree.kind {
             if ident.name == "_" {
                 gate_feature_post!(&self, underscore_imports, use_tree.span,
                                    "renaming imports with `_` is unstable");

src/libsyntax/fold.rs

@@ -315,8 +315,9 @@ pub fn noop_fold_use_tree<T: Folder>(use_tree: UseTree, fld: &mut T) -> UseTree
         span: fld.new_span(use_tree.span),
         prefix: fld.fold_path(use_tree.prefix),
         kind: match use_tree.kind {
-            UseTreeKind::Simple(rename) =>
+            UseTreeKind::Simple(rename, id1, id2) =>
-                UseTreeKind::Simple(rename.map(|ident| fld.fold_ident(ident))),
+                UseTreeKind::Simple(rename.map(|ident| fld.fold_ident(ident)),
+                                    fld.new_id(id1), fld.new_id(id2)),
             UseTreeKind::Glob => UseTreeKind::Glob,
             UseTreeKind::Nested(items) => UseTreeKind::Nested(items.move_map(|(tree, id)| {
                 (fld.fold_use_tree(tree), fld.new_id(id))

src/libsyntax/parse/parser.rs

@@ -7157,7 +7157,7 @@ impl<'a> Parser<'a> {
                     UseTreeKind::Nested(self.parse_use_tree_list()?)
                 }
             } else {
-                UseTreeKind::Simple(self.parse_rename()?)
+                UseTreeKind::Simple(self.parse_rename()?, ast::DUMMY_NODE_ID, ast::DUMMY_NODE_ID)
             }
         };
 

src/libsyntax/print/pprust.rs

@@ -2958,7 +2958,7 @@ impl<'a> State<'a> {
 
     pub fn print_use_tree(&mut self, tree: &ast::UseTree) -> io::Result<()> {
         match tree.kind {
-            ast::UseTreeKind::Simple(rename) => {
+            ast::UseTreeKind::Simple(rename, ..) => {
                 self.print_path(&tree.prefix, false, 0)?;
                 if let Some(rename) = rename {
                     self.s.space()?;

src/libsyntax/test.rs

@@ -492,7 +492,7 @@ fn mk_std(cx: &TestCtxt) -> P<ast::Item> {
         (ast::ItemKind::Use(P(ast::UseTree {
             span: DUMMY_SP,
             prefix: path_node(vec![id_test]),
-            kind: ast::UseTreeKind::Simple(None),
+            kind: ast::UseTreeKind::Simple(None, ast::DUMMY_NODE_ID, ast::DUMMY_NODE_ID),
         })),
          ast::VisibilityKind::Public, keywords::Invalid.ident())
     } else {
@@ -588,7 +588,7 @@ fn mk_test_module(cx: &mut TestCtxt) -> (P<ast::Item>, Option<P<ast::Item>>) {
         let use_path = ast::UseTree {
             span: DUMMY_SP,
             prefix: path_node(vec![mod_ident, Ident::from_str("main")]),
-            kind: ast::UseTreeKind::Simple(Some(rename)),
+            kind: ast::UseTreeKind::Simple(Some(rename), ast::DUMMY_NODE_ID, ast::DUMMY_NODE_ID),
         };
 
         expander.fold_item(P(ast::Item {

src/libsyntax/visit.rs

@@ -356,7 +356,8 @@ pub fn walk_use_tree<'a, V: Visitor<'a>>(
 ) {
     visitor.visit_path(&use_tree.prefix, id);
     match use_tree.kind {
-        UseTreeKind::Simple(rename) => {
+        UseTreeKind::Simple(rename, ..) => {
+            // the extra IDs are handled during HIR lowering
             if let Some(rename) = rename {
                 visitor.visit_ident(rename);
             }

src/test/rustdoc/namespaces.rs

@@ -0,0 +1,26 @@
+// Copyright 2016 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.
+
+// issue #34843: rustdoc prioritises documenting reexports from the type namespace
+
+mod inner {
+    pub mod sync {
+        pub struct SomeStruct;
+    }
+
+    pub fn sync() {}
+}
+
+// @has namespaces/sync/index.html
+// @has namespaces/fn.sync.html
+// @has namespaces/index.html '//a/@href' 'sync/index.html'
+// @has - '//a/@href' 'fn.sync.html'
+#[doc(inline)]
+pub use inner::sync;