[PATCH] D73484: [llvm-readobj] - Improve error message reported by DynRegionInfo.

[George Rimar via Phabricator via llvm-commits]

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