Lint unused pub items in binary crates#149509
Lint unused pub items in binary crates#149509mu001999 wants to merge 2 commits intorust-lang:mainfrom
Conversation
|
rustbot has assigned @jdonszelmann. Use |
-Ztreat-pub-as-pub-crate
-Ztreat-pub-as-pub-crate-Ztreat-pub-as-pub-crate
|
What are the plans for this flag? Being made the default? As just a flag, it's essentially useless, because no one will know about it. So I'd be opposed to just adding it without any plan for making it useful for everyone. |
6541f80 to
8ac52ba
Compare
|
Maybe this could be a separate lint, I haven't thought too clearly yet |
|
Yea I do agree with nora here, maybe open a (specific) zulip thread to make a proper plan for this? |
|
I guess I'd prefer this as a lint (attribute at the crate root) myself |
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
|
I guess that the use-case that I'm envisioning is to have an opt-in way to treat |
|
Why would the end goal be for this to be opt-in? I and others want to see #74970 become a default. I wouldn't opt into this, because that would make my codebase less idiomatic, diverging from how everyone else who hasn't opted in uses The reason this is useful as a default is because it doesn't semantically make sense to distinguish between The only reason I can see that someone might not want to opt into this is that one might use |
8ac52ba to
923596e
Compare
923596e to
1d89354
Compare
This comment has been minimized.
This comment has been minimized.
1d89354 to
e116866
Compare
|
Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
|
@rfcbot reviewed |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
Can you add a test to verify that this doesn't activate on |
|
Also, @mu001999, could you please confirm that this is in the |
This comment was marked as resolved.
This comment was marked as resolved.
@joshtriplett Nop, this lint is not in the If we add this into the |
|
If this isn't warn by default, my and Noratrieb's prior concerns remain unaddressed. |
Yes, but I hope we can add the implementation of this lint firstly. And then someone could use it, I believe this has been better than the unstable flag. Making it warn-by-default will lead to a lot of noise for this PR (like bless many tests). So I'd like to make it warn-by-default in a separate PR in the future. |
@tmandry Added now, you can see https://github.com/rust-lang/rust/pull/149509/changes#diff-c8a5a400e2fb95d67572098e39a9c21d70ee7c3c23acb0841d2210dd36991477 |
This comment has been minimized.
This comment has been minimized.
48cffd9 to
a4f8fce
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. |
View all comments
This PR adds a new unstable flag -Ztreat-pub-as-pub-crate as @Kobzol suggested.When compiling binary crates with this flag, the seed worklist will only contain the entry fn and won't contain other reachable items. Then we can do the dead code analysis for pub items just like they are pub(crate).Related zulip thread #general > pub/pub(crate) within a binary is a footgun.
Updated:
Adds a new lint
unused_pub_items_in_binary(crate-level, default allow for now) instead of the previous unstable flag to lint unusedpubitems for binary crates.See more details of implementation in #149509 (comment).
This lint is allowed by default, but I believe this has been better than the unstable flag. Making it warn-by-default will lead to a lot of noise for this PR (like bless many tests). So I'd like to make it warn-by-default in a separate PR in the future.