simark updated this revision to Diff 134097.
simark added a comment.

Add tests, work in progress

Can you take a look at the "TEST(DidChangeConfiguration, DifferentDeclaration)"
test, and tell me if you see anything wrong with it?  It seems to me like
changes in command line arguments (adding -DMACRO=1) are not taken into account
when changing configuration.

The other test (also very much work-in-progress) would be to verify that
changing configuration gets us the right diagnostics.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39571

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/DraftStore.cpp
  clangd/DraftStore.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===================================================================
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -257,6 +257,86 @@
                                    SourceAnnotations.range()}));
 }
 
+TEST(DidChangeConfiguration, DifferentDeclaration) {
+  Annotations SourceAnnotations(R"cpp(
+#ifndef MACRO
+$before[[static void bob() {}]]
+#else
+$after[[static void bob() {}]]
+#endif
+
+int main() { b^ob(); }
+)cpp");
+
+  MockCompilationDatabase CDB(/*UseRelPaths=*/true);
+  MockFSProvider FS;
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  FS.Files[FooCpp] = "";
+
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, DiagConsumer, FS, /*AsyncThreadsCount=*/0,
+                      /*StorePreambleInMemory=*/true);
+
+  Server.addDocument(FooCpp, SourceAnnotations.code());
+
+  /* Without MACRO defined.  */
+  auto Locations = Server.findDefinitions(FooCpp, SourceAnnotations.point());
+  EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+  EXPECT_THAT(Locations->Value,
+              ElementsAre(Location{URIForFile{FooCpp.str()},
+                                   SourceAnnotations.range("before")}));
+
+  CDB.ExtraClangFlags.push_back("-DMACRO=1");
+  Server.reparseOpenedFiles();
+
+  /* With MACRO defined.  */
+  Locations = Server.findDefinitions(FooCpp, SourceAnnotations.point());
+  EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+  EXPECT_THAT(Locations->Value,
+              ElementsAre(Location{URIForFile{FooCpp.str()},
+                                   SourceAnnotations.range("after")}));
+}
+
+class MyDiags : public DiagnosticsConsumer {
+  void onDiagnosticsReady(
+      PathRef File, Tagged<std::vector<DiagWithFixIts>> Diagnostics) override {
+    std::cout << "There are " << Diagnostics.Value.size() << std::endl;
+    for (DiagWithFixIts &Diag : Diagnostics.Value) {
+      std::cout << "  " << Diag.Diag.message << std::endl;
+    }
+  }
+};
+
+TEST(DidChangeConfiguration, Diagnostics) {
+  Annotations SourceAnnotations(R"cpp(
+#ifdef WITH_ERROR
+this is an error
+#endif
+
+int main() { return 0; }
+)cpp");
+
+  MockCompilationDatabase CDB(/*UseRelPaths=*/true);
+  CDB.ExtraClangFlags.push_back("-DWITH_ERROR=1");
+  MockFSProvider FS;
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  FS.Files[FooCpp] = "";
+
+  MyDiags DiagConsumer;
+  ClangdServer Server(CDB, DiagConsumer, FS, /*AsyncThreadsCount=*/0,
+                      /*StorePreambleInMemory=*/true);
+
+  Server.addDocument(FooCpp, SourceAnnotations.code());
+
+
+  CDB.ExtraClangFlags.clear();
+
+  Server.reparseOpenedFiles();
+
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/ProtocolHandlers.h
===================================================================
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -51,6 +51,7 @@
   virtual void onCommand(ExecuteCommandParams &Params) = 0;
   virtual void onRename(RenameParams &Parames) = 0;
   virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0;
+  virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out,
Index: clangd/ProtocolHandlers.cpp
===================================================================
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -71,4 +71,6 @@
   Register("workspace/executeCommand", &ProtocolCallbacks::onCommand);
   Register("textDocument/documentHighlight",
            &ProtocolCallbacks::onDocumentHighlight);
+  Register("workspace/didChangeConfiguration",
+           &ProtocolCallbacks::onChangeConfiguration);
 }
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -265,6 +265,20 @@
 };
 bool fromJSON(const json::Expr &, DidChangeWatchedFilesParams &);
 
+/// 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;
+};
+bool fromJSON(const json::Expr &, ClangdConfigurationParamsChange &);
+
+struct DidChangeConfigurationParams {
+  // We use this predefined struct because it is easier to use
+  // than the protocol specified type of 'any'.
+  ClangdConfigurationParamsChange settings;
+};
+bool fromJSON(const json::Expr &, DidChangeConfigurationParams &);
+
 struct FormattingOptions {
   /// Size of a tab in spaces.
   int tabSize;
Index: clangd/Protocol.cpp
===================================================================
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -400,5 +400,15 @@
   };
 }
 
+bool fromJSON(const json::Expr &Params, DidChangeConfigurationParams &CCP) {
+  json::ObjectMapper O(Params);
+  return O && O.map("settings", CCP.settings);
+}
+
+bool fromJSON(const json::Expr &Params, ClangdConfigurationParamsChange &CCPC) {
+  json::ObjectMapper O(Params);
+  return O && O.map("compilationDatabasePath", CCPC.compilationDatabasePath);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/GlobalCompilationDatabase.h
===================================================================
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -61,6 +61,9 @@
   /// Uses the default fallback command, adding any extra flags.
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
 
+  /// Set the compile commands directory to \p P.
+  void setCompileCommandsDir(Path P);
+
   /// Sets the extra flags that should be added to a file.
   void setExtraFlagsForFile(PathRef File, std::vector<std::string> ExtraFlags);
 
Index: clangd/GlobalCompilationDatabase.cpp
===================================================================
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -51,6 +51,12 @@
   return C;
 }
 
+void DirectoryBasedGlobalCompilationDatabase::setCompileCommandsDir(Path P) {
+  std::lock_guard<std::mutex> Lock(Mutex);
+  CompileCommandsDir = P;
+  CompilationDatabases.clear();
+}
+
 void DirectoryBasedGlobalCompilationDatabase::setExtraFlagsForFile(
     PathRef File, std::vector<std::string> ExtraFlags) {
   std::lock_guard<std::mutex> Lock(Mutex);
Index: clangd/DraftStore.h
===================================================================
--- clangd/DraftStore.h
+++ clangd/DraftStore.h
@@ -40,6 +40,12 @@
   /// \return version and contents of the stored document.
   /// For untracked files, a (0, None) pair is returned.
   VersionedDraft getDraft(PathRef File) const;
+
+  /// \return List of names of active drafts in this store.  Drafts that were
+  /// removed (which still have an entry in the Drafts map) are not returned by
+  /// this function.
+  std::vector<Path> getActiveFiles() const;
+
   /// \return version of the tracked document.
   /// For untracked files, 0 is returned.
   DocVersion getVersion(PathRef File) const;
Index: clangd/DraftStore.cpp
===================================================================
--- clangd/DraftStore.cpp
+++ clangd/DraftStore.cpp
@@ -21,6 +21,17 @@
   return It->second;
 }
 
+std::vector<Path> DraftStore::getActiveFiles() const {
+  std::lock_guard<std::mutex> Lock(Mutex);
+  std::vector<Path> ResultVector;
+
+  for (auto DraftIt = Drafts.begin(); DraftIt != Drafts.end(); DraftIt++)
+    if (DraftIt->second.Draft)
+      ResultVector.push_back(DraftIt->getKey());
+
+  return ResultVector;
+}
+
 DocVersion DraftStore::getVersion(PathRef File) const {
   std::lock_guard<std::mutex> Lock(Mutex);
 
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -162,6 +162,11 @@
   /// and AST and rebuild them from scratch.
   void forceReparse(PathRef File);
 
+  /// Calls forceReparse() on 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();
+
   /// Run code completion for \p File at \p Pos.
   /// Request is processed asynchronously.
   ///
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -500,6 +500,11 @@
                        std::move(Callback));
 }
 
+void ClangdServer::reparseOpenedFiles() {
+  for (const Path &FilePath : DraftMgr.getActiveFiles())
+    forceReparse(FilePath);
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -75,6 +75,7 @@
   void onFileEvent(DidChangeWatchedFilesParams &Params) override;
   void onCommand(ExecuteCommandParams &Params) override;
   void onRename(RenameParams &Parames) override;
+  void onChangeConfiguration(DidChangeConfigurationParams &Params) override;
 
   std::vector<TextEdit> getFixIts(StringRef File, const clangd::Diagnostic &D);
 
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -320,6 +320,18 @@
   reply(json::ary(Highlights->Value));
 }
 
+// FIXME: This function needs to be properly tested.
+void ClangdLSPServer::onChangeConfiguration(
+    DidChangeConfigurationParams &Params) {
+  ClangdConfigurationParamsChange &Settings = Params.settings;
+
+  // Compilation database change.
+  if (Settings.compilationDatabasePath.hasValue()) {
+    CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
+    Server.reparseOpenedFiles();
+  }
+}
+
 ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
                                  bool StorePreamblesInMemory,
                                  const clangd::CodeCompleteOptions &CCOpts,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to