labath added inline comments.
================
Comment at: lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp:757
+ bool is_scripted_process = m_process->GetPluginName() == "ScriptedProcess";
for (ThreadSP thread_sp : m_process->Threads()) {
----------------
mib wrote:
> JDevlieghere wrote:
> > Comparing the plugin name defeats the abstraction a plugin is meant to
> > provide. While we have other instances of LLDB breaking these abstractions,
> > I don't recall other places where we compare the plugin name. The way we
> > normally deal with this is extend the plugins capability (by adding a
> > method) and implementing it accordingly for all the plugins (or have a sane
> > default).
> >
> > Based on the description of the patch it's not clear to me why this is
> > special for scripted processes. If we need to special case this I'd like to
> > see a comment explaining why.
> I agree, but in this case, `Process::UpdateQueueListIfNeeded` calls the
> System Runtime plugin `PopulateQueueList` method and since queues are only
> supported on macOS, it does make sense in my opinion.
>
> I think we definitely shouldn't add another system runtime for scripted
> processes just to support that case, so adding a special case for scripted
> processes seems to be the less intrusive way to implement it.
I'm with Jonas here. Even though these Queue concepts are a big mystery do me,
I would like to understand why the scripted processes do can not go through the
same code path as "regular" ones.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139853/new/
https://reviews.llvm.org/D139853
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
