[clang-tools-extra] [clangd] [Modules] Add VFS to ASTUnit::LoadFromASTFile (PR #113879)

[kadir çetinkaya via cfe-commits]

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