[clang-tools-extra] r344787 - [clangd] Set workspace root when initializing ClangdServer, disallow mutation.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 19 08:42:24 PDT 2018


Author: sammccall
Date: Fri Oct 19 08:42:23 2018
New Revision: 344787

URL: http://llvm.org/viewvc/llvm-project?rev=344787&view=rev
Log:
[clangd] Set workspace root when initializing ClangdServer, disallow mutation.

Summary:
Rename instance variable to WorkspaceRoot to match what we call it internally.

Add fixme to set it automatically. Don't do it yet, clients have assumptions
that the constructor won't access the FS.

Don't second-guess the provided root.

Reviewers: ioeric

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

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

Modified:
    clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/clangd/ClangdServer.h
    clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=344787&r1=344786&r2=344787&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Oct 19 08:42:23 2018
@@ -261,6 +261,10 @@ void ClangdLSPServer::reply(llvm::json::
 
 void ClangdLSPServer::onInitialize(const InitializeParams &Params,
                                    Callback<json::Value> Reply) {
+  if (Params.rootUri && *Params.rootUri)
+    ClangdServerOpts.WorkspaceRoot = Params.rootUri->file();
+  else if (Params.rootPath && !Params.rootPath->empty())
+    ClangdServerOpts.WorkspaceRoot = *Params.rootPath;
   if (Server)
     return Reply(make_error<LSPError>("server already initialized",
                                       ErrorCode::InvalidRequest));
@@ -277,11 +281,6 @@ void ClangdLSPServer::onInitialize(const
     applyConfiguration(Opts.ParamsChange);
   }
 
-  if (Params.rootUri && *Params.rootUri)
-    Server->setRootPath(Params.rootUri->file());
-  else if (Params.rootPath && !Params.rootPath->empty())
-    Server->setRootPath(*Params.rootPath);
-
   CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
   DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
   DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=344787&r1=344786&r2=344787&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Oct 19 08:42:23 2018
@@ -109,6 +109,7 @@ ClangdServer::ClangdServer(const GlobalC
                      ? new FileIndex(Opts.URISchemes,
                                      Opts.HeavyweightDynamicSymbolIndex)
                      : nullptr),
+      WorkspaceRoot(Opts.WorkspaceRoot),
       PCHs(std::make_shared<PCHContainerOperations>()),
       // Pass a callback into `WorkScheduler` to extract symbols from a newly
       // parsed file and rebuild the file index synchronously each time an AST
@@ -131,18 +132,6 @@ ClangdServer::ClangdServer(const GlobalC
     Index = nullptr;
 }
 
-void ClangdServer::setRootPath(PathRef RootPath) {
-  auto FS = FSProvider.getFileSystem();
-  auto Status = FS->status(RootPath);
-  if (!Status)
-    elog("Failed to get status for RootPath {0}: {1}", RootPath,
-         Status.getError().message());
-  else if (Status->isDirectory())
-    this->RootPath = RootPath;
-  else
-    elog("The provided RootPath {0} is not a directory.", RootPath);
-}
-
 void ClangdServer::addDocument(PathRef File, StringRef Contents,
                                WantDiagnostics WantDiags) {
   DocVersion Version = ++InternalVersion[File];
@@ -495,7 +484,7 @@ void ClangdServer::onFileEvent(const Did
 void ClangdServer::workspaceSymbols(
     StringRef Query, int Limit, Callback<std::vector<SymbolInformation>> CB) {
   CB(clangd::getWorkspaceSymbols(Query, Limit, Index,
-                                 RootPath ? *RootPath : ""));
+                                 WorkspaceRoot.getValueOr("")));
 }
 
 void ClangdServer::documentSymbols(

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=344787&r1=344786&r2=344787&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Fri Oct 19 08:42:23 2018
@@ -86,6 +86,11 @@ public:
     /// If set, use this index to augment code completion results.
     SymbolIndex *StaticIndex = nullptr;
 
+    /// Clangd's workspace root. Relevant for "workspace" operations not bound
+    /// to a particular file.
+    /// FIXME: If not set, should use the current working directory.
+    llvm::Optional<std::string> WorkspaceRoot;
+
     /// The resource directory is used to find internal headers, overriding
     /// defaults and -resource-dir compiler flag).
     /// If None, ClangdServer calls CompilerInvocation::GetResourcePath() to
@@ -116,9 +121,6 @@ public:
                const FileSystemProvider &FSProvider,
                DiagnosticsConsumer &DiagConsumer, const Options &Opts);
 
-  /// Set the root path of the workspace.
-  void setRootPath(PathRef RootPath);
-
   /// Add a \p File to the list of tracked C++ files or update the contents if
   /// \p File is already tracked. Also schedules parsing of the AST for it on a
   /// separate thread. When the parsing is complete, DiagConsumer passed in
@@ -255,8 +257,7 @@ private:
       CachedCompletionFuzzyFindRequestByFile;
   mutable std::mutex CachedCompletionFuzzyFindRequestMutex;
 
-  // If set, this represents the workspace path.
-  llvm::Optional<std::string> RootPath;
+  llvm::Optional<std::string> WorkspaceRoot;
   std::shared_ptr<PCHContainerOperations> PCHs;
   /// Used to serialize diagnostic callbacks.
   /// FIXME(ibiryukov): get rid of an extra map and put all version counters

Modified: clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp?rev=344787&r1=344786&r2=344787&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp Fri Oct 19 08:42:23 2018
@@ -42,6 +42,7 @@ MATCHER_P(SymRange, Range, "") { return
 
 ClangdServer::Options optsForTests() {
   auto ServerOpts = ClangdServer::optsForTest();
+  ServerOpts.WorkspaceRoot = testRoot();
   ServerOpts.BuildDynamicSymbolIndex = true;
   ServerOpts.URISchemes = {"unittest", "file"};
   return ServerOpts;
@@ -53,7 +54,6 @@ public:
       : Server(CDB, FSProvider, DiagConsumer, optsForTests()) {
     // Make sure the test root directory is created.
     FSProvider.Files[testPath("unused")] = "";
-    Server.setRootPath(testRoot());
   }
 
 protected:




More information about the cfe-commits mailing list