[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib requested review of this revision.
mib added inline comments.


================
Comment at: lldb/include/lldb/Utility/ProcessInfo.h:90-110
+  bool IsScriptedProcess() const {
+    return !m_scripted_process_class_name.empty();
+  }
+
+  std::string GetScriptedProcessClassName() const {
+    return m_scripted_process_class_name;
+  }
----------------
JDevlieghere wrote:
> What's the relationship between this an the `ScriptedMetadata`? It seems like this is storing the same stuff. Also `IsScriptedProcess` is a pretty blatant violation of the scripted process being a plugin... 
I think you have a misunderstanding here: This is addition is not made in a process plugin, but rather in the `ProcessInfo` class, the base class for `ProcessLaunchInfo` and `ProcessAttachInfo`, so I don't think this violates in any way the plugin architecture.

Regarding the relationship with `ScriptedMetadata`: I guess we could replace `m_scripted_process_class_name` & `m_scripted_process_dictionary_sp` by a `m_scripted_metadata` object, but I didn't want to process info hold scripted process specific stuff. I don't have a strong opinion against it, either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143104