From 34fd28327d9b81bd3a4aa340a90869caccbddfc6 Mon Sep 17 00:00:00 2001
From: hjk <hjk@qt.io>
Date: Wed, 1 Mar 2023 16:27:47 +0100
Subject: [PATCH] CMake: Use callback-based environment iteration

Hides underlying data structure.

Task-number: QTCREATORBUG-28357
Change-Id: Ib0d7fb70afa820b1bd28d23e12b9379a6de6546b
Reviewed-by: Cristian Adam <cristian.adam@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
---
 .../cmakeprojectmanager/presetsmacros.cpp     | 55 ++++++++-----------
 .../cmakeprojectmanager/presetsparser.cpp     |  6 +-
 2 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/src/plugins/cmakeprojectmanager/presetsmacros.cpp b/src/plugins/cmakeprojectmanager/presetsmacros.cpp
index 4dd25c3b1d1..d267e295411 100644
--- a/src/plugins/cmakeprojectmanager/presetsmacros.cpp
+++ b/src/plugins/cmakeprojectmanager/presetsmacros.cpp
@@ -8,6 +8,8 @@
 #include <utils/filepath.h>
 #include <utils/hostosinfo.h>
 
+using namespace Utils;
+
 namespace CMakeProjectManager::Internal::CMakePresets::Macros {
 
 static QString getHostSystemName(Utils::OsType osType)
@@ -99,32 +101,26 @@ static QString expandMacroEnv(const QString &macroPrefix,
     return result;
 }
 
-static Utils::Environment getEnvCombined(const std::optional<Utils::Environment> &optPresetEnv,
-                                         const Utils::Environment &env)
+static Environment getEnvCombined(const std::optional<Environment> &optPresetEnv,
+                                  const Environment &env)
 {
-    Utils::Environment result = env;
+    Environment result = env;
 
-    if (!optPresetEnv)
-        return result;
-
-    Utils::Environment presetEnv = optPresetEnv.value();
-    for (auto it = presetEnv.constBegin(); it != presetEnv.constEnd(); ++it) {
-        result.set(it.key().name, it.value().first);
+    if (optPresetEnv) {
+        optPresetEnv->forEachEntry([&result](const QString &key, const QString &value, bool) {
+            result.set(key, value);
+        });
     }
 
     return result;
 }
 
 template<class PresetType>
-void expand(const PresetType &preset,
-            Utils::Environment &env,
-            const Utils::FilePath &sourceDirectory)
+void expand(const PresetType &preset, Environment &env, const FilePath &sourceDirectory)
 {
-    const Utils::Environment presetEnv = getEnvCombined(preset.environment, env);
-    for (auto it = presetEnv.constBegin(); it != presetEnv.constEnd(); ++it) {
-        const QString key = it.key().name;
-        QString value = it.value().first;
-
+    const Environment presetEnv = getEnvCombined(preset.environment, env);
+    presetEnv.forEachEntry([&](const QString &key, const QString &value_, bool) {
+        QString value = value_;
         expandAllButEnv(preset, sourceDirectory, value);
         value = expandMacroEnv("env", value, [presetEnv](const QString &macroName) {
             return presetEnv.value(macroName);
@@ -133,7 +129,7 @@ void expand(const PresetType &preset,
         QString sep;
         bool append = true;
         if (key.compare("PATH", Qt::CaseInsensitive) == 0) {
-            sep = Utils::OsSpecificAspects::pathListSeparator(env.osType());
+            sep = OsSpecificAspects::pathListSeparator(env.osType());
             const int index = value.indexOf("$penv{PATH}", 0, Qt::CaseInsensitive);
             if (index != 0)
                 append = false;
@@ -148,20 +144,15 @@ void expand(const PresetType &preset,
             env.appendOrSet(key, value, sep);
         else
             env.prependOrSet(key, value, sep);
-    }
+    });
 }
 
 template<class PresetType>
-void expand(const PresetType &preset,
-            Utils::EnvironmentItems &envItems,
-            const Utils::FilePath &sourceDirectory)
+void expand(const PresetType &preset, EnvironmentItems &envItems, const FilePath &sourceDirectory)
 {
-    const Utils::Environment presetEnv = preset.environment ? preset.environment.value()
-                                                            : Utils::Environment();
-    for (auto it = presetEnv.constBegin(); it != presetEnv.constEnd(); ++it) {
-        const QString key = it.key().name;
-        QString value = it.value().first;
-
+    const Environment presetEnv = preset.environment ? *preset.environment : Environment();
+    presetEnv.forEachEntry([&](const QString &key, const QString &value_, bool) {
+        QString value = value_;
         expandAllButEnv(preset, sourceDirectory, value);
         value = expandMacroEnv("env", value, [presetEnv](const QString &macroName) {
             if (presetEnv.hasKey(macroName))
@@ -169,12 +160,12 @@ void expand(const PresetType &preset,
             return QString("${%1}").arg(macroName);
         });
 
-        auto operation = Utils::EnvironmentItem::Operation::SetEnabled;
+        auto operation = EnvironmentItem::Operation::SetEnabled;
         if (key.compare("PATH", Qt::CaseInsensitive) == 0) {
-            operation = Utils::EnvironmentItem::Operation::Append;
+            operation = EnvironmentItem::Operation::Append;
             const int index = value.indexOf("$penv{PATH}", 0, Qt::CaseInsensitive);
             if (index != 0)
-                operation = Utils::EnvironmentItem::Operation::Prepend;
+                operation = EnvironmentItem::Operation::Prepend;
             value.replace("$penv{PATH}", "", Qt::CaseInsensitive);
         }
 
@@ -183,7 +174,7 @@ void expand(const PresetType &preset,
         });
 
         envItems.emplace_back(Utils::EnvironmentItem(key, value, operation));
-    }
+    });
 }
 
 template<class PresetType>
diff --git a/src/plugins/cmakeprojectmanager/presetsparser.cpp b/src/plugins/cmakeprojectmanager/presetsparser.cpp
index 8d1cc1610df..b6e8ddb205c 100644
--- a/src/plugins/cmakeprojectmanager/presetsparser.cpp
+++ b/src/plugins/cmakeprojectmanager/presetsparser.cpp
@@ -456,9 +456,9 @@ static QHash<QString, QString> merge(const QHash<QString, QString> &first,
 static Utils::Environment merge(const Utils::Environment &first, const Utils::Environment &second)
 {
     Utils::Environment result = first;
-    for (auto it = second.constBegin(); it != second.constEnd(); ++it) {
-        result.set(it.key().name, it.value().first);
-    }
+    second.forEachEntry([&](const QString &key, const QString &value, bool) {
+        result.set(key, value);
+    });
 
     return result;
 }