Rust: Async execution can get lost during refactoring

[ctiller]

I was recently refactoring some code that originally looked like this (minimized):

fn foo() {}
async fn bar() {
  Some(foo());
}

bar() was far away from the definition of foo().
I had need to refactor the code so that foo was also async, and so now I had:

async fn foo() {}
async fn bar() {
  Some(foo());
}

This gives no compiler diagnostic and instead the body of foo is not executed.

[jonas-schievink]

Why do you create an option and do nothing with it?

[ctiller]

did I minimize this example too far?... actual code was closer to:

some_option.map(|value| foo(value))

[tmandry]

The issue is that Option<T> is not #[must_use] even when T is. ~We can't change this backwards compatibly, but perhaps~ adding a warning is possible.

[jonas-schievink]

Also see https://github.com/rust-lang/rust/issues/67387

(#[must_use] is just a warning, so it can be added without breaking code)

[nikomatsakis]

Nominating for @rust-lang/lang consideration. This is another case where extending #[must_use] makes sense, I think -- in particular, an Option<impl Future> should warn when it is unused, just as a impl Future does.

I was pondering why it makes sense, and I think the reason is pretty clear -- an Option contains its T in a public field and doesn't in any sense try to "abstract over it" or "encapsulate it" (which were some of the reasons that we were reluctant to extend #[must_use] uniformly across all types).

[tmandry]

From wg-async-foundations triage: marking On Deck, because forgetting to await is a fairly common thing, and part of the learnability story for async/await is making sure you don't do that.

[joshtriplett]

How feasible would it be to detect taking a #[must_use] value and putting it in a value that subsequently never gets touched? That might give far less false positives than trying to make Option<impl Future> a #[must_use] type.

[ecstatic-morse]

What about making the enum variant constructors themselves #[must_use]? In this case that would be Option::Some (the function that returns an Option, not the variant itself).

[nikomatsakis]

Marking the variant constructor must_use seems like it would have too much fallout, since it would mean that code like Ok(22); would warn as well, wouldn't it?

Also, I think we might well want to warn on code like the following:

fn foo() -> Option<impl Future> { ... }

fn bar() {
   foo();
}

I'm not quite I understand what @joshtriplett is proposing. It sounds like saying "if you construct a enum or struct variant that contains a #[must_use] value, and that struct is unused". I'm reluctant to do that, because it implies that (a) it would apply to all enum/struct types, or at least "most", and we've historically felt that is too broad (which I still think is true), and also (b) it doesn't address cases like foo above, although I think those cases are a bit more borderline than the original example.

I'd like to ask: what is the resistance to making Option<T> be "must-use" if T is must-use? I guess it would be worth doing a bit of a crater run to see the effects before making a final decision, since it may be that cases like None; raise a bunch of false positives, but in general it seems ok to extend "must-use" to wrapper types like Option that don't "encapsulate" their contents.