[PATCH] D59060: Compute and Print Type and Section columns in "llvm-nm -f sysv" output. Round 2.
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 8 01:30:43 PST 2019
jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.
LGTM.
================
Comment at: tools/llvm-nm/llvm-nm.cpp:1113
+ if (!SecIOrErr) {
+ consumeError(SecIOrErr.takeError());
+ return '?';
----------------
Sunil_Srivastava wrote:
> Sunil_Srivastava wrote:
> > Sunil_Srivastava wrote:
> > > jhenderson wrote:
> > > > Could you add a comment explaining why we throw away the error rather than reporting it here, please?
> > > I have copied the usage from other places in the same file. I am assuming that consumeError is handling the error.
> > consumeError() is calling handleAllErrors which handles errors.
> I see now that `consumeError` is 'throwing away' errors in some sense.
>
> This function, `getNMSectionTagAndName` is the rename of `getNMTypeChar`. `getNMTypeChar` calls `getSymbolNMTypeChar` for different kinds of object files.
>
> Since I needed to do something special for ELF, I guarded my code for ELF, and mimicked the code in `getSymbolNMTypeChar`, the ELF version. `getSymbolNMTypeChar` for ELF (and also for COFF) has the same call to `getSection` and `consumeError`, without any comment indicating what they are doing.
>
> If you have any suggestion as to what comment should go here, I will be happy to add it.
>
>
Ah, I missed that this is how it is handled elsewhere. No need to worry for now. I might suggest some error handling improvements in the future, but they should be wider and not part of this change then.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59060/new/
https://reviews.llvm.org/D59060
More information about the llvm-commits
mailing list