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



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

Reply via email to