simark updated this revision to Diff 135153. simark added a comment. New version, take 2
The previous version contains the changes of already merged patches, I'm not sure what I did wrong. This is another try. 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/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp =================================================================== --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// +#include "Annotations.h" #include "ClangdLSPServer.h" #include "ClangdServer.h" #include "Matchers.h" @@ -77,6 +78,39 @@ 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; + } + + bool contains(PathRef P) { + std::lock_guard<std::mutex> Lock(Mutex); + return LastDiagsHadError.find(P) != LastDiagsHadError.end(); + } + + bool lastHadError(PathRef P) { + std::lock_guard<std::mutex> Lock(Mutex); + return LastDiagsHadError[P]; + } + + void clear() { + std::lock_guard<std::mutex> Lock(Mutex); + LastDiagsHadError.clear(); + } + +private: + std::mutex Mutex; + std::map<Path, 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 +447,78 @@ 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_TRUE(DiagConsumer.contains(FooCpp)); + EXPECT_TRUE(DiagConsumer.contains(BarCpp)); + EXPECT_TRUE(DiagConsumer.contains(BazCpp)); + EXPECT_FALSE(DiagConsumer.lastHadError(FooCpp)); + EXPECT_TRUE(DiagConsumer.lastHadError(BarCpp)); + EXPECT_FALSE(DiagConsumer.lastHadError(BazCpp)); + + 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_TRUE(DiagConsumer.contains(FooCpp)); + EXPECT_TRUE(DiagConsumer.contains(BarCpp)); + EXPECT_FALSE(DiagConsumer.contains(BazCpp)); + EXPECT_FALSE(DiagConsumer.lastHadError(FooCpp)); + EXPECT_FALSE(DiagConsumer.lastHadError(BarCpp)); + + 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: clangd/ProtocolHandlers.h =================================================================== --- clangd/ProtocolHandlers.h +++ 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: clangd/ProtocolHandlers.cpp =================================================================== --- clangd/ProtocolHandlers.cpp +++ clangd/ProtocolHandlers.cpp @@ -72,4 +72,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 @@ -318,6 +318,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: clangd/Protocol.cpp =================================================================== --- clangd/Protocol.cpp +++ 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: 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 @@ -556,6 +556,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 @@ -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: clangd/ClangdLSPServer.cpp =================================================================== --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -369,6 +369,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