sammccall added inline comments.

================
Comment at: clang-tools-extra/clangd/ClangdServer.h:46
-/// to allow reading tidy configs from the VFS used for parsing.
-using ClangTidyOptionsBuilder = std::function<tidy::ClangTidyOptions(
-    llvm::vfs::FileSystem &, llvm::StringRef /*File*/)>;
----------------
Hmm, I like the idea of avoiding a custom type and just reusing the standard 
one here, but:
- taking someone else's interface (which has existing implementations, some of 
the key ones being subtly non-threadsafe) and slapping "must be threadsafe" on 
it seems like a recipe for bugs
- ClangTidyOptionsProvider is a really horrible interface that exposes several 
things we don't need

anything wrong with just std::function<ClangTidyOptions(StringRef)>?
It's reasonable to use a ClangTidyOptionsProvider to create them, though I'm 
not sure it actually saves anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82002



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

Reply via email to