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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 6 03:09:39 PDT 2019


jhenderson added a comment.

Is there a test showing that we a) emit a warning only once per input, and b) multiple times, for multiple inputs (both archive members and when listing multiple objects on the command-line)?



================
Comment at: tools/llvm-readobj/ELFDumper.cpp:398
+  std::function<Error(const Twine &Msg)> WarningHandler;
+  std::set<std::string> Warnings;
+
----------------
grimar wrote:
> jhenderson wrote:
> > Should Warnings be private? Also set -> unordered_set.
> I made it private. Making `set` to be `unordered_set` was a good idea,
> but I can't do that now, when I have a set of pairs.
> (It just doesn't compile, the problem is that (taken from stackoverflow): 
> "std::unordered_set is using std::hash template to compute hashes for its entries and there is no std::hash specialization for pairs.")
> So I had to use `set`, it does not seem to be critical here.
You could always implement std::hash for pairs, but I agree that it's not important.


================
Comment at: tools/llvm-readobj/llvm-readobj.h:42
+
+  // TODO: This one is depricated. Use one with a Input name below.
   template <class T> T unwrapOrError(Expected<T> EO) {
----------------
depricated -> deprecated

Do you plan to remove this function soon? I don't see why you shouldn't (apart from time to write it...!)

What about the ErrorOr version? Should that be updated too?


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

https://reviews.llvm.org/D65515





More information about the llvm-commits mailing list