-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Change some pub
to pub(crate)
in std
#107901
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
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
pub
to pub(crate)
pub
to pub(crate)
in std
I have a feeling this is going to take a few iterations to get the windows CI to pass so |
af09e69
to
3c4a623
Compare
Decided not to touch |
Nominating for libs. I think this merits a decision on whether we want to do this or not at the team level given how impactful this is across the standard library. (If we do, then incrementally doing so seems ok, and then enabling the lint as we finish with most of the crate. I would probably enable earlier and allow it in e.g. the sys module tree though. |
We discussed this in today's libs meeting. We're generally in favor of doing this (thanks!), but we do want to enable the lint to make sure no new not-actually-pub Also, it seems there are a lot of items that could be |
3c4a623
to
ba7f8c2
Compare
I was able to get most of std automatically
I'll work on seeing how much of the |
fb72d8d
to
1dd36d8
Compare
1dd36d8
to
d272d6a
Compare
☔ The latest upstream changes (presumably #108538) made this pull request unmergeable. Please resolve the merge conflicts. |
@clubby789 any updates on this? |
I'm going to close this for now, as I'm not sure I have the time to make sure this is all done correctly right now 🙁
I think this would be a good step to approach first which I might find time to look into 😓 |
Most of this was done by enabling
warn(unreachable_pub)
and usingx fix library/std
. It's incomplete and doesn't leave the lint enabled ascargo fix
doesn't recurse through path dependencies, and only covers items which trigger on my system (x86-64 GNU Linux
).EDIT: Just wanted to note that the motivation for this was #107884, where a very unsound function, only used in tests, was both
pub
and not markedunsafe