Skip to content
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

#[must_use] on traits should apply to all types that implement the trait, not just for impl Trait placeholders #102238

Open
KarelPeeters opened this issue Sep 24, 2022 · 3 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@KarelPeeters
Copy link

KarelPeeters commented Sep 24, 2022

General issue

Currently, #[must_use] on a trait Foo does not extend to types that implement Foo, it only works on impl Foo return types, and Box<dyn Foo> as a special case:

#[must_use]
trait Foo {}

struct Bar;
impl Foo for Bar {}

fn build_foo() -> impl Foo { Bar }

fn build_box() -> Box<dyn Foo> { Box::new(Bar) }

fn build_bar() -> Bar { Bar }

fn main() {
    build_foo(); // warning: unused implementer of `Foo` that must be used
    build_box(); // warning: unused boxed `Foo` trait object that must be used
    build_bar(); // no warning!
}

(playground)

This does not make much sense, presumably if a trait must always be used all types that implement that trait must also be used.

Application to Future

I came across this when dealing with futures. The Future trait has a #[must_use] annotation, with the useful reminder to .await or poll them:

#[doc(notable_trait)]
#[must_use = "futures do nothing unless you `.await` or poll them"]
#[stable(feature = "futures_api", since = "1.36.0")]
#[lang = "future_trait"]
#[rustc_on_unimplemented(
label = "`{Self}` is not a future",
message = "`{Self}` is not a future",
note = "{Self} must be a future or must implement `IntoFuture` to be awaited"
)]
pub trait Future {

I was specifically using specifically BoxFuture, a type alias for Pin<Box<dyn Future>>. This type implements Future, but there is no warning message when you forget to use it. This was also the original justification in #67387, where a very different solution is proposed (extending must_use to Pin and other wrapper types).

Another effect of this missing warning is that any struct that implements Future has to repeat the same must_use annotation. Some examples:

If must_use on traits infected any type that implemented it, these duplicate annotations could be removed and there would be no risk of forgetting them for any future types.

@KarelPeeters KarelPeeters added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 24, 2022
@inquisitivecrystal inquisitivecrystal added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 25, 2022
@Rua
Copy link
Contributor

Rua commented Sep 26, 2022

Related: #102287.

@Noratrieb Noratrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
@kayabaNerve
Copy link

This is a bit worse than as initially described. Not only does must_use on trait not apply to generic instances, it erases the must_use which would be applied if not generic. While that makes sense (as otherwise you could instance a generic function with two different types, one must_use, one not, to distinct results), it means there's no workaround for this.

I unfortunately hit this and lost days of debugging over it (as I forgot to commit a rather critical DB transaction).

@traviscross
Copy link
Contributor

Specifically, what @kayabaNerve is referring to is this:

#[must_use]
trait Tr {}

#[must_use]
struct S;
impl Tr for S {}

fn id<T: Tr>(x: T) -> T { x }

fn foo<T: Tr>(x: T) {
    id(x); //~ No warning
}

fn main() {
    id(S);
    //~^ WARN unused `S` that must be used
    foo(S);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants