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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
