Skip to content

Commit 1486264

Browse files
bulbazordbulbazord
authored
[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.
1 parent fe98cce commit 1486264

File tree

9 files changed

+63
-71
lines changed

9 files changed

+63
-71
lines changed

‎lldb/include/lldb/Utility/StructuredData.h

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <functional>
2424
#include <map>
2525
#include <memory>
26+
#include <optional>
2627
#include <string>
2728
#include <type_traits>
2829
#include <utility>
@@ -247,23 +248,12 @@ class StructuredData {
247248
return success;
248249
}
249250

250-
bool GetItemAtIndexAsString(size_t idx, llvm::StringRef &result) const {
251-
ObjectSP value_sp = GetItemAtIndex(idx);
252-
if (value_sp.get()) {
253-
if (auto string_value = value_sp->GetAsString()) {
254-
result = string_value->GetValue();
255-
return true;
256-
}
251+
std::optional<llvm::StringRef> GetItemAtIndexAsString(size_t idx) const {
252+
if (auto item_sp = GetItemAtIndex(idx)) {
253+
if (auto *string_value = item_sp->GetAsString())
254+
return string_value->GetValue();
257255
}
258-
return false;
259-
}
260-
261-
bool GetItemAtIndexAsString(size_t idx, llvm::StringRef &result,
262-
llvm::StringRef default_val) const {
263-
bool success = GetItemAtIndexAsString(idx, result);
264-
if (!success)
265-
result = default_val;
266-
return success;
256+
return {};
267257
}
268258

269259
bool GetItemAtIndexAsDictionary(size_t idx, Dictionary *&result) const {

‎lldb/source/Breakpoint/Breakpoint.cpp

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,9 @@ lldb::BreakpointSP Breakpoint::CreateFromStructuredData(
205205
if (success && names_array) {
206206
size_t num_names = names_array->GetSize();
207207
for (size_t i = 0; i < num_names; i++) {
208-
llvm::StringRef name;
209-
Status error;
210-
success = names_array->GetItemAtIndexAsString(i, name);
211-
target.AddNameToBreakpoint(result_sp, name.str().c_str(), error);
208+
if (std::optional<llvm::StringRef> maybe_name =
209+
names_array->GetItemAtIndexAsString(i))
210+
target.AddNameToBreakpoint(result_sp, *maybe_name, error);
212211
}
213212
}
214213

@@ -238,11 +237,10 @@ bool Breakpoint::SerializedBreakpointMatchesNames(
238237
size_t num_names = names_array->GetSize();
239238

240239
for (size_t i = 0; i < num_names; i++) {
241-
llvm::StringRef name;
242-
if (names_array->GetItemAtIndexAsString(i, name)) {
243-
if (llvm::is_contained(names, name))
244-
return true;
245-
}
240+
std::optional<llvm::StringRef> maybe_name =
241+
names_array->GetItemAtIndexAsString(i);
242+
if (maybe_name && llvm::is_contained(names, *maybe_name))
243+
return true;
246244
}
247245
return false;
248246
}

‎lldb/source/Breakpoint/BreakpointOptions.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,9 @@ BreakpointOptions::CommandData::CreateFromStructuredData(
8888
if (success) {
8989
size_t num_elems = user_source->GetSize();
9090
for (size_t i = 0; i < num_elems; i++) {
91-
llvm::StringRef elem_string;
92-
success = user_source->GetItemAtIndexAsString(i, elem_string);
93-
if (success)
94-
data_up->user_source.AppendString(elem_string);
91+
if (std::optional<llvm::StringRef> maybe_elem_string =
92+
user_source->GetItemAtIndexAsString(i))
93+
data_up->user_source.AppendString(*maybe_elem_string);
9594
}
9695
}
9796

‎lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,14 @@ BreakpointResolverSP BreakpointResolverFileRegex::CreateFromStructuredData(
5656
if (success && names_array) {
5757
size_t num_names = names_array->GetSize();
5858
for (size_t i = 0; i < num_names; i++) {
59-
llvm::StringRef name;
60-
success = names_array->GetItemAtIndexAsString(i, name);
61-
if (!success) {
59+
std::optional<llvm::StringRef> maybe_name =
60+
names_array->GetItemAtIndexAsString(i);
61+
if (!maybe_name) {
6262
error.SetErrorStringWithFormat(
6363
"BRFR::CFSD: Malformed element %zu in the names array.", i);
6464
return nullptr;
6565
}
66-
names_set.insert(std::string(name));
66+
names_set.insert(std::string(*maybe_name));
6767
}
6868
}
6969

‎lldb/source/Breakpoint/BreakpointResolverName.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,9 @@ BreakpointResolverSP BreakpointResolverName::CreateFromStructuredData(
155155
std::vector<std::string> names;
156156
std::vector<FunctionNameType> name_masks;
157157
for (size_t i = 0; i < num_elem; i++) {
158-
llvm::StringRef name;
159-
160-
success = names_array->GetItemAtIndexAsString(i, name);
161-
if (!success) {
158+
std::optional<llvm::StringRef> maybe_name =
159+
names_array->GetItemAtIndexAsString(i);
160+
if (!maybe_name) {
162161
error.SetErrorString("BRN::CFSD: name entry is not a string.");
163162
return nullptr;
164163
}
@@ -168,7 +167,7 @@ BreakpointResolverSP BreakpointResolverName::CreateFromStructuredData(
168167
error.SetErrorString("BRN::CFSD: name mask entry is not an integer.");
169168
return nullptr;
170169
}
171-
names.push_back(std::string(name));
170+
names.push_back(std::string(*maybe_name));
172171
name_masks.push_back(static_cast<FunctionNameType>(fnt));
173172
}
174173

‎lldb/source/Commands/CommandObjectBreakpoint.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2235,9 +2235,9 @@ class CommandObjectBreakpointRead : public CommandObjectParsed {
22352235
size_t num_names = names_array->GetSize();
22362236

22372237
for (size_t i = 0; i < num_names; i++) {
2238-
llvm::StringRef name;
2239-
if (names_array->GetItemAtIndexAsString(i, name))
2240-
request.TryCompleteCurrentArg(name);
2238+
if (std::optional<llvm::StringRef> maybe_name =
2239+
names_array->GetItemAtIndexAsString(i))
2240+
request.TryCompleteCurrentArg(*maybe_name);
22412241
}
22422242
}
22432243
}

‎lldb/source/Core/SearchFilter.cpp

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -471,13 +471,13 @@ SearchFilterSP SearchFilterByModule::CreateFromStructuredData(
471471
return nullptr;
472472
}
473473

474-
llvm::StringRef module;
475-
success = modules_array->GetItemAtIndexAsString(0, module);
476-
if (!success) {
474+
std::optional<llvm::StringRef> maybe_module =
475+
modules_array->GetItemAtIndexAsString(0);
476+
if (!maybe_module) {
477477
error.SetErrorString("SFBM::CFSD: filter module item not a string.");
478478
return nullptr;
479479
}
480-
FileSpec module_spec(module);
480+
FileSpec module_spec(*maybe_module);
481481

482482
return std::make_shared<SearchFilterByModule>(target_sp, module_spec);
483483
}
@@ -596,14 +596,14 @@ SearchFilterSP SearchFilterByModuleList::CreateFromStructuredData(
596596
FileSpecList modules;
597597
size_t num_modules = modules_array->GetSize();
598598
for (size_t i = 0; i < num_modules; i++) {
599-
llvm::StringRef module;
600-
success = modules_array->GetItemAtIndexAsString(i, module);
601-
if (!success) {
599+
std::optional<llvm::StringRef> maybe_module =
600+
modules_array->GetItemAtIndexAsString(i);
601+
if (!maybe_module) {
602602
error.SetErrorStringWithFormat(
603603
"SFBM::CFSD: filter module item %zu not a string.", i);
604604
return nullptr;
605605
}
606-
modules.EmplaceBack(module);
606+
modules.EmplaceBack(*maybe_module);
607607
}
608608
return std::make_shared<SearchFilterByModuleList>(target_sp, modules);
609609
}
@@ -644,14 +644,14 @@ lldb::SearchFilterSP SearchFilterByModuleListAndCU::CreateFromStructuredData(
644644
if (success) {
645645
size_t num_modules = modules_array->GetSize();
646646
for (size_t i = 0; i < num_modules; i++) {
647-
llvm::StringRef module;
648-
success = modules_array->GetItemAtIndexAsString(i, module);
649-
if (!success) {
647+
std::optional<llvm::StringRef> maybe_module =
648+
modules_array->GetItemAtIndexAsString(i);
649+
if (!maybe_module) {
650650
error.SetErrorStringWithFormat(
651651
"SFBM::CFSD: filter module item %zu not a string.", i);
652652
return result_sp;
653653
}
654-
modules.EmplaceBack(module);
654+
modules.EmplaceBack(*maybe_module);
655655
}
656656
}
657657

@@ -666,14 +666,14 @@ lldb::SearchFilterSP SearchFilterByModuleListAndCU::CreateFromStructuredData(
666666
size_t num_cus = cus_array->GetSize();
667667
FileSpecList cus;
668668
for (size_t i = 0; i < num_cus; i++) {
669-
llvm::StringRef cu;
670-
success = cus_array->GetItemAtIndexAsString(i, cu);
671-
if (!success) {
669+
std::optional<llvm::StringRef> maybe_cu =
670+
cus_array->GetItemAtIndexAsString(i);
671+
if (!maybe_cu) {
672672
error.SetErrorStringWithFormat(
673673
"SFBM::CFSD: filter CU item %zu not a string.", i);
674674
return nullptr;
675675
}
676-
cus.EmplaceBack(cu);
676+
cus.EmplaceBack(*maybe_cu);
677677
}
678678

679679
return std::make_shared<SearchFilterByModuleListAndCU>(

‎lldb/source/Target/DynamicRegisterInfo.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -144,20 +144,21 @@ llvm::Expected<uint32_t> DynamicRegisterInfo::ByteOffsetFromComposite(
144144
uint32_t composite_offset = UINT32_MAX;
145145
for (uint32_t composite_idx = 0; composite_idx < num_composite_regs;
146146
++composite_idx) {
147-
llvm::StringRef composite_reg_name;
148-
if (!composite_reg_list.GetItemAtIndexAsString(composite_idx, composite_reg_name))
147+
std::optional<llvm::StringRef> maybe_composite_reg_name =
148+
composite_reg_list.GetItemAtIndexAsString(composite_idx);
149+
if (!maybe_composite_reg_name)
149150
return llvm::createStringError(
150151
llvm::inconvertibleErrorCode(),
151152
"\"composite\" list value is not a Python string at index %d",
152153
composite_idx);
153154

154155
const RegisterInfo *composite_reg_info =
155-
GetRegisterInfo(composite_reg_name);
156+
GetRegisterInfo(*maybe_composite_reg_name);
156157
if (!composite_reg_info)
157158
return llvm::createStringError(
158159
llvm::inconvertibleErrorCode(),
159160
"failed to find composite register by name: \"%s\"",
160-
composite_reg_name.str().c_str());
161+
maybe_composite_reg_name->str().c_str());
161162

162163
composite_offset =
163164
std::min(composite_offset, composite_reg_info->byte_offset);
@@ -205,10 +206,11 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict,
205206
if (dict.GetValueForKeyAsArray("sets", sets)) {
206207
const uint32_t num_sets = sets->GetSize();
207208
for (uint32_t i = 0; i < num_sets; ++i) {
208-
llvm::StringRef set_name;
209-
if (sets->GetItemAtIndexAsString(i, set_name) && !set_name.empty()) {
209+
std::optional<llvm::StringRef> maybe_set_name =
210+
sets->GetItemAtIndexAsString(i);
211+
if (maybe_set_name && !maybe_set_name->empty()) {
210212
m_sets.push_back(
211-
{ConstString(set_name).AsCString(), nullptr, 0, nullptr});
213+
{ConstString(*maybe_set_name).AsCString(), nullptr, 0, nullptr});
212214
} else {
213215
Clear();
214216
printf("error: register sets must have valid names\n");
@@ -345,12 +347,12 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict,
345347
const size_t num_regs = invalidate_reg_list->GetSize();
346348
if (num_regs > 0) {
347349
for (uint32_t idx = 0; idx < num_regs; ++idx) {
348-
llvm::StringRef invalidate_reg_name;
349350
uint64_t invalidate_reg_num;
350-
if (invalidate_reg_list->GetItemAtIndexAsString(
351-
idx, invalidate_reg_name)) {
351+
std::optional<llvm::StringRef> maybe_invalidate_reg_name =
352+
invalidate_reg_list->GetItemAtIndexAsString(idx);
353+
if (maybe_invalidate_reg_name) {
352354
const RegisterInfo *invalidate_reg_info =
353-
GetRegisterInfo(invalidate_reg_name);
355+
GetRegisterInfo(*maybe_invalidate_reg_name);
354356
if (invalidate_reg_info) {
355357
m_invalidate_regs_map[i].push_back(
356358
invalidate_reg_info->kinds[eRegisterKindLLDB]);
@@ -359,7 +361,7 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict,
359361
// format
360362
printf("error: failed to find a 'invalidate-regs' register for "
361363
"\"%s\" while parsing register \"%s\"\n",
362-
invalidate_reg_name.str().c_str(), reg_info.name);
364+
maybe_invalidate_reg_name->str().c_str(), reg_info.name);
363365
}
364366
} else if (invalidate_reg_list->GetItemAtIndexAsInteger(
365367
idx, invalidate_reg_num)) {

‎lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,12 @@ JThreadsInfo::parseRegisters(const StructuredData::Dictionary &Dict,
7676
auto KeysObj = Dict.GetKeys();
7777
auto Keys = KeysObj->GetAsArray();
7878
for (size_t i = 0; i < Keys->GetSize(); i++) {
79-
StringRef KeyStr, ValueStr;
80-
Keys->GetItemAtIndexAsString(i, KeyStr);
79+
std::optional<StringRef> MaybeKeyStr = Keys->GetItemAtIndexAsString(i);
80+
if (!MaybeKeyStr)
81+
return make_parsing_error("JThreadsInfo: Invalid Key at index {0}", i);
82+
83+
StringRef KeyStr = *MaybeKeyStr;
84+
StringRef ValueStr;
8185
Dict.GetValueForKeyAsString(KeyStr, ValueStr);
8286
unsigned int Register;
8387
if (!llvm::to_integer(KeyStr, Register, 10))

0 commit comments

Comments
 (0)