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

[David Goldman via Phabricator via cfe-commits]

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

- Swap to stringset and simpler way of tracking modified files


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
@@ -20,6 +20,7 @@
 #include "Transport.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringSet.h"
 #include <memory>
 
 namespace clang {
@@ -122,10 +123,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 llvm::StringSet<> &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,10 +1122,8 @@
 void ClangdLSPServer::applyConfiguration(
     const ConfigurationSettings &Settings) {
   // Per-file update to the compilation database.
-  bool ShouldReparseOpenFiles = false;
+  llvm::StringSet<> ModifiedFiles;
   for (auto &Entry : Settings.compilationDatabaseChanges) {
-    /// The opened files need to be reparsed only when some existing
-    /// entries are changed.
     PathRef File = Entry.first;
     auto Old = CDB->getCompileCommand(File);
     auto New =
@@ -1134,11 +1132,11 @@
                                 /*Output=*/"");
     if (Old != New) {
       CDB->setCompileCommand(File, std::move(New));
-      ShouldReparseOpenFiles = true;
+      ModifiedFiles.insert(File);
     }
   }
-  if (ShouldReparseOpenFiles)
-    reparseOpenedFiles();
+
+  reparseOpenedFiles(ModifiedFiles);
 }
 
 void ClangdLSPServer::publishSemanticHighlighting(
@@ -1391,10 +1389,15 @@
   notify("textDocument/clangd.fileStatus", Status.render(File));
 }
 
-void ClangdLSPServer::reparseOpenedFiles() {
+void ClangdLSPServer::reparseOpenedFiles(
+    const llvm::StringSet<> &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.239907.patch
Type: text/x-patch
Size: 3550 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200123/11d54009/attachment.bin>