The following produces no warnings or errors:
#[must_use = "function with no return type"]
fn foo() {}
fn main() {
foo();
}
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)
The fact that unused-must-use doesn't fire on the foo(); 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.)
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.
I concur. This should definitely cause unused_attributes to fire.
Should the lint unused_attributes also 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_use on 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.
@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.
Like @rkruppe said, custom ZSTs can have semantics attached to them so I don't think it's good to warn on #[must_use] for those.
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.
In other words I do not think lints should be designed to maximize their theoretical precision or be worded as generally as possible. They should be helpful and simple. Linting must_use functions that have the canonical "no return value" return type is both.
Yeah I can agree with that. It definitely makes it easier to implement so it keeps it as a good mentoring issue.
@abonander
Like @rkruppe said, custom ZSTs can have semantics attached to them so I don't think it's good to warn on #[must_use] for those.
I was saying the opposite of that; i.e. that when you write #[must_use] on any function then a warning should be emitted irrespective of type, i.e. including #[must_use] fn foo() {...}.
@Centril ah, you mean the unused_must_use warning should be emitted even if the function has no return type. You neglected to mention which warning you were talking about and we've been talking about the unused_attributes warning. That's what I was confused about.
You neglected to mention which warning you were talking about and we've been talking about the
unused_attributeswarning.
Well, they're not unrelated (to put it mildly): you would want unused-attributes to fire on functions with #[must_use] exactly when the #[must_use] attribute doesn't do anything because of the function's return type. As I mentioned upthread, currently, those return types are (), !, and the empty enum:
We could pull this code out into a common function that could also be used in a check_fn method in the UnusedAttributes pass, so that the behavior of #[must_use] and unused-attribute-policing-of-#[must_use] never got out of sync.
@zackmdavis yes but @Centril is talking about the exact opposite behavior, that the unused_must_use lint 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 trigger unused_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 a ty::Ty. :broken_heart: :crying_cat_face:
PR forthcoming anyway.
() would be handled by #[must_use] just like any other type.! and empty enums.(deleted comment that was based on, I believe, a misunderstanding of @varkor's comment above.)
This is my diagnosis of the issue:
(), ! and empty enums 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 (https://github.com/rust-lang/rust/issues/43806).#[must_use] uses the same method to detect unused results, so the same types end up getting special-cased, unintentionally.#[must_use], making this a straightforward bug fix.! and empty enums is incorrect, we should be using something like ty.conservative_is_uninhabited() (https://github.com/rust-lang/rust/pull/54125).)Per @varkor's notes re. bug-fix I've relabeled to T-compiler instead.
I've submitted https://github.com/rust-lang/rust/pull/54920 with what I think is the right fix here.