[PATCH] D72647: [clangd] Only re-open files if their flags changed

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 237791.
dgoldman added a comment.

- Fix broken did-change-configuration test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72647

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/test/did-change-configuration-params.test


Index: clang-tools-extra/clangd/test/did-change-configuration-params.test
===================================================================
--- clang-tools-extra/clangd/test/did-change-configuration-params.test
+++ clang-tools-extra/clangd/test/did-change-configuration-params.test
@@ -46,8 +46,6 @@
 # ERR: Updating file {{.*}}foo.c with command
 # ERR: [{{.*}}clangd-test2]
 # ERR: clang -c foo.c -Wall -Werror
-# Don't reparse the second file:
-# ERR: Skipping rebuild of the AST for {{.*}}bar.c
 ---
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -122,10 +122,10 @@
   /// produce '->' and '::', respectively.
   bool shouldRunCompletion(const CompletionParams &Params) const;
 
-  /// 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();
+  /// Forces a reparse of all currently opened files which were modified. As a
+  /// result, this method may be very expensive. This method is normally called
+  /// when the compilation database is changed.
+  void reparseOpenedFiles(const std::set<std::string> &ModifiedFiles);
   void applyConfiguration(const ConfigurationSettings &Settings);
 
   /// Sends a "publishSemanticHighlighting" notification to the LSP client.
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1122,7 +1122,11 @@
 void ClangdLSPServer::applyConfiguration(
     const ConfigurationSettings &Settings) {
   // Per-file update to the compilation database.
-  bool ShouldReparseOpenFiles = false;
+  std::set<std::string> ModifiedFiles;
+  auto Sub =
+      CDB->watch([&ModifiedFiles](const std::vector<std::string> Changes) {
+        ModifiedFiles.insert(Changes.begin(), Changes.end());
+      });
   for (auto &Entry : Settings.compilationDatabaseChanges) {
     /// The opened files need to be reparsed only when some existing
     /// entries are changed.
@@ -1134,11 +1138,10 @@
                                 /*Output=*/"");
     if (Old != New) {
       CDB->setCompileCommand(File, std::move(New));
-      ShouldReparseOpenFiles = true;
     }
   }
-  if (ShouldReparseOpenFiles)
-    reparseOpenedFiles();
+
+  reparseOpenedFiles(ModifiedFiles);
 }
 
 void ClangdLSPServer::publishSemanticHighlighting(
@@ -1391,10 +1394,15 @@
   notify("textDocument/clangd.fileStatus", Status.render(File));
 }
 
-void ClangdLSPServer::reparseOpenedFiles() {
+void ClangdLSPServer::reparseOpenedFiles(
+    const std::set<std::string> &ModifiedFiles) {
+  if (ModifiedFiles.empty())
+    return;
+  // Reparse only opened files that were modified.
   for (const Path &FilePath : DraftMgr.getActiveFiles())
-    Server->addDocument(FilePath, *DraftMgr.getDraft(FilePath),
-                        WantDiagnostics::Auto);
+    if (ModifiedFiles.find(FilePath) != ModifiedFiles.end())
+      Server->addDocument(FilePath, *DraftMgr.getDraft(FilePath),
+                          WantDiagnostics::Auto);
 }
 
 } // namespace clangd


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72647.237791.patch
Type: text/x-patch
Size: 3405 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200113/46d40c47/attachment.bin>