[Lldb-commits] [lldb] fdbe7c7 - [lldb] Refactor OptionValue to return a std::optional (NFC)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Mon May 1 21:08:30 PDT 2023
Author: Jonas Devlieghere
Date: 2023-05-01T21:08:23-07:00
New Revision: fdbe7c7faa547b16bf6da0fedbb7234b6ee3adef
URL: https://github.com/llvm/llvm-project/commit/fdbe7c7faa547b16bf6da0fedbb7234b6ee3adef
DIFF: https://github.com/llvm/llvm-project/commit/fdbe7c7faa547b16bf6da0fedbb7234b6ee3adef.diff
LOG: [lldb] Refactor OptionValue to return a std::optional (NFC)
Refactor OptionValue to return a std::optional instead of taking a fail
value. This allows the caller to handle situations where there's no
value, instead of being unable to distinguish between the absence of a
value and the value happening the match the fail value. When a fail
value is required, std::optional::value_or() provides the same
functionality.
Added:
Modified:
lldb/include/lldb/Interpreter/OptionValue.h
lldb/source/Commands/CommandObjectBreakpoint.cpp
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/CommandObjectRegister.cpp
lldb/source/Core/Disassembler.cpp
lldb/source/Interpreter/OptionValue.cpp
lldb/source/Interpreter/OptionValueArgs.cpp
lldb/source/Interpreter/OptionValueArray.cpp
lldb/source/Interpreter/OptionValueProperties.cpp
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Interpreter/OptionValue.h b/lldb/include/lldb/Interpreter/OptionValue.h
index 3c842d1f5ce7..562eab64b518 100644
--- a/lldb/include/lldb/Interpreter/OptionValue.h
+++ b/lldb/include/lldb/Interpreter/OptionValue.h
@@ -182,10 +182,6 @@ class OptionValue {
CreateValueFromCStringForTypeMask(const char *value_cstr, uint32_t type_mask,
Status &error);
- // Get this value as a uint64_t value if it is encoded as a boolean, uint64_t
- // or int64_t. Other types will cause "fail_value" to be returned
- uint64_t GetUInt64Value(uint64_t fail_value, bool *success_ptr);
-
OptionValueArch *GetAsArch();
const OptionValueArch *GetAsArch() const;
@@ -262,15 +258,15 @@ class OptionValue {
const OptionValueFormatEntity *GetAsFormatEntity() const;
- bool GetBooleanValue(bool fail_value = false) const;
+ std::optional<bool> GetBooleanValue() const;
bool SetBooleanValue(bool new_value);
- char GetCharValue(char fail_value) const;
+ std::optional<char> GetCharValue() const;
char SetCharValue(char new_value);
- int64_t GetEnumerationValue(int64_t fail_value = -1) const;
+ std::optional<int64_t> GetEnumerationValue() const;
bool SetEnumerationValue(int64_t value);
@@ -280,13 +276,11 @@ class OptionValue {
FileSpecList GetFileSpecListValue() const;
- lldb::Format
- GetFormatValue(lldb::Format fail_value = lldb::eFormatDefault) const;
+ std::optional<lldb::Format> GetFormatValue() const;
bool SetFormatValue(lldb::Format new_value);
- lldb::LanguageType GetLanguageValue(
- lldb::LanguageType fail_value = lldb::eLanguageTypeUnknown) const;
+ std::optional<lldb::LanguageType> GetLanguageValue() const;
bool SetLanguageValue(lldb::LanguageType new_language);
@@ -294,16 +288,15 @@ class OptionValue {
const RegularExpression *GetRegexValue() const;
- int64_t GetSInt64Value(int64_t fail_value = 0) const;
+ std::optional<int64_t> GetSInt64Value() const;
bool SetSInt64Value(int64_t new_value);
- llvm::StringRef GetStringValue(llvm::StringRef fail_value) const;
- llvm::StringRef GetStringValue() const { return GetStringValue(llvm::StringRef()); }
+ std::optional<llvm::StringRef> GetStringValue() const;
bool SetStringValue(llvm::StringRef new_value);
- uint64_t GetUInt64Value(uint64_t fail_value = 0) const;
+ std::optional<uint64_t> GetUInt64Value() const;
bool SetUInt64Value(uint64_t new_value);
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index 8c31630231b5..3debbb3b576e 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -1739,7 +1739,8 @@ class CommandObjectBreakpointNameConfigure : public CommandObjectParsed {
// check the error:
BreakpointSP bp_sp;
if (m_bp_id.m_breakpoint.OptionWasSet()) {
- lldb::break_id_t bp_id = m_bp_id.m_breakpoint.GetUInt64Value();
+ lldb::break_id_t bp_id =
+ m_bp_id.m_breakpoint.GetUInt64Value().value_or(0);
bp_sp = target.GetBreakpointByID(bp_id);
if (!bp_sp) {
result.AppendErrorWithFormatv("Could not find specified breakpoint {0}",
@@ -1755,7 +1756,8 @@ class CommandObjectBreakpointNameConfigure : public CommandObjectParsed {
if (!bp_name)
continue;
if (m_bp_id.m_help_string.OptionWasSet())
- bp_name->SetHelp(m_bp_id.m_help_string.GetStringValue().str().c_str());
+ bp_name->SetHelp(
+ m_bp_id.m_help_string.GetStringValue().value_or("").str().c_str());
if (bp_sp)
target.ConfigureBreakpointName(*bp_name, bp_sp->GetOptions(),
diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index 70dd6ea947be..903424336cd5 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -1047,18 +1047,20 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
DataBufferHeap buffer;
if (m_memory_options.m_string.OptionWasSet()) {
- llvm::StringRef str = m_memory_options.m_string.GetStringValue();
- if (str.empty()) {
+ std::optional<llvm::StringRef> str =
+ m_memory_options.m_string.GetStringValue();
+ if (!str) {
result.AppendError("search string must have non-zero length.");
return false;
}
- buffer.CopyData(str);
+ buffer.CopyData(*str);
} else if (m_memory_options.m_expr.OptionWasSet()) {
StackFrame *frame = m_exe_ctx.GetFramePtr();
ValueObjectSP result_sp;
if ((eExpressionCompleted ==
process->GetTarget().EvaluateExpression(
- m_memory_options.m_expr.GetStringValue(), frame, result_sp)) &&
+ m_memory_options.m_expr.GetStringValue().value_or(""), frame,
+ result_sp)) &&
result_sp) {
uint64_t value = result_sp->GetValueAsUnsigned(0);
std::optional<uint64_t> size =
diff --git a/lldb/source/Commands/CommandObjectRegister.cpp b/lldb/source/Commands/CommandObjectRegister.cpp
index a6ea64229ecc..80813cda04b8 100644
--- a/lldb/source/Commands/CommandObjectRegister.cpp
+++ b/lldb/source/Commands/CommandObjectRegister.cpp
@@ -171,8 +171,8 @@ class CommandObjectRegisterRead : public CommandObjectParsed {
const size_t set_array_size = m_command_options.set_indexes.GetSize();
if (set_array_size > 0) {
for (size_t i = 0; i < set_array_size; ++i) {
- set_idx = m_command_options.set_indexes[i]->GetUInt64Value(UINT32_MAX,
- nullptr);
+ set_idx = m_command_options.set_indexes[i]->GetUInt64Value().value_or(
+ UINT32_MAX);
if (set_idx < reg_ctx->GetRegisterSetCount()) {
if (!DumpRegisterSet(m_exe_ctx, strm, reg_ctx, set_idx)) {
if (errno)
diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp
index 0d9a83993e53..55ae412d3d43 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -908,7 +908,7 @@ bool Instruction::TestEmulation(Stream *out_stream, const char *file_name) {
return false;
}
- SetDescription(value_sp->GetStringValue());
+ SetDescription(value_sp->GetStringValue().value_or(""));
value_sp = data_dictionary->GetValueForKey(triple_key);
if (!value_sp) {
@@ -918,7 +918,7 @@ bool Instruction::TestEmulation(Stream *out_stream, const char *file_name) {
}
ArchSpec arch;
- arch.SetTriple(llvm::Triple(value_sp->GetStringValue()));
+ arch.SetTriple(llvm::Triple(value_sp->GetStringValue().value_or("")));
bool success = false;
std::unique_ptr<EmulateInstruction> insn_emulator_up(
diff --git a/lldb/source/Interpreter/OptionValue.cpp b/lldb/source/Interpreter/OptionValue.cpp
index 79bdf74b6fd9..218a473db5e6 100644
--- a/lldb/source/Interpreter/OptionValue.cpp
+++ b/lldb/source/Interpreter/OptionValue.cpp
@@ -15,26 +15,6 @@
using namespace lldb;
using namespace lldb_private;
-// Get this value as a uint64_t value if it is encoded as a boolean, uint64_t
-// or int64_t. Other types will cause "fail_value" to be returned
-uint64_t OptionValue::GetUInt64Value(uint64_t fail_value, bool *success_ptr) {
- if (success_ptr)
- *success_ptr = true;
- switch (GetType()) {
- case OptionValue::eTypeBoolean:
- return static_cast<OptionValueBoolean *>(this)->GetCurrentValue();
- case OptionValue::eTypeSInt64:
- return static_cast<OptionValueSInt64 *>(this)->GetCurrentValue();
- case OptionValue::eTypeUInt64:
- return static_cast<OptionValueUInt64 *>(this)->GetCurrentValue();
- default:
- break;
- }
- if (success_ptr)
- *success_ptr = false;
- return fail_value;
-}
-
Status OptionValue::SetSubValue(const ExecutionContext *exe_ctx,
VarSetOperationType op, llvm::StringRef name,
llvm::StringRef value) {
@@ -271,11 +251,10 @@ const OptionValueUUID *OptionValue::GetAsUUID() const {
return nullptr;
}
-bool OptionValue::GetBooleanValue(bool fail_value) const {
- const OptionValueBoolean *option_value = GetAsBoolean();
- if (option_value)
+std::optional<bool> OptionValue::GetBooleanValue() const {
+ if (const OptionValueBoolean *option_value = GetAsBoolean())
return option_value->GetCurrentValue();
- return fail_value;
+ return {};
}
bool OptionValue::SetBooleanValue(bool new_value) {
@@ -287,11 +266,10 @@ bool OptionValue::SetBooleanValue(bool new_value) {
return false;
}
-char OptionValue::GetCharValue(char fail_value) const {
- const OptionValueChar *option_value = GetAsChar();
- if (option_value)
+std::optional<char> OptionValue::GetCharValue() const {
+ if (const OptionValueChar *option_value = GetAsChar())
return option_value->GetCurrentValue();
- return fail_value;
+ return {};
}
char OptionValue::SetCharValue(char new_value) {
@@ -303,11 +281,10 @@ char OptionValue::SetCharValue(char new_value) {
return false;
}
-int64_t OptionValue::GetEnumerationValue(int64_t fail_value) const {
- const OptionValueEnumeration *option_value = GetAsEnumeration();
- if (option_value)
+std::optional<int64_t> OptionValue::GetEnumerationValue() const {
+ if (const OptionValueEnumeration *option_value = GetAsEnumeration())
return option_value->GetCurrentValue();
- return fail_value;
+ return {};
}
bool OptionValue::SetEnumerationValue(int64_t value) {
@@ -342,11 +319,10 @@ FileSpecList OptionValue::GetFileSpecListValue() const {
return FileSpecList();
}
-lldb::Format OptionValue::GetFormatValue(lldb::Format fail_value) const {
- const OptionValueFormat *option_value = GetAsFormat();
- if (option_value)
+std::optional<lldb::Format> OptionValue::GetFormatValue() const {
+ if (const OptionValueFormat *option_value = GetAsFormat())
return option_value->GetCurrentValue();
- return fail_value;
+ return {};
}
bool OptionValue::SetFormatValue(lldb::Format new_value) {
@@ -358,12 +334,10 @@ bool OptionValue::SetFormatValue(lldb::Format new_value) {
return false;
}
-lldb::LanguageType
-OptionValue::GetLanguageValue(lldb::LanguageType fail_value) const {
- const OptionValueLanguage *option_value = GetAsLanguage();
- if (option_value)
+std::optional<lldb::LanguageType> OptionValue::GetLanguageValue() const {
+ if (const OptionValueLanguage *option_value = GetAsLanguage())
return option_value->GetCurrentValue();
- return fail_value;
+ return {};
}
bool OptionValue::SetLanguageValue(lldb::LanguageType new_language) {
@@ -389,11 +363,10 @@ const RegularExpression *OptionValue::GetRegexValue() const {
return nullptr;
}
-int64_t OptionValue::GetSInt64Value(int64_t fail_value) const {
- const OptionValueSInt64 *option_value = GetAsSInt64();
- if (option_value)
+std::optional<int64_t> OptionValue::GetSInt64Value() const {
+ if (const OptionValueSInt64 *option_value = GetAsSInt64())
return option_value->GetCurrentValue();
- return fail_value;
+ return {};
}
bool OptionValue::SetSInt64Value(int64_t new_value) {
@@ -405,11 +378,10 @@ bool OptionValue::SetSInt64Value(int64_t new_value) {
return false;
}
-llvm::StringRef OptionValue::GetStringValue(llvm::StringRef fail_value) const {
- const OptionValueString *option_value = GetAsString();
- if (option_value)
+std::optional<llvm::StringRef> OptionValue::GetStringValue() const {
+ if (const OptionValueString *option_value = GetAsString())
return option_value->GetCurrentValueAsRef();
- return fail_value;
+ return {};
}
bool OptionValue::SetStringValue(llvm::StringRef new_value) {
@@ -421,11 +393,10 @@ bool OptionValue::SetStringValue(llvm::StringRef new_value) {
return false;
}
-uint64_t OptionValue::GetUInt64Value(uint64_t fail_value) const {
- const OptionValueUInt64 *option_value = GetAsUInt64();
- if (option_value)
+std::optional<uint64_t> OptionValue::GetUInt64Value() const {
+ if (const OptionValueUInt64 *option_value = GetAsUInt64())
return option_value->GetCurrentValue();
- return fail_value;
+ return {};
}
bool OptionValue::SetUInt64Value(uint64_t new_value) {
diff --git a/lldb/source/Interpreter/OptionValueArgs.cpp b/lldb/source/Interpreter/OptionValueArgs.cpp
index 0e20154907b0..e9f73e631dfd 100644
--- a/lldb/source/Interpreter/OptionValueArgs.cpp
+++ b/lldb/source/Interpreter/OptionValueArgs.cpp
@@ -15,10 +15,7 @@ using namespace lldb_private;
size_t OptionValueArgs::GetArgs(Args &args) const {
args.Clear();
- for (const auto &value : m_values) {
- llvm::StringRef string_value = value->GetStringValue();
- args.AppendArgument(string_value);
- }
-
+ for (const auto &value : m_values)
+ args.AppendArgument(value->GetStringValue().value_or(""));
return args.GetArgumentCount();
}
diff --git a/lldb/source/Interpreter/OptionValueArray.cpp b/lldb/source/Interpreter/OptionValueArray.cpp
index 40357d5f4b06..afd1f90aafe8 100644
--- a/lldb/source/Interpreter/OptionValueArray.cpp
+++ b/lldb/source/Interpreter/OptionValueArray.cpp
@@ -155,9 +155,9 @@ size_t OptionValueArray::GetArgs(Args &args) const {
args.Clear();
const uint32_t size = m_values.size();
for (uint32_t i = 0; i < size; ++i) {
- llvm::StringRef string_value = m_values[i]->GetStringValue();
- if (!string_value.empty())
- args.AppendArgument(string_value);
+ std::optional<llvm::StringRef> string_value = m_values[i]->GetStringValue();
+ if (string_value)
+ args.AppendArgument(*string_value);
}
return args.GetArgumentCount();
diff --git a/lldb/source/Interpreter/OptionValueProperties.cpp b/lldb/source/Interpreter/OptionValueProperties.cpp
index 46e677348352..4cd5e5984763 100644
--- a/lldb/source/Interpreter/OptionValueProperties.cpp
+++ b/lldb/source/Interpreter/OptionValueProperties.cpp
@@ -297,7 +297,7 @@ bool OptionValueProperties::GetPropertyAtIndexAsBoolean(
if (property) {
OptionValue *value = property->GetValue().get();
if (value)
- return value->GetBooleanValue(fail_value);
+ return value->GetBooleanValue().value_or(fail_value);
}
return fail_value;
}
@@ -330,7 +330,7 @@ int64_t OptionValueProperties::GetPropertyAtIndexAsEnumeration(
if (property) {
OptionValue *value = property->GetValue().get();
if (value)
- return value->GetEnumerationValue(fail_value);
+ return value->GetEnumerationValue().value_or(fail_value);
}
return fail_value;
}
@@ -433,7 +433,7 @@ int64_t OptionValueProperties::GetPropertyAtIndexAsSInt64(
if (property) {
OptionValue *value = property->GetValue().get();
if (value)
- return value->GetSInt64Value(fail_value);
+ return value->GetSInt64Value().value_or(fail_value);
}
return fail_value;
}
@@ -456,7 +456,7 @@ llvm::StringRef OptionValueProperties::GetPropertyAtIndexAsString(
if (property) {
OptionValue *value = property->GetValue().get();
if (value)
- return value->GetStringValue(fail_value);
+ return value->GetStringValue().value_or(fail_value);
}
return fail_value;
}
@@ -486,7 +486,7 @@ uint64_t OptionValueProperties::GetPropertyAtIndexAsUInt64(
if (property) {
OptionValue *value = property->GetValue().get();
if (value)
- return value->GetUInt64Value(fail_value);
+ return value->GetUInt64Value().value_or(fail_value);
}
return fail_value;
}
diff --git a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
index 752379308c48..a8cc86c3715a 100644
--- a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -14364,7 +14364,7 @@ bool EmulateInstructionARM::TestEmulation(Stream *out_stream, ArchSpec &arch,
out_stream->Printf("TestEmulation: Error reading opcode from test file.\n");
return false;
}
- test_opcode = value_sp->GetUInt64Value();
+ test_opcode = value_sp->GetUInt64Value().value_or(0);
if (arch.GetTriple().getArch() == llvm::Triple::thumb ||
arch.IsAlwaysThumbInstructions()) {
diff --git a/lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp b/lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
index e17e4f8ee125..fd7875599d88 100644
--- a/lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
+++ b/lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
@@ -267,7 +267,7 @@ bool EmulationStateARM::LoadRegistersStateFromDictionary(
OptionValueSP value_sp = reg_dict->GetValueForKey(sstr.GetString());
if (value_sp.get() == nullptr)
return false;
- uint64_t reg_value = value_sp->GetUInt64Value();
+ uint64_t reg_value = value_sp->GetUInt64Value().value_or(0);
StorePseudoRegisterValue(first_reg + i, reg_value);
}
@@ -296,7 +296,7 @@ bool EmulationStateARM::LoadStateFromDictionary(
if (value_sp.get() == nullptr)
return false;
else
- start_address = value_sp->GetUInt64Value();
+ start_address = value_sp->GetUInt64Value().value_or(0);
value_sp = mem_dict->GetValueForKey(data_key);
OptionValueArray *mem_array = value_sp->GetAsArray();
@@ -310,7 +310,7 @@ bool EmulationStateARM::LoadStateFromDictionary(
value_sp = mem_array->GetValueAtIndex(i);
if (value_sp.get() == nullptr)
return false;
- uint64_t value = value_sp->GetUInt64Value();
+ uint64_t value = value_sp->GetUInt64Value().value_or(0);
StoreToPseudoAddress(address, value);
address = address + 4;
}
@@ -330,7 +330,7 @@ bool EmulationStateARM::LoadStateFromDictionary(
value_sp = reg_dict->GetValueForKey(cpsr_name);
if (value_sp.get() == nullptr)
return false;
- StorePseudoRegisterValue(dwarf_cpsr, value_sp->GetUInt64Value());
+ StorePseudoRegisterValue(dwarf_cpsr, value_sp->GetUInt64Value().value_or(0));
// Load s/d Registers
// To prevent you giving both types in a state and overwriting
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 5a14234f81af..bfea299646e7 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -319,7 +319,8 @@ size_t ObjectFilePECOFF::GetModuleSpecifications(
llvm::Triple::EnvironmentType env;
if (module_env_option)
env =
- (llvm::Triple::EnvironmentType)module_env_option->GetEnumerationValue();
+ (llvm::Triple::EnvironmentType)module_env_option->GetEnumerationValue()
+ .value_or(0);
else
env = GetGlobalPluginProperties().ABI();
diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
index 9b29d1b8f158..faebe5dd34f8 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -887,7 +887,7 @@ class CommandObjectProcessKDPPacketSend : public CommandObjectParsed {
"the --command option must be set to a valid command byte");
} else {
const uint64_t command_byte =
- m_command_byte.GetOptionValue().GetUInt64Value(0);
+ m_command_byte.GetOptionValue().GetUInt64Value().value_or(0);
if (command_byte > 0 && command_byte <= UINT8_MAX) {
ProcessKDP *process =
(ProcessKDP *)m_interpreter.GetExecutionContext().GetProcessPtr();
More information about the lldb-commits
mailing list