[PATCH] D50807: [Error] Add FileError and DebugErrorInfo helpers

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 17 15:02:51 PDT 2018


zturner added a comment.

Is it possible for



================
Comment at: include/llvm/Support/Error.h:1217-1218
+///
+template <typename ThisErrT, typename EC, typename ECat>
+class DebugErrorInfo : public ErrorInfo<ThisErrT> {
+public:
----------------
I wonder if this should have a more general name.  There's nothing really specific to Debug Info in this implementation, it's basically just a wrapper around a `std::error_code` and `std::string`


================
Comment at: include/llvm/Support/Error.h:1248
+  std::string ErrMsg;
+  EC Code;
+};
----------------
I wonder if we can get away with removing the `EC` template argument.  A `std::error_condition` is essentially a wrapper around an error code and an error category, so ultimately when we call back into the category to format we're losing the info about the error code type anyway.  It doesn't matter what the enum type is, the only thing that matters is that we know which class can format it.

So I think we can just make this an `int`.

Furthermore, I wonder if we need the `ManagedStatic` at all.  All of these categories are cheap to construct, and I think they are all default constructible anyway, so perhaps our `convertToErrorCode()` function could just look like:

```
return std::error_code(Code, ECat{});
```

In all existing use cases, we never pass the same category with a different error code type, so we already havea  1:1 mapping anyway, and I don't think this is too restrictive.  What do you think?


Repository:
  rL LLVM

https://reviews.llvm.org/D50807





More information about the llvm-commits mailing list