[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

labath closed this revision. labath added a comment. Committed as a705cf1ac . It should be worth noting that this actually fixes TestPrintf.py (pr36715), because we no longer end up calling the member printf function. So

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

PatriosTheGreat updated this revision to Diff 237604. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70846/new/ https://reviews.llvm.org/D70846 Files: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

PatriosTheGreat added a comment. Made clang-format with last update of diff. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70846/new/ https://reviews.llvm.org/D70846 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

PatriosTheGreat marked an inline comment as done. PatriosTheGreat added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp:67 + +// FULL: Found 0 functions: shafik wrote: > You don't actually run a `FULL` test, is this

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

shafik added a comment. In D70846#1809837 , @labath wrote: > This looks fine to me. @shafik, @teemperor, do you have any more comments? Just a minor comment. Comment at: lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp:67 +

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This looks fine to me. @shafik, @teemperor, do you have any more comments? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70846/new/ https://reviews.llvm.org/D70846

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

PatriosTheGreat updated this revision to Diff 234899. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70846/new/ https://reviews.llvm.org/D70846 Files: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

labath added a comment. I wouldn't want to do (3). I think the ideal solution would be (2). AppleIndex should also be able to do this kind of filtering, only it'd have to be done differently -- after looking up the name in the accelerator table, it would go to the debug info, and check what

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

PatriosTheGreat added a comment. It seems like with this CL find-basic-functions logic became inconsistent for macos target and pc-linux. In cases of pc-linux it returns no function with --function-flags=full flag and in macos it returns 7 functions. I think there are 3 ways to fix this: 1.

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

shafik added a comment. In D70846#1767455 , @labath wrote: > I guess you were building darwin binaries, right? If that's the case, then > you weren't using this code at all, as apple uses AppleIndex by default. The > test in question uses all three

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

labath added a comment. I guess you were building darwin binaries, right? If that's the case, then you weren't using this code at all, as apple uses AppleIndex by default. The test in question uses all three indexes (it forces their generation by altering compile flags) and the failures you

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

shafik added a comment. In D70846#1766598 , @labath wrote: > In D70846#1766204 , @shafik wrote: > > > In D70846#1763731 , @labath wrote: > > > > > There's `lldb-shell ::

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

labath added a comment. In D70846#1766204 , @shafik wrote: > In D70846#1763731 , @labath wrote: > > > There's `lldb-shell :: SymbolFile/DWARF/find-basic-function.cpp`, which > > probably didn't get run for you as

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

shafik added a comment. In D70846#1763731 , @labath wrote: > There's `lldb-shell :: SymbolFile/DWARF/find-basic-function.cpp`, which > probably didn't get run for you as you didn't have lld enabled (cmake > -DLLVM_ENABLE_PROJECTS=...;lld). You'll need

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

shafik added subscribers: clayborg, shafik. shafik added a comment. In D70846#1763802 , @teemperor wrote: > This LGTM, but the TODO directly above the change in > ClangExpressionDeclMap.cpp worries me a bit. I am not sure how good our test > coverage is

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

teemperor added a comment. This LGTM, but the TODO directly above the change in ClangExpressionDeclMap.cpp worries me a bit. I am not sure how good our test coverage is for calling functions when there are instance methods in scope (e.g., we are in instance method and try to call another

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

labath added a comment. There's `lldb-shell :: SymbolFile/DWARF/find-basic-function.cpp`, which probably didn't get run for you as you didn't have lld enabled (cmake -DLLVM_ENABLE_PROJECTS=...;lld). You'll need to update that test to match the new behavior, but other than that, I think this is

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

PatriosTheGreat created this revision. PatriosTheGreat added reviewers: labath, jarin, aprantl. PatriosTheGreat added a project: LLDB. Herald added a subscriber: arphaman. This change is connected with https://reviews.llvm.org/D69843 In large codebases, we sometimes see Module::FindFunctions