From 741596481900347ef4c69d80efdffe710e79ff6b Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Thu, 19 Jan 2017 15:46:40 +0100 Subject: [PATCH] CppTools: Fix choosing project part after project open As long as there are project parts for a source file, always determine the best project part, instead of trying to stick to the previous one. This ensures the best project part at all times and simplifies the code. Change-Id: I25ea3eb43a5a3e6d93688d4b8965f596dc9ae22b Reviewed-by: David Schulz --- .../clangeditordocumentparser.cpp | 2 +- .../cpptools/baseeditordocumentparser.cpp | 4 +-- .../cpptools/baseeditordocumentparser.h | 8 ++--- .../cpptools/baseeditordocumentprocessor.cpp | 4 +-- .../cpptools/baseeditordocumentprocessor.h | 2 +- .../cpptools/builtineditordocumentparser.cpp | 2 +- .../cpptools/builtinindexingsupport.cpp | 3 +- src/plugins/cpptools/cppmodelmanager.cpp | 30 ++++++++++------ src/plugins/cpptools/cppmodelmanager.h | 2 +- src/plugins/cpptools/cppmodelmanager_test.cpp | 6 ++-- .../cpptools/cppprojectpartchooser.cpp | 26 +++++++------- src/plugins/cpptools/cppprojectpartchooser.h | 5 ++- src/plugins/cpptools/editordocumenthandle.h | 4 +-- .../unittest/cppprojectpartchooser-test.cpp | 34 ++++++++++--------- 14 files changed, 69 insertions(+), 63 deletions(-) diff --git a/src/plugins/clangcodemodel/clangeditordocumentparser.cpp b/src/plugins/clangcodemodel/clangeditordocumentparser.cpp index 9948efcccaf..0d4195c538f 100644 --- a/src/plugins/clangcodemodel/clangeditordocumentparser.cpp +++ b/src/plugins/clangcodemodel/clangeditordocumentparser.cpp @@ -41,7 +41,7 @@ void ClangEditorDocumentParser::updateImpl(const QFutureInterface &, state_.projectPartInfo, updateParams.activeProject, updateParams.languagePreference, - updateParams.hasActiveProjectChanged); + updateParams.projectsUpdated); emit projectPartInfoUpdated(state_.projectPartInfo); setState(state_); } diff --git a/src/plugins/cpptools/baseeditordocumentparser.cpp b/src/plugins/cpptools/baseeditordocumentparser.cpp index cd31bd9a074..1068a90e9f7 100644 --- a/src/plugins/cpptools/baseeditordocumentparser.cpp +++ b/src/plugins/cpptools/baseeditordocumentparser.cpp @@ -125,7 +125,7 @@ ProjectPartInfo BaseEditorDocumentParser::determineProjectPart( const ProjectPartInfo ¤tProjectPartInfo, const ProjectExplorer::Project *activeProject, Language languagePreference, - bool hasActiveProjectChanged) + bool projectsUpdated) { Internal::ProjectPartChooser chooser; chooser.setFallbackProjectPart([](){ @@ -145,7 +145,7 @@ ProjectPartInfo BaseEditorDocumentParser::determineProjectPart( preferredProjectPartId, activeProject, languagePreference, - hasActiveProjectChanged); + projectsUpdated); return chooserResult; } diff --git a/src/plugins/cpptools/baseeditordocumentparser.h b/src/plugins/cpptools/baseeditordocumentparser.h index 320627a8064..9a13aeb268c 100644 --- a/src/plugins/cpptools/baseeditordocumentparser.h +++ b/src/plugins/cpptools/baseeditordocumentparser.h @@ -56,18 +56,18 @@ public: UpdateParams(const WorkingCopy &workingCopy, const ProjectExplorer::Project *activeProject, Language languagePreference, - bool hasActiveProjectChanged) + bool projectsUpdated) : workingCopy(workingCopy) , activeProject(activeProject) , languagePreference(languagePreference) - , hasActiveProjectChanged(hasActiveProjectChanged) + , projectsUpdated(projectsUpdated) { } WorkingCopy workingCopy; const ProjectExplorer::Project *activeProject = nullptr; Language languagePreference = Language::Cxx; - bool hasActiveProjectChanged = false; + bool projectsUpdated = false; }; public: @@ -99,7 +99,7 @@ protected: const ProjectPartInfo ¤tProjectPartInfo, const ProjectExplorer::Project *activeProject, Language languagePreference, - bool hasActiveProjectChanged); + bool projectsUpdated); mutable QMutex m_stateAndConfigurationMutex; diff --git a/src/plugins/cpptools/baseeditordocumentprocessor.cpp b/src/plugins/cpptools/baseeditordocumentprocessor.cpp index 950d052793f..ad26fa653c3 100644 --- a/src/plugins/cpptools/baseeditordocumentprocessor.cpp +++ b/src/plugins/cpptools/baseeditordocumentprocessor.cpp @@ -56,7 +56,7 @@ BaseEditorDocumentProcessor::~BaseEditorDocumentProcessor() { } -void BaseEditorDocumentProcessor::run(bool hasActiveProjectChanged) +void BaseEditorDocumentProcessor::run(bool projectsUpdated) { const Language languagePreference = codeModelSettings()->interpretAmbigiousHeadersAsCHeaders() ? Language::C @@ -65,7 +65,7 @@ void BaseEditorDocumentProcessor::run(bool hasActiveProjectChanged) runImpl({CppModelManager::instance()->workingCopy(), ProjectExplorer::SessionManager::startupProject(), languagePreference, - hasActiveProjectChanged}); + projectsUpdated}); } TextEditor::QuickFixOperations diff --git a/src/plugins/cpptools/baseeditordocumentprocessor.h b/src/plugins/cpptools/baseeditordocumentprocessor.h index ec9df57ceed..2f2fdc3af20 100644 --- a/src/plugins/cpptools/baseeditordocumentprocessor.h +++ b/src/plugins/cpptools/baseeditordocumentprocessor.h @@ -54,7 +54,7 @@ public: BaseEditorDocumentProcessor(QTextDocument *textDocument, const QString &filePath); virtual ~BaseEditorDocumentProcessor(); - void run(bool hasActiveProjectChanged = false); + void run(bool projectsUpdated = false); virtual void semanticRehighlight() = 0; virtual void recalculateSemanticInfoDetached(bool force) = 0; virtual CppTools::SemanticInfo recalculateSemanticInfo() = 0; diff --git a/src/plugins/cpptools/builtineditordocumentparser.cpp b/src/plugins/cpptools/builtineditordocumentparser.cpp index ff09a635802..ac3cf3ee2a3 100644 --- a/src/plugins/cpptools/builtineditordocumentparser.cpp +++ b/src/plugins/cpptools/builtineditordocumentparser.cpp @@ -82,7 +82,7 @@ void BuiltinEditorDocumentParser::updateImpl(const QFutureInterface &futur baseState.projectPartInfo, updateParams.activeProject, updateParams.languagePreference, - updateParams.hasActiveProjectChanged); + updateParams.projectsUpdated); emit projectPartInfoUpdated(baseState.projectPartInfo); if (state.forceSnapshotInvalidation) { diff --git a/src/plugins/cpptools/builtinindexingsupport.cpp b/src/plugins/cpptools/builtinindexingsupport.cpp index a287012f5b2..c2ae6096c08 100644 --- a/src/plugins/cpptools/builtinindexingsupport.cpp +++ b/src/plugins/cpptools/builtinindexingsupport.cpp @@ -157,8 +157,7 @@ void indexFindErrors(QFutureInterface &future, const ParseParams params) // Parse the file as precisely as possible BuiltinEditorDocumentParser parser(file); parser.setReleaseSourceAndAST(false); - parser.update({CppModelManager::instance()->workingCopy(), nullptr, - Language::Cxx, false}); + parser.update({CppModelManager::instance()->workingCopy(), nullptr, Language::Cxx, false}); CPlusPlus::Document::Ptr document = parser.document(); QTC_ASSERT(document, return); diff --git a/src/plugins/cpptools/cppmodelmanager.cpp b/src/plugins/cpptools/cppmodelmanager.cpp index 330501d3b3e..0d9a3e67225 100644 --- a/src/plugins/cpptools/cppmodelmanager.cpp +++ b/src/plugins/cpptools/cppmodelmanager.cpp @@ -793,7 +793,7 @@ void CppModelManager::watchForCanceledProjectIndexer(QFuture future, watcher->setFuture(future); } -void CppModelManager::updateCppEditorDocuments(bool hasActiveProjectChanged) const +void CppModelManager::updateCppEditorDocuments(bool projectsUpdated) const { // Refresh visible documents QSet visibleCppEditorDocuments; @@ -802,7 +802,7 @@ void CppModelManager::updateCppEditorDocuments(bool hasActiveProjectChanged) con const QString filePath = document->filePath().toString(); if (CppEditorDocumentHandle *theCppEditorDocument = cppEditorDocument(filePath)) { visibleCppEditorDocuments.insert(document); - theCppEditorDocument->processor()->run(hasActiveProjectChanged); + theCppEditorDocument->processor()->run(projectsUpdated); } } } @@ -814,9 +814,9 @@ void CppModelManager::updateCppEditorDocuments(bool hasActiveProjectChanged) con foreach (Core::IDocument *document, invisibleCppEditorDocuments) { const QString filePath = document->filePath().toString(); if (CppEditorDocumentHandle *theCppEditorDocument = cppEditorDocument(filePath)) { - const CppEditorDocumentHandle::RefreshReason refreshReason = hasActiveProjectChanged - ? CppEditorDocumentHandle::ActiveProjectChange - : CppEditorDocumentHandle::Usual; + const CppEditorDocumentHandle::RefreshReason refreshReason = projectsUpdated + ? CppEditorDocumentHandle::ProjectUpdate + : CppEditorDocumentHandle::Other; theCppEditorDocument->setRefreshReason(refreshReason); } } @@ -905,7 +905,7 @@ QFuture CppModelManager::updateProjectInfo(const ProjectInfo &newProjectIn // However, on e.g. a session restore first the editor documents are created and then the // project updates come in. That is, there are no reasonable dependency tables based on // resolved includes that we could rely on. - updateCppEditorDocuments(); + updateCppEditorDocuments(/*projectsUpdated = */ true); // Trigger reindexing QFuture indexerFuture = updateSourceFiles(filesToReindex, ForcedProgressNotification); @@ -1044,9 +1044,18 @@ void CppModelManager::onAboutToRemoveProject(ProjectExplorer::Project *project) delayedGC(); } -void CppModelManager::onActiveProjectChanged(ProjectExplorer::Project *) +void CppModelManager::onActiveProjectChanged(ProjectExplorer::Project *project) { - updateCppEditorDocuments(true); + if (!project) + return; // Last project closed. + + { + QMutexLocker locker(&d->m_projectMutex); + if (!d->m_projectToProjectsInfo.contains(project)) + return; // Not yet known to us. + } + + updateCppEditorDocuments(); } void CppModelManager::onSourceFilesRefreshed() const @@ -1067,10 +1076,9 @@ void CppModelManager::onCurrentEditorChanged(Core::IEditor *editor) const CppEditorDocumentHandle::RefreshReason refreshReason = theCppEditorDocument->refreshReason(); if (refreshReason != CppEditorDocumentHandle::None) { + const bool projectsChanged = refreshReason == CppEditorDocumentHandle::ProjectUpdate; theCppEditorDocument->setRefreshReason(CppEditorDocumentHandle::None); - const bool hasActiveProjectChanged - = refreshReason == CppEditorDocumentHandle::ActiveProjectChange; - theCppEditorDocument->processor()->run(hasActiveProjectChanged); + theCppEditorDocument->processor()->run(projectsChanged); } } } diff --git a/src/plugins/cpptools/cppmodelmanager.h b/src/plugins/cpptools/cppmodelmanager.h index 4ce1ceed6cb..0ebaf40b926 100644 --- a/src/plugins/cpptools/cppmodelmanager.h +++ b/src/plugins/cpptools/cppmodelmanager.h @@ -88,7 +88,7 @@ public: QFuture updateSourceFiles(const QSet &sourceFiles, ProgressNotificationMode mode = ReservedProgressNotification); - void updateCppEditorDocuments(bool hasActiveProjectChanged = false) const; + void updateCppEditorDocuments(bool projectsUpdated = false) const; WorkingCopy workingCopy() const; QByteArray codeModelConfiguration() const; diff --git a/src/plugins/cpptools/cppmodelmanager_test.cpp b/src/plugins/cpptools/cppmodelmanager_test.cpp index 0248e3ac8ec..91970ae2973 100644 --- a/src/plugins/cpptools/cppmodelmanager_test.cpp +++ b/src/plugins/cpptools/cppmodelmanager_test.cpp @@ -879,8 +879,7 @@ void CppToolsPlugin::test_modelmanager_precompiled_headers() BaseEditorDocumentParser::Configuration config = parser->configuration(); config.usePrecompiledHeaders = true; parser->setConfiguration(config); - parser->update({CppModelManager::instance()->workingCopy(), nullptr, - Language::Cxx, false}); + parser->update({CppModelManager::instance()->workingCopy(), nullptr, Language::Cxx, false}); // Check if defines from pch are considered Document::Ptr document = mm->document(fileName); @@ -958,8 +957,7 @@ void CppToolsPlugin::test_modelmanager_defines_per_editor() BaseEditorDocumentParser::Configuration config = parser->configuration(); config.editorDefines = editorDefines.toUtf8(); parser->setConfiguration(config); - parser->update({CppModelManager::instance()->workingCopy(), nullptr, - Language::Cxx, false}); + parser->update({CppModelManager::instance()->workingCopy(), nullptr, Language::Cxx, false}); Document::Ptr doc = mm->document(main1File); QCOMPARE(nameOfFirstDeclaration(doc), firstDeclarationName); diff --git a/src/plugins/cpptools/cppprojectpartchooser.cpp b/src/plugins/cpptools/cppprojectpartchooser.cpp index 470fcfd4caf..a79b44540a7 100644 --- a/src/plugins/cpptools/cppprojectpartchooser.cpp +++ b/src/plugins/cpptools/cppprojectpartchooser.cpp @@ -128,12 +128,13 @@ private: bool m_isAmbiguous = false; }; -ProjectPartInfo ProjectPartChooser::choose(const QString &filePath, +ProjectPartInfo ProjectPartChooser::choose( + const QString &filePath, const ProjectPartInfo ¤tProjectPartInfo, const QString &preferredProjectPartId, const ProjectExplorer::Project *activeProject, Language languagePreference, - bool projectHasChanged) const + bool projectsUpdated) const { QTC_CHECK(m_projectPartsForFile); QTC_CHECK(m_projectPartsFromDependenciesForFile); @@ -144,7 +145,8 @@ ProjectPartInfo ProjectPartChooser::choose(const QString &filePath, QList projectParts = m_projectPartsForFile(filePath); if (projectParts.isEmpty()) { - if (projectPart && currentProjectPartInfo.hint == ProjectPartInfo::IsFallbackMatch) + if (!projectsUpdated && projectPart + && currentProjectPartInfo.hint == ProjectPartInfo::IsFallbackMatch) // Avoid re-calculating the expensive dependency table for non-project files. return {projectPart, ProjectPartInfo::IsFallbackMatch}; @@ -162,16 +164,14 @@ ProjectPartInfo ProjectPartChooser::choose(const QString &filePath, projectPart = prioritizer.projectPart(); } } else { - if (projectHasChanged || !projectParts.contains(projectPart)) { - ProjectPartPrioritizer prioritizer(projectParts, - preferredProjectPartId, - activeProject, - languagePreference); - projectPart = prioritizer.projectPart(); - hint = prioritizer.isAmbiguous() - ? ProjectPartInfo::IsAmbiguousMatch - : ProjectPartInfo::NoHint; - } + ProjectPartPrioritizer prioritizer(projectParts, + preferredProjectPartId, + activeProject, + languagePreference); + projectPart = prioritizer.projectPart(); + hint = prioritizer.isAmbiguous() + ? ProjectPartInfo::IsAmbiguousMatch + : ProjectPartInfo::NoHint; } return {projectPart, hint}; diff --git a/src/plugins/cpptools/cppprojectpartchooser.h b/src/plugins/cpptools/cppprojectpartchooser.h index 6cf59544cf4..62035ce8b6f 100644 --- a/src/plugins/cpptools/cppprojectpartchooser.h +++ b/src/plugins/cpptools/cppprojectpartchooser.h @@ -48,13 +48,12 @@ public: void setProjectPartsForFile(const ProjectPartsForFile &getter); void setProjectPartsFromDependenciesForFile(const ProjectPartsFromDependenciesForFile &getter); - ProjectPartInfo choose( - const QString &filePath, + ProjectPartInfo choose(const QString &filePath, const ProjectPartInfo ¤tProjectPartInfo, const QString &preferredProjectPartId, const ProjectExplorer::Project *activeProject, Language languagePreference, - bool projectHasChanged) const; + bool projectsUpdated) const; private: FallBackProjectPart m_fallbackProjectPart; diff --git a/src/plugins/cpptools/editordocumenthandle.h b/src/plugins/cpptools/editordocumenthandle.h index 16b69ab751f..ac9e9fa0763 100644 --- a/src/plugins/cpptools/editordocumenthandle.h +++ b/src/plugins/cpptools/editordocumenthandle.h @@ -40,8 +40,8 @@ public: enum RefreshReason { None, - Usual, // e.g. project configuration change or change of editor contents - ActiveProjectChange + ProjectUpdate, + Other, }; RefreshReason refreshReason() const; void setRefreshReason(const RefreshReason &refreshReason); diff --git a/tests/unit/unittest/cppprojectpartchooser-test.cpp b/tests/unit/unittest/cppprojectpartchooser-test.cpp index 278d43a93fa..b2f3918cbc2 100644 --- a/tests/unit/unittest/cppprojectpartchooser-test.cpp +++ b/tests/unit/unittest/cppprojectpartchooser-test.cpp @@ -53,8 +53,8 @@ protected: ProjectPartInfo::NoHint}; QString preferredProjectPartId; const ProjectExplorer::Project *activeProject = nullptr; - bool projectHasChanged = false; Language languagePreference = Language::Cxx; + bool projectsChanged = false; ::ProjectPartChooser chooser; QList projectPartsForFile; @@ -74,17 +74,6 @@ TEST_F(ProjectPartChooser, ChooseManuallySet) ASSERT_THAT(chosen, Eq(p2)); } -TEST_F(ProjectPartChooser, ForMultipleChoosePrevious) -{ - const ProjectPart::Ptr otherProjectPart; - projectPartsForFile += otherProjectPart; - projectPartsForFile += currentProjectPartInfo.projectPart; - - const ProjectPart::Ptr chosen = choose().projectPart; - - ASSERT_THAT(chosen, Eq(currentProjectPartInfo.projectPart)); -} - TEST_F(ProjectPartChooser, ForMultipleChooseFromActiveProject) { const QList projectParts = createProjectPartsWithDifferentProjects(); @@ -131,7 +120,6 @@ TEST_F(ProjectPartChooser, ForMultipleCheckIfActiveProjectChanged) projectPartsForFile += projectParts; currentProjectPartInfo.projectPart = firstProjectPart; activeProject = secondProjectPart->project; - projectHasChanged = true; const ProjectPart::Ptr chosen = choose().projectPart; @@ -213,7 +201,7 @@ TEST_F(ProjectPartChooser, ContinueUsingFallbackFromModelManagerIfProjectDoesNot ASSERT_THAT(chosen, Eq(fallbackProjectPart)); } -TEST_F(ProjectPartChooser, StopUsingFallbackFromModelManagerIfProjectChanges) +TEST_F(ProjectPartChooser, StopUsingFallbackFromModelManagerIfProjectChanges1) { fallbackProjectPart.reset(new ProjectPart); currentProjectPartInfo.projectPart = fallbackProjectPart; @@ -226,6 +214,20 @@ TEST_F(ProjectPartChooser, StopUsingFallbackFromModelManagerIfProjectChanges) ASSERT_THAT(chosen, Eq(addedProject)); } +TEST_F(ProjectPartChooser, StopUsingFallbackFromModelManagerIfProjectChanges2) +{ + fallbackProjectPart.reset(new ProjectPart); + currentProjectPartInfo.projectPart = fallbackProjectPart; + currentProjectPartInfo.hint = ProjectPartInfo::IsFallbackMatch; + const ProjectPart::Ptr addedProject(new ProjectPart); + projectPartsFromDependenciesForFile += addedProject; + projectsChanged = true; + + const ProjectPart::Ptr chosen = choose().projectPart; + + ASSERT_THAT(chosen, Eq(addedProject)); +} + TEST_F(ProjectPartChooser, IndicateFallbacktoProjectPartFromModelManager) { fallbackProjectPart.reset(new ProjectPart); @@ -237,7 +239,7 @@ TEST_F(ProjectPartChooser, IndicateFallbacktoProjectPartFromModelManager) void ProjectPartChooser::SetUp() { - chooser.setFallbackProjectPart([&](){ + chooser.setFallbackProjectPart([&]() { return fallbackProjectPart; }); chooser.setProjectPartsForFile([&](const QString &) { @@ -255,7 +257,7 @@ const ProjectPartInfo ProjectPartChooser::choose() const preferredProjectPartId, activeProject, languagePreference, - projectHasChanged); + projectsChanged); } QList ProjectPartChooser::createProjectPartsWithDifferentProjects()