[Lldb-commits] [PATCH] D159313: [lldb][NFCI] Remove use of ConstString in StructuredData

[Alex Langford via Phabricator via lldb-commits]

bulbazord marked an inline comment as done.
bulbazord added inline comments.


================
Comment at: lldb/include/lldb/Utility/StructuredData.h:448
-      if (!key.empty()) {
-        ConstString key_cs(key);
-        collection::const_iterator iter = m_dict.find(key_cs);
----------------
fdeazeve wrote:
> This line in particular also shows why I think this patch is appropriate: even queries were introducing strings into the pool
Yes, this also was happening in `HasKey` as well. 


================
Comment at: lldb/include/lldb/Utility/StructuredData.h:537
     void AddItem(llvm::StringRef key, ObjectSP value_sp) {
-      ConstString key_cs(key);
-      m_dict[key_cs] = std::move(value_sp);
+      m_dict.insert({key, std::move(value_sp)});
     }
----------------
fdeazeve wrote:
> This is not equivalent to the old code, right? The previous code would overwrite the contents if the key already existed.
Oh, good catch. I think what I want is `insert_or_assign`. Will update.


================
Comment at: lldb/source/Utility/StructuredData.cpp:173
+
+  llvm::sort(sorted_entries, [&](const Entry &lhs, const Entry &rhs) -> bool {
+    return lhs.first < rhs.first;
----------------
fdeazeve wrote:
> I thought pairs had `operator <` defined already?
Oh, I didn't even think to check. I'll try it and update if I don't need to define this lambda.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159313