jankratochvil added a comment.
I would separate/remove all [nfc] changes from this patch as they complicate it
a bit. Such cleaned up patch I have prepared for myself:
https://people.redhat.com/jkratoch/D74136-cleanup.patch
Then also did you notice there is a regression for (I do not know why):
FAIL: LLDB
(/home/jkratoch/redhat/llvm-monorepo-clangassert/bin/clang-11-x86_64) ::
test_scripted_resolver (TestScriptedResolver.TestScriptedResolver)
File
"/home/jkratoch/redhat/llvm-monorepo/lldb/test/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py",
line 23, in test_scripted_resolver
self.do_test()
File
"/home/jkratoch/redhat/llvm-monorepo/lldb/test/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py",
line 152, in do_test
self.assertEqual(wrong[i].GetNumLocations(), 0, "Breakpoint %d has
locations."%(i))
AssertionError: 1 != 0 : Breakpoint 1 has locations.
================
Comment at: lldb/source/Breakpoint/BreakpointResolverName.cpp:292
+ filter_by_function), // include symbols only if we
+ // aren't filtering by CU or function
include_inlines, func_list);
----------------
[nfc] It could use `include_symbols` here.
================
Comment at: lldb/source/Core/SearchFilter.cpp:713
+ if (!type)
+ return SearchFilterByModuleList::FunctionPasses(function);
+
----------------
If we cannot determine which file the function is from then rather ignore it
(the current call returns `true`):
```
return false;
```
================
Comment at: lldb/source/Core/SearchFilter.cpp:726
+ if (m_target_sp &&
+ m_target_sp->GetInlineStrategy() == eInlineBreakpointsHeaders) {
+ if (!sym_ctx.comp_unit) {
----------------
This value should be cached.
But besides that there should be `GetInlineStrategy() ==
eInlineBreakpointsNever || (GetInlineStrategy() == eInlineBreakpointsHeaders &&
cu_spec.IsSourceImplementationFile()modulosomeReverseRemapPath?`.
As for `eInlineBreakpointsAlways` the check is being delegated to
`FunctionPasses()`.
================
Comment at: lldb/source/Core/SearchFilter.cpp:732
+ FileSpec cu_spec;
+ if (sym_ctx.comp_unit) {
+ cu_spec = sym_ctx.comp_unit->GetPrimaryFile();
----------------
This condition is always `true` as there is already above:
```
if (!sym_ctx.comp_unit)
```
================
Comment at: lldb/source/Core/SearchFilter.cpp:835
+ if (m_target_sp &&
+ m_target_sp->GetInlineStrategy() == eInlineBreakpointsHeaders)
+ return flags | eSymbolContextCompUnit;
----------------
Here should be `eInlineBreakpointsNever || eInlineBreakpointsHeaders` (and
probably cache the `GetInlineStrategy()` value).
For `eInlineBreakpointsHeaders` we do not know for sure as we cannot call
`IsSourceImplementationFile()` for `cu_spec` which is not known yet here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74136/new/
https://reviews.llvm.org/D74136
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
