[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

Mon Feb 25 19:13:03 PST 2019

zturner added a comment.

In D58654#1410080 <https://reviews.llvm.org/D58654#1410080>, @jingham wrote:

> There aren't many more platforms (OpenVMS?) that are likely to show up and need support for lldb, so maybe this is making too much of the matter.  But having there be a well defined set of places where you need to look when porting lldb to a new host platform seems a useful design to me.  The fact that we weren't strict enough and allowed Host platform dependencies to creep in where they don't belong doesn't argue we should just abandon trying to keep the places where a port sockets into lldb regular.
>
> It's fine to have a staging area ("WillBeHost"?) to help pull out the classes that it is simple to pull out of host.  But I'd rather not glom them all into Utility, which is already becoming a little incoherent.

How about "System" then?  FTR, I'm also fine eventually renaming it back to Host once all the layering issues are addressed.  The important thing is that if it's really just a set of platform-specific abstractions, it shouldn't really depend on anything else (except perhaps Utility).

I guess one advantage to having them be together though is that often even the Utility code itself needs to make use of platform-specific abstractions.  You can already see this anywhere that lldb/Utility calls into llvm/Support to do things like read from the file system or start a thread (TBH, I'm actually not even sure it does any of those things, but it seems reasonable that it might want to).  So if we make a library called System, then it's possible that System ends up depending on Utility and Utility ends up depending on System.  Despite all of mine (and others') push for proper layering, this is one area where I'm actually ok with it.  LLVM has the same issue here, where ADT and Support depend on each other and just linked together into one big .lib, despite being in separate folders.

But it's something to keep in mind.

Anyway, does that seem reasonable?  Put it in a new folder called `System`?

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

https://reviews.llvm.org/D58654