[clang-tools-extra] r361178 - [clangd] Make it possible to use VFS from parsing for getting tidy options

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Mon May 20 10:30:46 PDT 2019


Author: ibiryukov
Date: Mon May 20 10:30:46 2019
New Revision: 361178

URL: http://llvm.org/viewvc/llvm-project?rev=361178&view=rev
Log:
[clangd] Make it possible to use VFS from parsing for getting tidy options

Summary:
To give an option for clangd embedders with snapshotted filesystem to
read config files from exact snapshots, possibly loosing some
performance from caching capabilities of the current implementations.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D62143

Modified:
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/clangd/ClangdServer.h
    clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=361178&r1=361177&r2=361178&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon May 20 10:30:46 2019
@@ -90,7 +90,7 @@ ClangdServer::ClangdServer(const GlobalC
       DynamicIdx(Opts.BuildDynamicSymbolIndex
                      ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex)
                      : nullptr),
-      ClangTidyOptProvider(Opts.ClangTidyOptProvider),
+      GetClangTidyOptions(Opts.GetClangTidyOptions),
       SuggestMissingIncludes(Opts.SuggestMissingIncludes),
       WorkspaceRoot(Opts.WorkspaceRoot),
       // Pass a callback into `WorkScheduler` to extract symbols from a newly
@@ -126,15 +126,18 @@ ClangdServer::ClangdServer(const GlobalC
 
 void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
                                WantDiagnostics WantDiags) {
+  auto FS = FSProvider.getFileSystem();
+
   ParseOptions Opts;
   Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
-  if (ClangTidyOptProvider)
-    Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File);
+  // FIXME: call tidy options builder on the worker thread, it can do IO.
+  if (GetClangTidyOptions)
+    Opts.ClangTidyOpts = GetClangTidyOptions(*FS, File);
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
 
   // Compile command is set asynchronously during update, as it can be slow.
   ParseInputs Inputs;
-  Inputs.FS = FSProvider.getFileSystem();
+  Inputs.FS = FS;
   Inputs.Contents = Contents;
   Inputs.Opts = std::move(Opts);
   Inputs.Index = Index;

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=361178&r1=361177&r2=361178&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Mon May 20 10:30:46 2019
@@ -50,6 +50,12 @@ public:
   virtual void onFileUpdated(PathRef File, const TUStatus &Status){};
 };
 
+/// When set, used by ClangdServer to get clang-tidy options for each particular
+/// file. Must be thread-safe. We use this instead of ClangTidyOptionsProvider
+/// to allow reading tidy configs from the VFS used for parsing.
+using ClangTidyOptionsBuilder = std::function<tidy::ClangTidyOptions(
+    llvm::vfs::FileSystem &, llvm::StringRef /*File*/)>;
+
 /// Manages a collection of source files and derived data (ASTs, indexes),
 /// and provides language-aware features such as code completion.
 ///
@@ -94,12 +100,12 @@ public:
     /// If set, use this index to augment code completion results.
     SymbolIndex *StaticIndex = nullptr;
 
-    /// If set, enable clang-tidy in clangd, used to get clang-tidy
+    /// If set, enable clang-tidy in clangd and use to it get clang-tidy
     /// configurations for a particular file.
     /// Clangd supports only a small subset of ClangTidyOptions, these options
     /// (Checks, CheckOptions) are about which clang-tidy checks will be
     /// enabled.
-    tidy::ClangTidyOptionsProvider *ClangTidyOptProvider = nullptr;
+    ClangTidyOptionsBuilder GetClangTidyOptions;
 
     /// Clangd's workspace root. Relevant for "workspace" operations not bound
     /// to a particular file.
@@ -278,8 +284,8 @@ private:
   // Storage for merged views of the various indexes.
   std::vector<std::unique_ptr<SymbolIndex>> MergedIdx;
 
-  // The provider used to provide a clang-tidy option for a specific file.
-  tidy::ClangTidyOptionsProvider *ClangTidyOptProvider = nullptr;
+  // When set, provides clang-tidy options for a specific file.
+  ClangTidyOptionsBuilder GetClangTidyOptions;
 
   // If this is true, suggest include insertion fixes for diagnostic errors that
   // can be caused by missing includes (e.g. member access in incomplete type).

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=361178&r1=361177&r2=361178&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Mon May 20 10:30:46 2019
@@ -18,6 +18,7 @@
 #include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -27,6 +28,7 @@
 #include <cstdlib>
 #include <iostream>
 #include <memory>
+#include <mutex>
 #include <string>
 #include <thread>
 
@@ -496,7 +498,9 @@ int main(int argc, char *argv[]) {
   }
 
   // Create an empty clang-tidy option.
-  std::unique_ptr<tidy::ClangTidyOptionsProvider> ClangTidyOptProvider;
+  std::mutex ClangTidyOptMu;
+  std::unique_ptr<tidy::ClangTidyOptionsProvider>
+      ClangTidyOptProvider; /*GUARDED_BY(ClangTidyOptMu)*/
   if (EnableClangTidy) {
     auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
     OverrideClangTidyOptions.Checks = ClangTidyChecks;
@@ -505,7 +509,13 @@ int main(int argc, char *argv[]) {
         /* Default */ tidy::ClangTidyOptions::getDefaults(),
         /* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
   }
-  Opts.ClangTidyOptProvider = ClangTidyOptProvider.get();
+  Opts.GetClangTidyOptions = [&](llvm::vfs::FileSystem &,
+                                 llvm::StringRef File) {
+    // This function must be thread-safe and tidy option providers are not.
+    std::lock_guard<std::mutex> Lock(ClangTidyOptMu);
+    // FIXME: use the FS provided to the function.
+    return ClangTidyOptProvider->getOptions(File);
+  };
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
   llvm::Optional<OffsetEncoding> OffsetEncodingFromFlag;
   if (ForceOffsetEncoding != OffsetEncoding::UnsupportedEncoding)




More information about the cfe-commits mailing list