https://github.com/PortalPete updated 
https://github.com/llvm/llvm-project/pull/74413

>From 5d991726fbb22bc79336becc08b53b49dc87396d Mon Sep 17 00:00:00 2001
From: Pete Lawrence <plawre...@apple.com>
Date: Mon, 4 Dec 2023 18:29:37 -1000
Subject: [PATCH] [lldb] Return index of element in ValueObject path instead of
 the element's value
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.

rdar://119169160
---
 lldb/include/lldb/Core/ValueObject.h |  4 ++--
 lldb/source/Core/ValueObject.cpp     | 11 +++++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index 20b3086138457..a158199e7fab1 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 a7f7ee64282d8..6cc4ba654e910 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;
   }
   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;
   }
   return root;
 }

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to