JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: source/Target/ThreadPlanStepThrough.cpp:91
+  if (!m_sub_plan_sp) {
+    for (unsigned lang = eLanguageTypeUnknown;
+         !m_sub_plan_sp && lang < eNumLanguageTypes; lang++) {
----------------
xiaobai wrote:
> JDevlieghere wrote:
> >  Given that we seem to have this pattern in a bunch of places, would it be 
> > worth extracting this into some kind of convenience function?
> Yeah, I'm getting tired of writing it. We could pull it into Process, into a 
> `ForEachLanguageRuntime` or something. I can either do that before this goes 
> in or in a follow up
Alright, either works for me. 


================
Comment at: source/Target/ThreadPlanStepThrough.cpp:96
+        m_sub_plan_sp =
+            runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
+      }
----------------
xiaobai wrote:
> JDevlieghere wrote:
> > Should there be a break here?
> I put a `!m_sub_plan_sp` in the for loop's condition. If you think it would 
> be more readable to put a break here, I could do that instead.
Aha, you're right, I missed that. I think the break is slightly more readable, 
but I'll leave it up to you. 


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

https://reviews.llvm.org/D61921



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

Reply via email to