[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Follow DW_AT_decl_file when setting breakpoint

[Pavel Labath via Phabricator via lldb-commits]

labath marked an inline comment as done.
labath added a comment.

In D74136#2085076 <https://reviews.llvm.org/D74136#2085076>, @kwk wrote:

> IMPORTANT: The behavior of `target.inline-breakpoint-strategy` when set to `headers` is still subject to change!
>
> I think the setting is not respected correctly...


Yes. I'm guessing you also need a call to `FileSpec::IsSourceImplementationFile` somewhere. The logic should be different only if `target.inline-breakpoint-strategy` is`headers` AND `FileSpec::IsSourceImplementationFile` is true.

To Jim: I'm still unable to decide whether we really should be respecting the inline-breakpoint-strategy setting or not in this case. I mean, this is an optimization for setting file+line breakpoints, but in this case it does not buy us anything, since we search for the functions by name. OTOH, it is nice to have a consistent treatment of the `--file` argument to `breakpoint set`...



================
Comment at: lldb/test/Shell/Breakpoint/search-support-files.test:77-80
+settings set target.inline-breakpoint-strategy headers
+breakpoint set -n function_in_header -f search-support-files.h
+# CHECK: (lldb) breakpoint set -n function_in_header -f search-support-files.h
+# CHECK-NEXT: Breakpoint 10: no locations (pending).
----------------
The function in a .h file should be found regardless of the value of the setting.


================
Comment at: lldb/test/Shell/Breakpoint/search-support-files.test:88-91
+settings set target.inline-breakpoint-strategy headers
+breakpoint set -n func -f search-support-files-func.cpp
+# CHECK: (lldb) breakpoint set -n func -f search-support-files-func.cpp
+# CHECK-NEXT: Breakpoint 12: no locations (pending).
----------------
This is right.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136