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

[Vedant Kumar via llvm-commits]

vsk added a comment.

I think I addressed Lang's review feedback.

If http://reviews.llvm.org/D20082 looks good, I'd like to rebase onto it. After that it seems like we'd be in good shape.


================
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;
+  }
 
----------------
lhames wrote:
> davidxl wrote:
> > vsk wrote:
> > > lhames wrote:
> > > > InstrProfError (and CoverageMappingError) need their own ID members. If they don't have them the Error RTTI will treat them both as EnumBasedErrors.
> > > Yikes! Is this true even if I define distinct ID's for them? Specifically:
> > > 
> > > ```
> > > template <> char EnumBasedError<instrprof_error, InstrProfError>::ID = 0; // InstrProf.cpp
> > > template <> char EnumBasedError<coveragemap_error, CoverageMapError>::ID = 0; // CoverageMapping.cpp
> > > ```
> > Can you elaborate on the RTTI issue?  Also what is the problem that can be caused with this (both Error treated as EnumBasedErrors)?
> My bad!  didn't stop to think about EnumBasedError being templated. I think having the ID on EnumBasedError like this should be safe.
@davidxl Here's an example where this could be a problem. Suppose classes A and B both inherit from class Parent, and that the storage for `ID` is shared. Then, a "B" error could be handled by a callback for "A" errors.


http://reviews.llvm.org/D19901