arphaman updated this revision to Diff 157762.
arphaman marked 2 inline comments as done.
arphaman added a comment.

Updated patch to address review comments:

- The compilation database updated are used only when 
'-in-memory-compile-commands' flag is used.
- It's now possible to set the working directory as well.


https://reviews.llvm.org/D49758

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/tool/ClangdMain.cpp
  test/clangd/did-change-configuration-params.test

Index: test/clangd/did-change-configuration-params.test
===================================================================
--- /dev/null
+++ test/clangd/did-change-configuration-params.test
@@ -0,0 +1,51 @@
+# RUN: clangd -in-memory-compile-commands -lit-test < %s 2> %t | FileCheck -strict-whitespace %s
+# RUN: cat %t | FileCheck --check-prefix=ERR %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabaseChanges":{"/clangd-test/foo.c": {"workingDirectory":"/clangd-test", "compilationCommand": ["clang", "-c", "foo.c"]}}}}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main() { int i; return i; }"}}}
+#      CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:    "diagnostics": [],
+# CHECK-NEXT:    "uri": "file://{{.*}}/foo.c"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///bar.c","languageId":"c","version":1,"text":"int main() { int i; return i; }"}}}
+#      CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:    "diagnostics": [],
+# CHECK-NEXT:    "uri": "file://{{.*}}/bar.c"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabaseChanges":{"/clangd-test/foo.c": {"workingDirectory":"/clangd-test2", "compilationCommand": ["clang", "-c", "foo.c", "-Wall", "-Werror"]}}}}}
+#      CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:    "diagnostics": [
+# CHECK-NEXT:      {
+# CHECK-NEXT:        "message": "variable 'i' is uninitialized when used here",
+# CHECK-NEXT:        "range": {
+# CHECK-NEXT:          "end": {
+# CHECK-NEXT:            "character": 28,
+# CHECK-NEXT:            "line": 0
+# CHECK-NEXT:          },
+# CHECK-NEXT:          "start": {
+# CHECK-NEXT:            "character": 27,
+# CHECK-NEXT:            "line": 0
+# CHECK-NEXT:          }
+# CHECK-NEXT:        },
+# CHECK-NEXT:        "severity": 1
+# CHECK-NEXT:      }
+# CHECK-NEXT:    ],
+# CHECK-NEXT:    "uri": "file://{{.*}}/foo.c"
+# CHECK-NEXT:  }
+#
+# ERR: Updating file /clangd-test/foo.c with command [/clangd-test2] clang -c foo.c -Wall -Werror
+# Don't reparse the second file:
+# ERR: Skipping rebuild of the AST for /clangd-test/bar.c
+---
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
+
Index: clangd/tool/ClangdMain.cpp
===================================================================
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -166,6 +166,11 @@
         "eventually. Don't rely on it."),
     llvm::cl::init(""), llvm::cl::Hidden);
 
+static llvm::cl::opt<bool>
+    InMemoryCompileCommands("in-memory-compile-commands",
+                            llvm::cl::desc("Use an in-memory compile commands"),
+                            llvm::cl::init(false), llvm::cl::Hidden);
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
@@ -278,7 +283,9 @@
   CCOpts.ShowOrigins = ShowOrigins;
 
   // Initialize and run ClangdLSPServer.
-  ClangdLSPServer LSPServer(Out, CCOpts, CompileCommandsDirPath, Opts);
+  ClangdLSPServer LSPServer(Out, CCOpts, CompileCommandsDirPath,
+                            /*ShouldUseInMemoryCDB=*/InMemoryCompileCommands,
+                            Opts);
   constexpr int NoShutdownRequestErrorCode = 1;
   llvm::set_thread_name("clangd.main");
   // Change stdin to binary to not lose \r\n on windows.
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -419,10 +419,26 @@
 };
 bool fromJSON(const llvm::json::Value &, DidChangeWatchedFilesParams &);
 
+/// Clangd extension that's used in the 'compilationDatabaseChanges' in
+/// workspace/didChangeConfiguration to record updates to the in-memory
+/// compilation database.
+struct ClangdConfigurationCompilationDatabaseUpdate {
+  std::string workingDirectory;
+  std::vector<std::string> compilationCommand;
+};
+bool fromJSON(const llvm::json::Value &,
+              ClangdConfigurationCompilationDatabaseUpdate &);
+
 /// Clangd extension to manage a workspace/didChangeConfiguration notification
 /// since the data received is described as 'any' type in LSP.
 struct ClangdConfigurationParamsChange {
   llvm::Optional<std::string> compilationDatabasePath;
+
+  // The changes that happened to the compilation database.
+  // The key of the map is a file name.
+  llvm::Optional<
+      std::map<std::string, ClangdConfigurationCompilationDatabaseUpdate>>
+      compilationDatabaseChanges;
 };
 bool fromJSON(const llvm::json::Value &, ClangdConfigurationParamsChange &);
 
Index: clangd/Protocol.cpp
===================================================================
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -591,10 +591,18 @@
   return O && O.map("settings", CCP.settings);
 }
 
+bool fromJSON(const llvm::json::Value &Params,
+              ClangdConfigurationCompilationDatabaseUpdate &CDbUpdate) {
+  json::ObjectMapper O(Params);
+  return O && O.map("workingDirectory", CDbUpdate.workingDirectory) &&
+         O.map("compilationCommand", CDbUpdate.compilationCommand);
+}
+
 bool fromJSON(const json::Value &Params,
               ClangdConfigurationParamsChange &CCPC) {
   json::ObjectMapper O(Params);
-  return O && O.map("compilationDatabasePath", CCPC.compilationDatabasePath);
+  return O && O.map("compilationDatabasePath", CCPC.compilationDatabasePath) &&
+         O.map("compilationDatabaseChanges", CCPC.compilationDatabaseChanges);
 }
 
 } // namespace clangd
