jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

The first code site did:  checking for thread plan success, then check for hit 
breakpoint, then for anything else (thread plan failed or other stop reason.)

The code in the first of your substitution sites retains the anything else case 
outside the bits you factored out, in the if clause you've retained after your 
function (5281 in the new version.)

But ThreadPlanDone returns true regardless of the success or failure of the 
plan (it gets used in a bunch of places, so it's no less well tested, just had 
different semantics...)  So if you use your new function in the second site, 
the code will no longer handle the case where the plan was completed but 
failed.  Be good to keep a check for that.

Other than that, this looks good.  Thanks for catching this.


https://reviews.llvm.org/D33283



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

Reply via email to