[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