[PATCH] D64014: [Object/ELF.h] - Improve error reporting.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: include/llvm/Object/ELF.h:63
+    return "[" + std::to_string(Sec - &(*TableOrErr).front()) + "]";
+  llvm::consumeError(TableOrErr.takeError());
+  return "[?]";
----------------
grimar wrote:
> jhenderson wrote:
> > Why not have this return an Expected, and let the call sites handle the error rather than throwing it away here? Of course, they could throw it away there.
> Two reasons:
> 1) To simplify the implementation. With the current version it is a very convenient helper that does not need additional wrapping.
>      It also allows to build the final string of view `[index X]` in one place.
> 2) I believe this error will *never* happen actually. Because our code always calls `sections()` earlier than this method is called,
> so it just should succeed here.
> But I still had to handle the error somehow and decided that because of all above the best way is just to return "[?]" as a section index in this
> really unlikely and untestable (I believe) case.
> 
The use of consumeError deserves a comment (the comment of the function says it may indicate a design problem)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64014/new/

https://reviews.llvm.org/D64014