[PATCH] D72973: [llvm-objdump] Use symbol index+symbol name + storage mapping class as label for -D

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.h:12
+
+#include "llvm/MC/MCDisassembler/MCDisassembler.h"
+#include "llvm/Object/XCOFFObjectFile.h"
----------------
What is this header being included from this header for?


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.h:13
+#include "llvm/MC/MCDisassembler/MCDisassembler.h"
+#include "llvm/Object/XCOFFObjectFile.h"
+
----------------
The function declarations do not require definitions for most of the types. Class types can be declared as incomplete in this header. `XCOFF::StorageMappingClass` comes from `llvm/BinaryFormat/XCOFF.h`.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.h:17
+namespace objdump {
+Optional<XCOFF::StorageMappingClass>
+getXCOFFSymbolCsectSMC(const object::XCOFFObjectFile *Obj,
----------------
Please include an interface header that is expected to provide `Optional` as part of its interface.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.h:24
+void printXCOFFSymbolDescription(const SymbolInfoTy &SymbolInfo,
+                                 StringRef SymbolName);
+
----------------
Same comment about interface header but for `StringRef`.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.h:28
+                                    const object::RelocationRef &RelRef,
+                                    llvm::SmallVectorImpl<char> &Result);
+} // namespace objdump
----------------
Ditto `SmallVectorImpl`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72973