Auto merge of #57922 - davidtwco:issue-57410, r=petrochenkov

Update visibility of intermediate use items.

Fixes #57410 and fixes #53925 and fixes #47816.

Currently, the target of a use statement will be updated with
the visibility of the use statement itself (if the use statement was
visible).

This PR ensures that if the path to the target item is via another
use statement then that intermediate use statement will also have the
visibility updated like the target. This silences incorrect
`unreachable_pub` lints with inactionable suggestions.

Author: bors

src/libcore/iter/adapters/mod.rs

@@ -11,6 +11,7 @@ mod flatten;
 mod zip;
 
 pub use self::chain::Chain;
+#[stable(feature = "rust1", since = "1.0.0")]
 pub use self::flatten::{FlatMap, Flatten};
 pub use self::zip::Zip;
 pub(crate) use self::zip::TrustedRandomAccess;

src/libcore/iter/traits/mod.rs

@@ -5,9 +5,11 @@ mod collect;
 mod accum;
 mod marker;
 
+#[stable(feature = "rust1", since = "1.0.0")]
 pub use self::iterator::Iterator;
 pub use self::double_ended::DoubleEndedIterator;
 pub use self::exact_size::ExactSizeIterator;
 pub use self::collect::{FromIterator, IntoIterator, Extend};
 pub use self::accum::{Sum, Product};
+#[stable(feature = "rust1", since = "1.0.0")]
 pub use self::marker::{FusedIterator, TrustedLen};

src/librustc/hir/def.rs

