From 077de5aab6a7d5cf829e42d850947ed7becd0ee7 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Thu, 8 Aug 2024 13:58:28 +0200 Subject: [PATCH] ProjectExplorer: Handle registering of auto-created bundle toolchains ... in the bundle itself, whenever possible. It's very annoying to have to add this stanza in all places where bundling takes place, and it's also easily forgotten, introducing memory leaks. This also nicely self-documents the expectations of the calling code as to whether new toolchains can or cannot be created in the given context as a side effect of bundling. Change-Id: I78d2d4cdfc1010568f61f201b0d930b01f79a88b Reviewed-by: hjk --- src/plugins/docker/kitdetector.cpp | 10 ++--- src/plugins/projectexplorer/gcctoolchain.cpp | 7 ++-- src/plugins/projectexplorer/kitaspects.cpp | 11 +++--- src/plugins/projectexplorer/kitmanager.cpp | 5 +-- src/plugins/projectexplorer/msvctoolchain.cpp | 3 +- src/plugins/projectexplorer/toolchain.cpp | 38 +++++++++++++------ src/plugins/projectexplorer/toolchain.h | 19 ++++++---- .../projectexplorer/toolchainoptionspage.cpp | 19 +++++----- src/plugins/qtsupport/qtkitaspect.cpp | 5 +-- 9 files changed, 66 insertions(+), 51 deletions(-) diff --git a/src/plugins/docker/kitdetector.cpp b/src/plugins/docker/kitdetector.cpp index 65565c2f3d5..e0c6142890e 100644 --- a/src/plugins/docker/kitdetector.cpp +++ b/src/plugins/docker/kitdetector.cpp @@ -262,9 +262,8 @@ Toolchains KitDetectorPrivate::autoDetectToolchains() .arg(toolchain->compilerCommand().toUserOutput())); toolchain->setDetectionSource(m_sharedId); } - const QList bundles = ToolchainBundle::collectBundles(newToolchains); - for (const ToolchainBundle &b : bundles) - ToolchainManager::registerToolchains(b.toolchains()); + const QList bundles + = ToolchainBundle::collectBundles(newToolchains, ToolchainBundle::AutoRegister::On); alreadyKnown.append(newToolchains); allNewToolchains.append(newToolchains); } @@ -356,9 +355,8 @@ void KitDetectorPrivate::autoDetect() const Toolchains toolchainCandidates = ToolchainManager::toolchains( [this](const Toolchain *tc) { return tc->detectionSource() == m_sharedId; }); - const QList bundles = ToolchainBundle::collectBundles(toolchainCandidates); - for (const ToolchainBundle &b : bundles) - ToolchainManager::registerToolchains(b.createdToolchains()); + const QList bundles + = ToolchainBundle::collectBundles(toolchainCandidates, ToolchainBundle::AutoRegister::On); // Try to find a matching Qt/Toolchain pair. bool match = false; diff --git a/src/plugins/projectexplorer/gcctoolchain.cpp b/src/plugins/projectexplorer/gcctoolchain.cpp index 4a19b2cbf5c..a51a445faf4 100644 --- a/src/plugins/projectexplorer/gcctoolchain.cpp +++ b/src/plugins/projectexplorer/gcctoolchain.cpp @@ -2090,10 +2090,9 @@ void GccToolchainConfigWidget::updateParentToolchainComboBox() Id parentBundleId = Id::fromSetting(m_parentToolchainCombo->currentData()); if (bundle().isAutoDetected() || m_parentToolchainCombo->count() == 0) parentBundleId = bundleIdFromId(bundle().get(&GccToolchain::parentToolchainId)); - const QList mingwBundles - = Utils::filtered(ToolchainBundle::collectBundles(), [](const ToolchainBundle &b) { - return b.type() == Constants::MINGW_TOOLCHAIN_TYPEID; - }); + const QList mingwBundles = Utils::filtered( + ToolchainBundle::collectBundles(ToolchainBundle::AutoRegister::NotApplicable), + [](const ToolchainBundle &b) { return b.type() == Constants::MINGW_TOOLCHAIN_TYPEID; }); const auto parentBundle = Utils::findOr(mingwBundles, std::nullopt, [parentBundleId](const ToolchainBundle &b) { return b.bundleId() == parentBundleId; diff --git a/src/plugins/projectexplorer/kitaspects.cpp b/src/plugins/projectexplorer/kitaspects.cpp index c4690f45b8d..c5478de4edf 100644 --- a/src/plugins/projectexplorer/kitaspects.cpp +++ b/src/plugins/projectexplorer/kitaspects.cpp @@ -271,8 +271,10 @@ private: return !tc->compilerCommand().isSameDevice(device->rootPath()); }); - const QList sameBundles = ToolchainBundle::collectBundles(same); - const QList otherBundles = ToolchainBundle::collectBundles(other); + const QList sameBundles + = ToolchainBundle::collectBundles(same, ToolchainBundle::AutoRegister::On); + const QList otherBundles + = ToolchainBundle::collectBundles(other, ToolchainBundle::AutoRegister::On); for (const ToolchainBundle &b : sameBundles) cb->addItem(b.displayName(), b.bundleId().toSetting()); @@ -453,9 +455,8 @@ static void setToolchainsFromAbis(Kit *k, const LanguagesAndAbis &abisByLanguage } // Get bundles. - const QList bundles = ToolchainBundle::collectBundles(); - for (const ToolchainBundle &b : bundles) - ToolchainManager::registerToolchains(b.createdToolchains()); + const QList bundles = ToolchainBundle::collectBundles( + ToolchainBundle::AutoRegister::On); // Set a matching bundle for each LanguageCategory/Abi pair, if possible. for (auto it = abisByCategory.cbegin(); it != abisByCategory.cend(); ++it) { diff --git a/src/plugins/projectexplorer/kitmanager.cpp b/src/plugins/projectexplorer/kitmanager.cpp index 37b537baa71..04425377efa 100644 --- a/src/plugins/projectexplorer/kitmanager.cpp +++ b/src/plugins/projectexplorer/kitmanager.cpp @@ -303,10 +303,9 @@ void KitManager::restoreKits() // On Linux systems, we usually detect a plethora of same-ish toolchains. The following // algorithm gives precedence to icecc and ccache and otherwise simply chooses the one with // the shortest path. - const QList bundles = ToolchainBundle::collectBundles(); + const QList bundles = ToolchainBundle::collectBundles( + ToolchainBundle::AutoRegister::On); for (const ToolchainBundle &bundle : bundles) { - ToolchainManager::registerToolchains(bundle.createdToolchains()); - auto &bestBundle = uniqueToolchains[bundle.targetAbi()][bundle.factory()->languageCategory()]; if (!bestBundle) { diff --git a/src/plugins/projectexplorer/msvctoolchain.cpp b/src/plugins/projectexplorer/msvctoolchain.cpp index dba9089f797..3859b83bb84 100644 --- a/src/plugins/projectexplorer/msvctoolchain.cpp +++ b/src/plugins/projectexplorer/msvctoolchain.cpp @@ -1640,7 +1640,8 @@ void ClangClToolchainConfigWidget::applyImpl() const QString displayedVarsBat = m_varsBatDisplayCombo->currentText(); Toolchains results = detectClangClToolChainInPath(clangClPath, {}, displayedVarsBat); - const QList bundles = ToolchainBundle::collectBundles(results); + const QList bundles + = ToolchainBundle::collectBundles(results, ToolchainBundle::AutoRegister::NotApplicable); if (bundles.isEmpty()) { bundle().set(&ClangClToolchain::resetVarsBat); diff --git a/src/plugins/projectexplorer/toolchain.cpp b/src/plugins/projectexplorer/toolchain.cpp index e242f198753..681f61fd480 100644 --- a/src/plugins/projectexplorer/toolchain.cpp +++ b/src/plugins/projectexplorer/toolchain.cpp @@ -879,7 +879,8 @@ void AsyncToolchainDetector::run() * - There is exactly one toolchain in the list for every language supported by the factory. * - If there is a C compiler, it comes first in the list. */ -ToolchainBundle::ToolchainBundle(const Toolchains &toolchains) : m_toolchains(toolchains) +ToolchainBundle::ToolchainBundle(const Toolchains &toolchains, AutoRegister autoRegister) + : m_toolchains(toolchains) { // Check pre-conditions. QTC_ASSERT(!m_toolchains.isEmpty(), return); @@ -892,7 +893,7 @@ ToolchainBundle::ToolchainBundle(const Toolchains &toolchains) : m_toolchains(to QTC_ASSERT(tc->bundleId() == toolchains.first()->bundleId(), return); } - addMissingToolchains(); + addMissingToolchains(autoRegister); // Check post-conditions. QTC_ASSERT(m_toolchains.size() == m_toolchains.first()->factory()->supportedLanguages().size(), @@ -905,12 +906,13 @@ ToolchainBundle::ToolchainBundle(const Toolchains &toolchains) : m_toolchains(to }); } -QList ToolchainBundle::collectBundles() +QList ToolchainBundle::collectBundles(AutoRegister autoRegister) { - return collectBundles(ToolchainManager::toolchains()); + return collectBundles(ToolchainManager::toolchains(), autoRegister); } -QList ToolchainBundle::collectBundles(const Toolchains &toolchains) +QList ToolchainBundle::collectBundles( + const Toolchains &toolchains, AutoRegister autoRegister) { QHash toolchainsPerBundleId; for (Toolchain * const tc : toolchains) @@ -919,12 +921,12 @@ QList ToolchainBundle::collectBundles(const Toolchains &toolcha QList bundles; if (const auto unbundled = toolchainsPerBundleId.constFind(Id()); unbundled != toolchainsPerBundleId.constEnd()) { - bundles = bundleUnbundledToolchains(*unbundled); + bundles = bundleUnbundledToolchains(*unbundled, autoRegister); toolchainsPerBundleId.erase(unbundled); } for (const Toolchains &tcs : toolchainsPerBundleId) - bundles << tcs; + bundles.emplaceBack(tcs, autoRegister); return bundles; } @@ -971,7 +973,8 @@ ToolchainBundle::Valid ToolchainBundle::validity() const return Valid::None; } -QList ToolchainBundle::bundleUnbundledToolchains(const Toolchains &unbundled) +QList ToolchainBundle::bundleUnbundledToolchains( + const Toolchains &unbundled, AutoRegister autoRegister) { QList bundles; QHash> unbundledByTypeAndLanguage; @@ -997,7 +1000,7 @@ QList ToolchainBundle::bundleUnbundledToolchains(const Toolchai const Id newBundleId = Id::generate(); for (Toolchain * const tc : nextBundle) tc->setBundleId(newBundleId); - bundles << nextBundle; + bundles.emplaceBack(nextBundle, autoRegister); } } @@ -1035,10 +1038,10 @@ ToolchainBundle ToolchainBundle::clone() const const Id newBundleId = Id::generate(); for (Toolchain * const tc : clones) tc->setBundleId(newBundleId); - return clones; + return ToolchainBundle(clones, ToolchainBundle::AutoRegister::NotApplicable); } -void ToolchainBundle::addMissingToolchains() +void ToolchainBundle::addMissingToolchains(AutoRegister autoRegister) { const QList missingLanguages = Utils::filtered(m_toolchains.first()->factory()->supportedLanguages(), [this](Id lang) { @@ -1046,12 +1049,23 @@ void ToolchainBundle::addMissingToolchains() return tc->language() == lang; }); }); + Toolchains createdToolchains; for (const Id lang : missingLanguages) { Toolchain * const tc = m_toolchains.first()->clone(); tc->setLanguage(lang); tc->setCompilerCommand(m_toolchains.first()->correspondingCompilerCommand(lang)); m_toolchains << tc; - m_createdToolchains << tc; + createdToolchains << tc; + } + + switch (autoRegister) { + case AutoRegister::On: + ToolchainManager::registerToolchains(createdToolchains); + case AutoRegister::Off: + break; + case AutoRegister::NotApplicable: + QTC_CHECK(createdToolchains.isEmpty()); + break; } } diff --git a/src/plugins/projectexplorer/toolchain.h b/src/plugins/projectexplorer/toolchain.h index ac1f4eb2519..40a384c0110 100644 --- a/src/plugins/projectexplorer/toolchain.h +++ b/src/plugins/projectexplorer/toolchain.h @@ -218,10 +218,16 @@ using Toolchains = QList; class PROJECTEXPLORER_EXPORT ToolchainBundle { public: - ToolchainBundle(const Toolchains &toolchains); + // Setting up a bundle may necessitate creating additional toolchains. + // Depending on the context, these should or should not be registered + // immediately with the ToolchainManager. + enum class AutoRegister { On, Off, NotApplicable }; - static QList collectBundles(); - static QList collectBundles(const Toolchains &toolchains); + ToolchainBundle(const Toolchains &toolchains, AutoRegister autoRegister); + + static QList collectBundles(AutoRegister autoRegister); + static QList collectBundles( + const Toolchains &toolchains, AutoRegister autoRegister); template R get(R (T:: *getter)(A...) const, A&&... args) const @@ -261,7 +267,6 @@ public: int size() const { return m_toolchains.size(); } const QList toolchains() const { return m_toolchains; } - const QList createdToolchains() const { return m_createdToolchains; } ToolchainFactory *factory() const; Utils::Id bundleId() const { return get(&Toolchain::bundleId); } QString displayName() const; @@ -301,11 +306,11 @@ public: } private: - void addMissingToolchains(); - static QList bundleUnbundledToolchains(const Toolchains &unbundled); + void addMissingToolchains(AutoRegister autoRegister); + static QList bundleUnbundledToolchains( + const Toolchains &unbundled, AutoRegister autoRegister); Toolchains m_toolchains; - Toolchains m_createdToolchains; }; class PROJECTEXPLORER_EXPORT BadToolchain diff --git a/src/plugins/projectexplorer/toolchainoptionspage.cpp b/src/plugins/projectexplorer/toolchainoptionspage.cpp index 67be9cac8a7..b9bea20a181 100644 --- a/src/plugins/projectexplorer/toolchainoptionspage.cpp +++ b/src/plugins/projectexplorer/toolchainoptionspage.cpp @@ -257,11 +257,10 @@ public: m_widgetStack = new QStackedWidget; m_container->setWidget(m_widgetStack); - const QList bundles = ToolchainBundle::collectBundles(); - for (const ToolchainBundle &b : bundles) { - ToolchainManager::registerToolchains(b.createdToolchains()); + const QList bundles = ToolchainBundle::collectBundles( + ToolchainBundle::AutoRegister::On); + for (const ToolchainBundle &b : bundles) insertBundle(b); - } auto buttonLayout = new QVBoxLayout; buttonLayout->setSpacing(6); @@ -392,11 +391,10 @@ void ToolChainOptionsWidget::handleToolchainsRegistered(const Toolchains &toolch return; } - const QList bundles = ToolchainBundle::collectBundles(toolchains); - for (const ToolchainBundle &bundle : bundles) { - ToolchainManager::registerToolchains(bundle.createdToolchains()); + const QList bundles + = ToolchainBundle::collectBundles(toolchains, ToolchainBundle::AutoRegister::On); + for (const ToolchainBundle &bundle : bundles) insertBundle(bundle); - } updateState(); } @@ -516,7 +514,8 @@ void ToolChainOptionsWidget::redetectToolchains() } // Step 4: Create new bundles and add items for them. - const QList newBundles = ToolchainBundle::collectBundles(toAdd); + const QList newBundles + = ToolchainBundle::collectBundles(toAdd, ToolchainBundle::AutoRegister::Off); for (const ToolchainBundle &bundle : newBundles) m_toAddList << insertBundle(bundle, true); } @@ -603,7 +602,7 @@ void ToolChainOptionsWidget::createToolchains(ToolchainFactory *factory, const Q toolchains << tc; } - const ToolchainBundle bundle(toolchains); + const ToolchainBundle bundle(toolchains, ToolchainBundle::AutoRegister::Off); ToolChainTreeItem * const item = insertBundle(bundle, true); m_toAddList << item; m_toolChainView->setCurrentIndex(m_sortModel.mapFromSource(m_model.indexForItem(item))); diff --git a/src/plugins/qtsupport/qtkitaspect.cpp b/src/plugins/qtsupport/qtkitaspect.cpp index eccff9490f8..e4dd95761be 100644 --- a/src/plugins/qtsupport/qtkitaspect.cpp +++ b/src/plugins/qtsupport/qtkitaspect.cpp @@ -229,9 +229,8 @@ void QtKitAspectFactory::fix(Kit *k) if (ToolchainKitAspect::cxxToolchain(k)) return; - QList bundles = ToolchainBundle::collectBundles(); - for (const ToolchainBundle &b : std::as_const(bundles)) - ToolchainManager::registerToolchains(b.createdToolchains()); + QList bundles = ToolchainBundle::collectBundles( + ToolchainBundle::AutoRegister::On); using ProjectExplorer::Constants::CXX_LANGUAGE_ID; bundles = Utils::filtered(bundles, [version](const ToolchainBundle &b) { if (!b.isCompletelyValid() || !b.factory()->languageCategory().contains(CXX_LANGUAGE_ID))