Author: simark Date: Thu Feb 22 06:00:39 2018 New Revision: 325784 URL: http://llvm.org/viewvc/llvm-project?rev=325784&view=rev Log: [clangd] DidChangeConfiguration Notification
Summary: Implementation of DidChangeConfiguration notification handling in clangd. This currently only supports changing one setting: the path of the compilation database to be used for the current project. In other words, it is no longer necessary to restart clangd with a different command line argument in order to change the compilation database. Reviewers: malaperle, krasimir, bkramer, ilya-biryukov Subscribers: jkorous-apple, ioeric, simark, klimek, ilya-biryukov, arphaman, rwols, cfe-commits Differential Revision: https://reviews.llvm.org/D39571 Signed-off-by: Simon Marchi <simon.mar...@ericsson.com> Signed-off-by: William Enright <william.enri...@polymtl.ca> Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp clang-tools-extra/trunk/clangd/ClangdLSPServer.h clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/clangd/DraftStore.cpp clang-tools-extra/trunk/clangd/DraftStore.h clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h clang-tools-extra/trunk/clangd/Protocol.cpp clang-tools-extra/trunk/clangd/Protocol.h clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp clang-tools-extra/trunk/clangd/ProtocolHandlers.h clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Feb 22 06:00:39 2018 @@ -370,6 +370,18 @@ void ClangdLSPServer::onHover(TextDocume }); } +// 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, Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Thu Feb 22 06:00:39 2018 @@ -76,6 +76,7 @@ private: void onCommand(ExecuteCommandParams &Params) override; void onRename(RenameParams &Parames) override; void onHover(TextDocumentPositionParams &Params) override; + void onChangeConfiguration(DidChangeConfigurationParams &Params) override; std::vector<TextEdit> getFixIts(StringRef File, const clangd::Diagnostic &D); Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Feb 22 06:00:39 2018 @@ -554,6 +554,11 @@ void ClangdServer::scheduleReparseAndDia WantDiags, 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. Modified: clang-tools-extra/trunk/clangd/ClangdServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.h Thu Feb 22 06:00:39 2018 @@ -163,6 +163,11 @@ public: /// 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. /// Modified: clang-tools-extra/trunk/clangd/DraftStore.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/DraftStore.cpp?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/DraftStore.cpp (original) +++ clang-tools-extra/trunk/clangd/DraftStore.cpp Thu Feb 22 06:00:39 2018 @@ -21,6 +21,17 @@ VersionedDraft DraftStore::getDraft(Path 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); Modified: clang-tools-extra/trunk/clangd/DraftStore.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/DraftStore.h?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/DraftStore.h (original) +++ clang-tools-extra/trunk/clangd/DraftStore.h Thu Feb 22 06:00:39 2018 @@ -40,6 +40,12 @@ public: /// \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; Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original) +++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Thu Feb 22 06:00:39 2018 @@ -51,6 +51,12 @@ DirectoryBasedGlobalCompilationDatabase: 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); Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h (original) +++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h Thu Feb 22 06:00:39 2018 @@ -61,6 +61,9 @@ public: /// 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); Modified: clang-tools-extra/trunk/clangd/Protocol.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Protocol.cpp (original) +++ clang-tools-extra/trunk/clangd/Protocol.cpp Thu Feb 22 06:00:39 2018 @@ -497,5 +497,15 @@ json::Expr toJSON(const DocumentHighligh }; } +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 Modified: clang-tools-extra/trunk/clangd/Protocol.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Protocol.h (original) +++ clang-tools-extra/trunk/clangd/Protocol.h Thu Feb 22 06:00:39 2018 @@ -324,6 +324,20 @@ struct DidChangeWatchedFilesParams { }; 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 = 0; Modified: clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp (original) +++ clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp Thu Feb 22 06:00:39 2018 @@ -72,4 +72,6 @@ void clangd::registerCallbackHandlers(JS Register("workspace/executeCommand", &ProtocolCallbacks::onCommand); Register("textDocument/documentHighlight", &ProtocolCallbacks::onDocumentHighlight); + Register("workspace/didChangeConfiguration", + &ProtocolCallbacks::onChangeConfiguration); } Modified: clang-tools-extra/trunk/clangd/ProtocolHandlers.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ProtocolHandlers.h?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ProtocolHandlers.h (original) +++ clang-tools-extra/trunk/clangd/ProtocolHandlers.h Thu Feb 22 06:00:39 2018 @@ -52,6 +52,7 @@ public: virtual void onRename(RenameParams &Parames) = 0; virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0; virtual void onHover(TextDocumentPositionParams &Params) = 0; + virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0; }; void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out, Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=325784&r1=325783&r2=325784&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Thu Feb 22 06:00:39 2018 @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// +#include "Annotations.h" #include "ClangdLSPServer.h" #include "ClangdServer.h" #include "Matchers.h" @@ -77,6 +78,43 @@ private: VFSTag LastVFSTag = VFSTag(); }; +/// For each file, record whether the last published diagnostics contained at +/// least one error. +class MultipleErrorCheckingDiagConsumer : public DiagnosticsConsumer { +public: + void + onDiagnosticsReady(PathRef File, + Tagged<std::vector<DiagWithFixIts>> Diagnostics) override { + bool HadError = diagsContainErrors(Diagnostics.Value); + + std::lock_guard<std::mutex> Lock(Mutex); + LastDiagsHadError[File] = HadError; + } + + /// Exposes all files consumed by onDiagnosticsReady in an unspecified order. + /// For each file, a bool value indicates whether the last diagnostics + /// contained an error. + std::vector<std::pair<Path, bool>> filesWithDiags() const { + std::vector<std::pair<Path, bool>> Result; + std::lock_guard<std::mutex> Lock(Mutex); + + for (const auto &it : LastDiagsHadError) { + Result.emplace_back(it.first(), it.second); + } + + return Result; + } + + void clear() { + std::lock_guard<std::mutex> Lock(Mutex); + LastDiagsHadError.clear(); + } + +private: + mutable std::mutex Mutex; + llvm::StringMap<bool> LastDiagsHadError; +}; + /// Replaces all patterns of the form 0x123abc with spaces std::string replacePtrsInDump(std::string const &Dump) { llvm::Regex RE("0x[0-9a-fA-F]+"); @@ -413,6 +451,72 @@ int main() { return 0; } EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); } +// Test ClangdServer.reparseOpenedFiles. +TEST_F(ClangdVFSTest, ReparseOpenedFiles) { + Annotations FooSource(R"cpp( +#ifdef MACRO +$one[[static void bob() {}]] +#else +$two[[static void bob() {}]] +#endif + +int main () { bo^b (); return 0; } +)cpp"); + + Annotations BarSource(R"cpp( +#ifdef MACRO +this is an error +#endif +)cpp"); + + Annotations BazSource(R"cpp( +int hello; +)cpp"); + + MockFSProvider FS; + MockCompilationDatabase CDB; + MultipleErrorCheckingDiagConsumer DiagConsumer; + ClangdServer Server(CDB, DiagConsumer, FS, + /*AsyncThreadsCount=*/0, + /*StorePreamblesInMemory=*/true); + + auto FooCpp = testPath("foo.cpp"); + auto BarCpp = testPath("bar.cpp"); + auto BazCpp = testPath("baz.cpp"); + + FS.Files[FooCpp] = ""; + FS.Files[BarCpp] = ""; + FS.Files[BazCpp] = ""; + + CDB.ExtraClangFlags = {"-DMACRO=1"}; + Server.addDocument(FooCpp, FooSource.code()); + Server.addDocument(BarCpp, BarSource.code()); + Server.addDocument(BazCpp, BazSource.code()); + + EXPECT_THAT(DiagConsumer.filesWithDiags(), + UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, true), + Pair(BazCpp, false))); + + auto Locations = runFindDefinitions(Server, FooCpp, FooSource.point()); + EXPECT_TRUE(bool(Locations)); + EXPECT_THAT(Locations->Value, ElementsAre(Location{URIForFile{FooCpp}, + FooSource.range("one")})); + + // Undefine MACRO, close baz.cpp. + CDB.ExtraClangFlags.clear(); + DiagConsumer.clear(); + Server.removeDocument(BazCpp); + Server.reparseOpenedFiles(); + + EXPECT_THAT(DiagConsumer.filesWithDiags(), + UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, false))); + + Locations = runFindDefinitions(Server, FooCpp, FooSource.point()); + EXPECT_TRUE(bool(Locations)); + EXPECT_THAT(Locations->Value, ElementsAre(Location{URIForFile{FooCpp}, + FooSource.range("two")})); +} + TEST_F(ClangdVFSTest, MemoryUsage) { MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits