[PATCH] D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC)
Pavel Labath via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 24 23:00:54 PDT 2019
labath added inline comments.
================
Comment at: lldb/source/Expression/IRExecutionUnit.cpp:601
if (log) {
+ LLDB_LOGF(log,
----------------
jingham wrote:
> labath wrote:
> > looks like there are some `if(log)`s still remaining. Maybe the `{}` around the printf confused your vim macro?
> There will always be some "if (log)" statements. Whenever consing up the log message involves work, you should always say:
>
> if (log) {
> /// Do expensive work
> LLDB_LOG...
> }
>
> Not saying that all the remaining "if (logs)" are for that reason, but then intent is NOT to remove all such statements as they serve a useful purpose.
I'm fine with `if(log)`s staying if they serve a purpose (or even if they're hard to remove in this patch). However, I would expect that with LLDB_LOG, they become much less frequently needed, because the formatter is much more powerful (e.g., there is no need to write a for loop just to print an array, etc.)
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65128/new/
https://reviews.llvm.org/D65128
More information about the llvm-commits
mailing list