From 71023394772e9194e3c7a2426364faef6c3ad2c4 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sat, 26 Jan 2019 20:30:52 +0100 Subject: [PATCH] Update visibility of intermediate use items. 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 commit 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. --- src/libcore/iter/adapters/mod.rs | 1 + src/libcore/iter/traits/mod.rs | 2 ++ src/librustc/hir/def.rs | 10 ++++++ src/librustc/middle/privacy.rs | 19 +++++++----- src/librustc_privacy/lib.rs | 47 +++++++++++++++++++++++++++-- src/libstd/sys/mod.rs | 2 ++ src/test/ui/issues/issue-57410-1.rs | 18 +++++++++++ src/test/ui/issues/issue-57410.rs | 17 +++++++++++ 8 files changed, 107 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/issues/issue-57410-1.rs create mode 100644 src/test/ui/issues/issue-57410.rs diff --git a/src/libcore/iter/adapters/mod.rs b/src/libcore/iter/adapters/mod.rs index f8d6bedeace25..bca1b76dbb975 100644 --- a/src/libcore/iter/adapters/mod.rs +++ b/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; diff --git a/src/libcore/iter/traits/mod.rs b/src/libcore/iter/traits/mod.rs index 000b9fad70b94..cf3013f423c94 100644 --- a/src/libcore/iter/traits/mod.rs +++ b/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}; diff --git a/src/librustc/hir/def.rs b/src/librustc/hir/def.rs index 8a74c51d3f723..f8f27992b3ea8 100644 --- a/src/librustc/hir/def.rs +++ b/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 { 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 { + 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 { diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs index 7736d5e795ea0..3baf0f0ea39ff 100644 --- a/src/librustc/middle/privacy.rs +++ b/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 { } impl AccessLevels { + /// 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) } diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 1bdc22b37d73b..05d20562d34e7 100644 --- a/src/librustc_privacy/lib.rs +++ b/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 . + 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(..) => { diff --git a/src/libstd/sys/mod.rs b/src/libstd/sys/mod.rs index f398a2a6225ce..99ef74179c2ba 100644 --- a/src/libstd/sys/mod.rs +++ b/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", diff --git a/src/test/ui/issues/issue-57410-1.rs b/src/test/ui/issues/issue-57410-1.rs new file mode 100644 index 0000000000000..dab77bd660ca0 --- /dev/null +++ b/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() {} diff --git a/src/test/ui/issues/issue-57410.rs b/src/test/ui/issues/issue-57410.rs new file mode 100644 index 0000000000000..0d697e5619d24 --- /dev/null +++ b/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() {}