Index: clangd/GlobalCompilationDatabase.h
===================================================================
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -113,6 +113,30 @@
       Cached; /* GUARDED_BY(Mut) */
 };
 
+/// Gets compile args from an in-memory mapping based on a filepath. Typically
+/// used by clients who provide the compile commands themselves.
+class InMemoryCompilationDb : public GlobalCompilationDatabase {
+public:
+  /// Gets compile command for \p File from the stored mapping.
+  llvm::Optional<tooling::CompileCommand>
+  getCompileCommand(PathRef File) const override;
+
+  /// Sets the compilation command for a particular file.
+  ///
+  /// \returns True if the File had no compilation command before.
+  bool setCompilationCommandForFile(PathRef File,
+                                    tooling::CompileCommand CompilationCommand);
+
+  /// Removes the compilation command for \p File if it's present in the
+  /// mapping.
+  void invalidate(PathRef File);
+
+private:
+  mutable std::mutex Mutex;
+  mutable llvm::StringMap<llvm::Optional<tooling::CompileCommand>>
+      Commands; /* GUARDED_BY(Mut) */
+};
+
 } // namespace clangd
 } // namespace clang
 
Index: clangd/GlobalCompilationDatabase.cpp
===================================================================
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -152,5 +152,28 @@
   Cached.clear();
 }
 
+llvm::Optional<tooling::CompileCommand>
+InMemoryCompilationDb::getCompileCommand(PathRef File) const {
+  std::lock_guard<std::mutex> Lock(Mutex);
+  auto It = Commands.find(File);
+  if (It == Commands.end())
+    return None;
+  return *It->second;
+}
+
+bool InMemoryCompilationDb::setCompilationCommandForFile(
+    PathRef File, tooling::CompileCommand CompilationCommand) {
+  std::unique_lock<std::mutex> Lock(Mutex);
+  llvm::Optional<tooling::CompileCommand> &Command = Commands[File];
+  bool IsNewEntry = !Command.hasValue();
+  Command = std::move(CompilationCommand);
+  return IsNewEntry;
+}
+
+void InMemoryCompilationDb::invalidate(PathRef File) {
+  std::unique_lock<std::mutex> Lock(Mutex);
+  Commands.erase(File);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -35,7 +35,7 @@
   /// for compile_commands.json in all parent directories of each file.
   ClangdLSPServer(JSONOutput &Out, const clangd::CodeCompleteOptions &CCOpts,
                   llvm::Optional<Path> CompileCommandsDir,
-                  const ClangdServer::Options &Opts);
+                  bool ShouldUseInMemoryCDB, const ClangdServer::Options &Opts);
 
   /// Run LSP server loop, receiving input for it from \p In. \p In must be
   /// opened in binary mode. Output will be written using Out variable passed to
@@ -103,6 +103,7 @@
   // before ClangdServer.
   DirectoryBasedGlobalCompilationDatabase NonCachedCDB;
   CachingCompilationDb CDB;
+  InMemoryCompilationDb InMemoryCDB;
 
   RealFileSystemProvider FSProvider;
   /// Options used for code completion
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -247,6 +247,7 @@
   PathRef File = Params.textDocument.uri.file();
   DraftMgr.removeDraft(File);
   Server.removeDocument(File);
+  InMemoryCDB.invalidate(File);
 }
 
 void ClangdLSPServer::onDocumentOnTypeFormatting(
@@ -411,15 +412,38 @@
 
     reparseOpenedFiles();
   }
+
+  // Update to the compilation database.
+  if (Settings.compilationDatabaseChanges) {
+    const auto &CompileCommandUpdates = *Settings.compilationDatabaseChanges;
+    bool ShouldReparseOpenFiles = false;
+    for (auto &Entry : CompileCommandUpdates) {
+      /// The opened files need to be reparsed only when some existing
+      /// entries are changed.
+      PathRef File = Entry.first;
+      if (!InMemoryCDB.setCompilationCommandForFile(
+              File, tooling::CompileCommand(
+                        std::move(Entry.second.workingDirectory),
+                        llvm::sys::path::filename(File),
+                        std::move(Entry.second.compilationCommand),
+                        /*Output=*/"")))
+        ShouldReparseOpenFiles = true;
+    }
+    if (ShouldReparseOpenFiles)
+      reparseOpenedFiles();
+  }
 }
 
 ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
                                  const clangd::CodeCompleteOptions &CCOpts,
                                  llvm::Optional<Path> CompileCommandsDir,
+                                 bool ShouldUseInMemoryCDB,
                                  const ClangdServer::Options &Opts)
     : Out(Out), NonCachedCDB(std::move(CompileCommandsDir)), CDB(NonCachedCDB),
       CCOpts(CCOpts), SupportedSymbolKinds(defaultSymbolKinds()),
-      Server(CDB, FSProvider, /*DiagConsumer=*/*this, Opts) {}
+      Server(ShouldUseInMemoryCDB ? (GlobalCompilationDatabase &)InMemoryCDB
+                                  : CDB,
+             FSProvider, /*DiagConsumer=*/*this, Opts) {}
 
 bool ClangdLSPServer::run(std::FILE *In, JSONStreamStyle InputStyle) {
   assert(!IsDone && "Run was called before");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to