[Lldb-commits] [PATCH] D66248: [JIT][Command] Add "inject-condition" flag to conditional breakpoints
Shafik Yaghmour via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Tue Aug 20 11:53:36 PDT 2019
shafik added inline comments.
Comment at: lldb/include/lldb/Breakpoint/BreakpointOptions.h:120
BreakpointOptions(const char *condition, bool enabled = true,
int32_t ignore = 0, bool one_shot = false,
> shafik wrote:
> > You have a lot of `bool` parameters, these are hard to distinguish when calling the function and easy to get mixed up during refactors and subsequent merge conflicts. It would probably be better to combine these `bool` options into a `struct` and then each option has an explicit name that that will be assigned to which makes it explicit which options are being chosen at the call site.
> I only added the `bool inject_condition` parameter to the `BreakpointOptions` constructor. I also added documentation that was missing for the other parameters. I don't think having a struct for those options is a necessary since that's what the `BreakpointOptions` class is for.
Sadly we won't have designated initializers until C++20 [see godbolt](https://godbolt.org/z/2xlti5) but instead of `bool` we can use enums and that would clarify the code at the calling site. As discussed this can be done as a patch after landing this change,
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the lldb-commits