Hacker News new | ask | show | jobs
by Twey 29 days ago
This would have been flagged by Clippy lints `let_underscore_untyped` or `let_underscore_must_use`, which sadly are not enabled by default.
2 comments

Or just by not writing let _ =
All recurrent people problems are system problems.
As seen by the fact that forcing the programmer to write let _ = to silence the warning did not fix the bug.

You know what might've solved this though? Using threads instead of async

I think that's more of a workaround than a fix. Relying on unstructured concurrency does mean you no longer have to understand the scheduling of your program's fibres (… until you do), but it introduces a bunch of new footguns around things like correct cancellation, error propagation, and predicting resource lifetimes.
Ehh, easy fix

    #[allow(clippy::let_underscore_untyped,clippy::let_underscore_must_use)]
    let _ = self.poll_flush(cx)?;
I said ‘flagged’, not ‘fixed’ :)

You can always write the wrong code if you want it enough. But hopefully a warning would have prompted someone to think harder about this flow.

But "let _ =" is already an explicit suppression of a must-use warning. Where does this arms race of "no, I really know what I am doing, compiler" versus "no, this really looks like a mistake, programmer" end?
That's an excellent question I don't have an answer for in general :)

IMHO the goal is usually for the compiler not to make these decisions but to provide the tools for the APIs people build to make them. That's kind of passing the buck, though.

I guess in this case the core problem is that the API for these I/O calls has no representation in the type system for what's happening to the buffer. Proxying it as ‘the programmer must think about this code path’ is a reasonable best-effort but, evidently, sometimes inadequate.

I do feel like Rust did enough to allow software engineers and their managers to make an explicit choice here.
I think it definitely sits at a reasonable point in the tradeoff space here, but it's not the only reasonable point. And it's the nature of tradeoffs that some cases will slip through the cracks.
And this is why you should warn on `clippy::allow_attributes_without_reason` in your projects.

    #[allow(clippy::allow_attributes_without_reason,clippy::let_underscore_untyped,clippy::let_underscore_must_use)]
    let _ = self.poll_flush(cx)?;
If someone working for me wrote that I would fire them.
You can set the lints to `forbid` instead of `deny`, which means they can't be `allowed` like that.
Yeah, but you must know about them and the possible bug first in order to allow them...
Hence ‘sadly’. IMNSHO both of these (or at least _untyped) should be enabled by default. Untyped `let _` is too big a footgun during refactorings.
At which point you wouldn't have written this bug in the first place; or the warnings would trigger immediately, you'd change _ to an actual variable and then remove the warning pragmas because now you don't assign to _.
`Poll` is marked `#[must_use]` so if you were assigning to something other than `_` you'd get a warning that you're ignoring the `Pending` path. The Clippy lint is only for `_` which Rust considers a use by default.
Not really. If I'm using a linter, I go and configure the strictest possible ruleset, and only disable rules when justified on a need-by-need basis. It's just a matter of discipline.