[PATCH] D55256: [clangd] Support clang-tidy configuration in clangd.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clangd/ClangdLSPServer.h:132
 
-  RealFileSystemProvider FSProvider;
   /// Options used for code completion
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > Could we instead call `getRealFS()` when we try to initialize a clang-tidy options provider in `main()` and avoid changing this?
> > To avoid adding extra non-real-fs "modes of operation" to `ClangdLSPServer`. Unless you see other uses for this.
> We already have out-of-tree modifications to ClangdLSPServer to use non-real FSes.
> Given that, I think this change is OK... though better still might be to move it into `ClangdServer::Options`
Yes, this is the main reason I did this change.


================
Comment at: clangd/ClangdUnit.h:83
+        IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+        tidy::ClangTidyOptions ClangTidyOpts);
 
----------------
sammccall wrote:
> Passing by value is OK here if deliberate, but let's try to avoid too many random copies below.
changed to `const &`


================
Comment at: clangd/tool/ClangdMain.cpp:204
 
+static llvm::cl::opt<std::string> ClangTidyChecks(
+    "clang-tidy-checks",
----------------
sammccall wrote:
> Maybe add a TODO or FIXME to respect .clang-tidy files?
didn't get the point of the comment -- in this patch, clangd will read configurations from `.clang-tidy` files (`FileOptionsProvider` provides this functionality). This command-line flag is used to overwrite the `.clang-tidy` configurations,.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55256