[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope
Med Ismail Bennani via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Wed May 24 15:53:17 PDT 2023
mib marked 3 inline comments as done.
mib added inline comments.
================
Comment at: lldb/source/Breakpoint/Watchpoint.cpp:103-104
+ TargetSP target_sp = exe_ctx.GetTargetSP();
+ if (!target_sp)
+ return false;
+
----------------
bulbazord wrote:
> Is it even possible to have an `ExecutionContext` created from a `StackFrame` that doesn't have a valid `Target`? I wonder if we should assert that we got one here if not.
There is no `ExecutionContext::IsValid` method, so if you created an `ExecutionContext` with a bogus frame, you might get a nullptr.
================
Comment at: lldb/source/Breakpoint/Watchpoint.cpp:165
+ process_sp->DisableWatchpoint(watch_sp.get());
+ return false;
+ }
----------------
bulbazord wrote:
> This should be `return true;` no? You could invert the condition to be consistent with the patterns above of "return false if some condition wasn't met".
We want to always `return false` since this is a breakpoint callback, we never want to stop.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151366/new/
https://reviews.llvm.org/D151366
More information about the lldb-commits
mailing list