bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

Seems ok to me, one small thought. Shouldn't hold this patch up though.



================
Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:270-271
 
+Status ScriptedProcess::EnableBreakpointSite(BreakpointSite *bp_site) {
+  assert(bp_site != nullptr);
+
----------------
I like the assert, but I can't help but wonder if we should change the 
signature of this method? Something like `EnableBreakpointSite(BreakpointSite 
&bp_site)` or something like this... Seems like the other overrides assume that 
it is not nullptr before using it and all the callsites verify it before 
calling it too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145296

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

Reply via email to