[Demangling] Add name and parameters range tracking when demangling a name

[charles-zablit]

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 of bar.foo() spans 4...7, while the parameters span 7...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 the print method of NodePrinter.cpp.

The previous behavior is still available when calling DemangleSymbolAsString without a printer, as this will result in a no op when startName 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

• We will need to track more than just the name and the parameters to implement the changes in the LLDB plugin. We will also need the qualifiers, return type and probably a catch all "suffix".
• It could be interesting to implement syntax highlighting in swift-demangle, as these changes make it fairly trivial to implement, and it would greatly help with readability.

[Michael137]

I don't mind adding these new callbacks into the demangler. Though I'll raise the same question Richard Smith raised on the Itanium demangler review: llvm/llvm-project#133249 (comment)

This might be overly specific to LLDB's use-case. A more general approach would be to override the print functions in the DemanglerPrinter. Then LLDB can do it's on tracking without putting any of it into the demangler logic. Wdyt?

[Michael137]

Having two separate members feels a bit weird. Could we just make Printer a std::unique_ptr<DemanglerPrinter>? And take a std::unique_ptr<DemanglerPrinter> in the constructor? And if it's nullptr we just make_unique

[charles-zablit]

It didn't feel right yes. I initially wanted to pass an std::optional, but couldn't because it wouldn't allow me to pass in a child class. I will give it another go with your suggestion.

[charles-zablit]

I think I managed to get it to a good state. I decided to use a template instead of changing all the Printer << ... calls to (*Printer) << .... I found out about decltype on this SO thread: https://stackoverflow.com/questions/18815221/what-is-decltype-and-how-is-it-used

[charles-zablit]

This might be overly specific to LLDB's use-case. A more general approach would be to override the print functions in the DemanglerPrinter. Then LLDB can do it's on tracking without putting any of it into the demangler logic. Wdyt?

I agree this would be very specific to LLDB.

When you say "override the print functions in DemanglerPrinter, are you talking about NodePrinter::printEntity for instance? My understanding is that we can have, let's say a NodePrinter::printName method which is called by NodePrinter::printEntity and our override would look like:

// in lldb
NodePointer NodePrinterOverride::printName() {
  startName();
  NodePrinter::printName();
  endName();
}

The logic inside startName and endName would be in the lldb codebase.

But I'm not sure how to implement tests for this. The ones in manglings.txt make sure that startName and endName are called at the correct places. If there is a change to where printName is called in NodePrinter.cpp, this will not break any tests in the swift codebase but will break some in LLDB.

Do you have any recommendations on how I could implement the tests?

[Michael137]

This might be overly specific to LLDB's use-case. A more general approach would be to override the print functions in the DemanglerPrinter. Then LLDB can do it's on tracking without putting any of it into the demangler logic. Wdyt?

I agree this would be very specific to LLDB.

When you say "override the print functions in DemanglerPrinter, are you talking about NodePrinter::printEntity for instance? My understanding is that we can have, let's say a NodePrinter::printName method which is called by NodePrinter::printEntity and our override would look like:

// in lldb
NodePointer NodePrinterOverride::printName() {
  startName();
  NodePrinter::printName();
  endName();
}

The logic inside startName and endName would be in the lldb codebase.

But I'm not sure how to implement tests for this. The ones in manglings.txt make sure that startName and endName are called at the correct places. If there is a change to where printName is called in NodePrinter.cpp, this will not break any tests in the swift codebase but will break some in LLDB.

Do you have any recommendations on how I could implement the tests?

You could have tests that take as input all the mangled names in manglings.txt. Then use the NodePrinterOverride to reconstruct the demangled name from the locations you tracked. And compare that name to what the swift-demangler returns. Those two should always be the same. That's what I'm currently proposing for the C++ version of this. See: llvm/llvm-project#137793

[charles-zablit]

I have moved the logic outside of the NodePrinter class. Now LLDB can override virtual functions by passing its own NodePrinter child class.

I have kept the tests for demangle.swift, and created a simple class that does range tracking in swift-demangle. I thought it would be a good way to ensure the functionality of the changes, as well as a starting point to implement syntax highlighting in swift-demangle.

I'm happy to remove this if that's outside the scope of this patch.

EDIT: I ended up removing the tests and the swift-demangle features, in order to split the PR.

[charles-zablit]

@swift-ci test