#[expect(unused_must_use)] does not work when applied directly to statement

[AnthonyMikh]

I tried this code:

#[must_use]
pub fn important() -> i32 {
    42
}

pub fn foo() {
    #[expect(unused_must_use)]
    important();
}

I expected #[expect] to catch unused_must_use lint and suppress it, as it should. Instead, compiler issues a diagnostic both about unused #[must_use] value and unfulfilled lint expectation:

warning: unused return value of `important` that must be used
 --> src/lib.rs:8:5
  |
8 |     important();
  |     ^^^^^^^^^^^
  |
  = note: `#[warn(unused_must_use)]` on by default
help: use `let _ = ...` to ignore the resulting value
  |
8 |     let _ = important();
  |     +++++++

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

It does work as expected though if #[expect] is applied to function.

Meta

I used the latest stable (Rust 1.81), but it is reproducible both on latest beta (2024-09-04 c7c49f4) and latest nightly (2024-09-05 9c01301).

@rustbot labels: +F-lint_reasons, +A-diagnostics

[gurry]

@rustbot claim

[gurry]

This issue is not limited to just expect. Others attrs like allow or deny don't work either. For instance, the following variant of the reproducer with expect replaced with allow emits the warning even though it shouldn't:

#[must_use]
pub fn important() -> i32 {
    42
}

pub fn foo() {
    #[allow(unused_must_use)]
    important();
}

Playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=5847d1b5165f0da09b093559c6d20f6e

This broken behaviour goes as far back as at least 2021 (couldn't build and test earlier rustc)

Underlying reason

We emit the lint on the Stmt HIR node corresponding to important();. However, the Expect or Allow levels, which are supposed to override the default Warn level of the lint, are associated with or bound to the Expr node underlying the Stmt as shown in the figure below:

Function body `Block`
|
+--- Semi Stmt corresponding to `important();` <--- This is what we trigger the lint on
          |
          +---- Call Expr corresponding to `important()`   <--- this is what `allow`, `expect` etc. are associated with

The end result is that the lint's Warn level is never knocked down to Expect or Allow since when searching for levels we start from the node on which the lint is triggered and go up the chain of parents. So in this case we look for the level on Stmt and then on the Block and so on. We never look for it on the Expr where the levels actually exist.

Possible fix

As it happens AttributeMap in the lowered HIR does have the expect/allow attrs for both the Stmt node and its child Expr node. However when adding the levels in the LintLevelBuilder we do not add anything for the Stmt node as shown here (note the comment):

rust/compiler/rustc_lint/src/levels.rs

Lines 258 to 263 in 6c65d4f

	fn visit_stmt(&mut self, e: &'tcx hir::Stmt<'tcx>) {
	// We will call `add_id` when we walk
	// the `StmtKind`. The outer statement itself doesn't
	// define the lint levels.
	intravisit::walk_stmt(self, e);
	}

If were to add the level for Stmt as well the issue will go away.

However, if we do that we will still have one problem in the case of expect which is that we will end up with two expectations and the one on the Expr will not be fullfilled and hence we will still see a warning about unfulfilled expectation. To prevent that I think we should take the following approach: when an attr like allow or expect etc. are present for more than one HIR node in the AttributeMap and if one node is the (direct or indirect) parent of the other node, then add the level only for parent node. This will take care of any lints on the child (since we always walk up the HIR hierachy as mentioned earlier) as well as the parent and it will not cause any warnings about missed expectations.

I am new to this area so if someone can review this idea or suggest alternatives that will be great. @cjgillot since you seemed to have worked in this area recently, can you please weigh in?

[cjgillot]

Your investigation is accurate, and the fix is simple: visit the statement node when building lint levels.

Do you mind making such a pr ?

I don't think we need any special adjustment for expect cases, duplicated attributes are already taken into account, but a test won't hurt.

[gurry]

Your investigation is accurate, and the fix is simple: visit the statement node when building lint levels.

Do you mind making such a pr ?

I don't think we need any special adjustment for expect cases, duplicated attributes are already taken into account, but a test won't hurt.

Thanks @cjgillot .

Yes, I'll put together a PR. My only concern is if all we do is add the level at Stmt, although it will work perfectly for allow, deny etc., in the case of expect we will end up with an unfulfilled expectation on the Expr node which will appear to the user as a warning.

But anyway let me have a go and see what happens.

[gurry]

Opened draft PR #130293 that adds the lint on Stmt. It does suffer from the extra warning problem.