From 4fbdbdb1ee382a582035a4e9256ca2b7d4f3b943 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Fri, 14 Oct 2016 13:05:44 +0200 Subject: [PATCH] Clang: Fix delayed reparse of dirty and visible but not current documents 1. Open document foo.h 2. Create a split and open foo.cpp (#including "foo.h") 3. Edit foo.h (e.g. by introducing a syntax error, so that foo.cpp will indicate header errors in the toolbar or as info bar) => Actual: foo.cpp will be reparsed immediately. Expected: foo.cpp should be reparsed after a delay. This saves resources (cpu time) and minimizes poping up of the header info bar while editing header files in splits. Regression introduced by commit 380d756a03346e5bbb83101f49ed6f6696c42c6a Clang: Hook up supportive translation unit on first edit Change-Id: Ib5fd90e49415dfc3aefacab7cd627b0e1937f5fc Reviewed-by: David Schulz --- .../ipcsource/clangcodemodelserver.cpp | 62 +++++++++++++++---- .../ipcsource/clangcodemodelserver.h | 17 +++-- .../clangbackend/ipcsource/clangdocuments.cpp | 16 +++++ .../clangbackend/ipcsource/clangdocuments.h | 4 ++ tests/unit/unittest/clangdocuments-test.cpp | 36 +++++++++++ tests/unit/unittest/clangipcserver-test.cpp | 1 + 6 files changed, 121 insertions(+), 15 deletions(-) diff --git a/src/tools/clangbackend/ipcsource/clangcodemodelserver.cpp b/src/tools/clangbackend/ipcsource/clangcodemodelserver.cpp index 8a921320ad3..62070b37b0b 100644 --- a/src/tools/clangbackend/ipcsource/clangcodemodelserver.cpp +++ b/src/tools/clangbackend/ipcsource/clangcodemodelserver.cpp @@ -50,6 +50,8 @@ #include #include +#include + #include #include @@ -57,16 +59,21 @@ namespace ClangBackEnd { ClangCodeModelServer::ClangCodeModelServer() : documents(projects, unsavedFiles) - , updateDocumentAnnotationsTimeOutInMs(1500) { updateDocumentAnnotationsTimer.setSingleShot(true); - QObject::connect(&updateDocumentAnnotationsTimer, &QTimer::timeout, [this]() { processJobsForDirtyAndVisibleDocuments(); }); + updateVisibleButNotCurrentDocumentsTimer.setSingleShot(true); + QObject::connect(&updateVisibleButNotCurrentDocumentsTimer, + &QTimer::timeout, + [this]() { + processJobsForDirtyAndVisibleButNotCurrentDocuments(); + }); + QObject::connect(documents.clangFileSystemWatcher(), &ClangFileSystemWatcher::fileChanged, [this](const Utf8String &filePath) { @@ -265,16 +272,44 @@ bool ClangCodeModelServer::isTimerRunningForTestOnly() const void ClangCodeModelServer::processJobsForDirtyAndVisibleDocuments() { - for (const auto &document : documents.documents()) { - if (document.isNeedingReparse() && document.isVisibleInEditor()) { - DocumentProcessor processor = documentProcessors().processor(document); - processor.addJob(createJobRequest(document, - JobRequest::Type::UpdateDocumentAnnotations, - PreferredTranslationUnit::PreviouslyParsed)); - } - } + processJobsForDirtyCurrentDocument(); + processTimerForVisibleButNotCurrentDocuments(); +} - documentProcessors().process(); +void ClangCodeModelServer::processJobsForDirtyCurrentDocument() +{ + const auto currentDirtyDocuments = documents.filtered([](const Document &document) { + return document.isNeedingReparse() && document.isUsedByCurrentEditor(); + }); + QTC_CHECK(currentDirtyDocuments.size() <= 1); + + addAndRunUpdateJobs(currentDirtyDocuments); +} + +void ClangCodeModelServer::addAndRunUpdateJobs(const std::vector &documents) +{ + for (const auto &document : documents) { + DocumentProcessor processor = documentProcessors().processor(document); + processor.addJob(createJobRequest(document, + JobRequest::Type::UpdateDocumentAnnotations, + PreferredTranslationUnit::PreviouslyParsed)); + processor.process(); + } +} + +void ClangCodeModelServer::processTimerForVisibleButNotCurrentDocuments() +{ + if (documents.dirtyAndVisibleButNotCurrentDocuments().empty()) { + updateVisibleButNotCurrentDocumentsTimer.stop(); + } else { + updateVisibleButNotCurrentDocumentsTimer.start( + updateVisibleButNotCurrentDocumentsTimeOutInMs); + } +} + +void ClangCodeModelServer::processJobsForDirtyAndVisibleButNotCurrentDocuments() +{ + addAndRunUpdateJobs(documents.dirtyAndVisibleButNotCurrentDocuments()); } void ClangCodeModelServer::processInitialJobsForDocuments(const std::vector &documents) @@ -332,6 +367,11 @@ void ClangCodeModelServer::setUpdateDocumentAnnotationsTimeOutInMsForTestsOnly(i updateDocumentAnnotationsTimeOutInMs = value; } +void ClangCodeModelServer::setUpdateVisibleButNotCurrentDocumentsTimeOutInMsForTestsOnly(int value) +{ + updateVisibleButNotCurrentDocumentsTimeOutInMs = value; +} + DocumentProcessors &ClangCodeModelServer::documentProcessors() { if (!documentProcessors_) { diff --git a/src/tools/clangbackend/ipcsource/clangcodemodelserver.h b/src/tools/clangbackend/ipcsource/clangcodemodelserver.h index ab4efdb7e29..b03b4327faf 100644 --- a/src/tools/clangbackend/ipcsource/clangcodemodelserver.h +++ b/src/tools/clangbackend/ipcsource/clangcodemodelserver.h @@ -65,16 +65,22 @@ public: // for tests int queueSizeForTestsOnly(); bool isTimerRunningForTestOnly() const; void setUpdateDocumentAnnotationsTimeOutInMsForTestsOnly(int value); + void setUpdateVisibleButNotCurrentDocumentsTimeOutInMsForTestsOnly(int value); DocumentProcessors &documentProcessors(); private: - void startDocumentAnnotationsTimerIfFileIsNotOpenAsDocument(const Utf8String &filePath); - void addJobRequestsForDirtyAndVisibleDocuments(); - void processJobsForDirtyAndVisibleDocuments(); + void processInitialJobsForDocuments(const std::vector &documents); void startInitializingSupportiveTranslationUnits(const std::vector &documents); + void processJobsForDirtyAndVisibleDocuments(); + void processJobsForDirtyCurrentDocument(); + void processTimerForVisibleButNotCurrentDocuments(); + void processJobsForDirtyAndVisibleButNotCurrentDocuments(); + + void addAndRunUpdateJobs(const std::vector &documents); + JobRequest createJobRequest(const Document &document, JobRequest::Type type, PreferredTranslationUnit preferredTranslationUnit @@ -88,7 +94,10 @@ private: QScopedPointer documentProcessors_; // Delayed initialization QTimer updateDocumentAnnotationsTimer; - int updateDocumentAnnotationsTimeOutInMs; + int updateDocumentAnnotationsTimeOutInMs = 1500; + + QTimer updateVisibleButNotCurrentDocumentsTimer; + int updateVisibleButNotCurrentDocumentsTimeOutInMs = 2000; }; } // namespace ClangBackEnd diff --git a/src/tools/clangbackend/ipcsource/clangdocuments.cpp b/src/tools/clangbackend/ipcsource/clangdocuments.cpp index 6077505b573..3c77089447a 100644 --- a/src/tools/clangbackend/ipcsource/clangdocuments.cpp +++ b/src/tools/clangbackend/ipcsource/clangdocuments.cpp @@ -33,6 +33,8 @@ #include #include +#include + #include #include @@ -143,6 +145,20 @@ const std::vector &Documents::documents() const return documents_; } +const std::vector Documents::filtered(const IsMatchingDocument &isMatchingDocument) const +{ + return Utils::filtered(documents_, isMatchingDocument); +} + +std::vector Documents::dirtyAndVisibleButNotCurrentDocuments() const +{ + return filtered([](const Document &document) { + return document.isNeedingReparse() + && document.isVisibleInEditor() + && !document.isUsedByCurrentEditor(); + }); +} + UnsavedFiles Documents::unsavedFiles() const { return unsavedFiles_; diff --git a/src/tools/clangbackend/ipcsource/clangdocuments.h b/src/tools/clangbackend/ipcsource/clangdocuments.h index 90c905f16c4..dc1699178aa 100644 --- a/src/tools/clangbackend/ipcsource/clangdocuments.h +++ b/src/tools/clangbackend/ipcsource/clangdocuments.h @@ -32,6 +32,7 @@ #include +#include #include namespace ClangBackEnd { @@ -57,6 +58,9 @@ public: bool hasDocumentWithFilePath(const Utf8String &filePath) const; const std::vector &documents() const; + using IsMatchingDocument = std::function; + const std::vector filtered(const IsMatchingDocument &isMatchingDocument) const; + std::vector dirtyAndVisibleButNotCurrentDocuments() const; UnsavedFiles unsavedFiles() const; diff --git a/tests/unit/unittest/clangdocuments-test.cpp b/tests/unit/unittest/clangdocuments-test.cpp index 58159924413..d4a6a0b1e55 100644 --- a/tests/unit/unittest/clangdocuments-test.cpp +++ b/tests/unit/unittest/clangdocuments-test.cpp @@ -289,6 +289,42 @@ TEST_F(Documents, HasNotDocument) ASSERT_FALSE(documents.hasDocument(filePath, projectPartId)); } +TEST_F(Documents, FilteredPositive) +{ + documents.create({{filePath, projectPartId}}); + const auto isMatchingFilePath = [this](const Document &document) { + return document.filePath() == filePath; + }; + + const bool hasMatches = !documents.filtered(isMatchingFilePath).empty(); + + ASSERT_TRUE(hasMatches); +} + +TEST_F(Documents, FilteredNegative) +{ + documents.create({{filePath, projectPartId}}); + const auto isMatchingNothing = [](const Document &) { + return false; + }; + + const bool hasMatches = !documents.filtered(isMatchingNothing).empty(); + + ASSERT_FALSE(hasMatches); +} + +TEST_F(Documents, DirtyAndVisibleButNotCurrentDocuments) +{ + documents.create({{filePath, projectPartId}}); + documents.updateDocumentsWithChangedDependency(filePath); + documents.setVisibleInEditors({filePath}); + documents.setUsedByCurrentEditor(Utf8String()); + + const bool hasMatches = !documents.dirtyAndVisibleButNotCurrentDocuments().empty(); + + ASSERT_TRUE(hasMatches); +} + TEST_F(Documents, isUsedByCurrentEditor) { documents.create({fileContainer}); diff --git a/tests/unit/unittest/clangipcserver-test.cpp b/tests/unit/unittest/clangipcserver-test.cpp index bcff6e95327..2a3c75dc2ac 100644 --- a/tests/unit/unittest/clangipcserver-test.cpp +++ b/tests/unit/unittest/clangipcserver-test.cpp @@ -375,6 +375,7 @@ void ClangClangCodeModelServer::SetUp() { clangServer.setClient(&mockClangCodeModelClient); clangServer.setUpdateDocumentAnnotationsTimeOutInMsForTestsOnly(0); + clangServer.setUpdateVisibleButNotCurrentDocumentsTimeOutInMsForTestsOnly(0); } void ClangClangCodeModelServer::TearDown()