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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits