[PATCH] D73484: [llvm-readobj] - Improve error message reported by DynRegionInfo.
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 29 02:12:46 PST 2020
grimar marked 4 inline comments as done.
grimar added inline comments.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:139
+ /// Error prefix. Used for error reporting to provide more information.
+ std::string ErrPrefix;
+ /// Region size name. Used for error reporting.
----------------
jhenderson wrote:
> Let's make this a little more generic. How about something like "InfoContext"?
Ok. Seems leaving the "Error prefix" sentence for the comment is fine?
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1877
+ ("section with index " +
+ Twine(DynamicSec - &cantFail(Obj->sections()).front()) + " has ")
+ .str();
----------------
jhenderson wrote:
> I'd move the " has " to the usage site, since it's always present, and the error message wouldn't make sense without it.
`ErrPrefix` is often empty. E.g. messages like:
`invalid DT_RELASZ value (24) or DT_RELAENT value (255)` does not suppose to have this prefix about section index.
Do you mean to add "has" to the usage site when `ErrPrefix` is not empty?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73484/new/
https://reviews.llvm.org/D73484
More information about the llvm-commits
mailing list