clayborg wrote: > > > Just to be clear, I need to find a solution for > > > SBTarget::CreateWatchpoint before this PR is ready to land. I'm inclined > > > towards adding a third bool parameter 'modify', but I'm not sure that's > > > the best choice. > > > > > > Anytime we keep adding more options to an API call we have two options: > > Yeah exactly, I wasn't restating the options as clearly in that last comment, > that's exactly what I meant. I can't think of other options we would want to > build in to a Watchpoint at creation time - but the argument for > SBWatchpointOptions is that ten-years-in-the-future-Jason may want another > flag.
The SBTarget::Launch(SBLaunchInfo) and SBTarget::Attach(SBAttachInfo) are some of the best example of using an options class. Breakpoints never tried to use an options class, we just kept adding new APIs which makes our API a bit messier, but we must keep APIs after they are added. So I would vote for the SBWatchpointOptions method. In the future we might have move complex ways to track watchpoints or want to track multiple address ranges with a single watchpoint? > > Probably the best thing is to look at the SBTarget BreakpointCreate* methods; > there are a dozen different methods for the different types of breakpoints > you might want to create (address breakpoint, file & line breakpoint, symbol > name breakpoint etc). An interesting aside is that none of the SB API methods > take a flag for whether breakpoint should be set using a software or hardware > breakpoint. Jonas added that feature to debugserver a few years ago for > x86_64 and aarch64, and I think he added the > `target.require-hardware-breakpoint` setting then. Otherwise the only way to > set a hardware breakpoint is through the commandline `breakpoint set` command. Yeah, we probably didn't want to add yet another API call to just add access to the flag. > > For a watchpoint, we only have `SBTarget::WatchAddress`, which takes and > address and size. If we were trying to follow the breakpoint API naming > style, we would add `SBTarget::WatchpointCreateByAddress`, > `SBTarget::WatchpointCreateByVariable`, and > `SBTarget::WatchpointCreateByExpression` methods. All of them would take the > same read/write/modify flags, which might be the strongest argument for an > options class even if it seems a little bit much for a few bools. Yeah, it isn't too hard and it allows us great flexibility in the future. I have a patch coming for improving the saving of core files that allows users to specify a custom memory region list. I am creating a new options class named SBProcessSaveCoreOptions for the SBProcess::SaveCore(SBProcessSaveCoreOptions options) function as we can always add more flags to this class. https://github.com/llvm/llvm-project/pull/66308 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
