[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