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 <hjk@qt.io>

src/plugins/docker/kitdetector.cpp

@@ -262,9 +262,8 @@ Toolchains KitDetectorPrivate::autoDetectToolchains()
                                   .arg(toolchain->compilerCommand().toUserOutput()));
             toolchain->setDetectionSource(m_sharedId);
         }
-        const QList<ToolchainBundle> bundles = ToolchainBundle::collectBundles(newToolchains);
+        const QList<ToolchainBundle> bundles
-        for (const ToolchainBundle &b : bundles)
+            = ToolchainBundle::collectBundles(newToolchains, ToolchainBundle::AutoRegister::On);
-            ToolchainManager::registerToolchains(b.toolchains());
         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<ToolchainBundle> bundles = ToolchainBundle::collectBundles(toolchainCandidates);
+        const QList<ToolchainBundle> bundles
-        for (const ToolchainBundle &b : bundles)
+            = ToolchainBundle::collectBundles(toolchainCandidates, ToolchainBundle::AutoRegister::On);
-            ToolchainManager::registerToolchains(b.createdToolchains());
 
         // Try to find a matching Qt/Toolchain pair.
         bool match = false;

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<ToolchainBundle> mingwBundles
+    const QList<ToolchainBundle> mingwBundles = Utils::filtered(
-        = Utils::filtered(ToolchainBundle::collectBundles(), [](const ToolchainBundle &b) {
+        ToolchainBundle::collectBundles(ToolchainBundle::AutoRegister::NotApplicable),
-              return b.type() == Constants::MINGW_TOOLCHAIN_TYPEID;
+        [](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;

src/plugins/projectexplorer/kitaspects.cpp

@@ -271,8 +271,10 @@ private:
                 return !tc->compilerCommand().isSameDevice(device->rootPath());
             });
 
-            const QList<ToolchainBundle> sameBundles = ToolchainBundle::collectBundles(same);
+            const QList<ToolchainBundle> sameBundles
-            const QList<ToolchainBundle> otherBundles = ToolchainBundle::collectBundles(other);
+                = ToolchainBundle::collectBundles(same, ToolchainBundle::AutoRegister::On);
+            const QList<ToolchainBundle> 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<ToolchainBundle> bundles = ToolchainBundle::collectBundles();
+    const QList<ToolchainBundle> bundles = ToolchainBundle::collectBundles(
-    for (const ToolchainBundle &b : bundles)
+        ToolchainBundle::AutoRegister::On);
-        ToolchainManager::registerToolchains(b.createdToolchains());
 
     // Set a matching bundle for each LanguageCategory/Abi pair, if possible.
     for (auto it = abisByCategory.cbegin(); it != abisByCategory.cend(); ++it) {

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<ToolchainBundle> bundles = ToolchainBundle::collectBundles();
+        const QList<ToolchainBundle> 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) {

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<ToolchainBundle> bundles = ToolchainBundle::collectBundles(results);
+    const QList<ToolchainBundle> bundles
+        = ToolchainBundle::collectBundles(results, ToolchainBundle::AutoRegister::NotApplicable);
 
     if (bundles.isEmpty()) {
         bundle().set(&ClangClToolchain::resetVarsBat);

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> ToolchainBundle::collectBundles()
+QList<ToolchainBundle> ToolchainBundle::collectBundles(AutoRegister autoRegister)
 {
-    return collectBundles(ToolchainManager::toolchains());
+    return collectBundles(ToolchainManager::toolchains(), autoRegister);
 }
 
-QList<ToolchainBundle> ToolchainBundle::collectBundles(const Toolchains &toolchains)
+QList<ToolchainBundle> ToolchainBundle::collectBundles(
+    const Toolchains &toolchains, AutoRegister autoRegister)
 {
     QHash<Id, Toolchains> toolchainsPerBundleId;
     for (Toolchain * const tc : toolchains)
@@ -919,12 +921,12 @@ QList<ToolchainBundle> ToolchainBundle::collectBundles(const Toolchains &toolcha
     QList<ToolchainBundle> 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> ToolchainBundle::bundleUnbundledToolchains(const Toolchains &unbundled)
+QList<ToolchainBundle> ToolchainBundle::bundleUnbundledToolchains(
+    const Toolchains &unbundled, AutoRegister autoRegister)
 {
     QList<ToolchainBundle> bundles;
     QHash<Id, QHash<Id, Toolchains>> unbundledByTypeAndLanguage;
@@ -997,7 +1000,7 @@ QList<ToolchainBundle> 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<Id> 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;
     }
 }
 

src/plugins/projectexplorer/toolchain.h

@@ -218,10 +218,16 @@ using Toolchains = QList<Toolchain *>;
 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<ToolchainBundle> collectBundles();
+    ToolchainBundle(const Toolchains &toolchains, AutoRegister autoRegister);
-    static QList<ToolchainBundle> collectBundles(const Toolchains &toolchains);
+
+    static QList<ToolchainBundle> collectBundles(AutoRegister autoRegister);
+    static QList<ToolchainBundle> collectBundles(
+        const Toolchains &toolchains, AutoRegister autoRegister);
 
     template<typename R, class T = Toolchain, typename... A>
     R get(R (T:: *getter)(A...) const, A&&... args) const
@@ -261,7 +267,6 @@ public:
     int size() const { return m_toolchains.size(); }
 
     const QList<Toolchain *> toolchains() const { return m_toolchains; }
-    const QList<Toolchain *> 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();
+    void addMissingToolchains(AutoRegister autoRegister);
-    static QList<ToolchainBundle> bundleUnbundledToolchains(const Toolchains &unbundled);
+    static QList<ToolchainBundle> bundleUnbundledToolchains(
+        const Toolchains &unbundled, AutoRegister autoRegister);
 
     Toolchains m_toolchains;
-    Toolchains m_createdToolchains;
 };
 
 class PROJECTEXPLORER_EXPORT BadToolchain

src/plugins/projectexplorer/toolchainoptionspage.cpp

@@ -257,11 +257,10 @@ public:
         m_widgetStack = new QStackedWidget;
         m_container->setWidget(m_widgetStack);
 
-        const QList<ToolchainBundle> bundles = ToolchainBundle::collectBundles();
+        const QList<ToolchainBundle> bundles = ToolchainBundle::collectBundles(
-        for (const ToolchainBundle &b : bundles) {
+            ToolchainBundle::AutoRegister::On);
-            ToolchainManager::registerToolchains(b.createdToolchains());
+        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<ToolchainBundle> bundles = ToolchainBundle::collectBundles(toolchains);
+    const QList<ToolchainBundle> bundles
-    for (const ToolchainBundle &bundle : bundles) {
+        = ToolchainBundle::collectBundles(toolchains, ToolchainBundle::AutoRegister::On);
-        ToolchainManager::registerToolchains(bundle.createdToolchains());
+    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<ToolchainBundle> newBundles = ToolchainBundle::collectBundles(toAdd);
+    const QList<ToolchainBundle> 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)));

src/plugins/qtsupport/qtkitaspect.cpp

@@ -229,9 +229,8 @@ void QtKitAspectFactory::fix(Kit *k)
     if (ToolchainKitAspect::cxxToolchain(k))
         return;
 
-    QList<ToolchainBundle> bundles = ToolchainBundle::collectBundles();
+    QList<ToolchainBundle> bundles = ToolchainBundle::collectBundles(
-    for (const ToolchainBundle &b : std::as_const(bundles))
+        ToolchainBundle::AutoRegister::On);
-        ToolchainManager::registerToolchains(b.createdToolchains());
     using ProjectExplorer::Constants::CXX_LANGUAGE_ID;
     bundles = Utils::filtered(bundles, [version](const ToolchainBundle &b) {
         if (!b.isCompletelyValid() || !b.factory()->languageCategory().contains(CXX_LANGUAGE_ID))