#[must_use] is permitted on functions without return type

[abonander]

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)

[zackmdavis]

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.)

#48486 is a similar issue with trait implementations.

[zackmdavis]

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.

[Havvy]

I concur. This should definitely cause unused_attributes to fire.

[Centril]

Should the lint unused_attributes also fire for an arbitrary unit type in that case?
There should be some consistency in the behavior.

[hanna-kruppe]

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.

[Centril]

@rkruppe I mean any terminal object in the category of Rust types, so struct Foo;, ((), ()), enum Foo { Bar }, and so on.

[abonander]

I would say an explicit () and any arity of tuple that's all ().

[Centril]

@abonander including recursively? e.g ((), ((), ()))

[abonander]

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.

[Centril]

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.

[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.

[hanna-kruppe]

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.

[hanna-kruppe]

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.

[abonander]

Yeah I can agree with that. It definitely makes it easier to implement so it keeps it as a good mentoring issue.

[Centril]

@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() {...}.

[abonander]

@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.

[zackmdavis]

You neglected to mention which warning you were talking about and we've been talking about the unused_attributes warning.

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:

rust/src/librustc_lint/unused.rs

Lines 63 to 68 in b8bea5a

	ty::Tuple(ref tys) if tys.is_empty() => return,
	ty::Never => return,
	ty::Adt(def, _) => {
	if def.variants.is_empty() {
	return;
	}

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.

[abonander]

@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.

[Centril]

@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.

[zackmdavis]

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. 💔 😿

PR forthcoming anyway.

[varkor]

• 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.

[pnkfelix]

(deleted comment that was based on, I believe, a misunderstanding of @varkor's comment above.)

[varkor]

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 (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 empty enums is incorrect, we should be using something like ty.conservative_is_uninhabited() (Less conservative uninhabitedness check #54125).)

[Centril]

Per @varkor's notes re. bug-fix I've relabeled to T-compiler instead.

[varkor]

I've submitted #54920 with what I think is the right fix here.