friss added a comment. In D74157#1863069 <https://reviews.llvm.org/D74157#1863069>, @jingham wrote:
> 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. I don't understand this comment. The fallback code we are talking about only ever triggers if there's a StopInfo. If the StopInfoSP was empty we wouldn't return a description. > 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"...) I removed this code and asserted that the description returned by StopInfos is not empty. The test suite passed fine. Do you have any example where this code would be needed? I think we should require that StopInfos return a non-empty description and get rid of it the fallback code. > 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. Do you have any example how to get into a problematic case? 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
