[PATCH] D44560: [DWARF] Rework debug line parsing to use llvm::Error and callbacks

[Rafael Avila de Espindola via Phabricator via llvm-commits]

espindola added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:262
   /// Get a pointer to a parsed line table corresponding to a compile unit.
-  const DWARFDebugLine::LineTable *getLineTableForUnit(DWARFUnit *cu);
+  const DWARFDebugLine::LineTable *getLineTableForUnit(DWARFUnit *U);
+
----------------
Please document what this version does when there is an error/warning.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugLine.h:44
+  // section.
+  bool isFatal() const { return IsFatal; }
+
----------------
I still think this is a design error.

Being fatal or not is something for the caller to decide. Looking at the code I think that the issue that makes this look necessary is that we just need two error types.

Imagine if a filesystem API didn't distinguish "no such file" and "permission denied" :-)

I have uploaded a somewhat hackish modified version to https://reviews.llvm.org/D45074.

The idea is to return a DebugLineLengthError when &Offset was not updated correctly and a StringError when it was.

Please update this patch to use something along those lines.
 


Repository:
  rL LLVM

https://reviews.llvm.org/D44560