-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Detect pub structs never constructed and unused associated constants #125572
Conversation
r? @wesleywiser rustbot has assigned @wesleywiser. Use |
r? @pnkfelix cc @shepmaster |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
2fc16d5
to
1390585
Compare
37bc658
to
cc7aa4d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some nits
☔ The latest upstream changes (presumably #125759) made this pull request unmergeable. Please resolve the merge conflicts. |
I found the state of this analysis interesting, especially the fact that it flags the struct as dead even when there's a This makes sense: the presence of clone does not provide evidence that the struct is live, because you need an instance of the struct in order to be able to enter the clone code in the first place, in order to call the But it led me to wonder: How far does this reasoning go? I.e., does it also filter out other variations of the same inductive reasoning, such as a variant trait that takes So I built a local copy, and soon determined: No, the reasoning does not go that far. That is, either of the following trait impls will suffice to treat a struct constructor as "live code": pub trait U { fn f(_: Self) -> Self; }
impl U for S { fn f(x: Self) -> Self { S(10) } }
pub trait W { fn f(_: &Self) -> Self; }
impl W for S { fn f(x: &Self) -> Self { S(10) } } Note: This is fine. I'd rather we start with a lint that doesn't attempt to catch such cases, than risk shipping an over-aggressive lint that ends up also accidentally treating things like |
These structs are never constructed, and the latest nightly no longer considers a trait impl as enough to keep the struct "live". rust-lang/rust#125572
This should be because struct Publicness {
ty_is_public: bool,
ty_and_all_fields_are_public: Option<bool>,
} Obtain whether each field of the structure is pub only when necessary. |
I think there is another source of extra time like #121752 (comment), more items in the worklist means that these items may be checked repeatedly. |
…hanges, r=pnkfelix Revert recent changes to dead code analysis This is a revert to recent changes to dead code analysis, namely: * efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov * a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix * 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix * 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix * 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov * 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc `@NobodyXu` who added these in #rust-lang#127153. Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs? I apologize for the size of the PR and the churn that it has on the codebase (and for reverting `@mu001999's` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way. cc `@mu001999` r? `@pnkfelix` Fixes rust-lang#128272 Fixes rust-lang#126169
…hanges, r=pnkfelix Revert recent changes to dead code analysis This is a revert to recent changes to dead code analysis, namely: * efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov * a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix * 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix * 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix * 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov * 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc `@NobodyXu` who added these in #rust-lang#127153. Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs? I apologize for the size of the PR and the churn that it has on the codebase (and for reverting `@mu001999's` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way. cc `@mu001999` r? `@pnkfelix` Fixes rust-lang#128272 Fixes rust-lang#126169
…hanges, r=pnkfelix Revert recent changes to dead code analysis This is a revert to recent changes to dead code analysis, namely: * efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov * a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix * 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix * 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix * 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov * 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc `@NobodyXu` who added these in #rust-lang#127153. Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs? I apologize for the size of the PR and the churn that it has on the codebase (and for reverting `@mu001999's` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way. cc `@mu001999` r? `@pnkfelix` Fixes rust-lang#128272 Fixes rust-lang#126169
…hanges, r=pnkfelix Revert recent changes to dead code analysis This is a revert to recent changes to dead code analysis, namely: * efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov * a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix * 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix * 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix * 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov * 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc ``@NobodyXu`` who added these in #rust-lang#127153. Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs? I apologize for the size of the PR and the churn that it has on the codebase (and for reverting ``@mu001999's`` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way. cc ``@mu001999`` r? ``@pnkfelix`` Fixes rust-lang#128272 Fixes rust-lang#126169
…hanges, r=pnkfelix Revert recent changes to dead code analysis This is a revert to recent changes to dead code analysis, namely: * efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov * a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix * 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix * 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix * 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov * 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc ```@NobodyXu``` who added these in #rust-lang#127153. Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs? I apologize for the size of the PR and the churn that it has on the codebase (and for reverting ```@mu001999's``` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way. cc ```@mu001999``` r? ```@pnkfelix``` Fixes rust-lang#128272 Fixes rust-lang#126169
…hanges, r=pnkfelix Revert recent changes to dead code analysis This is a revert to recent changes to dead code analysis, namely: * efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov * a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix * 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix * 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix * 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov * 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc ````@NobodyXu```` who added these in #rust-lang#127153. Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs? I apologize for the size of the PR and the churn that it has on the codebase (and for reverting ````@mu001999's```` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way. cc ````@mu001999```` r? ````@pnkfelix```` Fixes rust-lang#128272 Fixes rust-lang#126169
…nges, r=pnkfelix Revert recent changes to dead code analysis This is a revert to recent changes to dead code analysis, namely: * efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov * a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix * 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix * 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix * 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov * 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc `@NobodyXu` who added these in #rust-lang#127153. Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs? I apologize for the size of the PR and the churn that it has on the codebase (and for reverting `@mu001999's` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way. cc `@mu001999` r? `@pnkfelix` Fixes rust-lang#128272 Fixes rust-lang#126169
…nges-beta, r=cuviper Revert recent changes to dead code analysis (beta flavor) This PR cherry-picks the subset of rust-lang#128404 which actually applies to beta, namely: * 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix * 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix * 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov * 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix And then re-blesses tests. I opted to do a manual backport of that PR since preparing the revert correctly was quite a hassle. r? `@cuviper` (or anyone on release)
Lints never constructed public structs.
If we don't provide public methods to construct public structs with private fields, and don't construct them in the local crate. They would be never constructed. So that we can detect such public structs.
Update:
Also lints unused associated constants in traits.