Skip to content

Conversation

@mu001999
Copy link
Contributor

@mu001999 mu001999 commented Nov 7, 2025

This PR lints unused associated types, like associated consts and fns.

Different from assoc consts and fns, associated types correspond to QPath::TypeRelative, and Res::Err will be obtained in HIR. Therefore, it is necessary to traverse rustc_middle::ty::Ty to help propagate liveness to the corresponding associated type.

Fixes #127673
Fixes #110332

@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 Nov 7, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 7, 2025

r? @fee1-dead

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

@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
Copy link
Member

BoxyUwU commented Nov 7, 2025

does this handle assoc types used only from item signatures and not from within bodies?

@mu001999

This comment was marked as outdated.

@rustbot
Copy link
Collaborator

rustbot commented Nov 8, 2025

Some changes occurred in tests/ui/sanitizer

cc @rcvalle

@rustbot rustbot added the PG-exploit-mitigations Project group: Exploit mitigations label Nov 8, 2025
@fmease
Copy link
Member

fmease commented Nov 8, 2025

does this handle assoc types used only from item signatures and not from within bodies?

This also handles assoc types used from within bodies, by checking type of expr,

She meant the exact opposite. Assoc tys only used in non-bodies (aka "item signatures", i.e., places where we don't infer types, i.e., not fn or const bodies, but e.g., inside ADTs, type aliases, impl headers, etc.).

@mu001999
Copy link
Contributor Author

mu001999 commented Nov 8, 2025

@fmease Hi, I add some tests for such cases but I'm not sure they are right or good enough. Could you have a look or give some examples? Thanks!

@fmease
Copy link
Member

fmease commented Nov 8, 2025

Personally speaking, I was thinking of

trait Trait { type Ty; }

impl Trait for () { type Ty = (); }

pub struct Wrap(<() as Trait>::Ty); // <- use of QPath::Resolved `Ty` in a non-body only

impl Wrap {
    pub fn live(self) { _ = self.0; }
}

or

trait Trait { type Ty; }

impl Trait for () { type Ty = (); }

pub struct Wrap(Inner<()>);
struct Inner<T: Trait>(T::Ty); // <- use of QPath::TypeRelative `Ty` in a non-body only

impl Wrap {
    pub fn live(self) { _ = self.0; }
}

I'm sure my examples can be simplified. Also, I haven't really thought much about all of this yet as I'm just drive-by reviewing, so feel free to correct me anywhere :)

@mu001999 mu001999 force-pushed the dead-code/assoc-ty branch 2 times, most recently from 6a4d4a2 to 4f6cdd6 Compare November 8, 2025 00:58
@mu001999
Copy link
Contributor Author

mu001999 commented Nov 8, 2025

@fmease Added and passed.

But current implementation for struct fields is special. Simply speaking, types of fields in used struct will be marked live directly whether the field self is used or not.

The detail is described in https://github.com/rust-lang/rust/pull/148654/files#diff-719b4f36bbbf3b3bf48683429408f5d27e5633f70c1144a93554bd536d5bf655R753-R764

@mu001999
Copy link
Contributor Author

mu001999 commented Nov 8, 2025

does this handle assoc types used only from item signatures and not from within bodies?

Yes, such assoc types are handled by visiting predicates of the source items.

@mu001999
Copy link
Contributor Author

mu001999 commented Nov 9, 2025

@fmease I test and find this also fixes #110332

@fee1-dead
Copy link
Member

I am not confident enough that this code works well enough, I am worried about the false positives. E.g., what about projected types on impls:

pub struct A;

trait B {
    type Assoc;
}

impl B for A {
    type Assoc = A;
}

trait C {}

impl C for <A as B>::Assoc {}

What about types used in free-consts, e.g. const X: <A as B>::Assoc = A;?

This either needs to have solid coverage or land as an allowed-by-default lint for now.

@rustbot
Copy link
Collaborator

rustbot commented Nov 12, 2025

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
Copy link
Contributor Author

@fee1-dead Added test cases for projectd types on impl and free const.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lint unused assoc types although the trait is used dead_code: Inherent associated types are unconditionally considered dead

6 participants