[Lldb-commits] [PATCH] D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D61921#1503331 <https://reviews.llvm.org/D61921#1503331>, @xiaobai wrote:

> In D61921#1503282 <https://reviews.llvm.org/D61921#1503282>, @jingham wrote:
>
> > In D61921#1502502 <https://reviews.llvm.org/D61921#1502502>, @labath wrote:
> >
> > > If this iteration is going to be used a lot, I'd recommend taking a bit of time to implement an iterator abstraction over the language runtimes. It takes a bit longer to set up, but I hope we can all agree that `for (runtime: process->GetLanguageRuntimes()) runtime->foo();` is more readable than `process->ForEachLanguageRuntime([] (runtime) { runtime->foo(); })`. This is particularly true if you need some sort of a control flow construct (`continue`, `break`, `return`) in the loop "body".
> >
> >
> > +1
>
>
> Yeah this seems reasonable.
>
> In D61921#1503290 <https://reviews.llvm.org/D61921#1503290>, @jingham wrote:
>
> > On macOS, there are a handful of runtimes that the Plugin Manager knows about - C++, ObjC, maybe Swift.  But, for instance, we only load the ObjC language runtime into the process' m_language_runtimes array when we see libobjc.dylib get loaded.  A pure C++ program might never load libobjc.dylib, and so even though the Plugin Manager knows we have support for the ObjC language runtime, that plugin wouldn't be active in the current lldb_private::Process.  So there's a real difference between the "available" and the "currently loaded" runtimes. I was saying I didn't see any compelling reason to have an iterator over the available runtimes, just over the loaded ones.  Not that we didn't need one or the other iterator.
>
>
> Okay, that makes sense to me. Thanks for clearing that up. If I understand correctly (which I think I do), creating an iterator abstraction to iterate over the process' currently loaded runtimes instead of over every available runtime is the better option here. I'll follow up with that change later today or tomorrow unless somebody believes this is the wrong idea.


That seems the right choice to me and should be easy to implement - since it's just iterating over a map.  Iterable.h is some little dealie Sean whipped up a while ago that we use to make other contained maps iterable - e.g. the Types in a TypeMap.  You could just reuse that.


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

https://reviews.llvm.org/D61921