[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 2 inline comments as done.
wallace added inline comments.


================
Comment at: lldb/source/API/SBEnvironment.cpp:62-73
+      ConstString(std::next(m_opaque_up->begin(), index)->first())
+          .AsCString(""));
+}
+
+const char *SBEnvironment::GetValueAtIndex(size_t index) {
+  LLDB_RECORD_METHOD(const char *, SBEnvironment, GetValueAtIndex, (size_t),
+                     index);
----------------
clayborg wrote:
> labath wrote:
> > I don't think these need to const-stringified now, given that they are backed by the underlying map. We already have functions which return pointers to internal objects (e.g. SBStream::GetData).
> > 
> > @clayborg? 
> That's a tough one. I would like to think that any "const char *" that comes back from an API that returns strings could just be pointer compared if needed. So I like the idea that for strings that comes out of the API that they are all const-ed and could easily be compared. I am fine with SBStream::GetData() returning its own string because _it_ owns the data. So I would vote to ConstString() any returned results
I'm adding a Clear method. With that, the backing char* might be wiped out and the python reference to this string might be invalid unless we use ConstString


================
Comment at: lldb/source/API/SBEnvironment.cpp:80-81
+                     overwrite);
+  if (overwrite || m_opaque_up->find(name) == m_opaque_up->end()) {
+    m_opaque_up->insert_or_assign(name, std::string(value));
+    return LLDB_RECORD_RESULT(true);
----------------
labath wrote:
> how about `if(overwrite) insert_or_assign(name, value) else try_emplace(name, value)`? (avoiding two map lookups)
teach me more


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76111/new/

https://reviews.llvm.org/D76111