[PATCH] D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC.

[Victor Leschuk via Phabricator via llvm-commits]

vleschuk marked 2 inline comments as done.
vleschuk added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFListTable.h:213
   if (*OffsetPtr < HeaderOffset || *OffsetPtr >= End)
-    return createError("invalid %s list offset 0x%" PRIx32,
+    return createStringError(errc::invalid_argument,
+                       "invalid %s list offset 0x%" PRIx32,
----------------
JDevlieghere wrote:
> Although having an inconvertibleErrorCode (as per Lang's comment in the other diff) prevents conversion between errors and error codes, I'm not convinced it's actually needed here. If we don't need the conversion in libDebugInfo (which I think we don't) I'd rather have an  inconvertible one instead of whichever is close enough. 
During https://reviews.llvm.org/D49676 it was decided to use convertible error codes in such situations. I think we should have consistent behavior along code sections (in this case style should be similar in all DebugInfo/DWARF* classes).


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugRnglists.cpp:27
-
-namespace llvm {   // workaround for gcc bug
-template <>
----------------
JDevlieghere wrote:
> vleschuk wrote:
> > @wolfgangp I have compiled new code with gcc (7.2.1) and it compiles correctly and passes all tests. Could you please let me know if we can safely get rid of this workaround here?
> We officially support GCC 4.8.0 and later, so whether it compiles with 7.2.1 doesn't really tell us much. I believe we have bots running pretty old versions of GCC so you could try removing and see if any of them are failing (or you get complaints from downstream users). 
I have tested it, build fails if we just remove this part from code, however everything works fine if we fully apply the patch from this revision and replace calls to createError() with the createStringError(). Tested with clang 6.0, gcc 4.8, 4.9 and 7.2.


Repository:
  rL LLVM

https://reviews.llvm.org/D49964