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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D74136#1862200 <https://reviews.llvm.org/D74136#1862200>, @jingham wrote:

> Is there ever a reason other than performance why you would want NOT to consult both the Compile Unit name and also look for DW_AT_decl_file?  That doesn't seem clear to me.
>
> If the only worry is performance, and except for that you would really always want the full search, then I don't see what we gain by adding --compile-unit as distinct from --file.  We should use target.inline-breakpoint-strategy to allow people to turn on or off the searching for DW_AT_decl_file for folks whose projects are complicated enough AND who aren't doing a lot of inlining.  "break set" is already over-stuffed, I don't want to add to it unless we get a positive benefit.


Well, conceptually these are two different things. --compile-unit would restrict the search to the given compile unit no matter what is the file where the function was defined in. And --file would consider the defining file, and not care which compile unit hosts that. In theory, both can be useful, but I'm not sure if people would really use that flexibility.

My proposal was based on the fact that we want to preserve the ability to filter according to the defining cu. If we don't need to do that, then simply just redefining the meaning of the file+function combo is fine by me..

> On a side note, I really wish I had made "break set" be a multi-word command, so you could specify the main search criteria, and then the qualifiers could be used more flexibly and be better documented and and you wouldn't see options that weren't relevant.  Like:
> 
> break set name
>  break set source
>  break set address
> 
> That wouldn't make the command any harder to type, it would even be easier since:
> 
> break set name foo
> 
> would shorten to:
> 
> br s n foo
> 
> instead of
> 
> br s -n foo
> 
> I wonder if it is too late to make this change?

That would be a bigger change than what I proposed because it would affect every breakpoint command, instead of just the (rarely used, I think) file+function combo, though I'm not necessarily against that. However, if we redefine the meaning of the file+function combo, then I think the need for this will be diminished because the option will become less context sensitive and thus more predictable and more combinable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74136