simark created this revision.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov.

This patch adds support for watching for changes to
compile_commands.json, and reparsing files if needed.

The watching is done using the "workspace/didChangeWatchedFiles"
notification, so the actual file watching is the frontend's problem.

To keep things simple, we react to any change involving a file called
"compile_commands.json".

The chosen strategy tries to avoid useless reparses.  We don't want to
reparse a file if its compile commands don't change.  So when we get a
change notification, we:

1. Save the compile commands of all open files on the side.
2. Clear everything we know about compilation databases and compilation 
commands.
3. Query the compile commands again for all open files (which will go read the 
possibly updated compile commands).  If the commands are different than the 
saved ones, we reparse the file.

I updated the vscode-clangd extension.  If you don't feel inspired, you
can use this small .cpp to test it:

  #ifdef MACRO
  #warning "MACRO is defined"
  #else
  #warning "MACRO is not defined"
  #endif
  
  int main() {}

and this compile_commands.json:

  [{
    "directory": ".",
    "file": "test.cpp",
    "arguments": ["g++", "-c", "test.cpp", "-DMACRO=2"]
  }]

Adding and removing the "-DMACRO=2" argument, you should see a different


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49267

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/clients/clangd-vscode/src/extension.ts

Index: clangd/clients/clangd-vscode/src/extension.ts
===================================================================
--- clangd/clients/clangd-vscode/src/extension.ts
+++ clangd/clients/clangd-vscode/src/extension.ts
@@ -30,25 +30,32 @@
     }
     const serverOptions: vscodelc.ServerOptions = clangd;
 
-    const filePattern: string = '**/*.{' +
-        ['cpp', 'c', 'cc', 'cxx', 'c++', 'm', 'mm', 'h', 'hh', 'hpp', 'hxx', 'inc'].join() + '}';
+    const sourceFilePattern: string = '**/*.{' + [
+      'cpp', 'c', 'cc', 'cxx', 'c++', 'm', 'mm', 'h', 'hh', 'hpp', 'hxx', 'inc'
+    ].join() + '}';
+    const compileCommandsFilePattern: string = '**/compile_commands.json';
     const clientOptions: vscodelc.LanguageClientOptions = {
-        // Register the server for C/C++ files
-        documentSelector: [{ scheme: 'file', pattern: filePattern }],
-        synchronize: !syncFileEvents ? undefined : {
-            fileEvents: vscode.workspace.createFileSystemWatcher(filePattern)
-        },
-        // Resolve symlinks for all files provided by clangd.
-        // This is a workaround for a bazel + clangd issue - bazel produces a symlink tree to build in,
-        // and when navigating to the included file, clangd passes its path inside the symlink tree
-        // rather than its filesystem path.
-        // FIXME: remove this once clangd knows enough about bazel to resolve the
-        // symlinks where needed (or if this causes problems for other workflows).
-        uriConverters: {
-            code2Protocol: (value: vscode.Uri) => value.toString(),
-            protocol2Code: (value: string) =>
-                vscode.Uri.file(realpathSync(vscode.Uri.parse(value).fsPath))
-        }
+      // Register the server for C/C++ files
+      documentSelector : [ {scheme : 'file', pattern : sourceFilePattern} ],
+      synchronize : !syncFileEvents ? undefined : {
+        fileEvents : [
+          vscode.workspace.createFileSystemWatcher(sourceFilePattern),
+          vscode.workspace.createFileSystemWatcher(compileCommandsFilePattern),
+        ]
+      },
+      // Resolve symlinks for all files provided by clangd.
+      // This is a workaround for a bazel + clangd issue - bazel produces a
+      // symlink tree to build in,
+      // and when navigating to the included file, clangd passes its path inside
+      // the symlink tree
+      // rather than its filesystem path.
+      // FIXME: remove this once clangd knows enough about bazel to resolve the
+      // symlinks where needed (or if this causes problems for other workflows).
+      uriConverters : {
+        code2Protocol : (value: vscode.Uri) => value.toString(),
+        protocol2Code : (value: string) =>
+            vscode.Uri.file(realpathSync(vscode.Uri.parse(value).fsPath))
+      }
     };
 
     const clangdClient = new vscodelc.LanguageClient('Clang Language Server', serverOptions, clientOptions);
Index: clangd/ProtocolHandlers.h
===================================================================
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -48,7 +48,7 @@
   virtual void onSignatureHelp(TextDocumentPositionParams &Params) = 0;
   virtual void onGoToDefinition(TextDocumentPositionParams &Params) = 0;
   virtual void onSwitchSourceHeader(TextDocumentIdentifier &Params) = 0;
-  virtual void onFileEvent(DidChangeWatchedFilesParams &Params) = 0;
+  virtual void onDidChangeWatchedFiles(DidChangeWatchedFilesParams &Params) = 0;
   virtual void onCommand(ExecuteCommandParams &Params) = 0;
   virtual void onWorkspaceSymbol(WorkspaceSymbolParams &Params) = 0;
   virtual void onRename(RenameParams &Parames) = 0;
Index: clangd/ProtocolHandlers.cpp
===================================================================
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -68,7 +68,8 @@
   Register("textDocument/rename", &ProtocolCallbacks::onRename);
   Register("textDocument/hover", &ProtocolCallbacks::onHover);
   Register("textDocument/documentSymbol", &ProtocolCallbacks::onDocumentSymbol);
-  Register("workspace/didChangeWatchedFiles", &ProtocolCallbacks::onFileEvent);
+  Register("workspace/didChangeWatchedFiles",
+           &ProtocolCallbacks::onDidChangeWatchedFiles);
   Register("workspace/executeCommand", &ProtocolCallbacks::onCommand);
   Register("textDocument/documentHighlight",
            &ProtocolCallbacks::onDocumentHighlight);
Index: clangd/GlobalCompilationDatabase.h
===================================================================
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -68,6 +68,9 @@
   /// Sets the extra flags that should be added to a file.
   void setExtraFlagsForFile(PathRef File, std::vector<std::string> ExtraFlags);
 
+  /// Forget the compilation flags cached in this object.
+  void clear();
+
 private:
   tooling::CompilationDatabase *getCDBForFile(PathRef File) const;
   tooling::CompilationDatabase *getCDBInDirLocked(PathRef File) const;
Index: clangd/GlobalCompilationDatabase.cpp
===================================================================
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -66,6 +66,11 @@
   CompilationDatabases.clear();
 }
 
+void DirectoryBasedGlobalCompilationDatabase::clear() {
+  std::lock_guard<std::mutex> Lock(Mutex);
+  CompilationDatabases.clear();
+}
+
 void DirectoryBasedGlobalCompilationDatabase::setExtraFlagsForFile(
     PathRef File, std::vector<std::string> ExtraFlags) {
   std::lock_guard<std::mutex> Lock(Mutex);
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -69,19 +69,45 @@
   void onGoToDefinition(TextDocumentPositionParams &Params) override;
   void onSwitchSourceHeader(TextDocumentIdentifier &Params) override;
   void onDocumentHighlight(TextDocumentPositionParams &Params) override;
-  void onFileEvent(DidChangeWatchedFilesParams &Params) override;
+  void onDidChangeWatchedFiles(DidChangeWatchedFilesParams &Params) override;
   void onCommand(ExecuteCommandParams &Params) override;
   void onWorkspaceSymbol(WorkspaceSymbolParams &Params) override;
   void onRename(RenameParams &Parames) override;
   void onHover(TextDocumentPositionParams &Params) override;
   void onChangeConfiguration(DidChangeConfigurationParams &Params) override;
 
   std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
 
-  /// Forces a reparse of all currently opened files.  As a result, this method
-  /// may be very expensive.  This method is normally called when the
-  /// compilation database is changed.
-  void reparseOpenedFiles();
+  /// Structure used to save a "snapshot" of the compile commands for the open
+  /// files.  This is used to avoid unnecessary re-parses.
+  ///
+  /// When the compile commands configuration changes (either we point to a new
+  /// compile_commands.json, or a compile_commands.json file has changed), we
+  /// 1. Request the compile commands for all open files, save then in this
+  ///    structure.
+  /// 2. Clear our in-memory cache of the compile commands.
+  /// 3. Request the compile commands for all open files again.  If a file has
+  ///    different compile commands, we re-process it.
+
+  struct CompileCommandsChangeData {
+    struct FileNameCompileCommandPair {
+      FileNameCompileCommandPair(
+          std::string FileName,
+          llvm::Optional<tooling::CompileCommand> CompileCommand)
+          : FileName(std::move(FileName)),
+            CompileCommand(std::move(CompileCommand)) {}
+      std::string FileName;
+      llvm::Optional<tooling::CompileCommand> CompileCommand;
+    };
+    std::vector<FileNameCompileCommandPair> CompileCommands;
+  };
+
+  /// Take a snapshot of the compile commands for the currently open files.
+  CompileCommandsChangeData compileCommandsChangePre();
+  /// Compare the new compile comamnds of all open files with the commands
+  /// saved in \p Data.  If any file has different compile commands than before,
+  // re-parse/process it.
+  void compileCommandsChangePost(const CompileCommandsChangeData &Data);
 
   JSONOutput &Out;
   /// Used to indicate that the 'shutdown' request was received from the
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -167,8 +167,26 @@
   Server.addDocument(File, *Contents, WantDiags);
 }
 
