Lint unused pub items in binary crates

[mu001999]

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 unused pub items for binary crates.

[rustbot]

r? @jdonszelmann

rustbot has assigned @jdonszelmann.
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

[Noratrieb]

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.

[mu001999]

Maybe this could be a separate lint, I haven't thought too clearly yet

[jdonszelmann]

Yea I do agree with nora here, maybe open a (specific) zulip thread to make a proper plan for this?

[jdonszelmann]

I guess I'd prefer this as a lint (attribute at the crate root) myself

[jdonszelmann]

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[Kobzol]

I guess that the use-case that I'm envisioning is to have an opt-in way to treat pub stuff as pub(crate) in binary crates. If it was an allow-by-default lint, which could be manually denied, that would serve that use-case, I think.

[GrantGryczan]

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 pub.

The reason this is useful as a default is because it doesn't semantically make sense to distinguish between pub and pub(crate) in binary crates. This creates a footgun because, for example, pub items can't receive dead_code warnings. If it's just opt-in, then that doesn't remove the footgun for anyone who hasn't opted in, and I don't see a benefit to pub and pub(crate) being distinct in binary crates.

The only reason I can see that someone might not want to opt into this is that one might use pub to prevent dead_code warnings as opposed to pub(crate). But I think this intent might be better communicated using #[expect(dead_code)] rather than the implicit behavior of pub. Dead code detection is useful as a default. (The only downside to that is that you then have to remove the #[expect(dead_code)] if you later use it, but I'll leave it to others to decide if that coupling is an important consideration.)

[mu001999]

New lint unused_pub_items_in_binary (crate-level, default allow for now) is added and used to lint such items, instead of the unstable flag.

The main implementation and change in this PR is to try to split the dead-code analysis into two-phase analysis:

1. In the first phase, the worklist seeds won't contain items that are reachable or allowed by #[allow(dead_code)]. After the first phase, the snapshot of the result will be kept and used for the new unused_pub_items_in_binary lint.
2. Before the second phase, the worklist will be extended with these items. And finally, we could get the original analysis result for dead_code lint.

This two-phase process is to keep the behavior of the two lints independent of each other.

[jdonszelmann]

@rustbot author

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

[rustbot]

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.

[mu001999]

@rustbot review