[PATCH] D93021: [llvm-readelf/obj] - Minor cleanup of warning reporting.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 10 22:54:59 PST 2020


grimar added a comment.

In D93021#2446766 <https://reviews.llvm.org/D93021#2446766>, @MaskRay wrote:

> Is this desired? For
>
>   reportUniqueWarning(
>             "SHT_DYNSYM section header and DT_SYMTAB disagree about "
>             "the location of the dynamic symbol table");
>
> it is fine. But for the others, a bunch of `toString` are added.

It cleans up the API, leaving the single `reportUniqueWarning(const Twine &Msg)` method.
This method has different enough signature from regular global `reportWarning`.
It should help to encourage people to use `toString` and provide more context for error messages.

Generally I think that lines like
`this->reportUniqueWarning(toString(SymsOrErr.takeError()));` are not ideal.
Because most of the time messages could be better if the were like:
``this->reportUniqueWarning("some context here: " + toString(SymsOrErr.takeError()));``

I think a context could be added for many if not all of of places touched.


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

https://reviews.llvm.org/D93021



More information about the llvm-commits mailing list