[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

[Jim Ingham via lldb-commits]

> On Oct 31, 2017, at 9:05 AM, Zachary Turner <zturner at google.com> wrote:
> 
> This is a case where I think a new API *would* be warranted.  But it would not be an API specifically for the -m option.  It would instead be an API that introduces an `SBBreakpointOptions` class, and then add a method called `BreakpointCreateWithOptions(options)`.

That's almost right.  I want to keep the options that determine WHERE locations get set separate from the options that determine what happens WHEN a breakpoint gets hit.  The "reaction" options can be shared with the SBBreakpointName class and with Stop Hooks, and with a little fiddling should also be sharable with Watchpoints.  The only difference between watchpoints & breakpoints is the meaning of the ID they get passed however.  If you really wanted to share the options broadly, we'd probably have to also tell you what kind of stop this was for.  Anyway, for these reasons it is valuable to keep this part of the breakpoint specification separate.  But at the lldb_private layer these are already all gathered into their own class so promoting them would be straightforward.

The setting options aren't factored into their own class at the lldb_private layer.  They get used directly to make the search filters & kernels that actually implement the breakpoint, so that class wasn't really needed.  But it wouldn't be that hard to make an intermediary that gathers up these options for convenience.  Then you'd have:

SBBreakpoint BreakpointCreateWithOptions(SBBreakpointSettingOptions &setting_options, SBStoppointOptions &reaction_options);

> 
> Note that the general policy of the SB API is that you cannot delete or modify an existing method.  This kind of pattern provides an API that *can* be extended in the future, because you could, for example, add fields to SBBreakpointOptions and existing code wouldn't break, but new code could take advantage of the new options.
> 

Yes, we've been trying to convert all the complex information passing API's to this form both because it makes code using them easier to read and it's extensible.

Jim


> On Tue, Oct 31, 2017 at 8:32 AM Don Hinton <hintonda at gmail.com> wrote:
> Btw, is there a way to pass the '-m' option via the SB API?  I'd like to exclude matches in comments when using BreakpointCreateBySourceRegex.
> 
> On Tue, Oct 31, 2017 at 8:26 AM, Don Hinton <hintonda at gmail.com> wrote:
> On Tue, Oct 31, 2017 at 8:22 AM, Zachary Turner <zturner at google.com> wrote:
> 
> 
> On Tue, Oct 31, 2017 at 8:12 AM Don Hinton <hintonda at gmail.com> wrote:
> There have been a few suggestions that I could just use a script to solve this "problem" -- poor startup performance of clangdiag.
> 
> However, this patch was not submitted to solve a particular problem.  It was submitted in response to Jim's suggestion:
> 
> On Mon, Oct 23, 2017 at 6:25 PM, Jim Ingham <jingham at apple.com> wrote:
> Yeah, that would be easy to implement from the command line, maybe add a --file-is-regex flag or something.
> 
> From the SB API it would be better to have something like:
> 
> SBFileList SBTarget.GetFileListMatchingRegex("regex")
> 
> Please file an enhancement request for these of hack'em in if you're so motivated.
> 
> clangdiag was only provided as a motivating example.
> 
> Fair point, I had forgotten about that.  That said, I'm honestly still not that big of a fan.   It's no secret that I have a higher bar than others for options and API additions, but to me the potential use this particular option + SB API would get is not sufficient to justify the long term maintenance burden associated with having it.
> 
> I'm not that familiar with the SB API part, but will investigate, and try to come up with a better motivation.
> 
> Thanks again for the suggestions and feedback...
>  
> 
>