[PATCH] D51033: [DWARF] Move error printing from DWARF classes to Support library. NFC.

Victor Leschuk via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 22 05:06:31 PDT 2018


vleschuk added a comment.

In https://reviews.llvm.org/D51033#1208987, @jhenderson wrote:

> I'm thinking a bit more about this and I'm concerned that by adding these functions to Error.h, people will be more likely to use them from libraries, which can lead to various issues when the client of the library doesn't want to report errors and warnings the same way as this implements. The warn function was really only intended as a default implementation so that we didn't have to propagate things all the way out of DWARFContext.cpp, for the single client (llvm-dwarfdump) to consume.




> Given that, I think the right thing to do is to just move the DWARFDebugLine::warn function out into the DWARFContext::dump function, and pass it around as necessary (or make it a static function in the file).

I like that idea, however, we also have dsymutil which was using DWARFDebugLine's implementation. I think it would be correct to leave warning as function in DWARFContext.h, not as static function.

>   If you go that approach, I'd be tempted to do the same with an error function too (note that there are several places within DWARFContext.cpp that use `WithColor::error()`).

While warnings are used in one place only and their purpose is mainly to handleAllErrors, error reporting format is not that unified...


https://reviews.llvm.org/D51033





More information about the llvm-commits mailing list