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

Mon Aug 17 06:33:40 PDT 2020

jasonliu added inline comments.

================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:393

-    // The symbol's last auxiliary entry is a CSECT Auxiliary Entry.
-    printCsectAuxEnt32(XCOFFSymRef.getXCOFFCsectAuxEnt32());
+    printCsectAuxEnt(unwrapOrError(Obj.getFileName(), XCOFFSymRef.getXCOFFCsectAuxEnt()));
     break;
----------------
grimar wrote:
> jhenderson wrote:
> > @grimar has gone to a lot of effort to get rid of `unwrapOrError` from the ELF dumping code. I'd prefer it if we could avoid using it here too. It is generally better in dumping tools to report a warning and abort dumping the current section than to emit an error and terminate the program, since it gives the user more of the information they've asked for.
> Yeah. Having `unwrapOrError` available is my concern. I am trying to cleanup ELF dumper, but other files (e.g. COFF) are still using it, thought ideally I'd just remove this API from llvm-readobj code, it seems does more harm than good for a long term.
> At least I'd be happy if people stop adding more calls to the code.
Thanks. Agreed. Will avoid using `unwrapOrError` in future code.

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

https://reviews.llvm.org/D85774