[PATCH] D44382: [DWARF] Rework debug line parsing to use llvm::Error

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In https://reviews.llvm.org/D44382#1034669, @labath wrote:

> (though I'm not entirely sure that your usage of llvm::Error is completely idiomatic here).


Thanks for the comments. I am fairly new to llvm::Error myself, so I completely agree that this is probably a little unusual. I did consider the alternative which is to have a separate ErrorInfo class for each of the different severities, but that didn't fit particularly well in some situations, e.g. when treating Major amd Fatal errors the same, but distinct from Minor. I'm certainly open to suggestions for alternatives.



================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:42-43
+                  [&](DebugLineError &Info) {
+                    if (Info.getSeverity() == DebugLineError::None)
+                      return;
+
----------------
labath wrote:
> Why should we ever have an error with severity "none"?
You shouldn't (and I'm tempted to put an unreachable assert here). The problem is that we need a default value that indicates no error, so that a caller doesn't also have to check against Error::success().


Repository:
  rL LLVM

https://reviews.llvm.org/D44382