MsvcToolchain: Rework detection of supportedAbis

Commit 9912a409da made changes to
implement user MSVC toolchains allowing to set custom arguments to
vcvars bat file.

This commit introduced the possibility to have multiple supported Abis
for a MSVC toolchain.
But the supported ABI detection broke with older toolchain that are
initialized with a vcvars32.bat and the like (as defined in
MsvcPlatform platforms).

msvc2010 and VC2015 build tools got affected, the detected Abis were
empty and made Qt Creator to fail to load these toolchain from
toolchain.xml (as a non empty m_supportedAbis list was required).

The effect of this is that any Kit that was referring to these compilers
was reset to "No compiler" every time Qt Creator is started.
In that case, the user could workaround the issue by reconfiguring Kits
after each Qt Creator startup or never close QtCreator which is
impractical.

This commit is a rework of the fix introduced in
708179285d which was a minimal fix
introducted in 4.10.2.

This commit fix this issue by removing the notion of multiple ABI per
MSVC toolchain and instead iterate all detected toolchain for matching
ones when populating the ABI widget via setAbis.

This also makes only one proposed ABI in the mainComboBox of AbiWidget
for any selected varsBatArchCombo. So it is no more possible to have a
32 bits proposed ABI with a /x64 varsBatArchCombo for example.

At any time, if the user wants to set a different ABI for a given
varsBatPath/varsBatArch, he can still select the "custom" item and
manually configure each part of the ABI.

Task-number: QTCREATORBUG-22960
Change-Id: I753e96db3b0e83371fa39d91c4a3592bd1552b8e
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/projectexplorer/msvctoolchain.cpp

@@ -67,7 +67,6 @@ using namespace Utils;
 static const char varsBatKeyC[] = KEY_ROOT "VarsBat";
 static const char varsBatArgKeyC[] = KEY_ROOT "VarsBatArg";
 static const char supportedAbiKeyC[] = KEY_ROOT "SupportedAbi";
-static const char supportedAbisKeyC[] = KEY_ROOT "SupportedAbis";
 static const char environModsKeyC[] = KEY_ROOT "environmentModifications";
 
 enum { debug = 0 };
@@ -775,55 +774,6 @@ void MsvcToolChain::updateEnvironmentModifications(Utils::EnvironmentItems modif
     }
 }
 
-void MsvcToolChain::detectInstalledAbis()
-{
-    static QMap<QString, Abis> abiCache;
-    const QString vcVarsBase
-            = QDir::fromNativeSeparators(m_vcvarsBat).left(m_vcvarsBat.lastIndexOf('/'));
-    if (abiCache.contains(vcVarsBase)) {
-        m_supportedAbis = abiCache.value(vcVarsBase);
-    } else {
-        // Clear previously detected m_supportedAbis to repopulate it.
-        m_supportedAbis.clear();
-        const Abi baseAbi = targetAbi();
-        for (MsvcPlatform platform : platforms) {
-            bool toolchainInstalled = false;
-            QString perhapsVcVarsPath = vcVarsBase + QLatin1Char('/') + QLatin1String(platform.bat);
-            const Platform p = platform.platform;
-            if (QFileInfo(perhapsVcVarsPath).isFile()) {
-                toolchainInstalled = true;
-            } else {
-                // MSVC 2015 and below had various versions of vcvars scripts in subfolders. Try these
-                // as fallbacks.
-                perhapsVcVarsPath = vcVarsBase + platform.prefix + QLatin1Char('/')
-                                    + QLatin1String(platform.bat);
-                toolchainInstalled = QFileInfo(perhapsVcVarsPath).isFile();
-            }
-            if (hostSupportsPlatform(platform.platform) && toolchainInstalled) {
-                Abi newAbi(archForPlatform(p),
-                           baseAbi.os(),
-                           baseAbi.osFlavor(),
-                           baseAbi.binaryFormat(),
-                           wordWidthForPlatform(p));
-                if (!m_supportedAbis.contains(newAbi))
-                    m_supportedAbis.append(newAbi);
-            }
-        }
-
-        abiCache.insert(vcVarsBase, m_supportedAbis);
-    }
-
-    // Always add targetAbi in supportedAbis if it is empty.
-    // targetAbi is the abi with which the toolchain was detected.
-    // This is necessary for toolchains that don't have vcvars32.bat and the like in their
-    // vcVarsBase path, like msvc2010.
-    // Also, don't include that one in abiCache to avoid polluting it with values specific
-    // to one toolchain as the cache is global for a vcVarsBase path. For this reason, the
-    // targetAbi needs to be added manually.
-    if (m_supportedAbis.empty())
-        m_supportedAbis.append(targetAbi());
-}
-
 Utils::Environment MsvcToolChain::readEnvironmentSetting(const Utils::Environment &env) const
 {
     Utils::Environment resultEnv = env;
@@ -894,11 +844,6 @@ Abi MsvcToolChain::targetAbi() const
     return m_abi;
 }
 
-Abis MsvcToolChain::supportedAbis() const
-{
-    return m_supportedAbis;
-}
-
 void MsvcToolChain::setTargetAbi(const Abi &abi)
 {
     m_abi = abi;
@@ -978,7 +923,6 @@ QVariantMap MsvcToolChain::toMap() const
     if (!m_varsBatArg.isEmpty())
         data.insert(QLatin1String(varsBatArgKeyC), m_varsBatArg);
     data.insert(QLatin1String(supportedAbiKeyC), m_abi.toString());
-    data.insert(supportedAbisKeyC, Utils::transform<QStringList>(m_supportedAbis, &Abi::toString));
     Utils::EnvironmentItem::sort(&m_environmentModifications);
     data.insert(QLatin1String(environModsKeyC),
                 Utils::EnvironmentItem::toVariantList(m_environmentModifications));
@@ -994,14 +938,6 @@ bool MsvcToolChain::fromMap(const QVariantMap &data)
 
     const QString abiString = data.value(QLatin1String(supportedAbiKeyC)).toString();
     m_abi = Abi::fromString(abiString);
-    const QStringList abiList = data.value(supportedAbisKeyC).toStringList();
-    m_supportedAbis.clear();
-    for (const QString &a : abiList) {
-        Abi abi = Abi::fromString(a);
-        if (!abi.isValid())
-            continue;
-        m_supportedAbis.append(abi);
-    }
     m_environmentModifications = Utils::EnvironmentItem::itemsFromVariantList(
         data.value(QLatin1String(environModsKeyC)).toList());
     rescanForCompiler();
@@ -1011,11 +947,7 @@ bool MsvcToolChain::fromMap(const QVariantMap &data)
                                       m_vcvarsBat,
                                       m_varsBatArg));
 
-    // supported Abis were not stored in the map in previous versions of the settings. Re-detect
+    const bool valid = !m_vcvarsBat.isEmpty() && m_abi.isValid();
-    if (m_supportedAbis.isEmpty())
-        detectInstalledAbis();
-
-    const bool valid = !m_vcvarsBat.isEmpty() && m_abi.isValid() && !m_supportedAbis.isEmpty();
     if (valid)
         addToAvailableMsvcToolchains(this);
 
