Auto merge of #46419 - jseyfried:all_imports_in_metadata, r=nrc

Record all imports (`use`, `extern crate`) in the crate metadata

This PR adds non-`pub` `use` and `extern crate` imports in the crate metadata since hygienic macros invoked in other crates may use them. We already include all other non-`pub` items in the crate metadata. This improves import suggestions in some cases.

Fixes #42337.

r? @nrc

Author: bors

src/librustc/hir/def.rs

@@ -14,6 +14,7 @@ use syntax::ast;
 use syntax::ext::base::MacroKind;
 use syntax_pos::Span;
 use hir;
+use ty;
 
 #[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
 pub enum CtorKind {
@@ -126,6 +127,11 @@ pub struct Export {
     pub def: Def,
     /// The span of the target definition.
     pub span: Span,
+    /// The visibility of the export.
+    /// We include non-`pub` exports for hygienic macros that get used from extern crates.
+    pub vis: ty::Visibility,
+    /// True if from a `use` or and `extern crate`.
+    pub is_import: bool,
 }
 
 impl CtorKind {

src/librustc/ich/impls_hir.rs

@@ -1068,7 +1068,9 @@ for hir::def_id::DefIndex {
 impl_stable_hash_for!(struct hir::def::Export {
     ident,
     def,
-    span
+    vis,
+    span,
+    is_import
 });
 
 impl<'gcx> HashStable<StableHashingContext<'gcx>>

src/librustc/ty/mod.rs

@@ -307,6 +307,15 @@ impl Visibility {
 
         self.is_accessible_from(vis_restriction, tree)
     }
+
+    // Returns true if this item is visible anywhere in the local crate.
+    pub fn is_visible_locally(self) -> bool {
+        match self {
+            Visibility::Public => true,
+            Visibility::Restricted(def_id) => def_id.is_local(),
+            Visibility::Invisible => false,
+        }
+    }
 }
 
 #[derive(Clone, PartialEq, RustcDecodable, RustcEncodable, Copy)]

src/librustc_metadata/cstore_impl.rs

@@ -305,12 +305,12 @@ pub fn provide<'tcx>(providers: &mut Providers<'tcx>) {
                 let mut add_child = |bfs_queue: &mut VecDeque<_>,
                                      child: &def::Export,
                                      parent: DefId| {
-                    let child = child.def.def_id();
-
-                    if tcx.visibility(child) != ty::Visibility::Public {
+                    if child.vis != ty::Visibility::Public {
                         return;
                     }
 
+                    let child = child.def.def_id();
+
                     match visible_parent_map.entry(child) {
                         Entry::Occupied(mut entry) => {
                             // If `child` is defined in crate `cnum`, ensure

src/librustc_metadata/decoder.rs

@@ -639,7 +639,13 @@ impl<'a, 'tcx> CrateMetadata {
                         ext.kind()
                     );
                     let ident = Ident::with_empty_ctxt(name);
-                    callback(def::Export { ident: ident, def: def, span: DUMMY_SP });
+                    callback(def::Export {
+                        ident: ident,
+                        def: def,
+                        vis: ty::Visibility::Public,
+                        span: DUMMY_SP,
+                        is_import: false,
+                    });
                 }
             }
             return
@@ -676,7 +682,9 @@ impl<'a, 'tcx> CrateMetadata {
                                 callback(def::Export {
                                     def,
                                     ident: Ident::from_str(&self.item_name(child_index)),
+                                    vis: self.get_visibility(child_index),
                                     span: self.entry(child_index).span.decode((self, sess)),
+                                    is_import: false,
                                 });
                             }
                         }
@@ -693,23 +701,30 @@ impl<'a, 'tcx> CrateMetadata {
                 if let (Some(def), Some(name)) =
                     (self.get_def(child_index), def_key.disambiguated_data.data.get_opt_name()) {
                     let ident = Ident::from_str(&name);
-                    callback(def::Export { def: def, ident: ident, span: span });
+                    let vis = self.get_visibility(child_index);
+                    let is_import = false;
+                    callback(def::Export { def, ident, vis, span, is_import });
                     // For non-reexport structs and variants add their constructors to children.
                     // Reexport lists automatically contain constructors when necessary.
                     match def {
                         Def::Struct(..) => {
                             if let Some(ctor_def_id) = self.get_struct_ctor_def_id(child_index) {
                                 let ctor_kind = self.get_ctor_kind(child_index);
                                 let ctor_def = Def::StructCtor(ctor_def_id, ctor_kind);
-                                callback(def::Export { def: ctor_def, ident: ident, span: span });
+                                callback(def::Export {
+                                    def: ctor_def,
+                                    vis: self.get_visibility(ctor_def_id.index),
+                                    ident, span, is_import,
+                                });
                             }
                         }
                         Def::Variant(def_id) => {
                             // Braced variants, unlike structs, generate unusable names in
                             // value namespace, they are reserved for possible future use.
                             let ctor_kind = self.get_ctor_kind(child_index);
                             let ctor_def = Def::VariantCtor(def_id, ctor_kind);
-                            callback(def::Export { def: ctor_def, ident: ident, span: span });
+                            let vis = self.get_visibility(child_index);
+                            callback(def::Export { def: ctor_def, ident, vis, span, is_import });
                         }
                         _ => {}
                     }

src/librustc_metadata/encoder.rs

@@ -506,9 +506,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
 
         let data = ModData {
             reexports: match tcx.module_exports(def_id) {
-                Some(ref exports) if *vis == hir::Public => {
-                    self.lazy_seq_from_slice(exports.as_slice())
-                }
+                Some(ref exports) => self.lazy_seq_from_slice(exports.as_slice()),
                 _ => LazySeq::empty(),
             },
         };

src/librustc_privacy/lib.rs

@@ -331,7 +331,9 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
             if let Some(exports) = self.tcx.module_exports(def_id) {
                 for export in exports.iter() {
                     if let Some(node_id) = self.tcx.hir.as_local_node_id(export.def.def_id()) {
-                        self.update(node_id, Some(AccessLevel::Exported));
+                        if export.vis == ty::Visibility::Public {
+                            self.update(node_id, Some(AccessLevel::Exported));
+                        }
                     }
                 }
             }
@@ -365,6 +367,15 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
             for id in &module.item_ids {
                 self.update(id.id, level);
             }
+            let def_id = self.tcx.hir.local_def_id(module_id);
+            if let Some(exports) = self.tcx.module_exports(def_id) {
+                for export in exports.iter() {
+                    if let Some(node_id) = self.tcx.hir.as_local_node_id(export.def.def_id()) {
+                        self.update(node_id, level);
+                    }
+                }
+            }
+
             if module_id == ast::CRATE_NODE_ID {
                 break
             }

src/librustc_resolve/build_reduced_graph.rs

@@ -466,11 +466,8 @@ impl<'a> Resolver<'a> {
 
     /// Builds the reduced graph for a single item in an external crate.
     fn build_reduced_graph_for_external_crate_def(&mut self, parent: Module<'a>, child: Export) {
-        let ident = child.ident;
-        let def = child.def;
+        let Export { ident, def, vis, span, .. } = child;
         let def_id = def.def_id();
-        let vis = self.cstore.visibility_untracked(def_id);
-        let span = child.span;
         let expansion = Mark::root(); // FIXME(jseyfried) intercrate hygiene
         match def {
             Def::Mod(..) | Def::Enum(..) => {
@@ -674,7 +671,8 @@ impl<'a> Resolver<'a> {
             let ident = Ident::with_empty_ctxt(name);
             let result = self.resolve_ident_in_module(module, ident, MacroNS, false, false, span);
             if let Ok(binding) = result {
-                self.macro_exports.push(Export { ident: ident, def: binding.def(), span: span });
+                let (def, vis) = (binding.def(), binding.vis);
+                self.macro_exports.push(Export { ident, def, vis, span, is_import: true });
             } else {
                 span_err!(self.session, span, E0470, "reexported macro not found");
             }

src/librustc_resolve/lib.rs

@@ -1108,7 +1108,11 @@ impl<'a> NameBinding<'a> {
 
     // We sometimes need to treat variants as `pub` for backwards compatibility
     fn pseudo_vis(&self) -> ty::Visibility {
-        if self.is_variant() { ty::Visibility::Public } else { self.vis }
+        if self.is_variant() && self.def().def_id().is_local() {
+            ty::Visibility::Public
+        } else {
+            self.vis
+        }
     }
 
     fn is_variant(&self) -> bool {
@@ -3613,9 +3617,9 @@ impl<'a> Resolver<'a> {
             self.populate_module_if_necessary(in_module);
 
             in_module.for_each_child_stable(|ident, _, name_binding| {
-                // abort if the module is already found
-                if let Some(_) = result {
-                    return ();
+                // abort if the module is already found or if name_binding is private external
+                if result.is_some() || !name_binding.vis.is_visible_locally() {
+                    return
                 }
                 if let Some(module) = name_binding.module() {
                     // form the path

src/librustc_resolve/macros.rs

@@ -746,8 +746,13 @@ impl<'a> Resolver<'a> {
             }));
             if attr::contains_name(&item.attrs, "macro_export") {
                 let def = Def::Macro(def_id, MacroKind::Bang);
-                self.macro_exports
-                    .push(Export { ident: ident.modern(), def: def, span: item.span });
+                self.macro_exports.push(Export {
+                    ident: ident.modern(),
+                    def: def,
+                    vis: ty::Visibility::Public,
+                    span: item.span,
+                    is_import: false,
+                });
             } else {
                 self.unused_macros.insert(def_id);
             }

src/librustc_resolve/resolve_imports.rs

@@ -898,8 +898,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                 None => continue,
             };
 
-            if binding.vis == ty::Visibility::Public &&
-               (binding.is_import() || binding.is_macro_def()) {
+            if binding.is_import() || binding.is_macro_def() {
                 let def = binding.def();
                 if def != Def::Err {
                     if !def.def_id().is_local() {
@@ -915,7 +914,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                                 .emit();
                         }
                     }
-                    reexports.push(Export { ident: ident.modern(), def: def, span: binding.span });
+                    reexports.push(Export {
+                        ident: ident.modern(),
+                        def: def,
+                        span: binding.span,
+                        vis: binding.vis,
+                        is_import: true,
+                    });
                 }
             }
 

src/librustdoc/clean/inline.rs

@@ -392,7 +392,7 @@ fn build_module(cx: &DocContext, did: DefId) -> clean::Module {
         let mut visited = FxHashSet();
         for &item in cx.tcx.item_children(did).iter() {
             let def_id = item.def.def_id();
-            if cx.tcx.visibility(def_id) == ty::Visibility::Public {
+            if item.vis == ty::Visibility::Public {
                 if !visited.insert(def_id) { continue }
                 if let Some(i) = try_inline(cx, item.def, item.ident.name) {
                     items.extend(i)

src/librustdoc/visit_lib.rs

@@ -68,7 +68,9 @@ impl<'a, 'b, 'tcx> LibEmbargoVisitor<'a, 'b, 'tcx> {
         }
 
         for item in self.cx.tcx.item_children(def_id).iter() {
-            self.visit_item(item.def);
+            if !item.is_import || item.vis == Visibility::Public {
+                self.visit_item(item.def);
+            }
         }
     }
 

src/test/run-pass/hygiene/auxiliary/xcrate.rs

@@ -0,0 +1,38 @@
+// Copyright 2017 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.
+
+#![feature(decl_macro)]
+#![allow(unused)]
+
+pub use bar::test;
+
+extern crate std as foo;
+
+pub fn f() {}
+use f as f2;
+
+mod bar {
+    pub fn g() {}
+    use baz::h;
+
+    pub macro test() {
+        use std::mem;
+        use foo::cell;
+        ::f();
+        ::f2();
+        g();
+        h();
+        ::bar::h();
+    }
+}
+
+mod baz {
+    pub fn h() {}
+}

src/test/run-pass/hygiene/xcrate.rs

@@ -0,0 +1,21 @@
+// Copyright 2017 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.
+
+// ignore-pretty pretty-printing is unhygienic
+
+// aux-build:xcrate.rs
+
+#![feature(decl_macro)]
+
+extern crate xcrate;
+
+fn main() {
+    xcrate::test!();
+}