rust: use thiserror for error types

[wchargin]

Summary:
Our errors now have nice string formatting, easier From conversions,
and std::error::Error implementations.

Test Plan:
Included some tests for the Display implementations. They’re often not
necessary—one benefit of deriving traits is that you can be confident in
the implementation without manually testing it. But sometimes, if the
format string is non-trivial, it can be nice to actually see the full
text written out.

wchargin-branch: rust-use-thiserror

[psybuzz]

Is this comment still relevant for Debug, now that it writes Masked({}) without using the :#010x padding option?

[wchargin]

Yes, I think so. The point of the comment is to explain why we don’t
just write #[derive(Debug)]. This patch changes the implementation,
but not the behavior. If we derived Debug, the output would still be
MaskedCrc(4660) rather than MaskedCrc(0x1234), so the comment is
still as helpful as it was before.

In fact, I think it’s a bit more helpful than before, since before the
:#010x? was at least immediately following in the code. Now, if you
don’t look at the Display implementation, it’d be reasonable to ask,
“this just wraps the Display in the struct name; at that point, why
not just derive Debug?”—which the comment answers.

[wchargin]

I guess the tests answer it, too. :-)

[psybuzz]

I don't really understand why the '_ is needed in fmt::Formatter<'_>.

In Rust docs, some examples don't include it [1], while others do [2]. From what I gather, '_ is declaring a lifetime annotation for the Formatter, but we aren't using '_ anywhere else. Could you help me understand why it is needed?

[1] https://doc.rust-lang.org/std/fmt/struct.Formatter.html#examples
[2] https://doc.rust-lang.org/std/fmt/trait.UpperHex.html#examples

[wchargin]

You’re right—it isn’t needed here. You can read '_ as “infer the
correct lifetime”. The general feature is called lifetime elision, and
the '_ syntax was introduced in rust-lang/rfcs#2115; see conversation
for discussion, or the diff for the final text:

• You can write '_ to explicitly elide a lifetime, and it is deprecated to
  entirely leave off lifetime arguments for non-& types

The reason that it’s here is that I write trait implementations by
typing impl Display for MaskedCrc {} and then asking rust-analyzer to
fill out method stubs for all required methods (<Space>j1 in my
editor), which generates:

impl Display for Foo {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        todo!()
    }
}

Given that it’s optional, I don’t have a strong preference. In the
particular case of fmt::Formatter, I do actually find it helpful to
recall that Formatter is parameterized by a lifetime, since this
affects how you can use it and isn’t entirely obvious. As the RFC says:
“The '_ marker makes clear to the reader that borrowing is
happening, which might not otherwise be clear.”

I’m also weakly inclined to deem it acceptable to write the code that
rust-analyzer generates, so that I don’t have to always correct it. But
if we want to have a policy of eliding lifetimes implicitly rather than
implicitly absent a strong reason, I would be open to considering that.

[psybuzz]

Thanks for the links and explanation!

The RFC sheds a lot of light for me on why they decided it was good to explicitly use the '_ placeholder. I don't yet have an opinion on when we should do it, but Rust reference also says "For lifetimes in paths, using '_ is preferred", so in this case it's seems idiomatic.