Skip to content

Commit

Permalink
Rollup merge of #57922 - davidtwco:issue-57410, r=petrochenkov
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Centril authored Jan 28, 2019
2 parents b194206 + ad22de4 commit ae34f21
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/libcore/iter/adapters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/libcore/iter/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
10 changes: 10 additions & 0 deletions src/librustc/hir/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, _) |
Expand All @@ -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 {
Expand Down
19 changes: 12 additions & 7 deletions src/librustc/middle/privacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -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)
}
Expand Down
47 changes: 45 additions & 2 deletions src/librustc_privacy/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,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> {
Expand Down Expand Up @@ -522,8 +559,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(..) => {
Expand Down
1 change: 1 addition & 0 deletions src/libstd/sys/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
18 changes: 18 additions & 0 deletions src/test/ui/issues/issue-57410-1.rs
Original file line number Diff line number Diff line change
@@ -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() {}
17 changes: 17 additions & 0 deletions src/test/ui/issues/issue-57410.rs
Original file line number Diff line number Diff line change
@@ -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() {}

0 comments on commit ae34f21

Please sign in to comment.