[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

No reason for these strings to be in the ConstString pool, so that part is fine.

But if we're going to use this to store things like the env-vars, having no ordering guarantees when we dump them seems likely to be a bit annoying.  If the order of element output in `settings show env-vars` dump changes around every time I add an element to it, that would be make the results hard to follow.  Can we make the dumper for this option value pull the keys out, alphabetize the keys, then look the values up in that order?  I don't think printing these objects is ever going to be performance critical.



================
Comment at: lldb/source/Core/Disassembler.cpp:846
+        // Note: The reason we are making the data_type a uint64_t when value is
+        // uint32_t is that there is no eTypeUInt32 enum value.
+        if (llvm::StringRef(value) == "uint32_t")
----------------
That's kind of answering a question with a question:  Why isn't there an eTypeUInt32?


================
Comment at: lldb/source/Interpreter/OptionValueDictionary.cpp:39
 
-    for (pos = m_values.begin(); pos != end; ++pos) {
+    for (auto pos = m_values.begin(), end = m_values.end(); pos != end; ++pos) {
       OptionValue *option_value = pos->second.get();
----------------
Does the llvm::StringMap dingus not support `for (auto value : m_values) {` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149482