Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion for pub(crate) doesn't handle reexports well #57410

Closed
alexcrichton opened this issue Jan 7, 2019 · 2 comments · Fixed by #57922
Closed

Suggestion for pub(crate) doesn't handle reexports well #57410

alexcrichton opened this issue Jan 7, 2019 · 2 comments · Fixed by #57922
Assignees
Labels
A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`

Comments

@alexcrichton
Copy link
Member

First reported upstream this code:

#![warn(unreachable_pub)]

pub use self::m::f;

mod m {
    pub use self::imp::f;  // Recommends pub(crate), but that causes an error above.
    mod imp {
        pub fn f() {}
    }
}

gives the warning:

warning: unreachable `pub` item
 --> src/lib.rs:6:5
  |
6 |     pub use self::imp::f;  // Recommends pub(crate), but that causes an error above.
  |     ---^^^^^^^^^^^^^^^^^^
  |     |
  |     help: consider restricting its visibility: `pub(crate)`
  |
note: lint level defined here
 --> src/lib.rs:1:9
  |
1 | #![warn(unreachable_pub)]
  |         ^^^^^^^^^^^^^^^
  = help: or consider exporting it for use by other crates

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

but the suggestions, when applied, doesn't compile!

@alexcrichton alexcrichton added the A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` label Jan 7, 2019
@estebank
Copy link
Contributor

estebank commented Jan 7, 2019

Should it suggest pub mod m instead?

@ehuss
Copy link
Contributor

ehuss commented Jan 7, 2019

Should it suggest pub mod m instead?

Wouldn't that be a change in semantics (m would be reachable by other users)? The original code looks correct to me. The use self::imp::f; item is reachable from the root re-export. It seems to me that the lint shouldn't fire in this case.

@davidtwco davidtwco self-assigned this Jan 26, 2019
Centril added a commit to Centril/rust that referenced this issue Jan 28, 2019
Update visibility of intermediate use items.

Fixes rust-lang#57410 and fixes rust-lang#53925 and fixes rust-lang#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.
bors added a commit that referenced this issue Feb 3, 2019
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants