Allow #[must_use] on associated types to warn on unused values in generic contexts

[compiler-errors]

This PR enables #[must_use] on associated type definitions in traits. When the unused results visitor encounters a rigid associated type marked with #[must_use] (i.e. some T::Result in generic code), then it will trigger an unused warning.

This is particularly useful when users want to be generic over some Result associated type. In the compiler, our visitors typically return one of (), ControlFlow<T>, or Result<T, E>. In generic code, though, we are dealing with T::Result, which is a footgun since today we can't detect when a value of that rigid associated type goes unused.

trait Foo {
    #[must_use]
    type Result;

    fn process(arg: i32) -> Self::Result;
}

fn foo<T: Foo>() {
    T::process(10);
    //~^ WARNING unused value
}

This does not apply after an associated type can be normalized. For example, this code will not trigger the warning, since <NoOp as Foo>::Result is (), which is not #[must_use]:

trait Foo {
    #[must_use]
    type Result;

    fn process(arg: i32) -> Self::Result;
}

struct NoOp;
impl Foo for NoOp {
    type Result = ();

    fn process(_arg: i32) { println!("did nothing"); }
}

fn foo() {
    <NoOp as Foo>::process(10);
}

---

The commit 5e16f3a demonstrates some places in the compiler that had unhandled results, and so it should make it very clear that this change is useful.

[rustbot]

r? @BoxyUwU

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

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

[compiler-errors]

This needs T-lang sign-off.

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

[traviscross]

Makes sense to me. Thanks @compiler-errors for putting this together and putting it forward.

@rfcbot fcp merge

[rfcbot]

Team member @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.

[traviscross]

@rustbot labels +S-waiting-on-documentation

We do document the #[must_use] attribute in the Reference, so we'll want to update that.

cc @ehuss

[tmandry]

This seems about right, and I'm glad we have evidence for its usefulness. But are there use cases considered besides the one trait? I want to avoid overfitting to that specific use case if there are more.

The behavior strikes me as a little bit subtle. I think it's the best and most practical option, though there is a possibility of a mismatch in expectations, which would be clarified with something like #[must_use(when_unknown)]. We are in a generic context, which poses the question of how to apply the behavior to generic and non-generic contexts. From a user's perspective we could choose to apply the behavior more broadly than this PR does (implementation and design questions notwithstanding).

[compiler-errors]

But are there use cases considered besides the one trait? I want to avoid overfitting to that specific use case if there are more.

bb19361 is perhaps mistitled, because it applies the #[must_use] attribute to 7 different traits in the compiler, not just one. They are all visitor patterns, because that's where we've heavily leaned into generic control flow patterns, but I believe this is useful anywhere a Result associated type has been pulled out of generic code.

I'm not certain how this would be overfitted because there really isn't any other obvious semantics I could give to #[must_use] on an associated type.

We are in a generic context, which poses the question of how to apply the behavior to generic and non-generic contexts.

It doesn't really strike me as necessary to clarify this an additional disambiguator like with_unknown.

In the case that this type can be projected, then if the projected type is not #[must_use] (e.g. the unit type () if I wrote type Result = ()), then I don't really see why I would want it to be warned on, or how I could do that, since the type () has lost its connection to the Result associated type.

[scottmcm]

In the case that this type can be projected, then if the projected type is not #[must_use] (e.g. the unit type () if I wrote type Result = ()), then I don't really see why I would want it to be warned on

I could imagine that you want an Option warned on in these positions, for example. (Unit always being excluded certainly does make sense, though, like it does for #[must_use] on fn IIRC.)

I guess the statement is that this is like #[must_use] on a type, and if you want it on all such types that you'd need to put #[must_use] on the methods of the trait as well?

---

Regardless, this is a lint so can be tuned later, so from the perspective of "is it reasonable to allow the attribute here, even if we tweak the exact behaviour later" I'm happy to
@rfcbot reviewed

[nikomatsakis]

My take: This is the correct semantics and makes sense, but I think some folks will get confused, especially given the way we allow you to apply must_use to methods to "override" warnings. I think if you think it through deeply you kind of arrive at the semantics of this PR as the only logical thing we can do, but it'll cause some confusion.

@rfcbot reviewed

I think in particular the use case of "I want you to treat this as must-use in generic code where you MIGHT have to 'use' it (but in specific contexts, I don't want to worry about it)" will be common.

[rfcbot]

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