[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/lib/Object/XCOFFObjectFile.cpp:822
+    return createStringError(object_error::parse_failed,
+                             "this csect symbol contains no auxiliary entry");
 
----------------
Is this error user-facing (I'm assuming so)? Assuming it is, you should record here which symbol is causing the problem. Otherwise the user will be faced with an error along the lines of this:

```error: this csec symbol contains no auxiliary entry```

which is not really actionable (imagine the input had 100000 symbols in - the user can't realistically go through each to find the offending one).


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:851
+  return createStringError(object_error::parse_failed,
+                           "a csect auxiliary entry is not found");
+}
----------------
Similar to my above comment - which entry was not found? Give the user more context so that they can act on the problem.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:393
+                      [this](const StringError &SE) {
+                        W.startLine() << SE.getMessage() << "\n";
+                      });
----------------
This will write the error inline, rather than to stderr. Are you sure that's what you want? it isn't what most dumping tools do on failure.


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

https://reviews.llvm.org/D85774