[PATCH] D65643: unittests: Add a (centralized) ability to match std::error_code

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 2 04:05:23 PDT 2019


labath added a comment.

In D65643#1611969 <https://reviews.llvm.org/D65643#1611969>, @sammccall wrote:

> I'm lacking a bit of context here.
>  Why do we prefer this to `EXPECT_FALSE(err)` /`EXPECT_EQ(err, std::errc_address_in_use)` with an appropriate value printer for `std::error_code`?
>
> Apart from being a bit more obscure, here the Failed() matcher doesn't provide any overload to specify the code, so this api seems to encourage people not to specify the code even if they know it.


That's a good point. There wasn't a huge deal of thought going into this. I automatically reached for the Succeded/Failed matchers because:
a) I was involved in creating them
b) it seemed nice to have a single entity which can match all kinds of errors

(a) is not much of a reason but I think (b) has some value to it. Should the need arise we can also teach it to match `ErrorOr<T>`, and we can also come up with a way to match error codes or categories similar to how you can now say `Failed<InfoT>()` to match llvm::Errors of specific types.

OTOH, I agree that it would be nice if `std::error_code` came out "right" no matter how it ended up being used in the assertions. Then these matchers would be a purely optional add-on that one can use when it suits him. However, I am not sure if that is actually achievable. If I know my googletest correctly, then both `operator<<` and the `PrintTo` function need to be defined in the same namespace as the object they are printing, which in this case would be `std`...


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65643/new/

https://reviews.llvm.org/D65643





More information about the llvm-commits mailing list