Add a lint for forgetting futures by tdittr · Pull Request #17054 · rust-lang/rust-clippy

tdittr · 2026-05-22T15:18:45Z

View all comments

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

changelog: Add [forget_future] lint

tdittr · 2026-05-22T15:27:39Z

r? @llogiq

tdittr · 2026-05-22T15:47:18Z

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

samueltardieu · 2026-05-23T09:53:23Z

+                    if let Some(future_trait) = cx.tcx.lang_items().future_trait()
+                        && implements_trait(cx, arg_ty, future_trait, &[])
+                    {
+                        span_lint_and_then(cx, FORGET_FUTURE, expr.span, FORGET_FUTURE_SUMMARY, |diag| {
+                            diag.span_note(arg.span, format!("argument has type `{arg_ty}`"));
+                        });
+                    }


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

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

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

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.

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?

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

Yes, but leaving right after lunch.

tdittr · 2026-08-03T09:50:17Z

I finally found some time again to work on this after RustWeek and addressed the suggestions by @datdenkikniet (thanks those texts were much better than what I came up with).

The Drop glue that is generated still makes this lint complain about more cases than it strictly needs to.

datdenkikniet · 2026-08-03T16:55:57Z

                                 Dropping such a type only extends its contained lifetimes";
 const FORGET_NON_DROP_SUMMARY: &str = "call to `std::mem::forget` with a value that does not implement `Drop`. \
                                   Forgetting such a type is the same as dropping it";
+const FORGET_FUTURE_SUMMARY: &str = "forgetting a Future might cause problems with cancelation";


Not sure what clippy's stance is, but from googling: cancelation is used, but cancellation is much more common. We might want to switch to that instead.

View changes since the review

Co-authored-by: datdenkikniet <jcdra1@gmail.com>

rustbot · 2026-08-21T08:19:22Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

tdittr · 2026-08-21T10:37:38Z

Hey @llogiq I started this back at the All-Hands, would you still be available to review this?

llogiq

This looks good. The only drawback I see is that we might lint the same forget twice.

Can you fix that or should I merge and do a followup PR?

View changes since this review

llogiq · 2026-08-22T11:20:42Z

   = note: argument has type `std::string::String`

-error: aborting due to 4 previous errors
+error: usage of `mem::forget` on type with `Drop` fields


This means that we will lint both forget_future and mem_forget if the latter is active. I think we should not lint mem_forget if we already lint forget_future, as the latter is more specific.

The easiest way to do this is to merge both lint passes and return from check_expr right after linting forget_future and only otherwise lint mem_forget.

Oh I did not think about this. The only problem I see with this might be that if a project already set mem_forget to deny, then using mem::forget() on a future would go from an error to a warning. I guess I could also check if the lint level of mem_forget is higher than that of forget_future and in that case emit both or just mem_forget?

rustbot · 2026-08-22T11:21:49Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

tdittr · 2026-08-24T10:23:22Z

@rustbot ready

rustbot added the needs-fcp PRs that add, remove, or rename lints and need an FCP label May 22, 2026

tdittr force-pushed the mem_forget_impl_future branch from 3c86ae2 to 1a5451f Compare May 22, 2026 15:22

rustbot assigned llogiq May 22, 2026

tdittr force-pushed the mem_forget_impl_future branch 3 times, most recently from d0848b3 to b9bd4f3 Compare May 22, 2026 15:58

tdittr marked this pull request as ready for review May 23, 2026 09:10

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 23, 2026

samueltardieu reviewed May 23, 2026

View reviewed changes

datdenkikniet reviewed May 24, 2026

View reviewed changes

Comment thread clippy_lints/src/drop_forget_ref.rs

Comment thread clippy_lints/src/drop_forget_ref.rs Outdated

Comment thread clippy_lints/src/drop_forget_ref.rs Outdated

This comment has been minimized.

Sign in to view

tdittr force-pushed the mem_forget_impl_future branch from 49380f2 to ab5baf5 Compare August 3, 2026 09:33

This comment has been minimized.

Sign in to view

tdittr force-pushed the mem_forget_impl_future branch from ab5baf5 to 5e2808c Compare August 3, 2026 09:49

tdittr force-pushed the mem_forget_impl_future branch from 5e2808c to aeced90 Compare August 3, 2026 10:04

datdenkikniet reviewed Aug 3, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

tdittr force-pushed the mem_forget_impl_future branch from c7e8e43 to 0802178 Compare August 19, 2026 11:11

This comment has been minimized.

Sign in to view

tdittr added 7 commits August 21, 2026 10:18

Add a lint for forgetting futures

0b6f028

Use clippy_utils instead of manual impls check

eb965e8

Update lint description

2a10b52

Test that clippy::mem_forget still works on futures

0a80a0b

Fix that clippy::mem_forget still works on futures

fa4a298

Rename lint from mem_forget_future to forget_future

5c7657f

Only lint futures that implement drop

c850590

tdittr and others added 2 commits August 21, 2026 10:18

Improved lint description

e0e31c7

Co-authored-by: datdenkikniet <jcdra1@gmail.com>

Fix typo

9ae7fc0

tdittr force-pushed the mem_forget_impl_future branch from 0802178 to 9ae7fc0 Compare August 21, 2026 08:19

llogiq requested changes Aug 22, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Aug 22, 2026

Don't raise redundant mem_forget

dbad485

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Aug 24, 2026

Uh oh!

Uh oh!

Conversation

tdittr commented May 22, 2026 • edited by llogiq Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tdittr commented May 22, 2026

Uh oh!

tdittr commented May 22, 2026

Uh oh!

samueltardieu May 23, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

tdittr commented Aug 3, 2026

Uh oh!

datdenkikniet Aug 3, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Aug 21, 2026

Uh oh!

tdittr commented Aug 21, 2026

Uh oh!

llogiq left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Aug 22, 2026

Uh oh!

tdittr commented Aug 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tdittr commented May 22, 2026 •

edited by llogiq

Loading

samueltardieu May 23, 2026 •

edited by rustbot

Loading

datdenkikniet Aug 3, 2026 •

edited by rustbot

Loading

llogiq left a comment •

edited by rustbot

Loading