[PATCH] D80900: [clangd] Use different FS in PreambleThread
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 4 09:53:13 PDT 2020
kadircet added a comment.
In D80900#2071207 <https://reviews.llvm.org/D80900#2071207>, @sammccall wrote:
> That makes sense. However I don't think that contract is conceptually that great, having the ideas of "threadsafe FS" and "maybe-context-aware FS" coupled together is... weird. The benefit is that it's an easier contract to satisfy.
>
> It's already the case that the context could/should be plumbed through to the actual FS operations and other extension points. Things like tracing rely on it. I think saying "we always plumb context through to everything" is a better contract even if it's a bit stronger than we need and harder to prove satisfied. This frees up FileSystemProvider to be the conceptually-simple ThreadsafeFS that could be lifted from clangd/support to llvm/support if we want.
>
> WDYT?
Did that in D81173 <https://reviews.llvm.org/D81173>, there were two issues:
- Sometimes we create ParseInputs without an access to an FSProvider, overcome this one with a helper that wraps a VFS and returns an FSProvider for it.
- TestTU::inputs() needs to return a ParseInputs, but it is unclear which FSProvider it should point to :/ make it a parameter to the function. Since it needs to out-live the returned ParseInputs.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80900/new/
https://reviews.llvm.org/D80900
More information about the cfe-commits
mailing list