bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D149482
Files:
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
Index: lldb/unittests/Interpreter/TestOptionValue.cpp
===================================================================
--- lldb/unittests/Interpreter/TestOptionValue.cpp
+++ lldb/unittests/Interpreter/TestOptionValue.cpp
@@ -146,12 +146,12 @@
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);
Index: lldb/test/API/commands/settings/TestSettings.py
===================================================================
--- lldb/test/API/commands/settings/TestSettings.py
+++ lldb/test/API/commands/settings/TestSettings.py
@@ -60,7 +60,7 @@
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 @@
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")
@@ -604,6 +604,7 @@
self.expect(
"settings show target.env-vars",
SETTING_MSG("target.env-vars"),
+ ordered=False,
substrs=[
'target.env-vars (dictionary of strings) =',
'A=B',
@@ -640,7 +641,7 @@
def test_settings_remove_single(self):
# Set some environment variables and use 'remove' to delete them.
self.runCmd("settings set target.env-vars a=b c=d")
- self.expect("settings show target.env-vars", substrs=["a=b", "c=d"])
+ self.expect("settings show target.env-vars", ordered=False, substrs=["a=b", "c=d"])
self.runCmd("settings remove target.env-vars a")
self.expect("settings show target.env-vars", matching=False, substrs=["a=b"])
self.expect("settings show target.env-vars", substrs=["c=d"])
@@ -649,7 +650,7 @@
def test_settings_remove_multiple(self):
self.runCmd("settings set target.env-vars a=b c=d e=f")
- self.expect("settings show target.env-vars", substrs=["a=b", "c=d", "e=f"])
+ self.expect("settings show target.env-vars", ordered=False, substrs=["a=b", "c=d", "e=f"])
self.runCmd("settings remove target.env-vars a e")
self.expect("settings show target.env-vars", matching=False, substrs=["a=b", "e=f"])
self.expect("settings show target.env-vars", substrs=["c=d"])
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -300,20 +300,18 @@
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));
}
}
}
Index: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
===================================================================
--- lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
+++ 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"
Index: lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
===================================================================
--- lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
@@ -264,8 +264,7 @@
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::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 @@
// 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 @@
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;
Index: lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
===================================================================
--- lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
+++ 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>
Index: lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
===================================================================
--- lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ 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 @@
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);
Index: lldb/source/Interpreter/OptionValueDictionary.cpp
===================================================================
--- lldb/source/Interpreter/OptionValueDictionary.cpp
+++ lldb/source/Interpreter/OptionValueDictionary.cpp
@@ -33,12 +33,10 @@
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) {
+ for (auto pos = m_values.begin(), end = m_values.end(); pos != end; ++pos) {
OptionValue *option_value = pos->second.get();
if (one_line)
@@ -46,7 +44,7 @@
else
strm.EOL();
- strm.Indent(pos->first.GetStringRef());
+ strm.Indent(pos->first());
const uint32_t extra_dump_options = m_raw_value_dump ? eDumpOptionRaw : 0;
switch (dict_type) {
@@ -87,17 +85,16 @@
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 (auto pos = m_values.begin(), end = m_values.end(); pos != end; ++pos) {
StreamString strm;
- strm.Printf("%s=", pos->first.GetCString());
+ strm.Printf("%s=", pos->first().data());
pos->second->DumpValue(nullptr, strm, eDumpOptionValue | eDumpOptionRaw);
args.AppendArgument(strm.GetString());
}
@@ -178,7 +175,7 @@
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 +183,7 @@
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 +195,11 @@
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 +264,7 @@
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 +294,22 @@
}
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 +319,8 @@
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;
Index: lldb/source/Core/Disassembler.cpp
===================================================================
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -755,7 +755,7 @@
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,7 @@
return option_value_sp;
}
- ConstString const_key(key.c_str());
+ llvm::StringRef key_ref(key);
// Check value to see if it's the start of an array or dictionary.
lldb::OptionValueSP value_sp;
@@ -838,15 +838,16 @@
value_sp = std::make_shared<OptionValueString>(value.c_str(), "");
}
- if (const_key == encoding_key) {
+ if (key_ref == 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).
+ // 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")
data_type = OptionValue::eTypeUInt64;
} else
- option_value_sp->GetAsDictionary()->SetValueForKey(const_key, value_sp,
+ option_value_sp->GetAsDictionary()->SetValueForKey(key_ref, value_sp,
false);
}
}
Index: lldb/include/lldb/Interpreter/OptionValueDictionary.h
===================================================================
--- lldb/include/lldb/Interpreter/OptionValueDictionary.h
+++ 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 @@
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 @@
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;
};
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
