[PATCH] D19901: [ProfileData] (llvm) Use Error in InstrProf and Coverage

[Lang Hames via llvm-commits]

lhames added a comment.

I think EnumBasedError is problematic: at a minimum it needs to implement the convertToErrorCode method, since it's part of the current Error contract. Clients must be allowed to convert to std::error_code to allow interoperability with code that hasn't switched to Error yet. That shouldn't be too much a headache. You just need to create an error category and move your existing to-string methods for the enum over to that. See e.g. lib/Object/Error.cpp for a simple error_category definition.

EnumBasedError probably shouldn't have its own create method either - wherever you're creating a success value you should just use Error::success(). Where you're creating a failure value you should use make_error<ErrT>(<enum value>);


================
Comment at: include/llvm/ProfileData/InstrProf.h:303-321
@@ -282,5 +302,21 @@
 
-inline std::error_code make_error_code(instrprof_error E) {
-  return std::error_code(static_cast<int>(E), instrprof_category());
-}
+class InstrProfError : public EnumBasedError<instrprof_error, InstrProfError> {
+public:
+  InstrProfError(instrprof_error Err)
+      : EnumBasedError<instrprof_error, InstrProfError>(Err) {}
+
+  void log(raw_ostream &OS) const override;
+
+  std::string message() const override;
+
+  /// Consume an Error and return the raw enum value contained within it. The
+  /// Error must either be a success value, or contain a single InstrProfError.
+  static instrprof_error take(Error E) {
+    auto Err = instrprof_error::success;
+    handleAllErrors(std::move(E), [&Err](const InstrProfError &IPE) {
+      assert(Err == instrprof_error::success && "Multiple errors encountered");
+      Err = IPE.get();
+    });
+    return Err;
+  }
 
----------------
InstrProfError (and CoverageMappingError) need their own ID members. If they don't have them the Error RTTI will treat them both as EnumBasedErrors.


http://reviews.llvm.org/D19901