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

Move collection of items accessible directly or through reexports from rustc_privacy to rustc_resolve #82064

Open
petrochenkov opened this issue Feb 13, 2021 · 6 comments
Assignees
Labels
A-resolve Area: Name resolution A-visibility Area: Visibility / privacy T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented Feb 13, 2021

Directly accessible item AccessLevel::Public is an item with a fully public path from a crate root making it nameable from other crates.
AccessLevel::Exported item is an item with a fully public chain of reexports or modules making it nameable from other crates.
All such items can be found either by visiting the crate repeatedly and marking new and new items as Public/Exported until no items can be marked anymore, or doing the same thing by maintaining an item queue and pushing/popping things to/from it for processing until the queue is empty.

Sets of directly accessible and reexported items are build in rustc_privacy (EmbargoVisitor).
The problem is that information about reexport chains is lost at that point, so the produced sets do not contain all the necessary items.
fn update_visibility_of_intermediate_use_statements hack tries to mitigate that issue a bit, but it doesn't work correctly in presence of glob imports (see PR #57922 for more details).

What we need to do is to move the logic for determining AccessLevel::{Public,Exported}s to rustc_resolve where all the information about reexport chains is still available.
That logic should run either as a part of fn finalize_imports or after it.
It should build a table NodeId -> AccessLevel in Resolver, and that table should later move to struct ResolverOutputs and then to GlobalCtxt like all other ResolverOutputs fields.

Later in rustc_privacy the resolver-based table should be used as a seed for EmbargoVisitor which will then assign type-based accessibility levels like AccessLevel::Reachable etc.
fn update_visibility_of_intermediate_use_statements will need to be removed.

@petrochenkov petrochenkov added A-resolve Area: Name resolution A-visibility Area: Visibility / privacy T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 13, 2021
@petrochenkov
Copy link
Contributor Author

petrochenkov commented Feb 13, 2021

cc @jyn514 More precise AccessLevel table will fix some things for rustdoc, IIRC we've been talking about this on Zulip or some PR.
cc @davidtwco as the author of #57922.

@jyn514
Copy link
Member

jyn514 commented Feb 14, 2021

Is this meant to fix #64762? That would let me simplify parts of the doctree pass I think. The visibility issue we talked about was #77820 (comment) (tcx.visibility() returns semantic visibility, even if the pub keyword isn't accepted), which I'm not sure this would help with.

@petrochenkov
Copy link
Contributor Author

Is this meant to fix #64762?

Yes.

The visibility issue we talked about was #77820 (comment)

Nah, that's not what I meant. I think we briefly talked about #64762 or related issues at some point.

@lambinoo
Copy link
Contributor

(As part of #64762)
@rustbot claim

bors added a commit to rust-lang-ci/rust that referenced this issue Jan 10, 2022
…, r=petrochenkov

Fixes wrong unreachable_pub lints on nested and glob public reexport

Linked issues: rust-lang#64762 & rust-lang#82064
@lambinoo
Copy link
Contributor

Partially closed by #87487
except for Impl items and Reachable visibility which requires access to type information that are not available in rustc_resolve.

@petrochenkov
Copy link
Contributor Author

@lambinoo
I'll keep this open until the update_reachability_from_macro stuff is moved to resolve.
Unlike Impl items it shouldn't need access to type information and can be done in rustc_resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Area: Name resolution A-visibility Area: Visibility / privacy T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants