sammccall added a comment.

Hmm, having looked at this it is a bit of a mess, but I think partly because 
we're not converting *enough* code to use this abstraction (e.g. the preamble 
caching stuff, or prepareCompilerInstance).
We should work how how to get the bugfix part of this landed, but I think 
ultimately we should rename FSprovider and avoid most direct use of VFS in 
clangd, it bakes in this assumption about threadign that's very hard to deal 
with locally.



================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:184
   Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
+  auto VFS = FSProvider.getFileSystem();
   // FIXME: call tidy options builder on the worker thread, it can do IO.
----------------
As a question of style, I think we should inline these wherever we're handing 
off a VFS to another library and we don't want to reuse it for some reason.

We can shorten the names if needed to make this nice.



================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1069
     VFS = Input.Preamble.StatCache->getConsumingFS(std::move(VFS));
+  auto FSProvider = getFSProvider(VFS);
   ParseInputs ParseInput;
----------------
can we leave a fixme to propagate this change further instead?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81173/new/

https://reviews.llvm.org/D81173



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

Reply via email to