[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
Thu Aug 15 11:05:22 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,
----------------
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.
================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/fast_conditional_breakpoints/main.c:14
+
+#define asm __asm__
+
----------------
I don't think we need this macro.
================
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");
----------------
Can you explain why we need the `nop`s injected?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66248/new/
https://reviews.llvm.org/D66248
More information about the lldb-commits
mailing list