[PATCH] D82002: [clangd] Drop FS usage in ClangTidyOpts

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 17 08:03:52 PDT 2020


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





More information about the cfe-commits mailing list