It is too easy to lose `#[must_use]` annotations when using impl Trait, leading to footguns

[shepmaster]

If I return a direct type that is must_use, I get a helpful compiler warning:

use std::iter::{self, Empty, Skip};

fn x() -> Skip<Empty<i32>> {
    iter::empty().skip(1)
}

fn main() {
    x();
}

warning: unused `std::iter::Skip` which must be used
 --> src/main.rs:8:5
  |
8 |     x();
  |     ^^^^
  |
  = note: #[warn(unused_must_use)] on by default
  = note: iterator adaptors are lazy and do nothing unless consumed

The equivalent (and I believe preferred) code using impl Trait gives no such useful feedback:

use std::iter;

fn x() -> impl Iterator<Item = i32> {
    iter::empty().skip(1)
}

fn main() {
    x();
}

Amusingly, this isn't a new problem, as returning a boxed trait object also has the same problem. It's arguably worse there because you've performed an allocation for no good reason, but it's also less likely because people are more reluctant to introduce unneeded allocations.

[shepmaster]

Since Rust 1.27 stabilizes #[must_use] (on functions), one possible solution would be a lint that checks the compiler-known type underneath the impl Trait and the function returning it. If the type has the annotation but the function does not, it could suggest adding the attribute to the function.

[joshtriplett]

I think it would make sense for @rust-lang/lang to quickly review and agree on how we should handle this.

A lint seems plausible. Inside the same module, where we know the concrete underlying type, it would definitely make sense to pass through the #[must_use]. I'd love to know if it makes sense to automatically pass through the #[must_use] on the concrete type when used from outside the module, so that this can work automatically without needing a lint and a manual additional #[must_use] on the function.

[eddyb]

Why is the #[must_use] not on, say, Iterator?

[cramertj]

I agree with @eddyb that #[must_use] traits seem like the better solution here than trying to "leak" must-used-ness.

[oli-obk]

In that case, is it fine that returning an impl Debug of real type Result will drop the must-used-ness?

Maybe we should suggest to the author of the function to add the attribute to the function if the real type is #[must_use]

[eddyb]

@oli-obk Right, there's nothing meaningfully tying such a function to Result, if Result is hidden.

[zackmdavis]

Since Rust 1.27 stabilizes #[must_use]

The new thing being stabilized in 1.27 is specifically #[must_use] on functions and methods (RFC 1940). #[must_use] on types, including the iterator-adapter structs like Skip, are much older.

[zackmdavis]

We may need to add an arm for TyAnon in this match?

rust/src/librustc_lint/unused.rs

Lines 61 to 72 in 5205ae8

	let ty_warned = match t.sty {
	ty::TyTuple(ref tys) if tys.is_empty() => return,
	ty::TyNever => return,
	ty::TyAdt(def, _) => {
	if def.variants.is_empty() {
	return;
	} else {
	check_must_use(cx, def.did, s.span, "")
	}
	},
	_ => false,
	};

[joshtriplett]

So, I'd absolutely support adding #[must_use] for traits, which would have the same effect on a function returning -> impl Trait as #[must_use] on a type T would have for a function returning -> T.

However, I also think we ought to handle this case somehow, preferably in a way that doesn't require the user to manually propagate #[must_use] outward on every -> impl Trait function that returns a #[must_use] type.

#[must_use]
struct S { ... }

impl Trait for S { ... }

fn foo() -> impl Trait {
}

[withoutboats]

@joshtriplett I don't think we should. The type, including its must-use'dness, is not a part of the public API in that context. If you want the function return to be treated as must-use, the solution in that case is to tag the function as must-use.

[joshtriplett]

@withoutboats Then in that case, would it make sense to have a lint suggesting propagation of #[must_use] from the type to the function that returns that concrete type?

[withoutboats]

@joshtriplett I don't think so. The point of returning impl Trait is to hide the return type and the facts of its API, the fact that the type is must-use seems no different from other facts about the return type.

[zackmdavis]

As alluded to above, I argue that this is a bug in the must-use lint due to its antedating impl-Trait; I don't see sufficient cause here to complicate the language with another lint or feature. A pull request is forthcoming.