[Syntax Highlighting] Add name and parameters syntax highlighting in Swift backtraces #10710
Conversation
charles-zablit
changed the title
|
Are there some bits (such as the prefix and suffix) that we could do upstream? Even with an empty default implementation, to document the settings etc? |
llvm#140762 implements the "generic" parts of the PR that are a good fit for upstream. |
This PR is a subset of the commits made in swiftlang#10710. The most notable change is the addition of `PrefixRange` and `SuffixRange` which are a catch-all to track anything after or before a function's demangled name. In the case of Swift, this allows to add support for name highlighting without having to track the range of the scope and specifiers of a function (this will come in another PR).
This PR is a subset of the commits made in swiftlang/llvm-project#10710. The most notable change is the addition of `PrefixRange` and `SuffixRange` which are a catch-all to track anything after or before a function's demangled name. In the case of Swift, this allows to add support for name highlighting without having to track the range of the scope and specifiers of a function (this will come in another PR).
| return false; | ||
| } | ||
|
|
||
| bool SwiftLanguage::GetFunctionDisplayArgs( |
There was a problem hiding this comment.
Lets add this helper function in a separate patch. Makes it easier to review
There was a problem hiding this comment.
This method is needed for the implementation of bool SwiftLanguage::HandleFrameFormatVariable. I don't think I can remove it without copy pasting the logic at the call site.
Say this is PR A. Would it be OK to create a new PR B which introduces this helper function, merge PR B and then rebase the PR A on the changes introduced by PR B?
There was a problem hiding this comment.
Say this is PR A. Would it be OK to create a new PR B which introduces this helper function, merge PR B and then rebase the PR A on the changes introduced by PR B?
Yup that's the way to go
There was a problem hiding this comment.
Opened the PR here and ended up extracting 2 helper functions: #10766
| /// Returns \c true if this object holds a valid basename range. | ||
| bool hasBasename() const { | ||
| return BasenameRange.second > BasenameRange.first && | ||
| BasenameRange.second > 0; | ||
| return BasenameRange.second > BasenameRange.first; |
There was a problem hiding this comment.
Sorry if you've previously answered this, but why do we need this change?
There was a problem hiding this comment.
If we do require it, we should have that upstream too
There was a problem hiding this comment.
It was not needed but since I added the hasArguments method I wanted to remove the redundant >0 check.
Since the start and end range are unsigned, if end > start, then end is always greater than 0, which is why I removed the second check. I might be missing something.
I'm happy to remove the change to minimize changes in the PR.
There was a problem hiding this comment.
Ahh yes good catch. Can you fix this upstream and then cherry-pick it here?
My intention might've been to write BasenameRange.first > 0. Probably leftover from early iterations of that patch
| /// Indicates the [start, end) of the function's prefix. This is a | ||
| /// catch-all range for anything that is not tracked by the rest of | ||
| /// the pairs. | ||
| std::pair<size_t, size_t> PrefixRange{}; |
charles-zablit
force-pushed
the
charles-zablit/lldb/swift-mangling
branch
from
e2ae90b to
7113755
Compare
| /// Indicates the [start, end) of the function's suffix. This is a | ||
| /// catch-all range for anything that is not tracked by the rest of | ||
| /// the pairs. | ||
| std::pair<size_t, size_t> SuffixRange; |
There was a problem hiding this comment.
Can you cherry-pick/merge the upstream patch to 6.2? Then rebase this patch. So the diff is smaller
This PR is a subset of the commits made in #10710. The most notable change is the addition of `PrefixRange` and `SuffixRange` which are a catch-all to track anything after or before a function's demangled name. In the case of Swift, this allows to add support for name highlighting without having to track the range of the scope and specifiers of a function (this will come in another PR).
* [Demangling] Refactor Demangler range tracking (llvm#140762) This PR is a subset of the commits made in #10710. The most notable change is the addition of `PrefixRange` and `SuffixRange` which are a catch-all to track anything after or before a function's demangled name. In the case of Swift, this allows to add support for name highlighting without having to track the range of the scope and specifiers of a function (this will come in another PR). * add explicit default initialization to DemangledNameInfo to remove warning (llvm#141790) llvm#140762 introduces some compilation warnings in `lldb/unittests/Core/MangledTest.cpp`. This patch adds explicit default initialization to `DemangledNameInfo` to suppress those warnings. We only had the default initialization values to `PrefixRange` and `SuffixRange` because they are the only _optional_ fields of the structure.
This PR is a subset of the commits made in swiftlang#10710. The most notable change is the addition of `PrefixRange` and `SuffixRange` which are a catch-all to track anything after or before a function's demangled name. In the case of Swift, this allows to add support for name highlighting without having to track the range of the scope and specifiers of a function (this will come in another PR).
This PR adds function name syntax highlighting in LLDB Swift backtraces.
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.
Please find below an example of the same feature, ported to Swift (which this patch implements):
Before:
After:
Implementation details
swiftlang/swift#81511 implements a way to override the
NodePrinter, which allows to track relevant ranges. Once the correct ranges are tracked, we return a slice of the demangled string for the relevant part of the highlighting query.The rest of the implementation is a close copy of @Michael137 's work.
Testing
The tests are implemented using the
manglings.txtfile in the swift repo.Follow ups
prefixandsuffixranges. We could track those ranges as well to grey out some information that are not important, eg the fact the function is fileprivate.