fix: unfulfilled nested dead code lint - #161005

r? @folkertdev

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

r? JonathanBrouwer because lints?

I'm not sure we even want this though, it probably needs T-lang approval, or input at least

I feel a bit iffy about whether the original issue is really an issue, so I'll let the lang team decide.
Explanation for T-lang:

struct Foo {
    #[expect(unused)]
    x: u32,
}

Currently this example produces the following output

warning: struct `Foo` is never constructed
 --> src/main.rs:6:8
  |
6 | struct Foo {
  |        ^^^
  |
  = note: `#[warn(dead_code)]` (part of `#[warn(unused)]`) on by default

warning: this lint expectation is unfulfilled
 --> src/main.rs:7:14
  |
7 |     #[expect(unused)]
  |              ^^^^^^
  |
  = note: `#[warn(unfulfilled_lint_expectations)]` on by default

This is because if Foo is unused, we don't emit a lint that x is unused.
This PR changes that so that this example only produces the first warning.

This is implemented by recursively matching expectations of specifically the dead-code lint on children of dead items. Having such a special case for the expect attribute feels weird, since expect currently works the same way for all lints.

On the other hand, this does unlock the usecase from the original issue where expect can be used to really ensure that something is unused.

@rust-lang/lang @rust-lang/lang-advisors
How do you feel about this?

Some observations: we don't allow this, today:

#![deny(unfulfilled_lint_expectations)]

#[expect(unused)]
struct Root {
    #[expect(unused)] // ERROR: this lint expectation is unfulfilled
    leaf: u8,
}

And we don't allow:

#![deny(unfulfilled_lint_expectations)]

#[expect(dead_code)]
fn root() {
    #[expect(dead_code)] // ERROR: this lint expectation is unfulfilled
    fn leaf() {}
    leaf()
}

But we do allow this:

#![deny(unfulfilled_lint_expectations)]

#[expect(dead_code)]
fn root() { leaf() }

#[expect(dead_code)]
fn leaf() {} // OK.

Unless it already exists somewhere else, I think we'd want to also have/add the test case @JonathanBrouwer mentioned of

struct Foo {
    #[expect(unused)]
    x: u32,
}

to the test suite, where Foo is completely unused to make sure that the warning: struct Foo is never constructed itself does not disappear from the expect(unused) on the field.

We talked about this in the lang call today. On the call, our expectation was that all three of the examples in #161005 (comment) should be accepted (i.e., the expectation fulfilled), in principle. That's what we'd document as the intended behavior in the Reference. That said, we're cognizant of possible performance implications, and we leave to compiler the question of whether or when to implement this intended semantic on performance grounds.

@rfcbot fcp merge lang

@traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot reviewed

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot reviewed

@bors try @rust-timer queue

☀️ Try build successful (CI)
Build commit: 4169f58 (4169f58cb9e8bef1a3fe8b5df2da0227a5253aba)
Base parent: 4fcf397 (4fcf39725a9c99bd495d8c73af83628a256ff9a9)

Finished benchmarking commit (4169f58): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 476.065s -> 473.967s (-0.44%)
Artifact size: 401.18 MiB -> 401.09 MiB (-0.02%)

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.