[lldb] Retcon SBValue::GetChildAtIndex(synthetic=true)

The motivation here is being (un)able to treat pointer values as an array
consistently. This works for pointers to simple/scalar values, but for
aggregates, we get a very surprising result:
- GetChildAtIndex(x, ??, true) returns the `x` child of the zeroth array member
  (the one you get by dereferencing the pointer/array) for all `x` which are
  smaller than the number of children of that value.
- for other values of `x`, we get `v[x]`, where `v` is treated like a (C)
  pointer

This patch reimagines this interface so that the value of `true` always treats
(pointer and array) values as pointers. For `false`, we always dereference
pointers, while in the case of arrays, we only return the values as far as the
array bounds will allow.

This has the potential to break existing code, but I have a suspicion that code
was already broken to begin with, which is why I think this would be better
than introducing a new API and keeping the old (and surprising behavior). If
our own test coverage is any indication, breakage should be minimal.

Author: labath

lldb/include/lldb/API/SBValue.h

@@ -160,31 +160,26 @@ class LLDB_API SBValue {
   /// members (empty base classes are omitted), and all members of the
   /// current class will then follow the base classes.
   ///
-  /// Pointers differ depending on what they point to. If the pointer
-  /// points to a simple type, the child at index zero
-  /// is the only child value available, unless \a synthetic_allowed
-  /// is \b true, in which case the pointer will be used as an array
-  /// and can create 'synthetic' child values using positive or
-  /// negative indexes. If the pointer points to an aggregate type
-  /// (an array, class, union, struct), then the pointee is
-  /// transparently skipped and any children are going to be the indexes
-  /// of the child values within the aggregate type. For example if
-  /// we have a 'Point' type and we have a SBValue that contains a
-  /// pointer to a 'Point' type, then the child at index zero will be
-  /// the 'x' member, and the child at index 1 will be the 'y' member
-  /// (the child at index zero won't be a 'Point' instance).
+  /// For array and pointers the behavior of the function depends on the value
+  /// of the \a use_synthetic argument. If \b false, the function returns
+  /// members of the array as given by the array bounds. If the value is a
+  /// pointer to a simple type, the child at index zero is the only child
+  /// value available. If the pointer points to an aggregate type (an array,
+  /// class, union, etc.), then the pointee is transparently skipped and any
+  /// children are going to be the indexes of the child values within the
+  /// aggregate type. For example if we have a 'Point' type and we have a
+  /// SBValue that contains a pointer to a 'Point' type, then the child at
+  /// index zero will be the 'x' member, and the child at index 1 will be the
+  /// 'y' member (the child at index zero won't be a 'Point' instance). If \a
+  /// use_synthetic is \b true, pointer values will be used as a (C) array and
+  /// and the function will create 'synthetic' child values using positive or
+  /// negative indexes. In case of arrays, the function will return values
+  /// which are outside of the array bounds.
   ///
   /// If you actually need an SBValue that represents the type pointed
   /// to by a SBValue for which GetType().IsPointeeType() returns true,
   /// regardless of the pointee type, you can do that with SBValue::Dereference.
   ///
-  /// Arrays have a preset number of children that can be accessed by
-  /// index and will returns invalid child values for indexes that are
-  /// out of bounds unless the \a synthetic_allowed is \b true. In this
-  /// case the array can create 'synthetic' child values for indexes
-  /// that aren't in the array bounds using positive or negative
-  /// indexes.
-  ///
   /// \param[in] idx
   ///     The index of the child value to get
   ///
@@ -193,7 +188,7 @@ class LLDB_API SBValue {
   ///     and also if the target can be run to figure out the dynamic
   ///     type of the child value.
   ///
-  /// \param[in] can_create_synthetic
+  /// \param[in] use_synthetic
   ///     If \b true, then allow child values to be created by index
   ///     for pointers and arrays for indexes that normally wouldn't
   ///     be allowed.
@@ -202,7 +197,7 @@ class LLDB_API SBValue {
   ///     A new SBValue object that represents the child member value.
   lldb::SBValue GetChildAtIndex(uint32_t idx,
                                 lldb::DynamicValueType use_dynamic,
-                                bool can_create_synthetic);
+                                bool use_synthetic);
 
   // Matches children of this object only and will match base classes and
   // member names if this is a clang typed object.

lldb/source/API/SBValue.cpp

@@ -680,19 +680,18 @@ SBValue SBValue::GetChildAtIndex(uint32_t idx) {
 
 SBValue SBValue::GetChildAtIndex(uint32_t idx,
                                  lldb::DynamicValueType use_dynamic,
-                                 bool can_create_synthetic) {
-  LLDB_INSTRUMENT_VA(this, idx, use_dynamic, can_create_synthetic);
-
-  lldb::ValueObjectSP child_sp;
-
+                                 bool use_synthetic) {
+  LLDB_INSTRUMENT_VA(this, idx, use_dynamic, use_synthetic);
   ValueLocker locker;
   lldb::ValueObjectSP value_sp(GetSP(locker));
+
+  lldb::ValueObjectSP child_sp;
   if (value_sp) {
     const bool can_create = true;
-    child_sp = value_sp->GetChildAtIndex(idx);
-    if (can_create_synthetic && !child_sp) {
+    if (use_synthetic && (value_sp->IsPointerType() || value_sp->IsArrayType()))
       child_sp = value_sp->GetSyntheticArrayMember(idx, can_create);
-    }
+    else
+      child_sp = value_sp->GetChildAtIndex(idx);
   }
 
   SBValue sb_value;

lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py

@@ -21,3 +21,31 @@ def test_str(self):
         has_formatted = self.frame().FindVariable("has_foo")
         self.expect(str(formatted), exe=False, substrs=["synth_child"])
         self.expect(str(has_formatted), exe=False, substrs=["synth_child"])
+
+    def test_synth_arr(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(
+            self, "break here", lldb.SBFileSpec("main.cpp")
+        )
+        point_arr = self.frame().FindVariable("point_arr")
+        point_ptr = self.frame().FindVariable("point_ptr")
+        for v in [point_arr, point_ptr]:
+            for i in range(3):
+                child = v.GetChildAtIndex(i, lldb.eDynamicDontRunTarget, True)
+                check = ValueCheck(
+                    name=f"[{i}]",
+                    type="Point",
+                    children=[
+                        ValueCheck(name="x", value=str(2 * i + 1)),
+                        ValueCheck(name="y", value=str(2 * i + 2)),
+                    ],
+                )
+                check.check_value(self, child, f"{child}, child {i} of {v.GetName()}")
+
+        int_arr = self.frame().FindVariable("int_arr")
+        int_ptr = self.frame().FindVariable("int_ptr")
+        for v in [int_arr, int_ptr]:
+            for i in range(3):
+                child = v.GetChildAtIndex(i, lldb.eDynamicDontRunTarget, True)
+                check = ValueCheck(name=f"[{i}]", type="int", value=str(i + 1))
+                check.check_value(self, child, f"{child}, child {i} of {v.GetName()}")

lldb/test/API/python_api/sbvalue_synthetic/main.cpp

@@ -6,8 +6,17 @@ struct HasFoo {
   Foo f;
 };
 
+struct Point {
+  int x;
+  int y;
+};
+
 int main() {
   Foo foo;
   HasFoo has_foo;
+  Point point_arr[] = {{1, 2}, {3, 4}, {5, 6}};
+  int int_arr[] = {1, 2, 3, 4, 5, 6};
+  Point *point_ptr = point_arr;
+  int *int_ptr = int_arr;
   return 0; // break here
 }