This is an archive of the discontinued LLVM Phabricator instance.

[lldb] [gdb-remote] Isolate ClientBase from Communication
Needs ReviewPublic

Authored by mgorny on Oct 2 2022, 3:48 AM.

Details

Summary

Introduce an isolation layer between GDBRemoteClientBase
and GDBRemoteCommunication. This makes it possible to replace
the public packet reading/writing API with wrappers that will make
threaded access possible in the future.

Sponsored by: The FreeBSD Foundation

Diff Detail

Event Timeline

mgorny created this revision.Oct 2 2022, 3:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 2 2022, 3:48 AM
Herald added a subscriber: arichardson. · View Herald Transcript
labath added inline comments.Oct 3 2022, 7:43 AM
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
116–118

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.

mgorny added inline comments.Oct 3 2022, 9:30 AM
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
116–118

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.

labath added inline comments.Oct 4 2022, 8:09 AM
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
116–118

Ok, maybe that's fine. Let's figure out what to do with the other patch first.