[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Well.. first of let me say that the performance gain is impressive. I wouldn't have expected to gain that much with a relatively simple change.

Now, as for the implementation, I have two main questions:

- Do we really need the `GetQualifiedNameWithParams` function? My impression was that we've been moving towards ignoring function arguments for name matching, and `CPlusPlusLanguage::DemangledNameContainsPath` does not look like it is making use of that. Note that I don't think that switching to `GetQualifiedName` is the right approach either. I think we should use the mangled name (`GetMangledName`) first, and only fall back to `GetQualifiedName` if that is unavailable (you can look up how it works during SymbolContext construction, but I believe it is roughly like that. (Incidentally, that will bring in the function arguments through the back door for some/most functions, but that isn't my motivation.)
- This patch is duplicating some of the logic inside `Module::LookupInfo::Prune`. I get that we can't reuse it as-is (because we'd need a `SymbolContext` object), but it seems like that function only cares about the `GetFunctionName` portion of the object, so it seems like it should be possible to refactor it such that one can reuse it from within this new context (just pass in a name instead of a full SymbolContext). (I guess this part could be a separate patch)



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:819-822
+      if (iterated) {
+        storage.pop_back();
+        storage.pop_back();
+      }
----------------
This is an interesting approach, but I'd just use llvm::ListSeparator for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129682