[Lldb-commits] [PATCH] D135029: [lldb] [gdb-remote] Isolate ClientBase from Communication

Tue Oct 4 08:09:21 PDT 2022

labath added inline comments.

================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:116-118
+  GDBRemoteCommunication &GetCommunication() {
+    return m_comm;
+  }
----------------
mgorny wrote:
> labath wrote:
> > Is the plan to make this private/protected at some point, or something like that? Otherwise, I'm not really sure what's the benefit of this over the regular inheritance.
> > 
> > I like the idea of using composition instead of inheritance (I think we could do something similar with GDBRemoteCommunication and Communication), but right now this seems fairly messy, and the benefit is unclear.
> Ideally, yes. However, I don't think I'm going to pursue it that far, i.e. someone else will have to take up the effort. And yes, I honestly doubt anybody will.
> 
> The main goal is make ground for D135031, i.e. communication via separate thread. What I've been aiming at is leaving `GetCommunication()` for stuff that's unlikely to break when invoked cross-thread (or unlikely to be invoked cross-thread), while abstracting away the part of the API that needs to be reimplemented to communicate via the comm thread.
Ok, maybe that's fine. Let's figure out what to do with the other patch first.

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

https://reviews.llvm.org/D135029