Improve dead code analysis for structs and traits defined locally by mu001999 · Pull Request #128637 · rust-lang/rust (original) (raw)

This PR does some refactor and improvement on the dead code analysis, and doesn't lint pub structs.

refactors the two-phase check for impls and impl items of trait
1. checks them all later because we must use the trait/trait item and the adt defined locally firstly
2. makes the logic cleaner and doesn't require special checks about whether it's public or not
mark the adt live if it appears in pattern, like generic argument, it also implies the use of the adt
1. based on 1 and 2, we can detect unused private adts impl Default, without adding special logics for Default
2. so that we can remove rustc_trivial_field_reads on Default, and the logic in should_ignore_item
extends rules to impls for types which refer to adts, like &Foo/[Foo] things
lints unused assoc consts like assoc fns, and unused traits with assoc tys by marking assoc tys live only if the trait is live (is same to Mark assoc tys live only if the corresponding trait is live #126618 reverted in Revert recent changes to dead code analysis #128404)

Fixes #120770
Fixes #126729
Fixes #127911
Fixes #128839

rustbot added PG-exploit-mitigations

Project group: Exploit mitigations

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

Aug 4, 2024

Some changes occurred in tests/ui/sanitizer

cc @rust-lang/project-exploit-mitigations, @rcvalle

This comment has been minimized.

Please separate this into separate commits each implementing an individual tweak to the dead code analysis, with the tests adjusted at each commit so I can see the fallout from each change specifically. It's very difficult to map the changes to the UI tests to each code change without that. --- I want to think very hard about each of these changes to determine if there are any false positives that are caused by each change, and that is harder to do with a single commit.

Also, if you want, please open a separate PR that removes the dead code from the compiler/standard library that is now detected after these changes. That can be landed separately.

This comment has been minimized.

This comment was marked as resolved.

This comment has been minimized.

This comment was marked as resolved.

Could you submit 1317d54 as a separate PR?

I reviewed one of the predecessors to this PR and my experience can be summarized by this comment #122382 (comment)

And I still don't understand what you are trying to do with it in dead.rs, in general.

because neither the code itself nor its explanations were clear enough to understand what's going on, so I'm not really enthusiastic about reviewing it again.

@mu001999
Could you make a PR containing just test additions/modifications and refactorings not containing any functional changes?
After that we can reconsider the rest of the PR.
@rustbot author

rustbot added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

May 22, 2025

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

link to #141407, contains just test additions/modifications and refactorings

tgross35 added a commit to tgross35/rust that referenced this pull request

May 28, 2025

…r, r=petrochenkov

Refactor the two-phase check for impls and impl items

Refactor the two-phase dead code check to make the logic clearer and simpler:

adding assoc fn and impl into unsolved_items directly during the initial construction of the worklist
converge the logic of checking whether assoc fn and impl are used to item_should_be_checked, and the item is considered used only when its corresponding trait and Self adt are used

This PR only refactors as much as possible to avoid affecting the original functions. However, due to the adjustment of the order of checks, the test results are slightly different, but overall, there is no regression problem

Extracted from rust-lang#128637. r? petrochenkov

jhpratt added a commit to jhpratt/rust that referenced this pull request

May 28, 2025

…r, r=petrochenkov

Refactor the two-phase check for impls and impl items

Refactor the two-phase dead code check to make the logic clearer and simpler:

adding assoc fn and impl into unsolved_items directly during the initial construction of the worklist
converge the logic of checking whether assoc fn and impl are used to item_should_be_checked, and the item is considered used only when its corresponding trait and Self adt are used

Extracted from rust-lang#128637. r? petrochenkov

bors added a commit that referenced this pull request

May 28, 2025

Refactor the two-phase check for impls and impl items

Refactor the two-phase dead code check to make the logic clearer and simpler:

adding assoc fn and impl into unsolved_items directly during the initial construction of the worklist
converge the logic of checking whether assoc fn and impl are used to item_should_be_checked, and the item is considered used only when its corresponding trait and Self adt are used

Extracted from #128637. r? petrochenkov

try-job: dist-aarch64-linux

bors added a commit that referenced this pull request

May 28, 2025

Refactor the two-phase check for impls and impl items

Refactor the two-phase dead code check to make the logic clearer and simpler:

adding assoc fn and impl into unsolved_items directly during the initial construction of the worklist
converge the logic of checking whether assoc fn and impl are used to item_should_be_checked, and the item is considered used only when its corresponding trait and Self adt are used

Extracted from #128637. r? petrochenkov

try-job: dist-aarch64-linux

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

May 30, 2025

…r, r=petrochenkov

Refactor the two-phase check for impls and impl items

Refactor the two-phase dead code check to make the logic clearer and simpler:

adding assoc fn and impl into unsolved_items directly during the initial construction of the worklist
converge the logic of checking whether assoc fn and impl are used to item_should_be_checked, and the item is considered used only when its corresponding trait and Self adt are used

Fixes rust-lang#127911 Fixes rust-lang#128839

Extracted from rust-lang#128637. r? petrochenkov

try-job: dist-aarch64-linux

rust-timer added a commit that referenced this pull request

May 30, 2025

Rollup merge of #141407 - mu001999-contrib:dead-code/refactor, r=petrochenkov

Refactor the two-phase check for impls and impl items

Refactor the two-phase dead code check to make the logic clearer and simpler:

adding assoc fn and impl into unsolved_items directly during the initial construction of the worklist
converge the logic of checking whether assoc fn and impl are used to item_should_be_checked, and the item is considered used only when its corresponding trait and Self adt are used

Fixes #127911 Fixes #128839

Extracted from #128637. r? petrochenkov

try-job: dist-aarch64-linux

github-actions bot pushed a commit to rust-lang/miri that referenced this pull request

May 31, 2025

…ochenkov

Refactor the two-phase check for impls and impl items

Refactor the two-phase dead code check to make the logic clearer and simpler:

adding assoc fn and impl into unsolved_items directly during the initial construction of the worklist
converge the logic of checking whether assoc fn and impl are used to item_should_be_checked, and the item is considered used only when its corresponding trait and Self adt are used

Fixes rust-lang/rust#127911 Fixes rust-lang/rust#128839

Extracted from rust-lang/rust#128637. r? petrochenkov

try-job: dist-aarch64-linux

tgross35 added a commit to tgross35/rust that referenced this pull request

Jun 21, 2025

…tern, r=petrochenkov

Marks ADT live if it appears in pattern

Marks ADT live if it appears in pattern, it implies the construction of the ADT.

Then we can detect unused private ADTs impl Default, without special logics for Default and other std traits.
We can also remove rustc_trivial_field_reads on Default, and the logic in should_ignore_item (introduced by rust-lang#126302).

Fixes rust-lang#120770

Extracted from rust-lang#128637. r? @petrochenkov

rust-timer added a commit that referenced this pull request

Jun 21, 2025

Rollup merge of #142485 - mu001999-contrib:dead-code/adt-pattern, r=petrochenkov

Marks ADT live if it appears in pattern

Marks ADT live if it appears in pattern, it implies the construction of the ADT.

Then we can detect unused private ADTs impl Default, without special logics for Default and other std traits.
We can also remove rustc_trivial_field_reads on Default, and the logic in should_ignore_item (introduced by #126302).

Fixes #120770

Extracted from #128637. r? @petrochenkov

tautschnig pushed a commit to model-checking/verify-rust-std that referenced this pull request

Jul 3, 2025

…tern, r=petrochenkov

Marks ADT live if it appears in pattern

Marks ADT live if it appears in pattern, it implies the construction of the ADT.

Then we can detect unused private ADTs impl Default, without special logics for Default and other std traits.
We can also remove rustc_trivial_field_reads on Default, and the logic in should_ignore_item (introduced by rust-lang#126302).

Fixes rust-lang#120770

Extracted from rust-lang#128637. r? @petrochenkov