https://github.com/bulbazord created https://github.com/llvm/llvm-project/pull/71961
Similar to my previous patch (#71613) where I changed `GetItemAtIndexAsString`, this patch makes the same change to `GetItemAtIndexAsDictionary`. `GetItemAtIndexAsDictionary` now returns a std::optional that is either `std::nullopt` or is a valid pointer. Therefore, if the optional is populated, we consider the pointer to always be valid (i.e. no need to check pointer validity). >From d8e76ef15d431135003379e768bd69b98da1f4e7 Mon Sep 17 00:00:00 2001 From: Alex Langford <alangf...@apple.com> Date: Thu, 9 Nov 2023 15:07:22 -0800 Subject: [PATCH] [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsDictionary Similar to my previous patch (#71613) where I changed `GetItemAtIndexAsString`, this patch makes the same change to `GetItemAtIndexAsDictionary`. `GetItemAtIndexAsDictionary` now returns a std::optional that is either `std::nullopt` or is a valid pointer. Therefore, if the optional is populated, we consider the pointer to always be valid (i.e. no need to check pointer validity). --- lldb/include/lldb/Utility/StructuredData.h | 24 +++++++++++++------ .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 6 +++-- .../GDBRemoteCommunicationClient.cpp | 6 +++-- .../Process/scripted/ScriptedThread.cpp | 6 +++-- lldb/source/Target/DynamicRegisterInfo.cpp | 6 +++-- .../lldb-server/tests/MessageObjects.cpp | 7 +++--- 6 files changed, 37 insertions(+), 18 deletions(-) diff --git a/lldb/include/lldb/Utility/StructuredData.h b/lldb/include/lldb/Utility/StructuredData.h index 6e39bcff2c0af0b..8d0ae372f43c6bf 100644 --- a/lldb/include/lldb/Utility/StructuredData.h +++ b/lldb/include/lldb/Utility/StructuredData.h @@ -256,14 +256,24 @@ class StructuredData { return {}; } - bool GetItemAtIndexAsDictionary(size_t idx, Dictionary *&result) const { - result = nullptr; - ObjectSP value_sp = GetItemAtIndex(idx); - if (value_sp.get()) { - result = value_sp->GetAsDictionary(); - return (result != nullptr); + /// Retrieves the element at index \a idx from a StructuredData::Array if it + /// is a Dictionary. + /// + /// \param[in] idx + /// The index of the element to retrieve. + /// + /// \return + /// If the element at index \a idx is a Dictionary, this method returns a + /// valid pointer to the Dictionary wrapped in a std::optional. If the + /// element is not a Dictionary or the index is invalid, this returns + /// std::nullopt. Note that the underlying Dictionary pointer is never + /// nullptr. + std::optional<Dictionary *> GetItemAtIndexAsDictionary(size_t idx) const { + if (auto item_sp = GetItemAtIndex(idx)) { + if (auto *dict = item_sp->GetAsDictionary()) + return dict; } - return false; + return {}; } bool GetItemAtIndexAsArray(size_t idx, Array *&result) const { diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 3714e37ec5c57d0..1ea4f649427727f 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -5688,14 +5688,16 @@ bool ObjectFileMachO::GetCorefileThreadExtraInfos(std::vector<tid_t> &tids) { } const size_t num_threads = threads->GetSize(); for (size_t i = 0; i < num_threads; i++) { - StructuredData::Dictionary *thread; - if (!threads->GetItemAtIndexAsDictionary(i, thread) || !thread) { + std::optional<StructuredData::Dictionary *> maybe_thread = + threads->GetItemAtIndexAsDictionary(i); + if (!maybe_thread) { LLDB_LOGF(log, "Unable to read 'process metadata' LC_NOTE, threads " "array does not have a dictionary at index %zu.", i); return false; } + StructuredData::Dictionary *thread = *maybe_thread; tid_t tid = LLDB_INVALID_THREAD_ID; if (thread->GetValueForKeyAsInteger<tid_t>("thread_id", tid)) if (tid == 0) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 04d98b96acd6839..2cf8c29bf9d2fe9 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -2649,10 +2649,12 @@ size_t GDBRemoteCommunicationClient::QueryGDBServer( return 0; for (size_t i = 0, count = array->GetSize(); i < count; ++i) { - StructuredData::Dictionary *element = nullptr; - if (!array->GetItemAtIndexAsDictionary(i, element)) + std::optional<StructuredData::Dictionary *> maybe_element = + array->GetItemAtIndexAsDictionary(i); + if (!maybe_element) continue; + StructuredData::Dictionary *element = *maybe_element; uint16_t port = 0; if (StructuredData::ObjectSP port_osp = element->GetValueForKey(llvm::StringRef("port"))) diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp index aa2796db15cd00a..88a4ca3b0389fb9 100644 --- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp +++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp @@ -176,8 +176,9 @@ bool ScriptedThread::LoadArtificialStackFrames() { StackFrameListSP frames = GetStackFrameList(); for (size_t idx = 0; idx < arr_size; idx++) { - StructuredData::Dictionary *dict; - if (!arr_sp->GetItemAtIndexAsDictionary(idx, dict) || !dict) + std::optional<StructuredData::Dictionary *> maybe_dict = + arr_sp->GetItemAtIndexAsDictionary(idx); + if (!maybe_dict) return ScriptedInterface::ErrorWithMessage<bool>( LLVM_PRETTY_FUNCTION, llvm::Twine( @@ -185,6 +186,7 @@ bool ScriptedThread::LoadArtificialStackFrames() { llvm::Twine(idx) + llvm::Twine(") from stackframe array.")) .str(), error, LLDBLog::Thread); + StructuredData::Dictionary *dict = *maybe_dict; lldb::addr_t pc; if (!dict->GetValueForKeyAsInteger("pc", pc)) diff --git a/lldb/source/Target/DynamicRegisterInfo.cpp b/lldb/source/Target/DynamicRegisterInfo.cpp index cc2df5b97940fed..7469c1d4259afc2 100644 --- a/lldb/source/Target/DynamicRegisterInfo.cpp +++ b/lldb/source/Target/DynamicRegisterInfo.cpp @@ -231,13 +231,15 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict, // InvalidateNameMap; // InvalidateNameMap invalidate_map; for (uint32_t i = 0; i < num_regs; ++i) { - StructuredData::Dictionary *reg_info_dict = nullptr; - if (!regs->GetItemAtIndexAsDictionary(i, reg_info_dict)) { + std::optional<StructuredData::Dictionary *> maybe_reg_info_dict = + regs->GetItemAtIndexAsDictionary(i); + if (!maybe_reg_info_dict) { Clear(); printf("error: items in the 'registers' array must be dictionaries\n"); regs->DumpToStdout(); return 0; } + StructuredData::Dictionary *reg_info_dict = *maybe_reg_info_dict; // { 'name':'rcx' , 'bitsize' : 64, 'offset' : 16, // 'encoding':'uint' , 'format':'hex' , 'set': 0, 'ehframe' : 2, diff --git a/lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp b/lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp index ed1c8b92b3db74d..32833920ffc57f6 100644 --- a/lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp +++ b/lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp @@ -106,11 +106,12 @@ Expected<JThreadsInfo> JThreadsInfo::create(StringRef Response, return make_parsing_error("JThreadsInfo: JSON array"); for (size_t i = 0; i < array->GetSize(); i++) { - StructuredData::Dictionary *thread_info; - array->GetItemAtIndexAsDictionary(i, thread_info); - if (!thread_info) + std::optional<StructuredData::Dictionary *> maybe_thread_info = + array->GetItemAtIndexAsDictionary(i); + if (!maybe_thread_info) return make_parsing_error("JThreadsInfo: JSON obj at {0}", i); + StructuredData::Dictionary *thread_info = *maybe_thread_info; StringRef name, reason; thread_info->GetValueForKeyAsString("name", name); thread_info->GetValueForKeyAsString("reason", reason); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits