kwk marked an inline comment as done.
kwk added a comment.
@labath please see my inline comment.
================
Comment at: lldb/source/Breakpoint/BreakpointResolverName.cpp:315
+ if (filter_by_cu && filter_by_function) {
+ // Keep this symbol context if it is a function call to a function
+ // whose declaration is located in a file that passes. This is needed
----------------
labath wrote:
> This mention of a "function call" is confusing. Either a function is in a
> file or it isn't. Why do we care about who calls who?
@labath we have this `filter_by_function` as an indirection to let
`BreakpointResolverName::SearchCallback` know that the `SearchFilter` needs a
special handling. But while reviewing the code now I noticed that only the
implementations of `SearchFilter::GetFilterRequiredItems` currently either
return
- `eSymbolContextModule` (for `SearchFilterByModule` and
`SearchFilterByModuleList`) or
- `eSymbolContextModule | eSymbolContextCompUnit | eSymbolContextFunction`
(for `SearchFilterByModuleListAndCU`).
I thought this was clever at a time when the old
`SearchFilterByModuleListAndCU` which then only returned `eSymbolContextModule
| eSymbolContextCompUnit `. With my new search filter I wanted a special
handling which is why I wrote a new search filter class and had it return
`eSymbolContextModule | eSymbolContextCompUnit | eSymbolContextFunction`. But
when you confirmed my question to change the default behavior
(https://reviews.llvm.org/D74136#1869622), the new class became the default
implementation and there no longer is a need to distinguish the required items
for this search filter.
Makes sense?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74136/new/
https://reviews.llvm.org/D74136
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits