jarin marked an inline comment as done.
jarin added inline comments.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp:45
if (name_type_mask & eFunctionNameTypeFull) {
- dies.push_back(die);
- return;
+ if (!die.IsMethod() || die.GetMangledName(false) == name) {
+ dies.push_back(die);
----------------
jarin wrote:
> labath wrote:
> > I don't believe the `IsMethod` check is really needed here -- the mangled
> > name check should handle everything.
> >
> > In fact, looking at the implementation of
> > `DWARFDebugInfoEntry::GetMangledName`, I don't think you even need the
> > extra `substitute_name_allowed=false` part. The default value should do
> > exactly what we need. If the DIE has a linkage (mangled) name it will
> > return it and we will use that for comparison. For an `extern "C"` function
> > it will return the regular name, and we will compare that instead (this
> > check is somewhat redundant because if the name doesn't match, the function
> > should not be in the index in the first place, but I don't think it hurts
> > to check either).
> >
> > Am I missing something?
> I want to avoid returning methods to [[
> https://github.com/llvm/llvm-project/blob/808142876c10b52e7ee57cdc6dcf0acc5c97c1b7/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp#L1258
> | ClangExpressionDeclMap::LookupFunction ]], so that
> ClangExpressionDeclMap::LookupFunction does not do the expensive [[
> https://github.com/llvm/llvm-project/blob/808142876c10b52e7ee57cdc6dcf0acc5c97c1b7/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp#L1298
> | DeclContext parsing ]] just to find out the function [[
> https://github.com/llvm/llvm-project/blob/808142876c10b52e7ee57cdc6dcf0acc5c97c1b7/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp#L1304
> | is a method ]] and must be thrown away.
>
> The motivation is pretty much the same as for
> https://reviews.llvm.org/D70846.
>
> Does it make sense?
I think I see what you are saying - in the test case below, FULL should be
consistent with FULL-INDEXED. Let me remove the IsMethod check and merge the
FULL and FULL-INDEXED test cases below.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73191/new/
https://reviews.llvm.org/D73191
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