@@ -1250,7 +1182,6 @@ void MsvcToolChain::setupVarsBat(const Abi &abi, const QString &varsBat, const Q
     m_varsBatArg = varsBatArg;
 
     if (!varsBat.isEmpty()) {
-        detectInstalledAbis();
         initEnvModWatcher(Utils::runAsync(envModThreadPool(),
                                           &MsvcToolChain::environmentModifications,
                                           varsBat,
@@ -1410,34 +1341,66 @@ void MsvcToolChainConfigWidget::setFromMsvcToolChain()
     m_abiWidget->setAbis(tc->supportedAbis(), tc->targetAbi());
 }
 
-void MsvcToolChainConfigWidget::handleVcVarsChange(const QString &vcVars)
+void MsvcToolChainConfigWidget::updateAbis()
 {
-    const QString normalizedVcVars = QDir::fromNativeSeparators(vcVars);
+    const QString normalizedVcVars = QDir::fromNativeSeparators(m_varsBatPathCombo->currentText());
     const auto *currentTc = static_cast<const MsvcToolChain *>(toolChain());
     QTC_ASSERT(currentTc, return );
     const MsvcToolChain::Platform platform = m_varsBatArchCombo->currentData().value<MsvcToolChain::Platform>();
     const Abi::Architecture arch = archForPlatform(platform);
     const unsigned char wordWidth = wordWidthForPlatform(platform);
 
+    // Search the selected vcVars bat file in already detected MSVC compilers.
+    // For each variant of MSVC found, add its supported ABIs to the ABI widget so the user can
+    // choose one appropriately.
+    Abis supportedAbis;
+    Abi targetAbi;
     for (const MsvcToolChain *tc : g_availableMsvcToolchains) {
         if (tc->varsBat() == normalizedVcVars && tc->targetAbi().wordWidth() == wordWidth
-                && tc->targetAbi().architecture() == arch) {
+            && tc->targetAbi().architecture() == arch && tc->language() == currentTc->language()) {
-            m_abiWidget->setAbis(tc->supportedAbis(), tc->targetAbi());
+            // We need to filter out duplicates as there might be multiple toolchains with
-            break;
+            // same abi (like x86, amd64_x86 for example).
+            for (const Abi &abi : tc->supportedAbis()) {
+                if (!supportedAbis.contains(abi))
+                    supportedAbis.append(abi);
+            }
+            targetAbi = tc->targetAbi();
         }
     }
+
+    // If we didn't find an exact match, try to find a fallback according to varsBat only.
+    // This can happen when the toolchain does not support user-selected arch/wordWidth.
+    if (!targetAbi.isValid()) {
+        const MsvcToolChain *tc = Utils::findOrDefault(g_availableMsvcToolchains,
+                                                       [normalizedVcVars](const MsvcToolChain *tc) {
+                                                           return tc->varsBat() == normalizedVcVars;
+                                                       });
+        if (tc) {
+            targetAbi = Abi(arch,
+                            tc->targetAbi().os(),
+                            tc->targetAbi().osFlavor(),
+                            tc->targetAbi().binaryFormat(),
+                            wordWidth);
+        }
+    }
+
+    // Always set ABIs, even if none was found, to prevent stale data in the ABI widget.
+    // In that case, a custom ABI will be selected according to targetAbi.
+    m_abiWidget->setAbis(supportedAbis, targetAbi);
+
     emit dirty();
 }
 
+void MsvcToolChainConfigWidget::handleVcVarsChange(const QString &)
+{
+    updateAbis();
+}
+
 void MsvcToolChainConfigWidget::handleVcVarsArchChange(const QString &)
 {
-    Abi currentAbi = m_abiWidget->currentAbi();
+    // supportedAbi list in the widget only contains matching ABIs to whatever arch was selected.
-    const MsvcToolChain::Platform platform = m_varsBatArchCombo->currentData().value<MsvcToolChain::Platform>();
+    // We need to reupdate it from scratch with new arch parameters
-    Abi newAbi(archForPlatform(platform), currentAbi.os(), currentAbi.osFlavor(),
+    updateAbis();
-               currentAbi.binaryFormat(), wordWidthForPlatform(platform));
-    if (currentAbi != newAbi)
-        m_abiWidget->setAbis(m_abiWidget->supportedAbis(), newAbi);
-    emit dirty();
 }
 
 QString MsvcToolChainConfigWidget::vcVarsArguments() const

src/plugins/projectexplorer/msvctoolchain.h

@@ -63,7 +63,6 @@ public:
     ~MsvcToolChain() override;
 
     Abi targetAbi() const override;
-    Abis supportedAbis() const override;
     void setTargetAbi(const Abi &abi);
 
     bool isValid() const override;
@@ -148,7 +147,6 @@ protected:
 private:
     void updateEnvironmentModifications(Utils::EnvironmentItems modifications);
     void rescanForCompiler();
-    void detectInstalledAbis();
 
     mutable Utils::EnvironmentItems m_environmentModifications;
     mutable QFutureWatcher<GenerateEnvResult> m_envModWatcher;
@@ -160,7 +158,6 @@ private:
 
 protected:
     Abi m_abi;
-    Abis m_supportedAbis;
 
     QString m_vcvarsBat;
     QString m_varsBatArg; // Argument
@@ -271,6 +268,7 @@ private:
 
     void setFromMsvcToolChain();
 
+    void updateAbis();
     void handleVcVarsChange(const QString &vcVars);
     void handleVcVarsArchChange(const QString &arch);