[Lldb-commits] [lldb] c155269 - [lldb] Return index of element in ValueObject path instead of the element's value (#74413)

[via lldb-commits]

Author: Pete Lawrence
Date: 2023-12-13T15:26:05-08:00
New Revision: c1552695aedebe02e1973d489b93af4e64e9d1a8

URL: https://github.com/llvm/llvm-project/commit/c1552695aedebe02e1973d489b93af4e64e9d1a8
DIFF: https://github.com/llvm/llvm-project/commit/c1552695aedebe02e1973d489b93af4e64e9d1a8.diff

LOG: [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.

Added: 
    

Modified: 
    lldb/include/lldb/Core/ValueObject.h
    lldb/source/Core/ValueObject.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h
index 20b3086138457f..a158199e7fab1a 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -468,7 +468,7 @@ class ValueObject {
   virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx,
                                               bool can_create = true);
 
-  // this will always create the children if necessary
+  // The method always creates missing children in the path, if necessary.
   lldb::ValueObjectSP GetChildAtIndexPath(llvm::ArrayRef<size_t> idxs,
                                           size_t *index_of_error = nullptr);
 
@@ -476,7 +476,7 @@ class ValueObject {
   GetChildAtIndexPath(llvm::ArrayRef<std::pair<size_t, bool>> idxs,
                       size_t *index_of_error = nullptr);
 
-  // this will always create the children if necessary
+  // The method always creates missing children in the path, if necessary.
   lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef<llvm::StringRef> names);
 
   virtual lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,

diff  --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index a7f7ee64282d89..b13bffa0ca809b 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -398,13 +398,16 @@ ValueObject::GetChildAtIndexPath(llvm::ArrayRef<size_t> idxs,
   if (idxs.size() == 0)
     return GetSP();
   ValueObjectSP root(GetSP());
+
+  size_t current_index = 0;
   for (size_t idx : idxs) {
     root = root->GetChildAtIndex(idx);
     if (!root) {
       if (index_of_error)
-        *index_of_error = idx;
+        *index_of_error = current_index;
       return root;
     }
+    current_index += 1;
   }
   return root;
 }
@@ -414,13 +417,17 @@ lldb::ValueObjectSP ValueObject::GetChildAtIndexPath(
   if (idxs.size() == 0)
     return GetSP();
   ValueObjectSP root(GetSP());
+
+  size_t current_index = 0;
   for (std::pair<size_t, bool> idx : idxs) {
     root = root->GetChildAtIndex(idx.first, idx.second);
     if (!root) {
       if (index_of_error)
-        *index_of_error = idx.first;
+        *index_of_error = current_index;
       return root;
     }
+
+    current_index += 1;
   }
   return root;
 }