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

[Digger via Phabricator via llvm-commits]

DiggerLin marked 4 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:45
+  if (SymRef.hasCsectAuxEnt()) {
+    assert(SymRef.getNumberOfAuxEntries() >= 1 &&
+           "No CSECT Auxiliary Entry is found.");
----------------
hubert.reinterpretcast wrote:
> I would rather leave the asserts out since the TODO for `getXCOFFCsectAuxEnt32` will cover it. In particular, I'm not convinced that assertions are the right tool here.
since we have not implement the TODO, I think it is better to put a assert here.
when we implement the "TODO for getXCOFFCsectAuxEnt32" , I will remove the assert.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:59
+  if (SymRef.hasCsectAuxEnt()) {
+    assert(SymRef.getNumberOfAuxEntries() >= 1 &&
+           "No CSECT Auxiliary Entry is found.");
----------------
hubert.reinterpretcast wrote:
> Same comment re: the assertion.
since we have not implement the TODO, I think it is better to put a assert here.
when we implement the "TODO for getXCOFFCsectAuxEnt32" , I will remove the assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72973