-void ClangdLSPServer::onFileEvent(DidChangeWatchedFilesParams &Params) {
+void ClangdLSPServer::onDidChangeWatchedFiles(
+    DidChangeWatchedFilesParams &Params) {
   Server.onFileEvent(Params);
+
+  // Check if any created/modified/deleted file is called compile_commands.json.
+  bool CompileCommandsChanged = false;
+  for (const FileEvent &FE : Params.changes) {
+    StringRef FileName = llvm::sys::path::filename(FE.uri.file());
+    if (FileName == "compile_commands.json") {
+      CompileCommandsChanged = true;
+      break;
+    }
+  }
+
+  if (CompileCommandsChanged) {
+    auto CCChangeData = compileCommandsChangePre();
+    NonCachedCDB.clear();
+    CDB.clear();
+    compileCommandsChangePost(CCChangeData);
+  }
 }
 
 void ClangdLSPServer::onCommand(ExecuteCommandParams &Params) {
@@ -405,11 +423,11 @@
 
   // Compilation database change.
   if (Settings.compilationDatabasePath.hasValue()) {
+    auto CCChangeData = compileCommandsChangePre();
     NonCachedCDB.setCompileCommandsDir(
         Settings.compilationDatabasePath.getValue());
     CDB.clear();
-
-    reparseOpenedFiles();
+    compileCommandsChangePost(CCChangeData);
   }
 }
 
@@ -493,8 +511,39 @@
   });
 }
 
-void ClangdLSPServer::reparseOpenedFiles() {
-  for (const Path &FilePath : DraftMgr.getActiveFiles())
-    Server.addDocument(FilePath, *DraftMgr.getDraft(FilePath),
-                       WantDiagnostics::Auto);
+ClangdLSPServer::CompileCommandsChangeData
+ClangdLSPServer::compileCommandsChangePre() {
+  ClangdLSPServer::CompileCommandsChangeData Data;
+  std::vector<std::string> OpenFiles = this->DraftMgr.getActiveFiles();
+
+  for (std::string &File : OpenFiles) {
+    llvm::Optional<tooling::CompileCommand> Command =
+        this->CDB.getCompileCommand(File);
+
+    Data.CompileCommands.emplace_back(std::move(File), std::move(Command));
+  }
+
+  return Data;
+}
+
+void ClangdLSPServer::compileCommandsChangePost(
+    const ClangdLSPServer::CompileCommandsChangeData &Data) {
+  for (auto &Item : Data.CompileCommands) {
+    llvm::Optional<tooling::CompileCommand> NewCompileCommand =
+        this->CDB.getCompileCommand(Item.FileName);
+    const llvm::Optional<tooling::CompileCommand> &OldCompileCommand =
+        Item.CompileCommand;
+
+    if (NewCompileCommand != OldCompileCommand) {
+      Server.addDocument(Item.FileName, *DraftMgr.getDraft(Item.FileName),
+                         WantDiagnostics::Auto);
+      vlog("compileCommandsChangePost: File {0} requires reparse after compile "
+           "commands change.",
+           Item.FileName);
+    } else {
+      vlog("compileCommandsChangePost: File {0} does not require reparse after "
+           "compile commands change.",
+           Item.FileName);
+    }
+  }
 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to