-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Linting public reexport of private dependencies #143856
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
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @compiler-errors. Use |
r? petrochenkov perhaps |
☔ The latest upstream changes (presumably #143888) made this pull request unmergeable. Please resolve the merge conflicts. |
See calls to |
self.tcx.with_stable_hashing_context(|hcx| self.module_children.to_sorted(&hcx, false)) | ||
{ | ||
for import in module_children { | ||
if !import.vis.is_public() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to move the lint logic to check_hidden_glob_reexports
.
That function already has a very similar logic for iterating over bindings.
And also the effective visibility table is already populated there so you can do checks like self.effective_visibilities.is_exported(def_id)
instead of just relying on pub
and getting false positives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it there.
Should I rename the function to something like check_reexports
? I can't think of anything more specific but it already wasn't checking just for hidden globs and now I feel I'm making the naming situation worse.
070e53d
to
d2fc98c
Compare
@rustbot label -S-waiting-on-author +S-waiting-on-review |
Part of public/private dependencies #44663
Partially addresses #71043
I'm adding a warning for reexports of private dependencies into
rustc_resolve
. I get that this should not be a warning, but should instead be a lint to be controlled by the feature gate, but I did not figure out how exactly to do that at that point. I tried doing the same thing as is done inrustc_privacy
, but the linting system is not ready yet as far as I understand the error I got, so I made a warning for now instead. Some guidance on how to emit lints withdcx
would be appreciated.This also sets the
std_detect
crate as a public dependency ofstd
because some macros are reexported from there. I did not check closer, but the other option may be to allow the specific reexports instead.