[Lldb-commits] [PATCH] D66248: [JIT][Command] Add "inject-condition" flag to conditional breakpoints

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/fast_conditional_breakpoints/TestFastConditionalBreakpoints.py:45
+
+    def enable_injected_breakpoint_condition(self, cli):
+        exe = self.getBuildArtifact(self.binary)
----------------
mib wrote:
> JDevlieghere wrote:
> > What does `cli` stand for?
> CLI = Command Line Interface. In this case, it means running the command through the "REPL" instead of the SBAPI, to test if it works properly.
That's definitely not clear from the variable. How about `use_sbapi` or `use_interpreter` instead? 


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/fast_conditional_breakpoints/TestFastConditionalBreakpoints.py:79
+            # indeed, being set correctly.
+            # there's only one thread for the process.
+            breakpoint.SetThreadIndex(1)
----------------
> ... being set correctly and that there's only ...


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/fast_conditional_breakpoints/TestFastConditionalBreakpoints.py:93
+
+            # Set the condition on the breakpoint
+            location.SetCondition(self.condition)
----------------
Add full stop


================
Comment at: lldb/source/API/SBBreakpointLocation.cpp:214
+  BreakpointLocationSP loc_sp = GetSP();
+  if (loc_sp) {
+    std::lock_guard<std::recursive_mutex> guard(
----------------
Early return?


================
Comment at: lldb/source/API/SBBreakpointLocation.cpp:226
+  BreakpointLocationSP loc_sp = GetSP();
+  if (loc_sp) {
+    std::lock_guard<std::recursive_mutex> guard(
----------------
Early return?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66248/new/

https://reviews.llvm.org/D66248