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

fix: Implement missing logics in exported_private_dependencies lint #134176

Closed
wants to merge 2 commits into from

Conversation

ShoyuVanilla
Copy link
Member

@ShoyuVanilla ShoyuVanilla commented Dec 11, 2024

Fixes some cases of #71043

@rustbot
Copy link
Collaborator

rustbot commented Dec 11, 2024

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 11, 2024
@rust-log-analyzer

This comment has been minimized.

@ShoyuVanilla ShoyuVanilla changed the title fix: Implement missing exported_private_dependencies checks fix: Implement missing logics in exported_private_dependencies lint Feb 25, 2025
@ShoyuVanilla
Copy link
Member Author

r? compiler

@rustbot rustbot assigned BoxyUwU and unassigned petrochenkov Feb 25, 2025
@petrochenkov petrochenkov self-assigned this Feb 25, 2025
@petrochenkov
Copy link
Contributor

rustc_privacy is very unlikely to be the right place to process use and extern crate items.
I'm now back to reviews and was going to review this ~next week.

@ShoyuVanilla
Copy link
Member Author

ShoyuVanilla commented Feb 25, 2025

rustc_privacy is very unlikely to be the right place to process use and extern crate items.

Actually, I'm preparing another PR that fixes #119428 once and for all with some changes in ast path lowering and nameres resolves.
Should use and extern crate items be handled in rustc_resolve? Then I'll include them in that PR in proper locations.

@BoxyUwU BoxyUwU removed their assignment Feb 25, 2025
@petrochenkov
Copy link
Contributor

Should use and extern crate items be handled in rustc_resolve?

Yes, most likely.
In general all privacy/visibility checks are split into two parts - one in rustc_resolve for everything that doesn't require type checking, and rustc_privacy for things that do require type checking results.

See, for example, the effective_visibilities table that is build in 2 steps, or just regular privacy check.

@petrochenkov
Copy link
Contributor

I started reviewing this and I was sure that check_private_dep_leaks_only shouldn't exist, and the visitor should just always visit the impl's type and trait.
So I started figuring out why they weren't visited, and implemented a better solution (#138317) in the process.

Technically, the regular type privacy lints (not private dep leaks) should be reported there as well, it was just skipped due to many false positives.

Closing in favor of #138317.

@ShoyuVanilla ShoyuVanilla deleted the issue-71043 branch March 11, 2025 04:51
TaKO8Ki added a commit to TaKO8Ki/rust that referenced this pull request Mar 24, 2025
…r-errors

privacy: Visit types and traits in impls in type privacy lints

With one exception to avoid false positives.

Fixes the same issue as rust-lang#134176.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 25, 2025
…r-errors

privacy: Visit types and traits in impls in type privacy lints

With one exception to avoid false positives.

Fixes the same issue as rust-lang#134176.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2025
Rollup merge of rust-lang#138317 - petrochenkov:libsearch3, r=compiler-errors

privacy: Visit types and traits in impls in type privacy lints

With one exception to avoid false positives.

Fixes the same issue as rust-lang#134176.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants