[Lldb-commits] [PATCH] D83728: [lldb] Make `process connect` blocking in synchronous mode.

Tue Jul 14 08:33:42 PDT 2020

JDevlieghere marked 7 inline comments as done.
JDevlieghere added inline comments.

================
Comment at: lldb/source/Target/Platform.cpp:1834
   if (error.Fail())
     return nullptr;

----------------
labath wrote:
> JDevlieghere wrote:
> > jingham wrote:
> > > If you fail here you leave the process hijacked.  That doesn't matter because if "ConnectRemote" fails, you aren't going to have much to listen to anyway.  But it still looks odd.  I'm surprised we don't have some RAII-dingus for process hijacking, but anyway, it's good practice to undo this in the error branch.
> > Yeah, that's exactly my reasoning. You can't use a RAII object here, because the order of destruction is undefined, so you might end up calling `RestoreProcessEvents` after the shared pointer has been destructed. Anyway, I've added the call just for consistency. 
> I'm not sure what you mean by the undefined destruction order. In c++ the destruction order is always the reverse of the construction order. The only "exception" to that that I am aware of is the destruction order for function call arguments. But even there the destruction order is still reverse construction order -- just the construction order itself is not (fully) defined.
I was thinking of static destructors, but that is also reverse construction order, the construction order across TUs is just not defined. 

================
Comment at: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py:46
+
+class TestProcesConnect(GDBRemoteTestBase):
+    def test_gdb_remote_sync(self):
----------------
labath wrote:
> `s/s/ss` :P
My attempt to sneak in some Dutch spelling failed. (/jk)

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

https://reviews.llvm.org/D83728