unreachable_pub lint ignores nested re-exports

[CAD97]

Playground: https://play.rust-lang.org/?gist=d9e9fe07b83f07a3f9b908c0c7405a99&version=nightly

#![deny(unreachable_pub)]

mod one {
    mod two {
        pub struct S;
    }
    pub use self::two::S;
}
pub use self::one::S;

error: unreachable `pub` item
 --> src/lib.rs:7:5
  |
7 |     pub use self::two::S;
  |     ---^^^^^^^^^^^^^^^^^^
  |     |
  |     help: consider restricting its visibility: `pub(crate)`
  |
note: lint level defined here
 --> src/lib.rs:1:32
  |
1 | #![crate_type = "lib"] #![deny(unreachable_pub)]
  |                                ^^^^^^^^^^^^^^^
  = help: or consider exporting it for use by other crates

---

#![deny(unreachable_pub)]

mod one {
    mod two {
        pub struct S;
    }
    pub(crate) use self::two::S;
}
pub use self::one::S;

error[E0364]: `S` is private, and cannot be reexported
 --> src/lib.rs:9:9
  |
9 | pub use self::one::S;
  |         ^^^^^^^^^^^^
  |
note: consider marking `S` as `pub` in the imported module
 --> src/lib.rs:9:9
  |
9 | pub use self::one::S;
  |         ^^^^^^^^^^^^

---

A single level of re-export works:

#![deny(unreachable_pub)]

mod one {
    pub struct S;
}
pub use self::one::S;

 Finished dev [unoptimized + debuginfo] target(s) in 0.27 secs

[zackmdavis]

After inserting some logging statments, it looks like what's going on here is that use items within modules just don't get inserted into the AccessLevels map that the lint uses to determine reachability (but crate-level pub uses do) (because they inherit non-reachability from the private module that they're in).

 INFO 2018-07-29T19:12:20Z: rustc_lint::builtin: ZMD checking item Use(path(self::two::S), Single) with id NodeId(9)
 INFO 2018-07-29T19:12:20Z: rustc_lint::builtin: ZMD debug access levels map is {NodeId(0): Public, NodeId(8): Exported, NodeId(13): Public, NodeId(12): Public, NodeId(6): Exported}

It seems like this behavior is intended?—librustc_privacy's EmbargoVisitor::visit_item comments that reëxports are visit_mod's responsibility, but visit_mod looks at hir::def::Exports, which only stores information about the target, not the pub use item.

[zackmdavis]

↑ Unless someone understands how the reachability analysis needs to change, and given the increased prominence of this lint owing to the 2018 edition, we might want to quickly PR a change that declines to lint use statements (with a FIXME comment pointing to this issue), on the grounds that false-negatives (whose consequence is, at worst, slightly less idiomatic code) are less bad than false-positives (whose consequence is, at worst, undermining users' trust in the correctness of the compiler, causing them to abandon Rust, causing thousands or millions of lost QALYs as our civilization continues to depend on memory-unsafe software).