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