[Lldb-commits] [PATCH] D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false

[Jim Ingham via Phabricator via lldb-commits]

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