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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D74136#1862246 <https://reviews.llvm.org/D74136#1862246>, @labath wrote:

> 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..


Yeah, I'm not convinced the flexibility is worth adding another slot, especially since now --file needs to explain how it is different from --compile-unit, but ONLY when --name is specified.  When --file is the primary (for source breakpoints) that won't be true.  And why is there a --compile-unit that DOESN'T set file & line breakpoints, etc...  The fact that --file has different meanings depending on what the primary specification is is one of the reasons why it's a shame that they all share one common pool of documentable options.

> 
> 
>> 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.

Yes, I wasn't suggesting as blocking this change.  I just feel like at some point we're going to tip over into too much complexity as things are currently constituted, so someday we'll probably go crazy if we don't rework this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74136