[PATCH] D65515: [llvm-readobj/libObject] - Introduce a custom warning handler for `ELFFile<ELFT>` methods.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Thanks for this! I think it's a good approach, and reminds me of the approach I took in the DWARFDebugLine parser code.



================
Comment at: include/llvm/Object/ELF.h:101
+  // This is a callback that can be passed to a number of functions.
+  // It can be used to ignore non-critical errors (warnings), what is
+  // useful for dumpers, like llvm-readobj.
----------------
what -> which


================
Comment at: include/llvm/Object/ELF.h:104
+  // It accepts a warning message string and returns a success
+  // when the warning should be ignored or a error otherwise.
+  using WarningHandler = llvm::function_ref<Error(const Twine &Msg)>;
----------------
a error -> an error


================
Comment at: test/tools/llvm-readobj/elf-invalid-shstrndx.test:11
 # GNU-EMPTY:
-# GNU-NEXT:  error: '[[FILE]]': section header string table index 255 does not exist
+# GNU-NEXT:  error: section header string table index 255 does not exist
 
----------------
It makes me sad that we've lost the file name here. Can we do anything about it, by using an error function that prepends the file name?


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:353
+  DumpStyle(ELFDumper<ELFT> *Dumper) : Dumper(Dumper) {
+    // Dumper reports all non-critical errors as a warnings.
+    // It does not print the same warning more than once.
----------------
as a warnings -> as warnings


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:398
+  std::function<Error(const Twine &Msg)> WarningHandler;
+  std::set<std::string> Warnings;
+
----------------
Should Warnings be private? Also set -> unordered_set.


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

https://reviews.llvm.org/D65515