@@ -252,12 +252,14 @@ impl NonMacroAttrKind {
 }
 
 impl Def {
+    /// Return the `DefId` of this `Def` if it has an id, else panic.
     pub fn def_id(&self) -> DefId {
         self.opt_def_id().unwrap_or_else(|| {
             bug!("attempted .def_id() on invalid def: {:?}", self)
         })
     }
 
+    /// Return `Some(..)` with the `DefId` of this `Def` if it has a id, else `None`.
     pub fn opt_def_id(&self) -> Option<DefId> {
         match *self {
             Def::Fn(id) | Def::Mod(id) | Def::Static(id, _) |
@@ -284,6 +286,14 @@ impl Def {
         }
     }
 
+    /// Return the `DefId` of this `Def` if it represents a module.
+    pub fn mod_def_id(&self) -> Option<DefId> {
+        match *self {
+            Def::Mod(id) => Some(id),
+            _ => None,
+        }
+    }
+
     /// A human readable name for the def kind ("function", "module", etc.).
     pub fn kind_name(&self) -> &'static str {
         match *self {

src/librustc/middle/privacy.rs

@@ -11,16 +11,16 @@ use syntax::ast::NodeId;
 // Accessibility levels, sorted in ascending order
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]
 pub enum AccessLevel {
-    // Superset of Reachable used to mark impl Trait items.
+    /// Superset of `AccessLevel::Reachable` used to mark impl Trait items.
     ReachableFromImplTrait,
-    // Exported items + items participating in various kinds of public interfaces,
-    // but not directly nameable. For example, if function `fn f() -> T {...}` is
-    // public, then type `T` is reachable. Its values can be obtained by other crates
-    // even if the type itself is not nameable.
+    /// Exported items + items participating in various kinds of public interfaces,
+    /// but not directly nameable. For example, if function `fn f() -> T {...}` is
+    /// public, then type `T` is reachable. Its values can be obtained by other crates
+    /// even if the type itself is not nameable.
     Reachable,
-    // Public items + items accessible to other crates with help of `pub use` re-exports
+    /// Public items + items accessible to other crates with help of `pub use` re-exports
     Exported,
-    // Items accessible to other crates directly, without help of re-exports
+    /// Items accessible to other crates directly, without help of re-exports
     Public,
 }
 
@@ -31,12 +31,17 @@ pub struct AccessLevels<Id = NodeId> {
 }
 
 impl<Id: Hash + Eq> AccessLevels<Id> {
+    /// See `AccessLevel::Reachable`.
     pub fn is_reachable(&self, id: Id) -> bool {
         self.map.get(&id) >= Some(&AccessLevel::Reachable)
     }
+
+    /// See `AccessLevel::Exported`.
     pub fn is_exported(&self, id: Id) -> bool {
         self.map.get(&id) >= Some(&AccessLevel::Exported)
     }
+
+    /// See `AccessLevel::Public`.
     pub fn is_public(&self, id: Id) -> bool {
         self.map.get(&id) >= Some(&AccessLevel::Public)
     }

src/librustc_privacy/lib.rs

@@ -437,6 +437,43 @@ impl<'a, 'tcx> EmbargoVisitor<'a, 'tcx> {
             ev: self,
         }
     }
+
+
+    /// Given the path segments of a `ItemKind::Use`, then we need
+    /// to update the visibility of the intermediate use so that it isn't linted
+    /// by `unreachable_pub`.
+    ///
+    /// This isn't trivial as `path.def` has the `DefId` of the eventual target
+    /// of the use statement not of the next intermediate use statement.
+    ///
+    /// To do this, consider the last two segments of the path to our intermediate
+    /// use statement. We expect the penultimate segment to be a module and the
+    /// last segment to be the name of the item we are exporting. We can then
+    /// look at the items contained in the module for the use statement with that
+    /// name and update that item's visibility.
+    ///
+    /// FIXME: This solution won't work with glob imports and doesn't respect
+    /// namespaces. See <https://github.com/rust-lang/rust/pull/57922#discussion_r251234202>.
+    fn update_visibility_of_intermediate_use_statements(&mut self, segments: &[hir::PathSegment]) {
+        if let Some([module, segment]) = segments.rchunks_exact(2).next() {
+            if let Some(item) = module.def
+                .and_then(|def| def.mod_def_id())
+                .and_then(|def_id| self.tcx.hir().as_local_node_id(def_id))
+                .map(|module_node_id| self.tcx.hir().expect_item(module_node_id))
+             {
+                if let hir::ItemKind::Mod(m) = &item.node {
+                    for item_id in m.item_ids.as_ref() {
+                        let item = self.tcx.hir().expect_item(item_id.id);
+                        let def_id = self.tcx.hir().local_def_id(item_id.id);
+                        if !self.tcx.hygienic_eq(segment.ident, item.ident, def_id) { continue; }
+                        if let hir::ItemKind::Use(..) = item.node {
+                            self.update(item.id, Some(AccessLevel::Exported));
+                        }
+                    }
+                }
+            }
+        }
+    }
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
@@ -523,8 +560,14 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
             hir::ItemKind::ExternCrate(..) => {}
             // All nested items are checked by `visit_item`.
             hir::ItemKind::Mod(..) => {}
-            // Re-exports are handled in `visit_mod`.
-            hir::ItemKind::Use(..) => {}
+            // Re-exports are handled in `visit_mod`. However, in order to avoid looping over
+            // all of the items of a mod in `visit_mod` looking for use statements, we handle
+            // making sure that intermediate use statements have their visibilities updated here.
+            hir::ItemKind::Use(ref path, _) => {
+                if item_level.is_some() {
+                    self.update_visibility_of_intermediate_use_statements(path.segments.as_ref());
+                }
+            }
             // The interface is empty.
             hir::ItemKind::GlobalAsm(..) => {}
             hir::ItemKind::Existential(..) => {

src/libstd/sys/mod.rs

@@ -54,6 +54,7 @@ cfg_if! {
 cfg_if! {
     if #[cfg(any(unix, target_os = "redox"))] {
         // On unix we'll document what's already available
+        #[stable(feature = "rust1", since = "1.0.0")]
         pub use self::ext as unix_ext;
     } else if #[cfg(any(target_os = "cloudabi",
                         target_arch = "wasm32",
@@ -77,6 +78,7 @@ cfg_if! {
     if #[cfg(windows)] {
         // On windows we'll just be documenting what's already available
         #[allow(missing_docs)]
+        #[stable(feature = "rust1", since = "1.0.0")]
         pub use self::ext as windows_ext;
     } else if #[cfg(any(target_os = "cloudabi",
                         target_arch = "wasm32",

src/test/ui/issues/issue-57410-1.rs

@@ -0,0 +1,18 @@
+// compile-pass
+
+// Originally from #53925.
+// Tests that the `unreachable_pub` lint doesn't fire for `pub self::bar::Bar`.
+
+#![deny(unreachable_pub)]
+
+mod foo {
+    mod bar {
+        pub struct Bar;
+    }
+
+    pub use self::bar::Bar;
+}
+
+pub use foo::Bar;
+
+fn main() {}

src/test/ui/issues/issue-57410.rs

@@ -0,0 +1,17 @@
+// compile-pass
+
+// Tests that the `unreachable_pub` lint doesn't fire for `pub self::imp::f`.
+
+#![deny(unreachable_pub)]
+
+mod m {
+    mod imp {
+        pub fn f() {}
+    }
+
+    pub use self::imp::f;
+}
+
+pub use self::m::f;
+
+fn main() {}