Add a lint for forgetting futures

[tdittr]

This adds a new lint to check when a Future is passed to mem::forget, thus not running any cancellation logic.

changelog: Add [mem_forget_future] lint

Open question: Should it be called forget_future instead? Similar to forget_non_drop.

[tdittr]

r? @llogiq

[tdittr]

CC: @datdenkikniet we talked about this at the embedded unconf

[samueltardieu]

Aren't you also linting Future types even if they don't implement Drop? What would be the problem there?

View changes since the review

[tdittr]

Oh yes that is a very good point. I'll change that

[tdittr]

Weirdly the future created by an async block or async fn always implements Drop? No matter if it captures anything that implements Drop.

[samueltardieu]

Weirdly the future created by an async block or async fn always implements Drop? No matter if it captures anything that implements Drop.

I'd expect that, at least if any object inside the async block implements Drop and can be alive around any .await. You should try and check if this is easy to determine.

[tdittr]

It looks like the drop glue is always generated, but no explicit Drop impl is around. So for example this will fail to compile:

async fn foo() {
    let x = Box::new(());
    core::future::ready(()).await;
    let _ = x;
}

fn bar() -> impl Future<Output = ()> + Drop {
    foo()
}

Are you by any chance also at RustWeek at the moment?

[samueltardieu]

Are you by any chance also at RustWeek at the moment?

Yes, but leaving right after lunch.

[datdenkikniet]

Does this also catch other non-drop cases (i.e. ManuallyDrop<impl Future>)? If not, we should be more specific and say that "Checks whether std::mem::forget is called on a Future that implements Drop" or something like that..

View changes since the review

[datdenkikniet]

I've double-checked, and the following does indeed not generate the lint (which is fine, but supports my request for making this description more specific):

    #[expect(clippy::forget_non_drop)]
    let thing = std::mem::ManuallyDrop::new(async {});

[datdenkikniet]

It's fine in the sense that we can always add ManuallyDrop<impl Future> detection later, but not half-implying that this lint already does it would be good :D

[datdenkikniet]

I think we can be clearer here. The statement is not technically wrong, but it's arguing from the viewpoint of "we want to drop all Futures", as opposed to "we do not want to forget a Future".

Additionally, Drop doesn't necessarily signal cancellation: if a Future is Poll::Complete, there is likely "no" cancellation necessary.

I propose something like:

"When dropped, Futures may execute important cleanup logic, e.g. to signal that they have been cancelled to an executor/context/hardware/what-have-you if they have not completed. forget-ing a Future prevents this cleanup from occurring".

View changes since the review

[datdenkikniet]

Another "can we be more specific" request: "mem::forget on Future types prevents potential cleanup from being performed" or something like that

View changes since the review