mib marked 3 inline comments as done.
mib 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.
================
Comment at:
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/fast_conditional_breakpoints/main.c:30
+ printf("local_count = %d\n", local_count++); // Find the line number of
condition breakpoint for local_count
+ asm("nop");
+ asm("nop");
----------------
shafik wrote:
> Can you explain why we need the `nop`s injected?
I'm still working on dynamically patching the copied instructions, so I use a
`nop` sled for now to test my feature.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66248/new/
https://reviews.llvm.org/D66248
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
