labath created this revision. labath added reviewers: clayborg, wallace, jingham. Herald added a project: LLDB.
This function was documented to overwrite entries with D76111 <https://reviews.llvm.org/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 <https://reviews.llvm.org/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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D83306 Files: lldb/source/API/SBLaunchInfo.cpp lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py Index: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py =================================================================== --- lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py +++ lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py @@ -53,6 +53,11 @@ 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 @@ 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, []) Index: lldb/source/API/SBLaunchInfo.cpp =================================================================== --- lldb/source/API/SBLaunchInfo.cpp +++ lldb/source/API/SBLaunchInfo.cpp @@ -190,9 +190,10 @@ 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(); }
Index: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py =================================================================== --- lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py +++ lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py @@ -53,6 +53,11 @@ 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 @@ 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, []) Index: lldb/source/API/SBLaunchInfo.cpp =================================================================== --- lldb/source/API/SBLaunchInfo.cpp +++ lldb/source/API/SBLaunchInfo.cpp @@ -190,9 +190,10 @@ 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(); }
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits