Differential D154128
[lldb] Add log indicating which kind of data formatter Authored by kastiglione on Jun 29 2023, 11:17 AM.
Details The formatter logs include a function name, but these functions are mostly templates This change adds a new log which is printed upon entry of FormatManager::Get, which
Diff Detail
Event TimelineComment Actions I've bumped into this problem before, so really like this change. It's a bit sad that you'll only print the kind of data formatter once though, so someone reading the logs might have to jump back and forth to understand what kind of formatter they're dealing with. There's also __PRETTY_FUNCTION__, which shows the generic type parameters by default, but I think it's not a standard macro (although it's implemented by both clang and gcc). What do you think of having a macro like: #ifndef __PRETTY_FUNCTION__ #define __PRETTY_FUNCTION__ __FUNCTION__ #endif And replacing __FUNCTION__ with __PRETTY_FUNCTION__? Comment Actions I just gave __PRETTY_FUNCTION__ a test drive, and it's a bit verbose/noisy: For example: [ImplSP lldb_private::FormatManager::Get(lldb_private::ValueObject &, lldb::DynamicValueType) [ImplSP = std::shared_ptr<lldb_private::SyntheticChildren>]] Begin synthetic lookup. Here's an example of the formatter logs with my change. Note that these logs are output in "blocks", that is the first line is prefixed with two newlines (\n\n): [Get] Begin summary lookup. [GetCached] Looking into cache for type Swift.Optional<main.Item> [GetCached] Cache search failed. Going normal route [Get] candidate match = Swift.Optional<main.Item> no-strip-pointers no-strip-reference no-strip-typedef [Get] candidate match = main.Item? no-strip-pointers no-strip-reference no-strip-typedef [Get] candidate match = Swift.Optional<main.Item> no-strip-pointers no-strip-reference no-strip-typedef [Get] candidate match = Swift.Optional<main.Item> no-strip-pointers no-strip-reference no-strip-typedef [Get] candidate match = main.Item? no-strip-pointers no-strip-reference no-strip-typedef [Get] candidate match = Swift.Optional<main.Item> no-strip-pointers no-strip-reference no-strip-typedef [Get] Trying to use category default [Get] Trying to use category llvm [Get] Trying to use category VectorTypes [Get] Trying to use category runtime-synthetics [Get] Trying to use category system [Get] nothing found - returning empty SP [GetCached] Caching 0x0 for type Swift.Optional<main.Item> [Get] Search failed. Giving language a chance. [Get] Language search success. Returning. [Get] Begin synthetic lookup. ... At the beginning of the lines, those names in square brackets are the __FUNCTION__ macro. If they were replaced with __PRETTY_FUNCTION__, I think it would be too noisy.
I agree that this isn't ideal, As mentioned above, there are two newlines used to group and separate related log statements. This is why I put the new log at the top. Someone reading these logs can look at the first line in the block to know what kind of formatter is active. It's hard to strike a balance: if every line says the formatter kind, it's both convenient but also redundant/noisy. In summary:
Comment Actions Personally I vote for the formatter kind on every line, so we don't need to jump around the logs when reading them, up to you though. Comment Actions FWIW that's exactly what LLVM_PRETTY_FUNCTION does. It's implemented as you would expect (and you described): https://github.com/llvm/llvm-project/blob/5e2f0947c586042083cc1643f2aac90f2492bacb/llvm/include/llvm/Support/Compiler.h#L497 Comment Actions Our logging infrastructure already supports including the function name in the log message. That has always been the case for LLDB_LOG but LLDB_LOGF wasn't updated until 6099d519bb83 because the idea was that everyone would move to formatv-style logging LLDB_LOG but that never gained consensus. Comment Actions I'm fine with keeping the template argument, but we should remove the redundant FUNCTION argument from the log messages themselves |