kadircet wrote:

> So I prefer ASTUnit here especially clangd already depends on clangFrontend 
> already so I feel like I didn't introduce a new dependency.

It isn't at all about having this dependency in terms of "build graph", but 
rather about mental complexity and maintenance burden of that.

Clangd already has its own primitives for dealing with preambles, keeping 
ASTcontexts alive and even handling modules (and soon also module caches!). 
ASTUnit has its own way of doing all of these. So if we introduced any semantic 
dependency here, everytime someone is chaging those core primitives in clangd 
will need to think about how all of these going to interact with each other.

Moreover, if this was sent with a review, I would've surely objected to that.

>  For compiler instance, we need to provide the compiler invocation

We already have the compiler invocation available. I am not sure about the 
implications of loading a PCM with a "vanilla" compiler instance (that's what 
ASTUnit does underneath, creates a preprocessor, astcontext, diagsengine, ... 
with default options), I'd be much more comfortable if we created that 
ASTReader and loaded the AST with some relevant compilerinvocation that 
describes user's intent.

But you definitely know much more about modules, if you think that's fine, i 
think we can always create that astreader directly here, without a 
compilerinstance as well.

https://github.com/llvm/llvm-project/pull/113879
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to