[Lldb-commits] [PATCH] D74157: [lldb/API] NFC: Reformat SBThread::GetStopDescription()

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D74157#1862537 <https://reviews.llvm.org/D74157#1862537>, @labath wrote:

> Looks better. TBH, I'm not sure why/if we really need the case handling the situation where the thread does not have a stop description. Ideally I'd just move this code there (or delete it).


A thread that stops for no reason will have an empty StopInfoSP.  So somebody has to check for that.  I also don't want a thread that hasn't stopped for a reason to have a placeholder description like "no reason" because if you are using the API's you'd like to just print whatever the description is, and if you stop one thread at a breakpoint, having all the others say "no reason" is noisy and unhelpful.

But I agree that the SB layer is the wrong place to do this.  SBThread::GetStopDescription should just call Thread->GetStopDescription and return whatever that returned.  It really shouldn't even care whether the thread had a StopInfo or not, Thread::GetStopDescription should handle that case too.

I also really wish we didn't need the code to augment the Thread's stop description by using the stop reason.  You only get to this code if there IS a StopInfo, so the StopInfo should have provided a description if there's a reason for stopping.  Right now you can subclass StopInfo and return anything you want in GetStopDescription.  So somebody could make a StopInfo subclass that represents a stop because we hit a signal - returning eStopReasonSignal from GetStopReason - and then just decide not to print anything for the description.  But that's very unhelpful.  And these reduced descriptions are a bit bogus (like any plan completed calls itself "step"...)

It would be really nice to just ban that.  Maybe we could make the StopInfo virtual method be a DoGetStopDescription, and then have StopInfo::GetStopReason() not be virtual, and call DoGetStopDescription and assert if the StopReason in anything other than eStopReasonNone, but the description was empty.  That way we could force people who handle the stops to provide useful stop reasons.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74157