discussion/tracking issue for `#[must_use]` functions in the standard library

[zackmdavis]

RFC 1940 authorized use of the #[must_use] attribute on functions (with the result that invocations of the function behave as if the return type was #[must_use].

This has been implemented for a while under a fn_must_use feature gate (tracking issue), and PR #48925 proposes stabilization.

#[must_use] has been added to the comparison methods in the standard library (.eq, .lt, &c.), but we never got a consensus as to what other standard library methods, if any, should get it. In principle, any function or with no side effects could be must-use (e.g., .len() on collections), but adding dozens or hundreds of annotations to the standard library feels kind of dramatic: perhaps must-use should instead be reserved for functions with "unusually result-like" semantics—after all, users who want to be really sure they're not uselessly throwing away a return value can always opt in to the (allow-by-default) unused-results lint.

If we wanted to be very conservative, we could refuse to stabilize until we've made a decision on this: if we were to stabilize first, any new #[must_use] annotations in the standard library would be insta-stable. But, maybe this isn't a big deal (cargo passes cap-lints allow to dependencies, so tinkering with lints shouldn't break people's builds).

[scottmcm]

I'm not sure "result-like" is broad enough. Maybe something like "things that may have side effects, but which ought never be used for their side effects, especially if often expensive". As a canonical example of such a thing, Iterator::collect. Perhaps more controversially, Clone::clone.

I definitely don't think this needs to block stabilization, because any deny(warnings) impact will be the same whether with stabilization or after it.

[scottmcm]

As of #49533, this is now on Iterator::collect, Clone::clone, and ToOwned::to_owned.

[jonhoo]

I think the non-assigning arithmetic functions are good candidates for this (see #50124).

[shepmaster]

Another metric could be: "things that (are highly likely to) allocate".

[shepmaster]

There's a concrete question around String::from_raw_parts, if people are interested in chiming in. My position is that such a function is mostly used for the purpose of converting back from FFI to be dropped, and shouldn't use the attribute.

[scottmcm]

A thought: I agree we don't want to clutter code with #[must_use] everywhere. But would we be open to the warnings if they came free? Is there some cheap conservative heuristic that would give a bunch of the warnings without the annotation noise? Perhaps "any &self method on a : Freeze type"?

[shepmaster]

Perhaps "any &self method

I was starting to think down these lines:

Any function which:

• returns non-() (and maybe some others, like !?)
• has no &mut arguments of any kind (including and beyond self)

And then we opt-out of things with internal mutability?

We may still end up with a lot of attributes if we follow @Manishearth's suggestion to have explanatory text on every function. (Not sure if I agree with that or not).

[est31]

In principle, any function or with no side effects could be must-use (e.g., .len() on collections), but adding dozens or hundreds of annotations to the standard library feels kind of dramatic

The Rust language already has a concept of side-effect freedom which is const fn. So what about adding a warn-by-default lint to the compiler that complains about any unused result of a const fn invocation? This would remove the need to add #[must_use] everywhere, and wouldn't water it down.

MUST is a very strong word. When I see #[must_use], I think that if the result isn't used, this is most likely a dangerous bug. Using the attribute to lint for dead code seems wrong to me because it would water down the strong meaning of MUST. Of course, dead code might hide a bug as well so this distinction is a bit muddy.

[jonhoo]

Hmm, I'm not sure that warning on unused const fn results is sufficient. I could be mistaken, but isn't it currently impossible to write a const fn that, say, does allocation (e.g., one that returns HashMap::new())?

[sfackler]

HashMap::new doesn't allocate.