[Lldb-commits] [PATCH] D137098: [lldb] Support simplified template names in the manual index

Wed Nov 2 06:42:09 PDT 2022

labath added a comment.

Why is it that the other indexes don't need an equivalent fix? Could it be that you just haven't tried those code paths?

If they do need it, then it'd be good if we could make the fix in a single place. Possibly by putting the retry logic at a higher level?

In D137098#3897322 <https://reviews.llvm.org/D137098#3897322>, @dblaikie wrote:

> In D137098#3897289 <https://reviews.llvm.org/D137098#3897289>, @aeubanks wrote:
>
>> updated description with why this doesn't produce false positives with breakpoints
>>
>> this doesn't support regex function name lookup, not sure if we care enough about the interaction between regexes/function names under simple template names. if we do, we could instead add template parameters to the names we put in the manual index. I did take a quick look at doing that but it'd require more work
>
> Presumably that'd then lead to divergence between manual and automatic index - which would be bad. So if this behavior is the same between automatic and manual index with simplified template names, that's probably good.

In addition to the divergence, the building the manual index is possibly the most performance-critical part of lldb, so doing all this extra work to reconstruct the templated names is probably a non-starter. If we wanted to support that, then we'd have to come up with a completely different way to achieve implement this functionality.

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:450
   }
   if (name_type_mask & eFunctionNameTypeBase) {
     if (!m_set.function_basenames.Find(
----------------
Are you sure this doesn't need to be repeated for `eFunctionNameTypeBase`? that's where non-member functions end up...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137098