-
Notifications
You must be signed in to change notification settings - Fork 10.5k
add range tracking when demangling a name #82298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add range tracking when demangling a name #82298
Conversation
c95ffe3
to
51fd790
Compare
Are these cherry-picks from (In case you are wondering: yes, this is exactly the inverse workflow that LLDB uses). |
class DemanglerPrinter; | ||
|
||
class TrackingDemanglerPrinter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class DemanglerPrinter; | |
class TrackingDemanglerPrinter; |
Don't think these are needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
/// Demangle the given symbol. | ||
/// | ||
/// \param MangledName The mangled symbol string, which start a mangling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// \param MangledName The mangled symbol string, which start a mangling | |
/// \param MangledName The mangled symbol string, which start with a mangling |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure where this came from, probably a bad copy paste. Removed in:
/// prefix: _T, _T0, $S, _$S. | ||
/// \param printer The NodePrinter that will be used to demangle the symbol. | ||
/// | ||
/// \returns The demangled string. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function doesn't return anything though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adjusted the doxygen of this method and a few others in:
demangleSymbolAsString(llvm::StringRef MangledName, | ||
const DemangleOptions &Options = DemangleOptions()); | ||
|
||
/// Demangle the given symbol. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets briefly describe what the intended usage of this overload is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you separate out the changes that move the NodePrinter
class into a separate NFC PR? And then only include the "range tracking" parts here?
Oh I see you did that in two separate commits. That should be fine then |
@@ -236,6 +237,18 @@ class Node { | |||
public: | |||
Kind getKind() const { return NodeKind; } | |||
|
|||
bool shouldTrackNameRange() const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's used here: https://github.com/swiftlang/llvm-project/pull/10710/files#diff-5c0cd3ffc206ebb1e41b4984b7629c896aa79951708915cfafff560b1da51aa1
The code was moved there instead because it does not make sense to have it in the swift repo. It has been removed in:
@@ -3568,6 +3568,43 @@ NodePointer NodePrinter::printEntity(NodePointer Entity, unsigned depth, | |||
return PostfixContext; | |||
} | |||
|
|||
void NodePrinter::printFunctionName(bool hasName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has this function just been moved? Or has it been adjusted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a new function which was extracted from the printEntity
function.
public: | ||
NodePrinter(DemangleOptions options) : Options(options) {} | ||
|
||
virtual ~NodePrinter() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
virtual ~NodePrinter() {} | |
virtual ~NodePrinter() = default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved the PR here: This allows to target main without pinging a lot of reviewers. |
This PR implements range tracking for the name and parameters of a demangled function.
Motivation
In this patch, @Michael137 implemented name highlighting for methods in the C++ plugin of LLDB. This results in better readability when reading backtraces of functions with long scopes.
I am working on implementing the same feature for the Swift plugin in LLDB. Please see an example below:

Before:
After:

Notice how the name and parameters of the functions are highlighted differently than the rest.
(Please note that the difference in text contents are temporary, and should disappear before for the final patch).
Implementation details
To implement this feature in the Swift plugin of LLDB, we need to provide LLDB with the ranges of the "components" of the demangled name. For instance the
baseName
ofbar.foo()
spans4...7
, while theparameters
span7...9
.This information allows LLDB to build the name that is displayed in the backtraces.
Original implementation
This patch introduces a new class called
TrackingDemanglerPrinter
which stores the information needed to track the ranges. It can later be extended to track more ranges. The methods it defines are called in theprint
method ofNodePrinter.cpp
.The previous behavior is still available when calling
DemangleSymbolAsString
without aprinter
, as this will result in a no op whenstartName
is called for instance.Current implementation
2 methods were made virtual in
NodePrinter
, so that any consumer can override them and track the ranges they need. This is less lldb specific than the previous method.Testing (removed, relevant only for the original implementation)
To implement the tests for the range tracking, I used the
manglings.txt
file. It now contains the ranges for the name and parameters where that's relevant. The format I chose to serialize the ranges is arbitrary and I'm happy to discuss any change, especially when considering we might want to add more tracked ranges.I also developed a Python script to help review the test cases by coloring them. Please find it here for reference. It could be interesting to add it to the repo.
Follow ups
swift-demangle
, as these changes make it fairly trivial to implement, and it would greatly help with readability.Please note that it was originally opened here #81511 but I moved to a different target branch.
This PR depends on #82303