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