@jankratochvil out of curiosity. Can you give a pointer to what the C program looks like that produces the object code? I know GCC might change and no longer produce this code so it's better to have the obj code instead. But still I'm not nearly close to understanding

Fixup

Done. Thank you!

Incorporated Galina's feedback

LGTM but a test would indeed be nice.

In D83580#2144871, @serge-sans-paille wrote:

Possible followup: shouldn't all the other Get* be flagged as const too?

• Simplify logic

• Add newlines

• bring back logic to keep a symbol context when a function passes and add a comment as Jan suggested
• remove test from scripted resolver that calls SearchFilterByModulesAndSupportFiles::AddressPasses
  • before the test checked that a non existing file doesn't cause setting a breakpoint location
  • the logic in AddressPasses before was to identify the CU of a symbol context and check if it's file is in the list of files that are allowed to pass. We agreed to change this logic so that not only CU's are checked but also files in which a function is declared. SearchFilterByModulesAndSupportFiles::FunctionPasses now does exactly that but it is not called from the BreakPointResolverScripted class, only from BreakpointResolverName.
  • It is an open question how to deal with this and I hope Jim can help here.
  • Shall we maybe take another file from the SymbolContext to see if we can filter by that in AddressPasses? Here's a dump of the filter context for the removed test:

• Remove old logic that was no longer needed since my search filter now adaptively adds eSymbolContextCompUnit and not always returns it.

@labath @jingham @jankratochvil The test change suggested by @labath is now in place and it works. @jankratochvil, I've removed the logic that checks seomthing with the CU from AddressPasses. That logic now lives in FunctionPasses where it logically makes more sense to me. The whole filtering by CU logic from AddressPasses is gone now and I wonder if @jingham has some thoughts on this.

• Align tests with reviewer expectations

@labath I've applied all the ideas we ping-ponged yesterday and I decided to go with alternating the target.inline-breakpoint-strategy from always (the default) to headers. This way you can exactly see in the test file how things are behaving. So before going into the actual code review I'd like to ask you and @jingham to take a look at the test file. Is it the behavior described there the desired outcome? Then we can discuss the implementation.

• remove debug output from test

• Fix comment

• remove commented out code

• Outsource parts into SearchFilterByModulesAndSupportFiles::FunctionPasses
• Tests with alternating setting target.inline-breakpoint-strategy between "always" and "headers"
• Respecting target.inline-breakpoint-strategy setting in SearchFilterByModulesAndSupportFiles and maded adjustments in BreakpointResolverName::SearchCallback
• Renamed SearchFilterByModuleListAndCUList to SearchFilterByModulesAndSupportFiles

In D80531#2069383, @njames93 wrote:

LGTM with 2 small nits, but I'd still give a few days see if anyone else wants to weight in.

• Return macro name by reference
• Add colon to paragraph showing code

Done.

Rebased

Marked more comments as "done" after going through the code again.

@njames93 I've addressed all your comments and hope the patch is good to land now :)

• Don't store SourceManager but get it from Preprocessor
• Use getName instead of getNameStart
• Remove FIXME
• change warning message
• doxygen: \a -> \p
• Make MacroName private and add getter

@labath please see my inline comment.

@njames93 and @Eugene.Zelenko do you guys think this patch is ready to be approved and land?

• don't hard-code --color and --dump-input on FileCheck invocation

• make test CHECKs less strict

• use %t in files created in tests

• rebase
• Rename function in test from inlined_42 to function_in_header
• Typo: compulation -> compilation

@Eugene.Zelenko thank you for your patience with me. I fixed the link.

• fix link in doc

• Adjust link to documentation

• Refer to modernize-use-equals-delete for warning about deleted functions in private sections

Thanks @njames93 and @Eugene.Zelenko for your review. Most of it, I addressed but for some I have comments. See inline.

• Avoid constructing string
• CamelCase for vars
• more readable snippets for documentation
• Added empty line after header in documentation
• Remove sentence from doc
• Remove unimportant tests

@njames93 I've implemented everything you've mentioned. This simplified a lot.

• Wrap RUN lines
• Use CamelCase variable name
• Simplify access to options
• Make check options const
• Use two-line placeholder
• Automate placement of semicolon at the end

[lldb] Manual remove of DISALLOW_COPY_AND_ASSIGN def and one expansion

Sorry, wrong revisions uploaded

Thank you @njames93 for the review. I'm implementing what you wrote.

• Move comment about FinalizeWithSemicolon into code

@Eugene.Zelenko thank you for the review. I've fixed all places that you've mentioned. And have a question about one thing in particular. See inline.

• Make ReplaceDisallowCopyAndAssignMacroCheck only work in C++11 and above
• Remove unrelated changes from clang-tools-extra/docs/clang-tidy/checks/list.rst
• documentation simplification
• Move comment into code
• use single ticks

I've marked some mistakes that were not addressed yet but are marked as "Done".

The only code that is currently after addDecl is changing whether the special members are
defaulted/trivial. I'm not sure if this actually fixes anything but it's more correct than what we
did before.

In D75750#1988330, @labath wrote:

lldb-dev is indeed a better place for the architectural discussion. However, moving the discussion there does not automatically unblock this patch. "get something in now and improve the architecture later" almost never works out in practice. In fact I would say that adding debuginfod is a good way to cement the status quo.

All tests pass. I first thought that the lldb/test/Shell/Reproducer/TestGDBRemoteRepro.test test didn't work but it seems to be flaky.