I do think at some point we will need to break up Host, but whether the best way is to move stuff into Utility or to somewhere else is an open question.  Host also depends on Target, Symbol, process plugins, and various other things.  So we will need a way to group together all the stuff in Host that has no other dependencies.  This could then remove the dependency from X -> Host for many values of X.<br><br>I agree that Core is bigger, but unfortunately with deps it's kind of all or nothing.  You don't see any benefit from removing 99 if there's still 1 more.<br><br>If you don't want to do it now that's fine as it does indeed require moving many other things, but moving it to Host seems like a lateral move at best - nothing gained nothing lost.  What about moving the base class to Utility?  At least then you wouldn't need to link Host to use Connection, which could encourage a factory-like pattern<br><div class="gmail_quote"><div dir="ltr">On Tue, Jun 20, 2017 at 7:00 AM Pavel Labath <<a href="mailto:labath@google.com">labath@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 20 June 2017 at 14:28, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br>
> I had actually been considering going the other way. Moving connection and<br>
> all implementations to Utility. Host is a big contributor to the dependency<br>
> problems, so putting more stuff in seems like the wrong direction.<br>
<br>
Well, I'd say that Core is an even bigger contributor to the cycles<br>
than Host (e.g. it has Timer.cpp, which depends on nothing, and<br>
Debugger.cpp, which depends on everything). With this, I was looking<br>
at breaking the Host->Core dependency. After this patch the only Core<br>
files which are included from Host are:<br>
Core/ArchSpec.h<br>
Core/Communication.h<br>
Core/Module.h<br>
Core/ModuleSpec.h<br>
Core/RegisterValue.h<br>
Core/State.h<br>
Core/StreamFile.h<br>
Core/StructuredData.h<br>
Core/Timer.h<br>
Core/TraceOptions.h<br>
<br>
Of these, only the Module dependency worries me (but I haven't yet<br>
looked at why exactly it's needed) -- I believe all the others can be<br>
solved fairly easily by moving the relevant files to Host or Utility.<br>
<br>
<br>
><br>
> The end state i had in mind was similar to llvm support (which is more less<br>
> the equivalent of lldb host), where it has all the host specific<br>
> functionality but contains no deps.<br>
><br>
> With that in mind, thoughts on moving everything to Utility?<br>
<br>
I am not categorically against it, but it's not my preferred solution<br>
either. For example, if we were to move Connection there now, it would<br>
require at least moving all of the socket library and pipes which is a<br>
large chunk of host-specific code. I like how currently we have this<br>
separation where Utility contains some fairly generic classes and<br>
algorithms, and Host contains things for interfacing with the host<br>
system (which tends to be ugly and full of ifdefs -- I'd like to avoid<br>
ifdefs in Utlity if at all possible).<br>
<br>
I realize it's possible I may run into a wall with this philosophy,<br>
but I'd like to try and give it a go. I think the fact that we have<br>
the llvm support library under us, where we can hide a lot of the<br>
non-debugger-specific host code, means that there is a fair chance<br>
this could succeed.<br>
<br>
<br>
><br>
> On Tue, Jun 20, 2017 at 5:56 AM Pavel Labath via Phabricator<br>
> <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br>
>><br>
>> labath created this revision.<br>
>> Herald added a subscriber: mgorny.<br>
>><br>
>> All implementations of the connection interface live in the Host module<br>
>> already, so it makes sense for the interface itself to be defined there.<br>
>><br>
>><br>
>> <a href="https://reviews.llvm.org/D34400" rel="noreferrer" target="_blank">https://reviews.llvm.org/D34400</a><br>
>><br>
>> Files:<br>
>>   include/lldb/Core/Connection.h<br>
>>   include/lldb/Host/Connection.h<br>
>>   include/lldb/Host/posix/ConnectionFileDescriptorPosix.h<br>
>>   include/lldb/Host/windows/ConnectionGenericFileWindows.h<br>
>>   source/Core/CMakeLists.txt<br>
>>   source/Core/Communication.cpp<br>
>>   source/Core/Connection.cpp<br>
>>   source/Host/CMakeLists.txt<br>
>>   source/Host/common/Connection.cpp<br>
>>   source/Host/posix/ConnectionFileDescriptorPosix.cpp<br>
>>   tools/lldb-server/Acceptor.h<br>
>><br>
><br>
</blockquote></div>