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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
