https://github.com/bulbazord created
https://github.com/llvm/llvm-project/pull/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.
>From 5081b19801932866997fe19719df0184a68694d6 Mon Sep 17 00:00:00 2001
From: Alex Langford <alangf...@apple.com>
Date: Tue, 7 Nov 2023 16:56:35 -0800
Subject: [PATCH] [lldb] Change interface of
StructuredData::Array::GetItemAtIndexAsString
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.
---
lldb/include/lldb/Utility/StructuredData.h | 22 ++++---------
lldb/source/Breakpoint/Breakpoint.cpp | 16 ++++------
lldb/source/Breakpoint/BreakpointOptions.cpp | 7 ++--
.../BreakpointResolverFileRegex.cpp | 8 ++---
.../Breakpoint/BreakpointResolverName.cpp | 9 +++---
.../Commands/CommandObjectBreakpoint.cpp | 6 ++--
lldb/source/Core/SearchFilter.cpp | 32 +++++++++----------
lldb/source/Target/DynamicRegisterInfo.cpp | 26 ++++++++-------
.../lldb-server/tests/MessageObjects.cpp | 8 +++--
9 files changed, 63 insertions(+), 71 deletions(-)
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 ff4f195ea30909e..28a6a16d45907ef 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
