[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?
| /// 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.
| /// 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
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.