Author: Alex Langford Date: 2023-11-09T13:35:35-08:00 New Revision: 1486264d5fad0ef7d76a44f6358390cf41218c7e
URL: https://github.com/llvm/llvm-project/commit/1486264d5fad0ef7d76a44f6358390cf41218c7e DIFF: https://github.com/llvm/llvm-project/commit/1486264d5fad0ef7d76a44f6358390cf41218c7e.diff LOG: [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsString (#71613) This patch changes the interface of StructuredData::Array::GetItemAtIndexAsString to return a `std::optional<llvm::StringRef>` instead of taking an out parameter. More generally, this commit serves as proposal that we change all of the sibling APIs (`GetItemAtIndexAs`) to do the same thing. The reason this isn't one giant patch is because it is rather unwieldy changing just one of these, so if this is approved, I will do all of the other ones as individual follow-ups. Added: Modified: lldb/include/lldb/Utility/StructuredData.h lldb/source/Breakpoint/Breakpoint.cpp lldb/source/Breakpoint/BreakpointOptions.cpp lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp lldb/source/Breakpoint/BreakpointResolverName.cpp lldb/source/Commands/CommandObjectBreakpoint.cpp lldb/source/Core/SearchFilter.cpp lldb/source/Target/DynamicRegisterInfo.cpp lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Utility/StructuredData.h b/lldb/include/lldb/Utility/StructuredData.h index 279238f9af76e01..6e39bcff2c0af0b 100644 --- a/lldb/include/lldb/Utility/StructuredData.h +++ b/lldb/include/lldb/Utility/StructuredData.h @@ -23,6 +23,7 @@ #include <functional> #include <map> #include <memory> +#include <optional> #include <string> #include <type_traits> #include <utility> @@ -247,23 +248,12 @@ class StructuredData { return success; } - bool GetItemAtIndexAsString(size_t idx, llvm::StringRef &result) const { - ObjectSP value_sp = GetItemAtIndex(idx); - if (value_sp.get()) { - if (auto string_value = value_sp->GetAsString()) { - result = string_value->GetValue(); - return true; - } + std::optional<llvm::StringRef> GetItemAtIndexAsString(size_t idx) const { + if (auto item_sp = GetItemAtIndex(idx)) { + if (auto *string_value = item_sp->GetAsString()) + return string_value->GetValue(); } - return false; - } - - bool GetItemAtIndexAsString(size_t idx, llvm::StringRef &result, - llvm::StringRef default_val) const { - bool success = GetItemAtIndexAsString(idx, result); - if (!success) - result = default_val; - return success; + return {}; } bool GetItemAtIndexAsDictionary(size_t idx, Dictionary *&result) const { diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp index a94498a23a0fb27..6e6b51b562496c6 100644 --- a/lldb/source/Breakpoint/Breakpoint.cpp +++ b/lldb/source/Breakpoint/Breakpoint.cpp @@ -205,10 +205,9 @@ lldb::BreakpointSP Breakpoint::CreateFromStructuredData( if (success && names_array) { size_t num_names = names_array->GetSize(); for (size_t i = 0; i < num_names; i++) { - llvm::StringRef name; - Status error; - success = names_array->GetItemAtIndexAsString(i, name); - target.AddNameToBreakpoint(result_sp, name.str().c_str(), error); + if (std::optional<llvm::StringRef> maybe_name = + names_array->GetItemAtIndexAsString(i)) + target.AddNameToBreakpoint(result_sp, *maybe_name, error); } } @@ -238,11 +237,10 @@ bool Breakpoint::SerializedBreakpointMatchesNames( size_t num_names = names_array->GetSize(); for (size_t i = 0; i < num_names; i++) { - llvm::StringRef name; - if (names_array->GetItemAtIndexAsString(i, name)) { - if (llvm::is_contained(names, name)) - return true; - } + std::optional<llvm::StringRef> maybe_name = + names_array->GetItemAtIndexAsString(i); + if (maybe_name && llvm::is_contained(names, *maybe_name)) + return true; } return false; } diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp b/lldb/source/Breakpoint/BreakpointOptions.cpp index 268c52341dfed6e..6c6037dd9edd3a4 100644 --- a/lldb/source/Breakpoint/BreakpointOptions.cpp +++ b/lldb/source/Breakpoint/BreakpointOptions.cpp @@ -88,10 +88,9 @@ BreakpointOptions::CommandData::CreateFromStructuredData( if (success) { size_t num_elems = user_source->GetSize(); for (size_t i = 0; i < num_elems; i++) { - llvm::StringRef elem_string; - success = user_source->GetItemAtIndexAsString(i, elem_string); - if (success) - data_up->user_source.AppendString(elem_string); + if (std::optional<llvm::StringRef> maybe_elem_string = + user_source->GetItemAtIndexAsString(i)) + data_up->user_source.AppendString(*maybe_elem_string); } } diff --git a/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp b/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp index 38de16a56155e96..13c7f17fd807ee5 100644 --- a/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp +++ b/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp @@ -56,14 +56,14 @@ BreakpointResolverSP BreakpointResolverFileRegex::CreateFromStructuredData( if (success && names_array) { size_t num_names = names_array->GetSize(); for (size_t i = 0; i < num_names; i++) { - llvm::StringRef name; - success = names_array->GetItemAtIndexAsString(i, name); - if (!success) { + std::optional<llvm::StringRef> maybe_name = + names_array->GetItemAtIndexAsString(i); + if (!maybe_name) { error.SetErrorStringWithFormat( "BRFR::CFSD: Malformed element %zu in the names array.", i); return nullptr; } - names_set.insert(std::string(name)); + names_set.insert(std::string(*maybe_name)); } } diff --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp b/lldb/source/Breakpoint/BreakpointResolverName.cpp index ef61df51ba16600..0097046cf511b5d 100644 --- a/lldb/source/Breakpoint/BreakpointResolverName.cpp +++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp @@ -155,10 +155,9 @@ BreakpointResolverSP BreakpointResolverName::CreateFromStructuredData( std::vector<std::string> names; std::vector<FunctionNameType> name_masks; for (size_t i = 0; i < num_elem; i++) { - llvm::StringRef name; - - success = names_array->GetItemAtIndexAsString(i, name); - if (!success) { + std::optional<llvm::StringRef> maybe_name = + names_array->GetItemAtIndexAsString(i); + if (!maybe_name) { error.SetErrorString("BRN::CFSD: name entry is not a string."); return nullptr; } @@ -168,7 +167,7 @@ BreakpointResolverSP BreakpointResolverName::CreateFromStructuredData( error.SetErrorString("BRN::CFSD: name mask entry is not an integer."); return nullptr; } - names.push_back(std::string(name)); + names.push_back(std::string(*maybe_name)); name_masks.push_back(static_cast<FunctionNameType>(fnt)); } diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp index e1d1c5e42c32a03..63492590d32d665 100644 --- a/lldb/source/Commands/CommandObjectBreakpoint.cpp +++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp @@ -2235,9 +2235,9 @@ class CommandObjectBreakpointRead : public CommandObjectParsed { size_t num_names = names_array->GetSize(); for (size_t i = 0; i < num_names; i++) { - llvm::StringRef name; - if (names_array->GetItemAtIndexAsString(i, name)) - request.TryCompleteCurrentArg(name); + if (std::optional<llvm::StringRef> maybe_name = + names_array->GetItemAtIndexAsString(i)) + request.TryCompleteCurrentArg(*maybe_name); } } } diff --git a/lldb/source/Core/SearchFilter.cpp b/lldb/source/Core/SearchFilter.cpp index 4f9519b5cc9a784..b3ff73bbf58f04a 100644 --- a/lldb/source/Core/SearchFilter.cpp +++ b/lldb/source/Core/SearchFilter.cpp @@ -471,13 +471,13 @@ SearchFilterSP SearchFilterByModule::CreateFromStructuredData( return nullptr; } - llvm::StringRef module; - success = modules_array->GetItemAtIndexAsString(0, module); - if (!success) { + std::optional<llvm::StringRef> maybe_module = + modules_array->GetItemAtIndexAsString(0); + if (!maybe_module) { error.SetErrorString("SFBM::CFSD: filter module item not a string."); return nullptr; } - FileSpec module_spec(module); + FileSpec module_spec(*maybe_module); return std::make_shared<SearchFilterByModule>(target_sp, module_spec); } @@ -596,14 +596,14 @@ SearchFilterSP SearchFilterByModuleList::CreateFromStructuredData( FileSpecList modules; size_t num_modules = modules_array->GetSize(); for (size_t i = 0; i < num_modules; i++) { - llvm::StringRef module; - success = modules_array->GetItemAtIndexAsString(i, module); - if (!success) { + std::optional<llvm::StringRef> maybe_module = + modules_array->GetItemAtIndexAsString(i); + if (!maybe_module) { error.SetErrorStringWithFormat( "SFBM::CFSD: filter module item %zu not a string.", i); return nullptr; } - modules.EmplaceBack(module); + modules.EmplaceBack(*maybe_module); } return std::make_shared<SearchFilterByModuleList>(target_sp, modules); } @@ -644,14 +644,14 @@ lldb::SearchFilterSP SearchFilterByModuleListAndCU::CreateFromStructuredData( if (success) { size_t num_modules = modules_array->GetSize(); for (size_t i = 0; i < num_modules; i++) { - llvm::StringRef module; - success = modules_array->GetItemAtIndexAsString(i, module); - if (!success) { + std::optional<llvm::StringRef> maybe_module = + modules_array->GetItemAtIndexAsString(i); + if (!maybe_module) { error.SetErrorStringWithFormat( "SFBM::CFSD: filter module item %zu not a string.", i); return result_sp; } - modules.EmplaceBack(module); + modules.EmplaceBack(*maybe_module); } } @@ -666,14 +666,14 @@ lldb::SearchFilterSP SearchFilterByModuleListAndCU::CreateFromStructuredData( size_t num_cus = cus_array->GetSize(); FileSpecList cus; for (size_t i = 0; i < num_cus; i++) { - llvm::StringRef cu; - success = cus_array->GetItemAtIndexAsString(i, cu); - if (!success) { + std::optional<llvm::StringRef> maybe_cu = + cus_array->GetItemAtIndexAsString(i); + if (!maybe_cu) { error.SetErrorStringWithFormat( "SFBM::CFSD: filter CU item %zu not a string.", i); return nullptr; } - cus.EmplaceBack(cu); + cus.EmplaceBack(*maybe_cu); } return std::make_shared<SearchFilterByModuleListAndCU>( diff --git a/lldb/source/Target/DynamicRegisterInfo.cpp b/lldb/source/Target/DynamicRegisterInfo.cpp index 0372fd48feae687..cc2df5b97940fed 100644 --- a/lldb/source/Target/DynamicRegisterInfo.cpp +++ b/lldb/source/Target/DynamicRegisterInfo.cpp @@ -144,20 +144,21 @@ llvm::Expected<uint32_t> DynamicRegisterInfo::ByteOffsetFromComposite( uint32_t composite_offset = UINT32_MAX; for (uint32_t composite_idx = 0; composite_idx < num_composite_regs; ++composite_idx) { - llvm::StringRef composite_reg_name; - if (!composite_reg_list.GetItemAtIndexAsString(composite_idx, composite_reg_name)) + std::optional<llvm::StringRef> maybe_composite_reg_name = + composite_reg_list.GetItemAtIndexAsString(composite_idx); + if (!maybe_composite_reg_name) return llvm::createStringError( llvm::inconvertibleErrorCode(), "\"composite\" list value is not a Python string at index %d", composite_idx); const RegisterInfo *composite_reg_info = - GetRegisterInfo(composite_reg_name); + GetRegisterInfo(*maybe_composite_reg_name); if (!composite_reg_info) return llvm::createStringError( llvm::inconvertibleErrorCode(), "failed to find composite register by name: \"%s\"", - composite_reg_name.str().c_str()); + maybe_composite_reg_name->str().c_str()); composite_offset = std::min(composite_offset, composite_reg_info->byte_offset); @@ -205,10 +206,11 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict, if (dict.GetValueForKeyAsArray("sets", sets)) { const uint32_t num_sets = sets->GetSize(); for (uint32_t i = 0; i < num_sets; ++i) { - llvm::StringRef set_name; - if (sets->GetItemAtIndexAsString(i, set_name) && !set_name.empty()) { + std::optional<llvm::StringRef> maybe_set_name = + sets->GetItemAtIndexAsString(i); + if (maybe_set_name && !maybe_set_name->empty()) { m_sets.push_back( - {ConstString(set_name).AsCString(), nullptr, 0, nullptr}); + {ConstString(*maybe_set_name).AsCString(), nullptr, 0, nullptr}); } else { Clear(); printf("error: register sets must have valid names\n"); @@ -345,12 +347,12 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict, const size_t num_regs = invalidate_reg_list->GetSize(); if (num_regs > 0) { for (uint32_t idx = 0; idx < num_regs; ++idx) { - llvm::StringRef invalidate_reg_name; uint64_t invalidate_reg_num; - if (invalidate_reg_list->GetItemAtIndexAsString( - idx, invalidate_reg_name)) { + std::optional<llvm::StringRef> maybe_invalidate_reg_name = + invalidate_reg_list->GetItemAtIndexAsString(idx); + if (maybe_invalidate_reg_name) { const RegisterInfo *invalidate_reg_info = - GetRegisterInfo(invalidate_reg_name); + GetRegisterInfo(*maybe_invalidate_reg_name); if (invalidate_reg_info) { m_invalidate_regs_map[i].push_back( invalidate_reg_info->kinds[eRegisterKindLLDB]); @@ -359,7 +361,7 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict, // format printf("error: failed to find a 'invalidate-regs' register for " "\"%s\" while parsing register \"%s\"\n", - invalidate_reg_name.str().c_str(), reg_info.name); + maybe_invalidate_reg_name->str().c_str(), reg_info.name); } } else if (invalidate_reg_list->GetItemAtIndexAsInteger( idx, invalidate_reg_num)) { diff --git a/lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp b/lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp index da4dc10d4b87c2a..ed1c8b92b3db74d 100644 --- a/lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp +++ b/lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp @@ -76,8 +76,12 @@ JThreadsInfo::parseRegisters(const StructuredData::Dictionary &Dict, auto KeysObj = Dict.GetKeys(); auto Keys = KeysObj->GetAsArray(); for (size_t i = 0; i < Keys->GetSize(); i++) { - StringRef KeyStr, ValueStr; - Keys->GetItemAtIndexAsString(i, KeyStr); + std::optional<StringRef> MaybeKeyStr = Keys->GetItemAtIndexAsString(i); + if (!MaybeKeyStr) + return make_parsing_error("JThreadsInfo: Invalid Key at index {0}", i); + + StringRef KeyStr = *MaybeKeyStr; + StringRef ValueStr; Dict.GetValueForKeyAsString(KeyStr, ValueStr); unsigned int Register; if (!llvm::to_integer(KeyStr, Register, 10)) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits