[clang-tools-extra] 60249c2 - [clangd] Only re-open files if their flags changed

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2020-01-27T10:58:20-05:00
New Revision: 60249c2c3b9e268af6ade0a4be3c883d7d567940

URL: https://github.com/llvm/llvm-project/commit/60249c2c3b9e268af6ade0a4be3c883d7d567940
DIFF: https://github.com/llvm/llvm-project/commit/60249c2c3b9e268af6ade0a4be3c883d7d567940.diff

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

Reviewers: sammccall

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

Tags: #clang

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 0f87a81227a9..982e05f68fdd 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1126,10 +1126,8 @@ void ClangdLSPServer::onResolveTypeHierarchy(
 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 =
@@ -1138,11 +1136,11 @@ void ClangdLSPServer::applyConfiguration(
                                 /*Output=*/"");
     if (Old != New) {
       CDB->setCompileCommand(File, std::move(New));
-      ShouldReparseOpenFiles = true;
+      ModifiedFiles.insert(File);
     }
   }
-  if (ShouldReparseOpenFiles)
-    reparseOpenedFiles();
+
+  reparseOpenedFiles(ModifiedFiles);
 }
 
 void ClangdLSPServer::publishSemanticHighlighting(
@@ -1463,10 +1461,15 @@ void ClangdLSPServer::onFileUpdated(PathRef File, const TUStatus &Status) {
   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

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index c436f7114610..a186d05f08f9 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/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 {
@@ -123,10 +124,10 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
   /// 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.

diff  --git a/clang-tools-extra/clangd/test/did-change-configuration-params.test b/clang-tools-extra/clangd/test/did-change-configuration-params.test
index bd8ffafcd592..6f9b537e5414 100644
--- a/clang-tools-extra/clangd/test/did-change-configuration-params.test
+++ b/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"}
 ---