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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Thanks for the comments everybody. As the two error types is not far off what I have at the moment, I will try implementing that first. I will then take a look at returning an iterator and providing an error-handling callback (and probably post it in a separate review). Similar to some of the earlier ideas I actually believe this would need to be two callbacks - one for errors and one for warnings. Signatures would be void(Error) (for errors) and void(StringRef) (for warnings). Default behaviour would be to simply print the message. Encountering a "fatal" error would cause the internal indicator for the iterator to be set to the end of the section, in addition to feeding an Error to the callback.



================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugLine.h:44
+  // section.
+  bool isFatal() const { return IsFatal; }
+
----------------
espindola wrote:
> 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.
>  
Thanks for the idea. I think this is a good thought.


Repository:
  rL LLVM

https://reviews.llvm.org/D44560