[lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

Summary:
This function was documented to overwrite entries with D76111, which was
adding a couple of similar functions. However, this function (unlike the
functions added in that patch) was/is not actually overwriting variables
-- any pre-existing variables would get ignored.

This behavior does not seem to be intentional. In fact, before the refactor in
D41359, this function could introduce duplicate entries, which could
have very surprising effects both inside lldb and on other applications
(some applications would take the first value, some the second one; in
lldb, attempting to unset a variable could make the second variable
become active, etc.).

Overwriting seems to be the most reasonable behavior here, so change the
code to match documentation.

Reviewers: clayborg, wallace, jingham

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D83306

diff --git a/lldb/source/API/SBLaunchInfo.cpp b/lldb/source/API/SBLaunchInfo.cpp
index ba13072..cda8134 100644 (file)
--- a/lldb/source/API/SBLaunchInfo.cpp
+++ b/lldb/source/API/SBLaunchInfo.cpp
@@ -190,9 +190,10 @@ void SBLaunchInfo::SetEnvironment(const SBEnvironment &env, bool append) {
   LLDB_RECORD_METHOD(void, SBLaunchInfo, SetEnvironment,
                      (const lldb::SBEnvironment &, bool), env, append);
   Environment &refEnv = env.ref();
-  if (append)
-    m_opaque_sp->GetEnvironment().insert(refEnv.begin(), refEnv.end());
-  else
+  if (append) {
+    for (auto &KV : refEnv)
+      m_opaque_sp->GetEnvironment().insert_or_assign(KV.first(), KV.second);
+  } else
     m_opaque_sp->GetEnvironment() = refEnv;
   m_opaque_sp->RegenerateEnvp();
 }

diff --git a/lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py b/lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
index c1937f9..6389854 100644 (file)
--- a/lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
+++ b/lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
@@ -53,6 +53,11 @@ class SBEnvironmentAPICase(TestBase):
         launch_info.SetEnvironment(env, append=True)
         self.assertEqual(launch_info.GetEnvironment().GetNumValues(), env_count + 1)
 
+        env.Set("FOO", "baz", overwrite=True)
+        launch_info.SetEnvironment(env, append=True)
+        self.assertEqual(launch_info.GetEnvironment().GetNumValues(), env_count + 1)
+        self.assertEqual(launch_info.GetEnvironment().Get("FOO"), "baz")
+
         # Make sure we can replace the launchInfo's environment
         env.Clear()
         env.Set("BAR", "foo", overwrite=True)
@@ -120,6 +125,11 @@ class SBEnvironmentAPICase(TestBase):
         env.SetEntries(entries, append=False)
         self.assertEqualEntries(env, ["X=x", "Y=y"])
 
+        entries.Clear()
+        entries.AppendList(["X=y", "Y=x"], 2)
+        env.SetEntries(entries, append=True)
+        self.assertEqualEntries(env, ["X=y", "Y=x"])
+
         # Test clear
         env.Clear()
         self.assertEqualEntries(env, [])