I find myself frequently writing let _: () = blah_blah_blah.await?; and I am wondering if this is a code smell.
So starting out, I write: blah_blah_blah
but this code does nothing, because it returns a Future.
Therefore, we write blah_blah_blah.await
Now, because async can have errors all over the place (trying to send, but can't; trying to receive, but empty), the above returns a Result<T, E> and we want to eject on failure, so now we have: blah_blah_blah.await?
Now, not all async fns actually return something useful. Some funcs, like send, just return (), to indicate the send was successful, so now we do: let _ : () = blah_blah_blah.await?;
The above, when repeated many times, looks like a code smell of some sort.
I actually write a lot of let () = ...; statements (with or w/o .await), and I find it quite nice to let it be known "nothing of value was lost" in that statement. So, on the contrary, as with any type annotation, you're just making your code mode explicit and, imho, readable!
Similarly, and especially in unsafe code, whenever I'm calling a diverging function, I write
if ptr.is_null() {
enum Diverged {}
let _: Diverged = alloc::handle_alloc_error();
}
to make sure the compiler guarantees that function does indeed diverge.
(If Infallible hadn't had that ridiculously ill-fitted name (Result<Infallible, ()> is actually not only fallible, but actually doomed to fail, etc.) and had instead been called Unreachable, then I might have used that one rather than an ad-hoc empty enum. I guess we'll have to wait for never sigil !)
Finally, once we get used to making it explicit the intent of discarding values, I find that writing #![deny(unused_must_use)] becomes quite handy
This may be a flaw in my development process, but I generally have 10-20 warnings flying around due to development churn (comment out this function, and now suddenly there are lots of unused exports, dead code, unused function), to the point I generally only fix them when the code works.
I was scracthing my head since I vaguely remembered there being such a lint. That one is a bit too much, maybe, although for a codebase where I'm alone I might try enabling it
I do hope that one day we'll get a middle-ground lint -- and maybe clippy already has one -- for things like "hey, that method only takes &s to no-interior-mutability things; maybe don't call it if you don't need the result?" without needing to #[must_use]everything.
Using #![deny(lint_name)] or #![forbid(lint_name)] (depending on whether you want to let more fine-grained interior modules undo the deny with an [allow(lint_name)] or [warn(lint_name)]) at the root of the src/lib.rs is the usual way to do that; you can also just do it in a fine-grained / per-module or function basis with #[deny(...)].
error: unused implementer of `Future` that must be used
--> src/main.rs:7:5
|
7 | async_fn(); // Error
| ^^^^^^^^^^^
|
note: the lint level is defined here
--> src/main.rs:1:9
|
1 | #![deny(unused_must_use)]
| ^^^^^^^^^^^^^^^
= note: futures do nothing unless you `.await` or poll them
error: unused `Result` that must be used
--> src/main.rs:8:5
|
8 | async_fn().await; // Error
| ^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
.ok() turns a Result into an Option. It's useful, but it's not the behavior the OP is talking about, which is bubbling the error case with ?, and ensuring the success type is unimportant (())
fn emit_smth() -> Result<(), …> { … }
fn log_stuff() -> (/* must be infallible */) {
// This triggers the `unused_must_use` lint:
emit_smth();
// To silence it, rather than doing:
let _ = emit_smth();
// parasyte was mentioning that they preferred doing:
emit_smth().ok(); // <- Does not trigger `unused_must_use`!
}
I personally like the approach, but I'm not super convinced by .ok() being the right method for this (although if enough people do it, it would become, by sheer usage, idiomatic / meaningful; in the same way as doing let _ currently is). I'd love to see a .ignore_err() method being used instead:
