Clang: Take over jobs if document gets new project part

We could loose jobs if e.g. the user switched to another parse context
or shuffled project files between targets/products/project-parts while
there were still jobs in the queue.

Previously, changing the project part id of a document was a two step
process:
  1) Unregister document with old project part id
  2) Register document with new project part id
On 1), we have thrown the document processors (and thus the job queue)
away. On 2), we have created a new document. Due to this separation the
backend could not take over jobs to the new document (processor) - it
could not know that these commands belong together.

Now, we avoid the explicit unregister command. On a register command the
backend has enough context to find out what to do, it can take over
relevant jobs.

Task-number: QTCREATORBUG-18856
Change-Id: Ib68a8e62140fcfdb2de58dcd2ae955b4c2e15166
Reviewed-by: Ivan Donchevskii <ivan.donchevskii@qt.io>

src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp

@@ -435,7 +435,6 @@ void ClangEditorDocumentProcessor::registerTranslationUnitForEditor(CppTools::Pr
     if (m_projectPart) {
         if (projectPart->id() == m_projectPart->id())
             return;
-        unregisterTranslationUnitForEditor();
     }
 
     m_communicator.registerTranslationUnitsForEditor(

src/tools/clangbackend/source/clangcodemodelserver.cpp

@@ -84,21 +84,36 @@ void ClangCodeModelServer::end()
     QCoreApplication::exit();
 }
 
+static std::vector<Document> operator+(const std::vector<Document> &a,
+                                       const std::vector<Document> &b)
+{
+    std::vector<Document> result = a;
+    result.insert(result.end(), b.begin(), b.end());
+    return result;
+}
+
+// TODO: Rename to createOrUpdate...
 void ClangCodeModelServer::registerTranslationUnitsForEditor(const ClangBackEnd::RegisterTranslationUnitForEditorMessage &message)
 {
     qCDebug(serverLog) << "########## registerTranslationUnitsForEditor";
     TIME_SCOPE_DURATION("ClangCodeModelServer::registerTranslationUnitsForEditor");
 
     try {
-        auto createdDocuments = documents.create(message.fileContainers());
+        DocumentResetInfos toReset;
+        QVector<FileContainer> toCreate;
+        categorizeFileContainers(message.fileContainers(), toCreate, toReset);
+
+        const std::vector<Document> createdDocuments = documents.create(toCreate);
         for (const auto &document : createdDocuments)
             documentProcessors().create(document);
+        const std::vector<Document> resetDocuments_ = resetDocuments(toReset);
+
         unsavedFiles.createOrUpdate(message.fileContainers());
         documents.setUsedByCurrentEditor(message.currentEditorFilePath());
         documents.setVisibleInEditors(message.visibleEditorFilePaths());
-        processSuspendResumeJobs(documents.documents());
 
-        processInitialJobsForDocuments(createdDocuments);
+        processSuspendResumeJobs(documents.documents());
+        processInitialJobsForDocuments(createdDocuments + resetDocuments_);
     } catch (const std::exception &exception) {
         qWarning() << "Error in ClangCodeModelServer::registerTranslationUnitsForEditor:" << exception.what();
     }
@@ -147,6 +162,14 @@ void ClangCodeModelServer::unregisterTranslationUnitsForEditor(const ClangBackEn
     }
 }
 
+static DocumentResetInfos toDocumentResetInfos(const std::vector<Document> &documents)
+{
+    DocumentResetInfos infos;
+    for (const auto &d : documents)
+        infos.push_back(DocumentResetInfo{d, d.fileContainer()});
+    return infos;
+}
+
 void ClangCodeModelServer::registerProjectPartsForEditor(const RegisterProjectPartsForEditorMessage &message)
 {
     qCDebug(serverLog) << "########## registerProjectPartsForEditor";
@@ -156,19 +179,7 @@ void ClangCodeModelServer::registerProjectPartsForEditor(const RegisterProjectPa
         projects.createOrUpdate(message.projectContainers());
         std::vector<Document> affectedDocuments = documents.setDocumentsDirtyIfProjectPartChanged();
 
-        for (Document &document : affectedDocuments) {
+        resetDocuments(toDocumentResetInfos(affectedDocuments));
-            documents.remove({document.fileContainer()});
-
-            Document newDocument = *documents.create({document.fileContainer()}).begin();
-            newDocument.setDirtyIfDependencyIsMet(document.filePath());
-            newDocument.setIsUsedByCurrentEditor(document.isUsedByCurrentEditor());
-            newDocument.setIsVisibleInEditor(document.isVisibleInEditor(), document.visibleTimePoint());
-            newDocument.setResponsivenessIncreaseNeeded(document.isResponsivenessIncreased());
-
-            documentProcessors().reset(document, newDocument);
-
-            QTC_CHECK(document.useCount() == 1);
-        }
 
         processJobsForDirtyAndVisibleDocuments();
     } catch (const std::exception &exception) {
@@ -419,6 +430,45 @@ void ClangCodeModelServer::processSuspendResumeJobs(const std::vector<Document>
     }
 }
 
+void ClangCodeModelServer::categorizeFileContainers(const QVector<FileContainer> &fileContainers,
+                                                    QVector<FileContainer> &toCreate,
+                                                    DocumentResetInfos &toReset) const
+{
+    for (const FileContainer &fileContainer : fileContainers) {
+        const std::vector<Document> matching = documents.filtered([&](const Document &document) {
+            return document.filePath() == fileContainer.filePath();
+        });
+        if (matching.empty())
+            toCreate.push_back(fileContainer);
+        else
+            toReset.push_back(DocumentResetInfo{*matching.begin(), fileContainer});
+    }
+}
+
+std::vector<Document> ClangCodeModelServer::resetDocuments(const DocumentResetInfos &infos)
+{
+    std::vector<Document> newDocuments;
+
+    for (const DocumentResetInfo &info : infos) {
+        const Document &document = info.documentToRemove;
+        QTC_CHECK(document.filePath() == info.fileContainer.filePath());
+
+        documents.remove({document.fileContainer()});
+
+        Document newDocument = *documents.create({info.fileContainer}).begin();
+        newDocument.setDirtyIfDependencyIsMet(document.filePath());
+        newDocument.setIsUsedByCurrentEditor(document.isUsedByCurrentEditor());
+        newDocument.setIsVisibleInEditor(document.isVisibleInEditor(), document.visibleTimePoint());
+        newDocument.setResponsivenessIncreaseNeeded(document.isResponsivenessIncreased());
+
+        documentProcessors().reset(document, newDocument);
+
+        newDocuments.push_back(newDocument);
+    }
+
+    return newDocuments;
+}
+
 void ClangCodeModelServer::processInitialJobsForDocuments(const std::vector<Document> &documents)
 {
     for (const auto &document : documents) {

src/tools/clangbackend/source/clangcodemodelserver.h

@@ -42,6 +42,12 @@
 
 namespace ClangBackEnd {
 
+struct DocumentResetInfo {
+    Document documentToRemove;
+    FileContainer fileContainer;
+};
+using DocumentResetInfos = QVector<DocumentResetInfo>;
+
 class ClangCodeModelServer : public ClangCodeModelServerInterface,
                              public IpcClientProvider<ClangCodeModelClientInterface>
 {
@@ -79,6 +85,11 @@ private:
     void processTimerForVisibleButNotCurrentDocuments();
     void processSuspendResumeJobs(const std::vector<Document> &documents);
 
+    void categorizeFileContainers(const QVector<FileContainer> &fileContainers,
+                                  QVector<FileContainer> &toCreate,
+                                  DocumentResetInfos &toReset) const;
+    std::vector<Document> resetDocuments(const DocumentResetInfos &infos);
+
     void addAndRunUpdateJobs(std::vector<Document> documents);
 
 private:

src/tools/clangbackend/source/clangdocumentprocessors.cpp

@@ -86,22 +86,34 @@ void DocumentProcessors::remove(const Document &document)
         throw DocumentProcessorDoesNotExist(document.filePath(), document.projectPart().id());
 }
 
+static JobRequests jobsToTakeOver(const JobRequests &jobsStillInQueue,
+                                  const Utf8String &updatedProjectPartId)
+{
+    JobRequests jobs = Utils::filtered(jobsStillInQueue, [](const JobRequest &job) {
+        return job.isTakeOverable();
+    });
+
+    for (JobRequest &job : jobs)
+        job.projectPartId = updatedProjectPartId;
+
+    return jobs;
+}
+
 void DocumentProcessors::reset(const Document &oldDocument, const Document &newDocument)
 {
     // Wait until the currently running jobs finish and remember the not yet
     // processed job requests for the new processor...
-    JobRequests jobsToTakeOver = processor(oldDocument).stop();
+    const JobRequests jobsStillInQueue = processor(oldDocument).stop();
     // ...but do not take over irrelevant ones.
-    jobsToTakeOver = Utils::filtered(jobsToTakeOver, [](const JobRequest &job){
+    const JobRequests jobsForNewProcessor = jobsToTakeOver(jobsStillInQueue,
-        return job.isTakeOverable();
+                                                           newDocument.projectPart().id());
-    });
 
     // Remove current processor
     remove(oldDocument);
 
     // Create new processor and take over not yet processed jobs.
     DocumentProcessor newProcessor = create(newDocument);
-    for (const JobRequest &job : jobsToTakeOver)
+    for (const JobRequest &job : jobsForNewProcessor)
         newProcessor.addJob(job);
 }
 

tests/unit/unittest/clangcodemodelserver-test.cpp

@@ -102,6 +102,7 @@ protected:
     bool waitUntilAllJobsFinished(int timeOutInMs = 10000);
 
     void registerProjectPart();
+    void registerProjectPart(const Utf8String &projectPartId);
     void changeProjectPartArguments();
 
     void registerProjectAndFile(const Utf8String &filePath,
@@ -111,6 +112,8 @@ protected:
     void registerProjectAndFilesAndWaitForFinished(int expectedDocumentAnnotationsChangedMessages = 2);
     void registerFile(const Utf8String &filePath,
                       int expectedDocumentAnnotationsChangedMessages = 1);
+    void registerFile(const Utf8String &filePath, const Utf8String &projectPartId,
+                      int expectedDocumentAnnotationsChangedMessages = 1);
     void registerFiles(int expectedDocumentAnnotationsChangedMessages);
     void registerFileWithUnsavedContent(const Utf8String &filePath, const Utf8String &content);
 
@@ -155,6 +158,7 @@ protected:
     ClangBackEnd::ClangCodeModelServer clangServer;
     const ClangBackEnd::Documents &documents = clangServer.documentsForTestOnly();
     const Utf8String projectPartId = Utf8StringLiteral("pathToProjectPart.pro");
+    const Utf8String projectPartId2 = Utf8StringLiteral("otherPathToProjectPart.pro");
 
     const Utf8String filePathA = Utf8StringLiteral(TESTDATA_DIR"/complete_extractor_function.cpp");
     const QString filePathAUnsavedVersion1
@@ -418,6 +422,18 @@ TEST_F(ClangCodeModelServerSlowTest, TakeOverJobsOnProjectPartChange)
     updateVisibilty(filePathC, filePathC); // Enable processing jobs
 }
 
+TEST_F(ClangCodeModelServerSlowTest, TakeOverJobsOnProjectPartIdChange)
+{
+    registerProjectPart(projectPartId);
+    registerProjectPart(projectPartId2);
+    registerFile(filePathC, projectPartId, 0);
+    requestReferences();
+
+    expectReferences();
+
+    registerFile(filePathC, projectPartId2); // Here we do not want to loose the RequestReferences job
+}
+
 void ClangCodeModelServer::SetUp()
 {
     clangServer.setClient(&mockClangCodeModelClient);
@@ -452,6 +468,13 @@ void ClangCodeModelServer::registerProjectAndFilesAndWaitForFinished(
 
 void ClangCodeModelServer::registerFile(const Utf8String &filePath,
                                   int expectedDocumentAnnotationsChangedMessages)
+{
+    registerFile(filePath, projectPartId, expectedDocumentAnnotationsChangedMessages);
+}
+
+void ClangCodeModelServer::registerFile(const Utf8String &filePath,
+                                        const Utf8String &projectPartId,
+                                        int expectedDocumentAnnotationsChangedMessages)
 {
     const FileContainer fileContainer(filePath, projectPartId);
     const RegisterTranslationUnitForEditorMessage message({fileContainer}, filePath, {filePath});
@@ -683,6 +706,11 @@ void ClangCodeModelServer::unregisterFile(const Utf8String &filePath)
 }
 
 void ClangCodeModelServer::registerProjectPart()
+{
+    registerProjectPart(projectPartId);
+}
+
+void ClangCodeModelServer::registerProjectPart(const Utf8String &projectPartId)
 {
     RegisterProjectPartsForEditorMessage message({ProjectPartContainer(projectPartId)});