[PATCH] D72973: [llvm-objdump] Use symbol index+symbol name + storage mapping class as label for -D
Hubert Tong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 1 15:47:56 PDT 2020
hubert.reinterpretcast added a comment.
It seems we are still expecting some more comments to be addressed? @DiggerLin, in the future please clearly acknowledge all comments that have not yet been addressed if you post an updated patch that does not address all comments.
================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:45
+ if (SymRef.hasCsectAuxEnt()) {
+ assert(SymRef.getNumberOfAuxEntries() >= 1 &&
+ "No CSECT Auxiliary Entry is found.");
----------------
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.
================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:59
+ if (SymRef.hasCsectAuxEnt()) {
+ assert(SymRef.getNumberOfAuxEntries() >= 1 &&
+ "No CSECT Auxiliary Entry is found.");
----------------
Same comment re: the assertion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72973/new/
https://reviews.llvm.org/D72973
More information about the llvm-commits
mailing list