[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

Fri Oct 28 01:05:06 PDT 2022

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

In D134378#3890302 <https://reviews.llvm.org/D134378#3890302>, @aeubanks wrote:

> regarding the failure in TestCPPBreakpointLocations.py (added recently in https://reviews.llvm.org/D135921, seems like it's catching real issues with this patch), it looks like the manual dwarf index needs a similar fix to this
>
> In D134378#3888462 <https://reviews.llvm.org/D134378#3888462>, @labath wrote:
>
>> I can't say I fully understand all of this code, but I also don't know who would, so I guess I'll just say it "looks good" :)
>>
>> I am wondering about the testing situation though. If I understand correctly, you've run the test suite with hardcoded simplified names, and it all passed (?)
>
> yes, that was true until https://reviews.llvm.org/D135921, which catches some missing functionality in this patch around breakpoints (I'd still like to defer that to a separate patch)

Yes, sounds good.

Can you elaborate on the manual index issue. My impression that it just takes the strings from inside DW_AT_name, and so it should behave similarly (identically?) to a compiler index -- though, in this case, we have direct control over it, so we could e.g. make it always store simplified names regardless of the debug info.

>> I am definitely not suggesting we add a new test suite mode for that, but maybe we could extend this one test case with extra check that look at the type names in other contexts than in name lookup (e.g. expression evaluation, backtraces, ???) -- just to make sure that something doesn't break there in the future. WDYT?
>
> I've added expression evaluation to the test. do you have examples of backtrace tests?

I don't have anything particular in mind. I figured you'd just stop the middle of some templated member function and run `bt`. Though, when I think of it now, these names come from the demangler, so they're probably not particularly interesting here. (Unless you build with -gsce, but in that case, they should come from the same code that is doing the type templatification).

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2509
+        TypeSP Ty = GetTypeForDIE(die);
+        llvm::StringRef qualName = Ty->GetBaseName().AsCString();
+        auto it = qualName.find('<');
----------------
This isn't a qualified name anymore. I guess every instance of the string "qual" in this block should be replaced or deleted.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134378/new/

https://reviews.llvm.org/D134378