[Lldb-commits] [PATCH] D11150: [NativeProcessLinux] Integrate MainLoop

Pavel Labath labath at google.com
Tue Jul 14 03:17:38 PDT 2015


labath added inline comments.

================
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:19
@@ -18,2 +18,3 @@
 #include "lldb/Host/Mutex.h"
+#include "lldb/Host/MainLoop.h"
 #include "llvm/ADT/StringRef.h"
----------------
ovyalov wrote:
> Can you use forward declaration here?
The simple forward declaration `class MainLoop;` will not work because MainLoop is a typedef and you will get redefinition errors. We could try to do something clever here, but I am not sure it is worth it. I could also have the interface take MainLoopBase and forward declare that, but then I would have to upcast the object to MainLoopPosix in NPL, which i also don't like much.

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:102
@@ -101,3 +101,3 @@
 //----------------------------------------------------------------------
 GDBRemoteCommunicationServerLLGS::~GDBRemoteCommunicationServerLLGS()
 {
----------------
ovyalov wrote:
> Since you're removing Terminate method from it's seems feasible to remove this destructor impl as well.
Note that I haven't removed the base method NativeProcessProtocol::Terminate(). However, I think this is a good idea, so I will interpret this comment as a request to remove that as well. :)


http://reviews.llvm.org/D11150







More information about the lldb-commits mailing list