[Lldb-commits] [lldb] e53e1de - [lldb] Change ObjectValueDictionary to use a StringMap
Alex Langford via lldb-commits
lldb-commits at lists.llvm.org
Mon May 1 16:18:34 PDT 2023
Author: Alex Langford
Date: 2023-05-01T16:17:24-07:00
New Revision: e53e1de57eccda49a93c4368eabbd95d01c5b854
URL: https://github.com/llvm/llvm-project/commit/e53e1de57eccda49a93c4368eabbd95d01c5b854
DIFF: https://github.com/llvm/llvm-project/commit/e53e1de57eccda49a93c4368eabbd95d01c5b854.diff
LOG: [lldb] Change ObjectValueDictionary to use a StringMap
llvm has a structure for maps where the key's type is a string. Using
that also means that the keys for OptionValueDictionary don't stick
around forever in ConstString's StringPool (even after they are gone).
The only thing we lose here is ordering: iterating over the map where the keys
are ConstStrings guarantees that we iterate in alphabetical order.
StringMap makes no guarantees about the ordering when you iterate over
the entire map.
Differential Revision: https://reviews.llvm.org/D149482
Added:
Modified:
lldb/include/lldb/Interpreter/OptionValueDictionary.h
lldb/source/Core/Disassembler.cpp
lldb/source/Interpreter/OptionValueDictionary.cpp
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/test/API/commands/settings/TestSettings.py
lldb/unittests/Interpreter/TestOptionValue.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Interpreter/OptionValueDictionary.h b/lldb/include/lldb/Interpreter/OptionValueDictionary.h
index 1fdccdeccef50..4c250468b3cda 100644
--- a/lldb/include/lldb/Interpreter/OptionValueDictionary.h
+++ b/lldb/include/lldb/Interpreter/OptionValueDictionary.h
@@ -9,11 +9,11 @@
#ifndef LLDB_INTERPRETER_OPTIONVALUEDICTIONARY_H
#define LLDB_INTERPRETER_OPTIONVALUEDICTIONARY_H
-#include <map>
-
#include "lldb/Interpreter/OptionValue.h"
#include "lldb/lldb-private-types.h"
+#include "llvm/ADT/StringMap.h"
+
namespace lldb_private {
class OptionValueDictionary
@@ -58,7 +58,7 @@ class OptionValueDictionary
size_t GetNumValues() const { return m_values.size(); }
- lldb::OptionValueSP GetValueForKey(ConstString key) const;
+ lldb::OptionValueSP GetValueForKey(llvm::StringRef key) const;
lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
llvm::StringRef name, bool will_modify,
@@ -67,21 +67,19 @@ class OptionValueDictionary
Status SetSubValue(const ExecutionContext *exe_ctx, VarSetOperationType op,
llvm::StringRef name, llvm::StringRef value) override;
- bool SetValueForKey(ConstString key,
- const lldb::OptionValueSP &value_sp,
+ bool SetValueForKey(llvm::StringRef key, const lldb::OptionValueSP &value_sp,
bool can_replace = true);
- bool DeleteValueForKey(ConstString key);
+ bool DeleteValueForKey(llvm::StringRef key);
size_t GetArgs(Args &args) const;
Status SetArgs(const Args &args, VarSetOperationType op);
protected:
- typedef std::map<ConstString, lldb::OptionValueSP> collection;
uint32_t m_type_mask;
OptionEnumValues m_enum_values;
- collection m_values;
+ llvm::StringMap<lldb::OptionValueSP> m_values;
bool m_raw_value_dump;
};
diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp
index 570332f582df2..0d9a83993e53a 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -755,7 +755,7 @@ OptionValueSP Instruction::ReadDictionary(FILE *in_file, Stream *out_stream) {
char buffer[1024];
auto option_value_sp = std::make_shared<OptionValueDictionary>();
- static ConstString encoding_key("data_encoding");
+ static constexpr llvm::StringLiteral encoding_key("data_encoding");
OptionValue::Type data_type = OptionValue::eTypeInvalid;
while (!done) {
@@ -802,7 +802,6 @@ OptionValueSP Instruction::ReadDictionary(FILE *in_file, Stream *out_stream) {
return option_value_sp;
}
- ConstString const_key(key.c_str());
// Check value to see if it's the start of an array or dictionary.
lldb::OptionValueSP value_sp;
@@ -838,15 +837,14 @@ OptionValueSP Instruction::ReadDictionary(FILE *in_file, Stream *out_stream) {
value_sp = std::make_shared<OptionValueString>(value.c_str(), "");
}
- if (const_key == encoding_key) {
+ if (key == encoding_key) {
// A 'data_encoding=..." is NOT a normal key-value pair; it is meta-data
- // indicating the
- // data type of an upcoming array (usually the next bit of data to be
- // read in).
- if (strcmp(value.c_str(), "uint32_t") == 0)
+ // indicating the data type of an upcoming array (usually the next bit
+ // of data to be read in).
+ if (llvm::StringRef(value) == "uint32_t")
data_type = OptionValue::eTypeUInt64;
} else
- option_value_sp->GetAsDictionary()->SetValueForKey(const_key, value_sp,
+ option_value_sp->GetAsDictionary()->SetValueForKey(key, value_sp,
false);
}
}
diff --git a/lldb/source/Interpreter/OptionValueDictionary.cpp b/lldb/source/Interpreter/OptionValueDictionary.cpp
index d18510166eeb7..29253161352ea 100644
--- a/lldb/source/Interpreter/OptionValueDictionary.cpp
+++ b/lldb/source/Interpreter/OptionValueDictionary.cpp
@@ -33,20 +33,24 @@ void OptionValueDictionary::DumpValue(const ExecutionContext *exe_ctx,
if (dump_mask & eDumpOptionType)
strm.PutCString(" =");
- collection::iterator pos, end = m_values.end();
-
if (!one_line)
strm.IndentMore();
- for (pos = m_values.begin(); pos != end; ++pos) {
- OptionValue *option_value = pos->second.get();
+ // m_values is not guaranteed to be sorted alphabetically, so for
+ // consistentcy we will sort them here before dumping
+ std::map<llvm::StringRef, OptionValue *> sorted_values;
+ for (const auto &value : m_values) {
+ sorted_values[value.first()] = value.second.get();
+ }
+ for (const auto &value : sorted_values) {
+ OptionValue *option_value = value.second;
if (one_line)
strm << ' ';
else
strm.EOL();
- strm.Indent(pos->first.GetStringRef());
+ strm.Indent(value.first);
const uint32_t extra_dump_options = m_raw_value_dump ? eDumpOptionRaw : 0;
switch (dict_type) {
@@ -87,18 +91,17 @@ llvm::json::Value
OptionValueDictionary::ToJSON(const ExecutionContext *exe_ctx) {
llvm::json::Object dict;
for (const auto &value : m_values) {
- dict.try_emplace(value.first.GetCString(), value.second->ToJSON(exe_ctx));
+ dict.try_emplace(value.first(), value.second->ToJSON(exe_ctx));
}
return dict;
}
size_t OptionValueDictionary::GetArgs(Args &args) const {
args.Clear();
- collection::const_iterator pos, end = m_values.end();
- for (pos = m_values.begin(); pos != end; ++pos) {
+ for (const auto &value : m_values) {
StreamString strm;
- strm.Printf("%s=", pos->first.GetCString());
- pos->second->DumpValue(nullptr, strm, eDumpOptionValue | eDumpOptionRaw);
+ strm.Printf("%s=", value.first().data());
+ value.second->DumpValue(nullptr, strm, eDumpOptionValue | eDumpOptionRaw);
args.AppendArgument(strm.GetString());
}
return args.GetArgumentCount();
@@ -178,7 +181,7 @@ Status OptionValueDictionary::SetArgs(const Args &args,
if (error.Fail())
return error;
m_value_was_set = true;
- SetValueForKey(ConstString(key), enum_value, true);
+ SetValueForKey(key, enum_value, true);
} else {
lldb::OptionValueSP value_sp(CreateValueFromCStringForTypeMask(
value.str().c_str(), m_type_mask, error));
@@ -186,7 +189,7 @@ Status OptionValueDictionary::SetArgs(const Args &args,
if (error.Fail())
return error;
m_value_was_set = true;
- SetValueForKey(ConstString(key), value_sp, true);
+ SetValueForKey(key, value_sp, true);
} else {
error.SetErrorString("dictionaries that can contain multiple types "
"must subclass OptionValueArray");
@@ -198,11 +201,11 @@ Status OptionValueDictionary::SetArgs(const Args &args,
case eVarSetOperationRemove:
if (argc > 0) {
for (size_t i = 0; i < argc; ++i) {
- ConstString key(args.GetArgumentAtIndex(i));
+ llvm::StringRef key(args.GetArgumentAtIndex(i));
if (!DeleteValueForKey(key)) {
error.SetErrorStringWithFormat(
"no value found named '%s', aborting remove operation",
- key.GetCString());
+ key.data());
break;
}
}
@@ -267,7 +270,7 @@ OptionValueDictionary::GetSubValue(const ExecutionContext *exe_ctx,
return nullptr;
}
- value_sp = GetValueForKey(ConstString(key));
+ value_sp = GetValueForKey(key);
if (!value_sp) {
error.SetErrorStringWithFormat(
"dictionary does not contain a value for the key name '%s'",
@@ -297,22 +300,22 @@ Status OptionValueDictionary::SetSubValue(const ExecutionContext *exe_ctx,
}
lldb::OptionValueSP
-OptionValueDictionary::GetValueForKey(ConstString key) const {
+OptionValueDictionary::GetValueForKey(llvm::StringRef key) const {
lldb::OptionValueSP value_sp;
- collection::const_iterator pos = m_values.find(key);
+ auto pos = m_values.find(key);
if (pos != m_values.end())
value_sp = pos->second;
return value_sp;
}
-bool OptionValueDictionary::SetValueForKey(ConstString key,
+bool OptionValueDictionary::SetValueForKey(llvm::StringRef key,
const lldb::OptionValueSP &value_sp,
bool can_replace) {
// Make sure the value_sp object is allowed to contain values of the type
// passed in...
if (value_sp && (m_type_mask & value_sp->GetTypeAsMask())) {
if (!can_replace) {
- collection::const_iterator pos = m_values.find(key);
+ auto pos = m_values.find(key);
if (pos != m_values.end())
return false;
}
@@ -322,8 +325,8 @@ bool OptionValueDictionary::SetValueForKey(ConstString key,
return false;
}
-bool OptionValueDictionary::DeleteValueForKey(ConstString key) {
- collection::iterator pos = m_values.find(key);
+bool OptionValueDictionary::DeleteValueForKey(llvm::StringRef key) {
+ auto pos = m_values.find(key);
if (pos != m_values.end()) {
m_values.erase(pos);
return true;
diff --git a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
index 115527a1ec02d..752379308c48a 100644
--- a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -18,7 +18,6 @@
#include "lldb/Interpreter/OptionValueDictionary.h"
#include "lldb/Symbol/UnwindPlan.h"
#include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/Stream.h"
#include "Plugins/Process/Utility/ARMDefines.h"
@@ -14353,9 +14352,9 @@ bool EmulateInstructionARM::TestEmulation(Stream *out_stream, ArchSpec &arch,
return false;
}
- static ConstString opcode_key("opcode");
- static ConstString before_key("before_state");
- static ConstString after_key("after_state");
+ static constexpr llvm::StringLiteral opcode_key("opcode");
+ static constexpr llvm::StringLiteral before_key("before_state");
+ static constexpr llvm::StringLiteral after_key("after_state");
OptionValueSP value_sp = test_data->GetValueForKey(opcode_key);
diff --git a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
index 7b8849ede9c7d..1260b45b9d97b 100644
--- a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
+++ b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
@@ -11,7 +11,6 @@
#include "Plugins/Process/Utility/ARMDefines.h"
#include "lldb/Core/EmulateInstruction.h"
-#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/Status.h"
#include <optional>
diff --git a/lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp b/lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
index ca118375b8abc..e17e4f8ee125b 100644
--- a/lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
+++ b/lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
@@ -264,8 +264,7 @@ bool EmulationStateARM::LoadRegistersStateFromDictionary(
for (int i = 0; i < num; ++i) {
sstr.Clear();
sstr.Printf("%c%d", kind, i);
- OptionValueSP value_sp =
- reg_dict->GetValueForKey(ConstString(sstr.GetString()));
+ OptionValueSP value_sp = reg_dict->GetValueForKey(sstr.GetString());
if (value_sp.get() == nullptr)
return false;
uint64_t reg_value = value_sp->GetUInt64Value();
@@ -277,8 +276,8 @@ bool EmulationStateARM::LoadRegistersStateFromDictionary(
bool EmulationStateARM::LoadStateFromDictionary(
OptionValueDictionary *test_data) {
- static ConstString memory_key("memory");
- static ConstString registers_key("registers");
+ static constexpr llvm::StringLiteral memory_key("memory");
+ static constexpr llvm::StringLiteral registers_key("registers");
if (!test_data)
return false;
@@ -288,8 +287,8 @@ bool EmulationStateARM::LoadStateFromDictionary(
// Load memory, if present.
if (value_sp.get() != nullptr) {
- static ConstString address_key("address");
- static ConstString data_key("data");
+ static constexpr llvm::StringLiteral address_key("address");
+ static constexpr llvm::StringLiteral data_key("data");
uint64_t start_address = 0;
OptionValueDictionary *mem_dict = value_sp->GetAsDictionary();
@@ -327,7 +326,7 @@ bool EmulationStateARM::LoadStateFromDictionary(
if (!LoadRegistersStateFromDictionary(reg_dict, 'r', dwarf_r0, 16))
return false;
- static ConstString cpsr_name("cpsr");
+ static constexpr llvm::StringLiteral cpsr_name("cpsr");
value_sp = reg_dict->GetValueForKey(cpsr_name);
if (value_sp.get() == nullptr)
return false;
diff --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
index c0321b6a2b59f..a0b02d6b848f7 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
+++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
@@ -12,7 +12,6 @@
#include "lldb/Core/PluginManager.h"
#include "lldb/Symbol/UnwindPlan.h"
#include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/RegisterValue.h"
#include "lldb/Utility/Stream.h"
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 515c43ec6a720..5a14234f81af8 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -300,20 +300,18 @@ size_t ObjectFilePECOFF::GetModuleSpecifications(
if (!module_env_option) {
// Step 2: Try with the file name in lowercase.
auto name_lower = name.GetStringRef().lower();
- module_env_option =
- map->GetValueForKey(ConstString(llvm::StringRef(name_lower)));
+ module_env_option = map->GetValueForKey(llvm::StringRef(name_lower));
}
if (!module_env_option) {
// Step 3: Try with the file name with ".debug" suffix stripped.
auto name_stripped = name.GetStringRef();
if (name_stripped.consume_back_insensitive(".debug")) {
- module_env_option = map->GetValueForKey(ConstString(name_stripped));
+ module_env_option = map->GetValueForKey(name_stripped);
if (!module_env_option) {
// Step 4: Try with the file name in lowercase with ".debug" suffix
// stripped.
auto name_lower = name_stripped.lower();
- module_env_option =
- map->GetValueForKey(ConstString(llvm::StringRef(name_lower)));
+ module_env_option = map->GetValueForKey(llvm::StringRef(name_lower));
}
}
}
diff --git a/lldb/test/API/commands/settings/TestSettings.py b/lldb/test/API/commands/settings/TestSettings.py
index e50a4f385344d..82d3d886276ba 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -60,7 +60,7 @@ def cleanup(setting):
self.assertEqual(self.dbg.GetSelectedTarget().GetNumBreakpoints(), 1)
def test_append_target_env_vars(self):
- """Test that 'append target.run-args' works."""
+ """Test that 'append target.env-vars' works."""
# Append the env-vars.
self.runCmd('settings append target.env-vars MY_ENV_VAR=YES')
# And add hooks to restore the settings during tearDown().
@@ -268,7 +268,7 @@ def do_test_run_args_and_env_vars(self, use_launchsimple):
found_env_var = True
break
self.assertTrue(found_env_var,
- "MY_ENV_VAR was not set in LunchInfo object")
+ "MY_ENV_VAR was not set in LaunchInfo object")
self.assertEqual(launch_info.GetNumArguments(), 3)
self.assertEqual(launch_info.GetArgumentAtIndex(0), "A")
diff --git a/lldb/unittests/Interpreter/TestOptionValue.cpp b/lldb/unittests/Interpreter/TestOptionValue.cpp
index 9bdbb22240eae..9e32e270d4cac 100644
--- a/lldb/unittests/Interpreter/TestOptionValue.cpp
+++ b/lldb/unittests/Interpreter/TestOptionValue.cpp
@@ -146,12 +146,12 @@ TEST(TestProperties, DeepCopy) {
ASSERT_TRUE(dict_copy_ptr->OptionWasSet());
ASSERT_EQ(dict_copy_ptr->GetNumValues(), 2U);
- auto value_ptr = dict_copy_ptr->GetValueForKey(ConstString("A"));
+ auto value_ptr = dict_copy_ptr->GetValueForKey("A");
ASSERT_TRUE(value_ptr);
ASSERT_EQ(value_ptr->GetParent().get(), dict_copy_ptr);
ASSERT_EQ(value_ptr->GetUInt64Value(), 1U);
- value_ptr = dict_copy_ptr->GetValueForKey(ConstString("B"));
+ value_ptr = dict_copy_ptr->GetValueForKey("B");
ASSERT_TRUE(value_ptr);
ASSERT_EQ(value_ptr->GetParent().get(), dict_copy_ptr);
ASSERT_EQ(value_ptr->GetUInt64Value(), 2U);
More information about the lldb-commits
mailing list