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

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Feb 26 12:03:27 PST 2019


labath added a comment.

BTW, what's the reason that you have to have this split now? I understand its attractiveness from a Zen perspective, but it seems to me that at the end of the day, it doesn't have that much impact on the new code you're about to write. It seems to me you could still enforce a clean layering on the new code by just saying "only ever include stuff from Host or Utility". The fact that Host transitively pulls in the kitchen sink is unfortunate, but I don't think it should impact you much. And when we finally clean up Host (which is something that we'll do eventually, and I hope not too far from now), then the new code will magically stop depending on the whole world without any extra effort.

In D58654#1410852 <https://reviews.llvm.org/D58654#1410852>, @zturner wrote:

> In D58654#1410817 <https://reviews.llvm.org/D58654#1410817>, @JDevlieghere wrote:
>
> > I'm not a fan of introducing another library for this. Without a clear timeline of "fixing" Host it's basically as temporary as anything else in software development. I also think it's confusing (e,g, new files should go where?). If you really want a clean slate we should move everything in System that doesn't depend on anything other than Utility until Host is empty, but I'm sure everyone agrees that unrealistic and not worth the churn.
> >
> > For the record I also very much support a Host library instead of having everything in Utility. That being said, I could see the config file being special enough (in that everything depends on it) to be the exception.
>


I would actually say that almost nothing should depend on the config file :). I mean this in the sense that the dependencies should be encapsulated similarly to how llvm encapsulates all of the external dependencies and the rest of the code should just use those abstractions.

However, I too have come to the conclusion that the config file could be an exception. My reason for that is that for instance our XML abstraction seems to be misplaced in the Host library. It's kind of true that whether libxml is present is a feature of the host system, but that's a fairly odd way of looking at things. Right not I am incubating this idea of creating a "Formats" library which would house the minidump parser, gdb-remote protocol support code and similar, XML support seems like it could fit nicely there. Having the LIBXML_AVAILABLE macro in an easily accessible place would help make that happen.

>> It seems like we could drop the dependencies on Core and Symbol by moving Symbols somewhere else. How much work would there be left after Pavel's patch?

Yea, symbols is going to be the trickiest part. I've been preparing myself (both code-wise and mentally) to tackle that by first removing all the other easy obstacles first. I have a sort of an idea on how to handle that, but I don't want to go into that here.

I suppose one way to make progress here would be to move Symbols into some weird temporary package. Then cleaning up the rest of Host should be easy, and the temporary package could disappear as soon as Symbols has been dealt with. It might make my job of cleaning it up easier, as I could just move the modularized functionality back into Host piece by piece. It shouldn't even create additional history churn, as that code will need to be rewritten anyway.


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

https://reviews.llvm.org/D58654





More information about the lldb-commits mailing list