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

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

I still think it's a little weird that you can create LaunchInfo or AttachInfo, call SetScriptedProcessDictionary, and still have the ScriptedMetadata be "invalid", but I suppose it makes no sense if there is no class name anyway.

Just a few small things. Everything else looks fine to me.



================
Comment at: lldb/source/API/SBAttachInfo.cpp:272
 void SBAttachInfo::SetScriptedProcessClassName(const char *class_name) {
   LLDB_INSTRUMENT_VA(this, class_name);
+  ScriptedMetadataSP metadata_sp = m_opaque_sp->GetScriptedMetadata();
----------------
Add a new line after the instrument macro


================
Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:203-208
+  SetPrivateState(eStateRunning);
+
+  if (error.Fail())
+    return error;
+
+  SetPrivateState(eStateStopped);
----------------
mib wrote:
> bulbazord wrote:
> > I'm not sure I understand what the point of setting the state to `Running` is and only setting it to `Stopped` if the attach failed? Should we be mucking with state at all if the attach failed?
> I guess we can do it subsequently
I'm not sure this was addressed and maybe I don't understand something but why do we check to see if the error failed only after setting the state to running? If we actually did fail to attach, does it make sense to think of the ScriptedProcess as "running"?


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

https://reviews.llvm.org/D143104