mib 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()) {
----------------
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.
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
