[lldb-dev] Is GetLogIf**All**CategoriesSet useful?

Jonas Devlieghere via lldb-dev lldb-dev at lists.llvm.org
Wed Jan 19 10:42:33 PST 2022



> On Jan 19, 2022, at 10:25 AM, Jim Ingham <jingham at apple.com> wrote:
> 
> 
> 
>> On Jan 19, 2022, at 6:40 AM, Pavel Labath <pavel at labath.sk> wrote:
>> 
>> Hi all,
>> 
>> In case you haven't noticed, I'd like to draw your attention to the in-flight patches (https://reviews.llvm.org/D117382, https://reviews.llvm.org/D117490) whose goal clean up/improve/streamline the logging infrastructure.
>> 
>> I'm don't want go into technical details here (they're on the patch), but the general idea is to replace statements like GetLogIf(Any/All)CategoriesSet(LIBLLDB_LOG_CAT1 | LIBLLDB_LOG_CAT2)
>> with
>> GetLogIf(Any/All)(LLDBLog::Cat1 | LLDBLog::Cat2)
>> i.e., drop macros and make use of templates to make the function calls shorter and safer.
>> 
>> The reason I'm writing this email is to ask about the "All" versions of these logging functions. Do you find them useful in practice?
>> 
>> I'm asking that because I've never used this functionality. While I can't find anything wrong with the concept in theory, practically I think it's just confusing to have some log message appear only for some combination of enabled channels. It might have made some sense when we had a "verbose" logging channel, but that one is long gone (we still have a verbose logging *flag*).
>> 
>> In fact, out of all our GetLogIf calls (1203), less than 1% (11*) uses the GetLogIfAll form with more than one category. Of those, three are in tests, one is definitely a bug (it combines the category with LLDB_LOG_OPTION_VERBOSE), and the others (7) are of questionable usefulness (to me anyway).
>> 
>> If we got rid of this, we could simplify the logging calls even further and have something like:
>> Log *log = GetLog(LLDBLog::Process);
>> everywhere.
> 
> The only time I’ve ever “used” GetLogIfAll was when I added another LOG option to a log call, not noticing it was “All”, finding the new log didn’t work, and going back to switch “All” to “Any”.
> 
> I vote for removing it.

+1 

> 
> Jim
> 
> 
>> 
>> cheers,
>> pl
>> 
>> (*) I used this command to count:
>> $ git grep -e LogIfAll -A 1 | fgrep -e '|' | wc -l

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20220119/9e4fe479/attachment.html>


More information about the lldb-dev mailing list