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

globally introduce #![deny(unreachable_pub)] #2902

Closed
wants to merge 1 commit into from

Conversation

Rustin170506
Copy link
Member

@Rustin170506 Rustin170506 commented Nov 14, 2021

close #2730

I know we don't want to use pub(crate) in pub(crate) struct, but it looks like this lint suggests we do. Do you have any comments on how to handle it?

@Rustin170506 Rustin170506 changed the title globally introduce #! [deny(unreachable_pub)] globally introduce #![deny(unreachable_pub)] Nov 14, 2021
@kinnison
Copy link
Contributor

kinnison commented Dec 2, 2021

I'm not sure what the right answer is here - the extra (crate) is entirely redundant as far as I understand it. Perhaps we need to check in with that lint author to find out?

@Rustin170506
Copy link
Member Author

@Nemo157 I remember you helped to modify this lint last year, do you have any suggestions for using create field in the crate structure? Thanks!

@Nemo157
Copy link
Member

Nemo157 commented Dec 3, 2021

🤔 I did? I don't remember that. It does make sense to me that fields could be excluded from this lint, they are not items which are what it's meant to lint against, but it also makes some sense to lint against them too and have a fully consistent "pub means external users can access it" so that locally you can tell whether things are part of the public API (though this matters less for this usecase where it looks like the unreachable_pub lint is being used as a manual implementation of rust-lang/rust#74970).

@Rustin170506
Copy link
Member Author

Maybe this doesn't make much sense for field checking. So I close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Detect dead code better
3 participants