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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to