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

We should use crate visibility in implementation details like std::sys. #69737

Open
eddyb opened this issue Mar 5, 2020 · 4 comments
Open

We should use crate visibility in implementation details like std::sys. #69737

eddyb opened this issue Mar 5, 2020 · 4 comments
Labels
A-visibility Area: Visibility / privacy C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Mar 5, 2020

@pnkfelix brought up during the compiler meeting that #67705 is "adding a pub fn to std::sys::windows", which turned out not to be a concern as std::sys itself is private.

However I think it would be helpful (and prevent accidental reexports), if everything under std::sys (and anything like it) had crate-level visibility instead of pub.

cc @rust-lang/libs

@jonas-schievink jonas-schievink added A-visibility Area: Visibility / privacy C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 5, 2020
@Amanieu
Copy link
Member

Amanieu commented Mar 5, 2020

This pattern (pub in a private module) is actually very common in Rust code. If this is something that we want to discourage then we may want a lint for this: e.g. public item is not exported outside the current crate, considering using pub(crate) instead.

@sfackler
Copy link
Member

sfackler commented Mar 5, 2020

I think that style of lint could be interesting.

@jhpratt
Copy link
Member

jhpratt commented Mar 5, 2020

Isn't that the unreachable_pub lint?

@eddyb
Copy link
Member Author

eddyb commented Mar 5, 2020

This pattern (pub in a private module) is actually very common in Rust code.

And sometimes it's needed, e.g. bounds on pub traits that are never exported.
Also, crate-level visibility is much newer than the pattern, so I'm not surprised.

I'm not as concerned about user code, I just think the standard library should be aggressive about hiding implementation details since it's not versioned in the same way as libraries on crates.io are.

The stability system probably prevents any accidental reexports anyway, so this isn't as important to that end, I suppose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-visibility Area: Visibility / privacy C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants