Skip to content

Commit

Permalink
Recommit "[lldb/API] Overwrite variables with SBLaunchInfo::SetEnviro…
Browse files Browse the repository at this point in the history
…nment(append=true)"

The patch was reverted 27d52cd because of failures in
TestWeakSymbols.py. These have now been addressed in D83552.

The original commit message was:
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.

Differential Revision: https://reviews.llvm.org/D83306
  • Loading branch information
labath committed Jul 23, 2020
1 parent 697c6d8 commit 9cdd68e
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
7 changes: 4 additions & 3 deletions lldb/source/API/SBLaunchInfo.cpp
Expand Up @@ -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();
}
Expand Down
10 changes: 10 additions & 0 deletions lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
Expand Up @@ -53,6 +53,11 @@ def test_launch_info(self):
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)
Expand Down Expand Up @@ -120,6 +125,11 @@ def test_creating_and_modifying_environment(self):
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, [])

0 comments on commit 9cdd68e

Please sign in to comment.