Repository navigation
#[must_use] is permitted on functions without return type #54828
Description
Activity
The fact that
unused-must-usedoesn't fire on thefoo();is a consequence of the lint pass returning early for()and!.I agree that it's useless to put
#[must_use]on a function that returns(), but I don't think anyone thought ahead and decided it should (or shouldn't) be an error. (I wouldn't want to write a dedicated lint for such an edge-case, and I think these days we tend to frown on non-lint warnings on account of their unsilenceability.)#48486 is a similar issue with trait implementations.
I wouldn't want to write a dedicated lint for such an edge-case
... well, come to think of it, I see a good case that this should trigger
unused_attributes, as @petrochenkov recently argued with respect to empty#[derive()]s.Reacted by Michael LamparskiI concur. This should definitely cause
unused_attributesto fire.Should the lint
unused_attributesalso fire for an arbitrary unit type in that case?
There should be some consistency in the behavior.By "unit type" do you mean
struct Foo;? I'd say no,must_useon user-defined types is always potentially sensible, e.g. ZSTs can be used as tokens for something that the caller should not usually forget to do.Reacted by Austin Bonander@rkruppe I mean any terminal object in the category of Rust types, so
struct Foo;,((), ()),enum Foo { Bar }, and so on.I would say an explicit
()and any arity of tuple that's all().@abonander including recursively? e.g
((), ((), ()))Probably, because those types typically don't have any meaning or semantics that can be added to them by the user. I guess it's possible with extension traits but that's going to be a very weird corner case.
I'm down with that. It seems to me not too arbitrary a rule.
On the other hand, if we could get
#[must_use]on all types to emit a warning that could work as well.I was skeptical about linting even () but forgetting to fill in the return type is at least a plausible scenario where such a lint could help. Going any further does not seem likely to help anyone in any scenario. Types like ((), ()) are rarely even constructed much less written in function signatures.
5 remaining items
@zackmdavis yes but @Centril is talking about the exact opposite behavior, that the
unused_must_uselint should trigger regardless of the function's return type.@abonander except for
!and the empty enum.As @zackmdavis I'm suggesting that if the
#[must_use]annotation has no effect, then a warning should be emitted. But a way to achieve that is for#[must_use] fn foo() {}have an effect. The other way is for#[must_use] fn foo() {}to triggerunused_attributes.We could pull this code out into a common function [...] so that the behavior of #[must_use] and unused-attribute-policing-of-#[must_use] never got out of sync.
Unfortunately, this turns out to not be easy because the function signature gives us
hir::Ty, whereas the UnusedResults pass is looking for aty::Ty. 💔 😿PR forthcoming anyway.
- added a commit that references this issue
on Oct 7, 2018 - I'd rather functions returning
()would be handled by#[must_use]just like any other type. - Uninhabited types are irrelevant to the warning, but the current check is wrong regardless and should apply to any uninhabited types: not just
!and empty enums.
Reacted by Mazdak Farrokhzad- I'd rather functions returning
This is my diagnosis of the issue:
(),!and emptyenums are special cased here so that the#![deny(unused_results)]lint wouldn't fire in cases that don't really make sense for it to fire (Unused results lint fails on trivial program #43806).#[must_use]uses the same method to detect unused results, so the same types end up getting special-cased, unintentionally.- Therefore, we should just remove the special casing for these types for
#[must_use], making this a straightforward bug fix. - (Also, special casing
!and emptyenums is incorrect, we should be using something likety.conservative_is_uninhabited()(Less conservative uninhabitedness check #54125).)
Reacted by Zack M. Davis
The following produces no warnings or errors:
Is this intended? Shouldn't it produce a lint warning on the attribute? This is a good mentoring issue, I can bang out instructions once I get an answer (fortunately this doesn't seem to involve hygiene this time so hopefully I should get it right... @petrochenkov)