[PATCH] D113825: [llvm-readobj][XCOFF] dump auxiliary symbols.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:1116-1117
+  if (!ExpCsectAuxEnt) {
+    // If we could not get the CSECT auxiliary entry, then this symbol should
+    // not be a function. So consume the error and return `false` to move on.
+    consumeError(ExpCsectAuxEnt.takeError());
----------------
Is "should" appropriate here? Is it never a function? Or is it potentially a function that we can't identify? If the former, delete the word. If the latter, replace the phrase "then this symbol should not be a function" with "then treat this symbol as if it isn't a function".


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:523
+          continue;
+        else if (Type == XCOFF::SymbolAuxType::AUX_FCN) {
+          const XCOFFFunctionAuxEnt64 *AuxEntPtr =
----------------
No need for `else` after `continue`


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:563-565
+    if (NumberOfAuxEntries > 1)
+      reportUniqueWarning("the C_DWARF symbol at index " + Twine(SymbolIdx) +
+                          " should not have more than 1 auxiliary entry");
----------------
There are several checks identical to this one. I think it would be useful to pull them into a helper lambda that takes a C_* enum and reports the message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113825