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

Sam McCall via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 9 14:45:49 PDT 2019


sammccall added a comment.

In D65643#1616608 <https://reviews.llvm.org/D65643#1616608>, @labath wrote:

> @sammccall, what do you think of the `operator<<(raw_ostream&, std::error_code)` idea? Is that the direction I should go in? If so, where would the implementation of that function live? (next to `raw_ostream`, somewhere under `llvm/Testing`, or under `gtest/internal/custom`)


Sorry about taking a while to get back to this.
So I think we should have `operator<<` no matter what: even if we go with `ASSERT_THAT(x, Succeeded())` being able to print `x` still improves the output.
And it'll make `std::error_code` work with `llvm::errs() <<` and `formatv` and so on.
I think it's fine to put it in `raw_ostream.h` along with the others - this isn't test-specific. We might need an overload for `std::errc` too?

Generally, I'd rather have the "plain" styles of assertions if the readability + errors are as good, since there's less to learn.
We still have a lot of `ASSERT_TRUE(X == Y)` in llvm! I think it's important this stuff work as well as possible without special knowledge.
`Failed()` and `Succeeded()` seem mostly useful when wrapping values (ErrorOr --> `Failed(some_error_matcher)` or `Succeeded(some_value_matcher)`) or to deal with the terror that is `llvm::Error` move-semantics. But I don't feel strongly here, if you'd like to be able to use Failed() with plain std::error_code, that seems fine in Testing/Error.h, though I probably won't use it myself.


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