This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL325784: [clangd] DidChangeConfiguration Notification (authored by simark, committed by ). Herald added a subscriber: llvm-commits.
Changed prior to commit: https://reviews.llvm.org/D39571?vs=135229&id=135407#toc Repository: rL LLVM https://reviews.llvm.org/D39571 Files: 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
Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// +#include "Annotations.h" #include "ClangdLSPServer.h" #include "ClangdServer.h" #include "Matchers.h" @@ -77,6 +78,43 @@ 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 @@ 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; Index: clang-tools-extra/trunk/clangd/ProtocolHandlers.h =================================================================== --- clang-tools-extra/trunk/clangd/ProtocolHandlers.h +++ clang-tools-extra/trunk/clangd/ProtocolHandlers.h @@ -52,6 +52,7 @@ 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, Index: clang-tools-extra/trunk/clangd/ClangdServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -163,6 +163,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: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp =================================================================== --- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp +++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/clangd/DraftStore.h =================================================================== --- clang-tools-extra/trunk/clangd/DraftStore.h +++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/clangd/ClangdLSPServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h @@ -76,6 +76,7 @@ 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); Index: clang-tools-extra/trunk/clangd/Protocol.cpp =================================================================== --- clang-tools-extra/trunk/clangd/Protocol.cpp +++ clang-tools-extra/trunk/clangd/Protocol.cpp @@ -497,5 +497,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: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h =================================================================== --- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h +++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp +++ clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp @@ -72,4 +72,6 @@ Register("workspace/executeCommand", &ProtocolCallbacks::onCommand); Register("textDocument/documentHighlight", &ProtocolCallbacks::onDocumentHighlight); + Register("workspace/didChangeConfiguration", + &ProtocolCallbacks::onChangeConfiguration); } Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -554,6 +554,11 @@ 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. Index: clang-tools-extra/trunk/clangd/DraftStore.cpp =================================================================== --- clang-tools-extra/trunk/clangd/DraftStore.cpp +++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/clangd/Protocol.h =================================================================== --- clang-tools-extra/trunk/clangd/Protocol.h +++ clang-tools-extra/trunk/clangd/Protocol.h @@ -324,6 +324,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 = 0; Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp @@ -370,6 +370,18 @@ }); } +// 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