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

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

Only some minor nits, otherwise LGTM. I haven't attempted to review the test coverage for all the new code, as I don't feel like I'm in a good position to do that, not being an XCOFF developer. I assume someone else has though.



================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:481
+      : OwningObjectPtr(OwningObjectPtr) {
+    assert(OwningObjectPtr && "OwningObjectPtr can not be nullptr!");
+    assert(SymEntDataRef.p != 0 &&
----------------



================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:483
+    assert(SymEntDataRef.p != 0 &&
+           "Symbol table entry pointer can not be nullptr!");
+
----------------



================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:791
+  // A function definition should be a label definition.
+  // FIXME: This is not necessary the case when -ffunction-sections is enabled.
+  if (!CsectAuxRef.isLabel())
----------------



================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:801
+  if (!SI) {
+    consumeError(SI.takeError());
+    return false;
----------------
Basically any time you use `consumeError`, add a comment explaining why it's justified that we don't report the error to the user.


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

https://reviews.llvm.org/D85774