[Lldb-commits] [PATCH] D66241: stop-hooks don't fire on "step-out"

[Greg Clayton via Phabricator via lldb-commits]

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