Skip to content

Extend dead code analysis of impls and impl items to non-ADTs #142968

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mu001999
Copy link
Contributor

Then we can lint more unused structs and traits like the following:

struct Foo; //~ ERROR struct `Foo` is never constructed

trait Trait { //~ ERROR trait `Trait` is never used
    fn foo(&self) {}
}

impl Trait for &Foo {}

Extracted from #128637.
r? @petrochenkov

@rustbot rustbot added 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. labels Jun 24, 2025
|| assoc_item.trait_item_def_id.is_none()
}
_ => true,
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this change related to supporting non-ADTs?
Isn't this reverting a part of changes from #141407?
The logic is again spread randomly between the various analysis stages, what is the principle behind all this, what is filtered away or analyzed at what stage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this and edited the reachable analysis in the new change

Copy link
Contributor Author

@mu001999 mu001999 Jun 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here is removing impl ReachableTrait for ReachableTy<PrivateTy> from the seed worklist. Because the reachable analysis considers impl ReachableTrait for ReachableTy<UnreachableTy> reachable due to the trait and the "shallow" type are both reachable, but this will also mark impl ReachableTrait for ReachableTy<PrivateTy> and impl ReachableTrait for [PrivateTy]reachable. The latter is not expected.

Copy link
Contributor

@petrochenkov petrochenkov Jun 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

impl ReachableTrait for ReachableTy<PrivateTy> and impl ReachableTrait for [PrivateTy] may indeed be reachable due to type inference, even it's not intuitive.
(But they will produce an error on use.)

I need to check though, whether we mark private items as reachable in other, non-impl cases.
If we do, then private impls can probably be treated as non-reachable as well.

This is a change that may have language implications.

@petrochenkov petrochenkov 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 Jun 24, 2025
@mu001999
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 26, 2025
@petrochenkov
Copy link
Contributor

The cases reported by dead code lint in this PR are most similar to this case.

struct Priv;

pub fn leak_priv() -> Priv { Priv }

leak_priv is considered reachable here despite the fact that it's unusable from other crates due to Priv in its primary interface.
However, potentially we can report it as dead code if it's not used otherwise.

On the other hand marking it reachable is exactly what allows the private_interfaces lint to work correctly.

warning: type `Priv` is more private than the item `leak_priv`

So what we really need is two different "effective visibility" tables, one with the rules making leak_priv reachable and another making it non-reachable.
These tables would then both be used in various places depending on context.

@petrochenkov
Copy link
Contributor

(I'll think about this more tomorrow.)

@mu001999
Copy link
Contributor Author

mu001999 commented Jun 27, 2025

I'm thinking about whether it's correct to extend to non-ADT.

Considering the following:

#![deny(dead_code)]

struct T;

fn foo(_: &[T]) {}

struct U;

pub trait Bar {
    fn bar(&self);
}

impl Bar for [U] {
    fn bar(&self) {}
}

fn main() {
    foo(&[]);
    [].bar();
}

This PR will mark U as never constructed, different to T. Although U has not been constructed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants