Skip to content

Commit 95be024

Browse files
committed
[lldb] Return index of element in ValueObject path instead of the element's value (llvm#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 2a0affb commit 95be024

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
@@ -473,15 +473,15 @@ class ValueObject {
473473
virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx,
474474
bool can_create = true);
475475

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

480480
lldb::ValueObjectSP
481481
GetChildAtIndexPath(llvm::ArrayRef<std::pair<size_t, bool>> idxs,
482482
size_t *index_of_error = nullptr);
483483

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

487487
lldb::ValueObjectSP

lldb/source/Core/ValueObject.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -485,13 +485,16 @@ ValueObject::GetChildAtIndexPath(llvm::ArrayRef<size_t> idxs,
485485
if (idxs.size() == 0)
486486
return GetSP();
487487
ValueObjectSP root(GetSP());
488+
489+
size_t current_index = 0;
488490
for (size_t idx : idxs) {
489491
root = root->GetChildAtIndex(idx);
490492
if (!root) {
491493
if (index_of_error)
492-
*index_of_error = idx;
494+
*index_of_error = current_index;
493495
return root;
494496
}
497+
current_index += 1;
495498
}
496499
return root;
497500
}
@@ -501,13 +504,17 @@ lldb::ValueObjectSP ValueObject::GetChildAtIndexPath(
501504
if (idxs.size() == 0)
502505
return GetSP();
503506
ValueObjectSP root(GetSP());
507+
508+
size_t current_index = 0;
504509
for (std::pair<size_t, bool> idx : idxs) {
505510
root = root->GetChildAtIndex(idx.first, idx.second);
506511
if (!root) {
507512
if (index_of_error)
508-
*index_of_error = idx.first;
513+
*index_of_error = current_index;
509514
return root;
510515
}
516+
517+
current_index += 1;
511518
}
512519
return root;
513520
}

0 commit comments

Comments
 (0)