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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to