[lldb] Add new per-language frame-format variables for formatting function names#131836
Closed
Michael137 wants to merge 5 commits intollvm:mainfrom
Closed
[lldb] Add new per-language frame-format variables for formatting function names#131836Michael137 wants to merge 5 commits intollvm:mainfrom
Michael137 wants to merge 5 commits intollvm:mainfrom
Conversation
| bool GetFunctionDisplayName( | ||
| const SymbolContext *sc, const ExecutionContext *exe_ctx, | ||
| FunctionNameRepresentation representation, Stream &s, | ||
| const FormatEntity::Entry::HighlightSettings &) override; |
Collaborator
There was a problem hiding this comment.
I feel like the idea here was to have the "out" parameters come last?
1e58b86 to
5656cdd
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
5656cdd to
d262063
Compare
libcxxabi/src/demangle/Utility.h
Outdated
| return std::string_view(Buffer, CurrentPosition); | ||
| } | ||
|
|
||
| // Stores information about parts of a demangled function name. |
Collaborator
There was a problem hiding this comment.
Suggested change
| // Stores information about parts of a demangled function name. | |
| /// Stores information about parts of a demangled function name. |
libcxxabi/src/demangle/Utility.h
Outdated
|
|
||
| // Stores information about parts of a demangled function name. | ||
| struct FunctionNameInfo { | ||
| ///< A [start, end) pair for the function basename. |
Collaborator
There was a problem hiding this comment.
Suggested change
| ///< A [start, end) pair for the function basename. | |
| /// A [start, end) pair for the function basename. |
libcxxabi/src/demangle/Utility.h
Outdated
| ///< ^ ^ | ||
| ///< Start End | ||
| ///< \endcode | ||
| std::pair<size_t, size_t> ScopeLocs; |
Collaborator
There was a problem hiding this comment.
Maybe Range instead of Locs?
| return true; | ||
| } | ||
|
|
||
| struct HighlightSettings { |
Collaborator
There was a problem hiding this comment.
/// comment for the structure?
Collaborator
There was a problem hiding this comment.
maybe explain what prefix and suffix should be?
d262063 to
57e65ac
Compare
f06c493 to
e40c950
Compare
748989a to
3c34325
Compare
Member
Author
|
Will give others some time to look over Any preference on how to land this? Would be nice to keep the commits separate and not get them squashed. Perhaps push them to HEAD without the github UI but mention the PR in each commit message? |
…tion (llvm#131836) This patch implements a new `TrackingOutputBuffer` which tracks where the scope/basename/arguments begin and end in the demangled string. The idea that a function name can be decomposed into <scope, base, arguments>. The assumption is that given the ranges of those three elements and the demangled name, LLDB will be able to to reconstruct the full demangled name. The tracking of those ranges is pretty simple. We don’t ever deal with nesting, so whenever we recurse into a template argument list or another function type, we just stop tracking any positions. Once we recursed out of those, and are back to printing the top-level function name, we continue tracking the positions. We introduce a new structure `FunctionNameInfo` that holds all this information and is stored in the new `TrackingOutputBuffer` class. Tests are in `ItaniumDemangleTest.cpp`. llvm#131836
…#131836) Add version of GetDemangledName that will force re-demangling. This is required because LLDB will SetDemangledName without going through the demangler. So we need a way to force demangling to set the m_demangled_info member when we need it. llvm#131836
Uses the `TrackingOutputBuffer` to populate the new member `Mangled::m_demangled_info`. `m_demangled_info` is lazily popluated by `GetDemangledInfo`. To ensure `m_demangled` and `m_demangled_info` are in-sync we clear `m_demangled_info` anytime `m_demangled` is set/cleared. llvm#131836
llvm#131836) Adds new frame-format variables and implements them in the CPlusPlusLanguage plugin. We use the `DemangledNameInfo` type to retrieve the necessary part of the demangled name. llvm#131836
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Apr 23, 2025
…setting (llvm#131836) Adds the new `plugin.cplusplus.language.function-name-format` setting and makes the `${function.name-with-args}` query it for formatting the function name. One caveat is that the setting can't itself be set to `${function.name-with-args}` because that would cause infinite recursion and blow the stack. I added an XFAILed test-case for it and will address it in a follow-up patch. llvm#131836
3c34325 to
d95c0df
Compare
d95c0df to
4b63bd9
Compare
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Apr 24, 2025
…etting (llvm#131836) Adds the new `plugin.cplusplus.display.function-name-format` setting and makes the `${function.name-with-args}` query it for formatting the function name. One caveat is that the setting can't itself be set to `${function.name-with-args}` because that would cause infinite recursion and blow the stack. I added an XFAILed test-case for it and will address it in a follow-up patch. llvm#131836
4b63bd9 to
f1bd052
Compare
…etting (llvm#131836) Adds the new `plugin.cplusplus.display.function-name-format` setting and makes the `${function.name-with-args}` query it for formatting the function name. One caveat is that the setting can't itself be set to `${function.name-with-args}` because that would cause infinite recursion and blow the stack. I added an XFAILed test-case for it and will address it in a follow-up patch. llvm#131836
f1bd052 to
53af742
Compare
Member
Author
Contributor
https://lab.llvm.org/buildbot/#/builders/195/builds/8106 https://lab.llvm.org/buildbot/#/builders/197/builds/4461 |
Member
Author
Fixed this in a follow-up commit |
Contributor
| bool valid_basename = true; | ||
| }; | ||
|
|
||
| DemanglingPartsTestCase g_demangling_parts_test_cases[] = { |
Collaborator
There was a problem hiding this comment.
Some part/all of this lot is creating warnings compiling on Linux with clang 19:
$ ninja check-lldb
[244/267] Building CXX object tools/lldb/unit...MakeFiles/LLDBCoreTests.dir/MangledTest.cpp.o
/home/david.spickett/llvm-project/lldb/unittests/Core/MangledTest.cpp:416:8: warning: designated initializers are a C++20 extension [-Wc++20-designator]
416 | { .BasenameRange = {92, 98}, .ScopeRange = {36, 92}, .ArgumentsRange = { 108, 158 },
| ^
/home/david.spickett/llvm-project/lldb/unittests/Core/MangledTest.cpp:418:6: warning: mixture of designated and non-designated initializers in the same initializer list is a C99 extension [-Wc99-designator]
418 | .basename = "method",
| ^~~~~~~~~~~~~~~~~~~~
/home/david.spickett/llvm-project/lldb/unittests/Core/MangledTest.cpp:415:6: note: first non-designated initializer is here
415 | { "_ZNVKO3BarIN2ns3QuxIiEEE1CIPFi3FooIS_IiES6_EEE6methodIS6_EENS5_IT_SC_E5InnerIiEESD_SD_",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/david.spickett/llvm-project/lldb/unittests/Core/MangledTest.cpp:423:8: warning: designated initializers are a C++20 extension [-Wc++20-designator]
423 | { .BasenameRange = {6, 13}, .ScopeRange = {6, 6}, .ArgumentsRange = { 20, 27 }, .QualifiersRange = {38, 38} },
<...>
Member
Author
There was a problem hiding this comment.
Argh that's unfortunate. Let me see what we can do about that
Contributor
|
https://lab.llvm.org/buildbot/#/builders/197/builds/4470 is still broken |
Member
Author
i'll have a look |
Contributor
|
https://lab.llvm.org/buildbot/#/builders/141 is also broken. |
Contributor
|
The new failure |
Contributor
|
e6f7e34 broke lldb-remote-linux-win again. |
Member
Author
Yea i saw that. Will need to not generate PDB in those tests i guess |
Member
|
This was referenced Apr 27, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a series of patches that implements the infrastructure to enable highlighted/more compact C++ backtraces in LLDB (see this RFC).
We add a new per-language plugin setting (implemented for C++ as
plugin.cplusplus.display.function-name-format) which takes aframe-format-style string that dictates how to print a function name for frames in that language. By default${function.name-with-args}will try to ask the language plugin if it can print a name, if not it falls back to the previous way of printing names. Additionally we introduce several new frame-format variables (${function.scope},${function.basename}, etc.) that will only print that respective part of a demangled function name (this is also handled within a language plugin).The default format of the new
plugin.cplusplus.display.function-name-formatsetting is:which should reconstruct the demangled name in its entirety. A user can then choose to omit certain parts of the name or add arbitrary color highlighting.
Example highlighted backtrace:
Changes
TrackingOutputBufferGetDemangledNamethat will force re-demangling. This is required because LLDB willSetDemangledNamewithout going through the demangler. So we need a way to force demangling to set them_demangled_infomember when we need itGetDemangledNameto use theTrackingOutputBufferand populate them_demangled_infomemberframe-formatvariables and implements them in theCPlusPlusLanguagepluginplugin.cplusplus.display.function-name-formatsetting and makes the${function.name-with-args}query it for formatting the function name