clayborg added inline comments.

================
Comment at: source/Target/StopInfo.cpp:550
         thread_sp->ResetStopInfo();
+        thread_sp->SetStopInfo(thread_sp->GetStopInfo());
       }
----------------
jingham wrote:
> clayborg wrote:
> > Can you clarify what is going on here? Does this force a recalculation of 
> > the stop info? This looks really goofy from a code perspective.
> This is replacing the raw stop info (a StopInfoBreakpoint) with the public - 
> cooked - stop info (StopInfoThreadPlan).
> 
> I think the handling of "public and private" stop info has gotten a bit 
> muddled up.  I swear when I first did this there were distinct Public and 
> Private stop infos.  But (I think when the StopInfo's acquired the Stop 
> Actions many years back) we went to having just one stop info, and when you 
> wanted to produce a different StopInfo from the one that you got from the 
> Process plugin you overwrite it.
> 
> I think it would be worth playing around with keeping these two separate.  It 
> would make this sort of code much clearer, since then you wouldn't muck with 
> the truth you got from the Process plugin, but just layer the public facing 
> Info over top of it.  But that's a much bigger change.
ok, it does look quite goofy from a API usage scenario. Seems like you are 
getting and then setting to the same value. If we had an API that returned an 
integer, it would be like:

```
foo->SetInt(foo->GetInt());
```

Can we just do this in ResetStopInfo()? Or rename it to "UpdateStopInfo" or 
something clearer? I would like to see this:

```
thread_sp->ResetStopInfo();
thread_sp->SetStopInfo(thread_sp->GetStopInfo());
```

become:
```
thread_sp->UpdateStopInfo();
```




Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66241



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

Reply via email to