[PATCH] D85774: [XCOFF][AIX] Enable tooling support for 64 bit symbol table parsing

[Jason Liu via Phabricator via llvm-commits]

jasonliu added inline comments.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:391-392
+    if (!ErrOrCsectAuxRef) {
+      handleAllErrors(ErrOrCsectAuxRef.takeError(), [this](ErrorInfoBase &EI) {
+        report_fatal_error(EI.message());
+      });
----------------
jhenderson wrote:
> Use `reportError` or `reportWarning` so that the error is reported in a clean manner and consistent with other llvm-readobj varieties, and not `report_fatal_error` which looks like a crash. General rule of thumb: try to avoid using `report_fatal_error`, especially in tool code where it is easy to report errors properly.
> 
> In llvm-readobj for ELF, we try to avoid even using `reportError` where possible, as that stops the tool from continuing dumping, which can be problematic occasionally. We prefer `reportWarning` (or more specifically the local `reportUniqueWarning` which avoids reporting the same warning multiple times) and bailing out of the current routine. Take a look at ELFDumper.cpp for examples.
Thanks for the elaboration. That clears up things a lot for me. I will use reportUniqueWarning here.


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

https://reviews.llvm.org/D85774