Comment by logicchains

12 hours ago

[flagged]

    let _ = …?

This is the Rust idiom for “I am intentionally ignoring this return value”. The linter would have caught

    self.poll_read()?;

and in fact one of the options the linter itself suggests in this case is exactly this “let underscore equals” idiom. (Arguably, this code exists because of the linter, not due to its absence!)

In any case, the return value is being “handled” - the question mark examines the result and breaks the loop if the result is not `Ok(…)`, ie if the call is not successful.

Intentionally ignoring the successful return value isn’t necessarily terrible, either - you could be calling the function for its side effect, and you don’t care what the specific result of that effect is, just as long as there is some effect. E.g. maybe you have a state machine, and this is the code that repeatedly drives it.

(Not coincidentally, polling is what you do to Futures, and Futures are state machines that you need to repeatedly drive…)

In conclusion, I do not think this is prima facie terrible code, nor is it an obvious bug. Async rust is subtle and complicated, and not always fully understood by those who nevertheless have to use it.

  • >This is the Rust idiom for “I am intentionally ignoring this return value”.

    That doesn't make the code any less awful, it just makes idiomatic Rust sound awful. Discarding a return value without even a comment to explain why shouldn't be allowed in any critical project, and the linter should be perfectly capable of ensuring that a comment accompanies the discard and complaining loudly when it doesn't.

    • This is missing that it’s a human issue though. If someone is determined to discard an error and not do anything about it, they’ll just put in a dummy comment to appease the linter any way.

      Force people to handle errors and you end up with the exception fiasco in eg Java where everything ends up being a runtime exception to avoid it

It is an explicit way to discard return values; `self.poll_read(cx)?` etc. alone would warn. Or in this case, `Poll<Result<(), Error>>` is unwrapped once and `Result<(), Error>` is being discarded. The decision to discard `Result<(), Error>` should have been intentional, albeit turned out to be not always the case.

  • If they're not going to handle the return values, they should change the function signature to reflect this aspirational contract, that that function "never fails".

    I see in the article they did change the poll_flush to run just-in-time at poll_shutdown. So they definitely can make a "best effort" poll_flush version that just does not return any errors for use in that loop.

    But all in all? Amateur hour.

    • You're missing how rust works. The function is explicitly allowed to fail, which is why it returns a Result<(), Error>. They're using the function calls within for their side effects. The ? at the end of each line signals that the function will short-circuit return with an error if the function call fails, and only if it is successful it returns the actual value: they just don't care about this value, hence the let _ =. Basically, they are doing the equivalent of:

        let _, err = function_call();
        if err {
          return err
        }
        ...

      2 replies →

Assigning to _ in Rust specifically means that you intentionally want to discard the value, and the clippy linter and the Rust compiler both know that.