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

[James Henderson via Phabricator via llvm-commits]

jhenderson 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());
+      });
----------------
jasonliu wrote:
> 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.
`reportUniqueWarning` can take an `Error` directly, so you can just do:
```
    if (!ErrOrCsectAuxRef)
      reportUniqueWarning(ErrOrCsectAuxRef.takeError());
```

Also, be careful, as the program continues, so referencing `ErrOrCsectAuxRef` after this may result in things going wrong...


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

https://reviews.llvm.org/D85774