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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h:26
+  uint64_t Index;
+  bool ShowSymDesc;
+  XCOFFSymbolInfo(Optional<XCOFF::StorageMappingClass> Smc, uint64_t Idx,
----------------
jasonliu wrote:
> I wonder if this bool is really necessary. Could you instead check if it is label in getXCOFFSymbolCsectSMC? If it is a label, then return the empty Optional. 
> In other words, is it necessary to get the SMC from a label at the first place?
I am not a fan of //this// `bool` and I am not a fan of using the empty optional either. I do not believe that display-related decisions should be stored in something named `XCOFFSymbolInfo`. I would suggest to store a `bool` for whether the symbol is known to be a label.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72973