Skip to content

Commit c155269

Browse files
authored
[lldb] Return index of element in ValueObject path instead of the element's value (#74413)
It's more meaningful and actionable to indicate which element in the array has an issue by returning that element's index instead of its value. The value can be ambiguous if at least one other element has the same value. The first parameter for these methods is `idxs`, an array of indices that represent a path from a (root) parent to on of its descendants, typically though intermediate descendants. When the path leads to a descendant that doesn't exist, the method is supposed to indicate where things went wrong by setting an index to `&index_of_error`, the second parameter. The problem is the method sets `*index_of_error` to the index of the most recent parent's child in the hierarchy, which isn't very useful if there's more one index with the same value in the path. In this example, each element in the path has a value that's the same as another element. ```cpp GetChildAtIndexPath({1, 2, 3, 3, 1, 1, 2}, &index_of_error); ``` Say the the second `1` in the path (the 5th element at `[4]`) doesn't exist and the code returns a `nullptr`. In that situation, the code sets `*index_of_error` to `1`, but that's an ambiguous hint can implicate the 1st, 5th, or 6th element (at `[0]`, `[4]`, or `[5]`). It’s more helpful to set `*index_of_error` to `4` to clearly indicate which element in `idxs` has the issue.
1 parent e52c941 commit c155269

File tree

2 files changed

+11
-4
lines changed

2 files changed

+11
-4
lines changed

lldb/include/lldb/Core/ValueObject.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -468,15 +468,15 @@ class ValueObject {
468468
virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx,
469469
bool can_create = true);
470470

471-
// this will always create the children if necessary
471+
// The method always creates missing children in the path, if necessary.
472472
lldb::ValueObjectSP GetChildAtIndexPath(llvm::ArrayRef<size_t> idxs,
473473
size_t *index_of_error = nullptr);
474474

475475
lldb::ValueObjectSP
476476
GetChildAtIndexPath(llvm::ArrayRef<std::pair<size_t, bool>> idxs,
477477
size_t *index_of_error = nullptr);
478478

479-
// this will always create the children if necessary
479+
// The method always creates missing children in the path, if necessary.
480480
lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef<llvm::StringRef> names);
481481

482482
virtual lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,

lldb/source/Core/ValueObject.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,13 +398,16 @@ ValueObject::GetChildAtIndexPath(llvm::ArrayRef<size_t> idxs,
398398
if (idxs.size() == 0)
399399
return GetSP();
400400
ValueObjectSP root(GetSP());
401+
402+
size_t current_index = 0;
401403
for (size_t idx : idxs) {
402404
root = root->GetChildAtIndex(idx);
403405
if (!root) {
404406
if (index_of_error)
405-
*index_of_error = idx;
407+
*index_of_error = current_index;
406408
return root;
407409
}
410+
current_index += 1;
408411
}
409412
return root;
410413
}
@@ -414,13 +417,17 @@ lldb::ValueObjectSP ValueObject::GetChildAtIndexPath(
414417
if (idxs.size() == 0)
415418
return GetSP();
416419
ValueObjectSP root(GetSP());
420+
421+
size_t current_index = 0;
417422
for (std::pair<size_t, bool> idx : idxs) {
418423
root = root->GetChildAtIndex(idx.first, idx.second);
419424
if (!root) {
420425
if (index_of_error)
421-
*index_of_error = idx.first;
426+
*index_of_error = current_index;
422427
return root;
423428
}
429+
430+
current_index += 1;
424431
}
425432
return root;
426433
}

0 commit comments

Comments
 (0)