[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D47235#1109219, @labath wrote:

> In a way, this makes sense, but in a lot of other ways, it actually makes things worse :)
>
> My long-term goal is to be able to build lldb-server without even having clang checked out (because it doesn't really need anything clang-related), and this would certainly make that impossible. Having Core depend on clang does not worry me much because it depends on so many things already, so the way I was thinking of resolving that is to move low-level things out of it (my tentative list includes things like Event/Listener/Broadcaster, State, Communication). That would leave Core with containing only the high-level stuff for which a clang dependency is not that surprising (although I agree that a ModuleList is not the best place for such a dependency).


Agree, but just because `Core` is already a problem doesn't mean we should just ignore it IMO.  At some point we're going to have to do something about it, even if it's not that surprising for `Core` to have a dependency on clang at some point in the future, it will *almost always* be surprising for two things to have a dependency on each other.  So from that angle, we need to be vigilant in outgoing dependencies from `Core`.

> I guess it would be nice to encapsulate this in some sort of a plugin (since the setting is used from the clang expression parser plugin, I guess this would be the natural home for it) , but I haven't looked in detail at could that work. What I do know is that we already have the ability to inject settings from within a plugin (see SymbolFileDWARF::DebuggerInitialize). Maybe that would work here too?

I agree that a clang plugin seems like the "real" solution, I had come to the same conclusion yesterday.  But that is a significant amount of work obviously.

That's why I proposed in the other thread the idea of having a function called `Module::setDefaultClangModulesPath(const Twine&)` and just call that method in `SystemInitializerFull`.  That's very similar in spirit to how the plugins work anyway.  During initialization we call global functions on all of the plugins to have them initialize themselves, and they're free to muck with the world during this initialization.  So this solution is, IMO, an incremental step towards the "real" solution while still not making the problem worse.  If/when someone makes a real clang plugin, they just copy this code into that plugins initialize method.


https://reviews.llvm.org/D47235