[Lldb-commits] [PATCH] D105470: [lldb] Clear children of ValueObject on value update

[Andy Yankovsky via Phabricator via lldb-commits]

werat updated this revision to Diff 357710.
werat added a comment.

Add more test cases


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105470/new/

https://reviews.llvm.org/D105470

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/test/API/python_api/value/change_values/TestChangeValueAPI.py
  lldb/test/API/python_api/value/change_values/main.c


Index: lldb/test/API/python_api/value/change_values/main.c
===
--- lldb/test/API/python_api/value/change_values/main.c
+++ lldb/test/API/python_api/value/change_values/main.c
@@ -8,7 +8,12 @@
   uint32_t  second_val;
   uint64_t  third_val;
 };
-  
+
+struct bar
+{
+  int value;
+};
+
 int main ()
 {
   int val = 100;
@@ -18,6 +23,11 @@
   ptr->second_val = ;
   ptr->third_val = ;
 
+  struct bar _b1 = {.value = 1};
+  struct bar _b2 = {.value = 2};
+  struct bar *b1 = &_b1;
+  struct bar *b2 = &_b2;
+
   // Stop here and set values
   printf ("Val - %d Mine - %d, %d, %llu. Ptr - %d, %d, %llu\n", val, 
   mine.first_val, mine.second_val, mine.third_val,
Index: lldb/test/API/python_api/value/change_values/TestChangeValueAPI.py
===
--- lldb/test/API/python_api/value/change_values/TestChangeValueAPI.py
+++ lldb/test/API/python_api/value/change_values/TestChangeValueAPI.py
@@ -130,6 +130,41 @@
 self.assertEquals(actual_value, 98765,
 "Got the right changed value from ptr->second_val")
 
+# Test updating the children after updating the parent value.
+def test_update_parent_value(parent):
+self.assertEquals(
+parent.GetValue(),
+frame0.FindVariable("b1").GetValue())
+self.assertEquals(parent.GetChildAtIndex(0).GetValue(), "1")
+
+result = parent.SetValueFromCString(
+frame0.FindVariable("b2").GetValue())
+self.assertTrue(result, "Success setting {}".format(parent.name))
+self.assertEquals(
+parent.GetValue(),
+frame0.FindVariable("b2").GetValue())
+self.assertEquals(parent.GetChildAtIndex(0).GetValue(), "2")
+
+# Test for value returned by SBFrame::EvaluateExpression.
+test_update_parent_value(
+frame0.EvaluateExpression("auto $b_0 = b1; $b_0"))
+
+# Test for value _created_ by SBFrame::EvaluateExpression.
+frame0.EvaluateExpression("auto $b_0 = b1")
+test_update_parent_value(
+frame0.FindValue('$b_0', lldb.eValueTypeConstResult))
+
+# Test for value created by SBTarget::CreateValueFromData.
+b1 = frame0.FindVariable("b1")
+b1_size = b1.GetByteSize()
+b1_value = b1.GetValueAsUnsigned()
+b1_addr_bytes = b1_value.to_bytes(b1_size, 'little')
+error = lldb.SBError()
+data = lldb.SBData()
+data.SetData(error, b1_addr_bytes, lldb.eByteOrderLittle, b1_size)
+test_update_parent_value(
+target.CreateValueFromData("b", data, b1.GetType()))
+
 # gcc may set multiple locations for breakpoint
 breakpoint.SetEnabled(False)
 
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -231,6 +231,10 @@
   // We have to clear the value string here so ConstResult children will notice
   // if their values are changed by hand (i.e. with SetValueAsCString).
   ClearUserVisibleData(eClearUserVisibleDataItemsValue);
+  // Children have to be re-computed after updating the parent value.
+  m_flags.m_children_count_valid = false;
+  m_children.Clear();
+  SetSyntheticChildren(lldb::SyntheticChildrenSP());
 }
 
 void ValueObject::ClearDynamicTypeInformation() {


Index: lldb/test/API/python_api/value/change_values/main.c
===
--- lldb/test/API/python_api/value/change_values/main.c
+++ lldb/test/API/python_api/value/change_values/main.c
@@ -8,7 +8,12 @@
   uint32_t  second_val;
   uint64_t  third_val;
 };
-  
+
+struct bar
+{
+  int value;
+};
+
 int main ()
 {
   int val = 100;
@@ -18,6 +23,11 @@
   ptr->second_val = ;
   ptr->third_val = ;
 
+  struct bar _b1 = {.value = 1};
+  struct bar _b2 = {.value = 2};
+  struct bar *b1 = &_b1;
+  struct bar *b2 = &_b2;
+
   // Stop here and set values
   printf ("Val - %d Mine - %d, %d, %llu. Ptr - %d, %d, %llu\n", val, 
   mine.first_val, mine.second_val, mine.third_val,
Index: lldb/test/API/python_api/value/change_values/TestChangeValueAPI.py
===
--- lldb/test/API/python_api/value/change_values/TestChangeValueAPI.py
+++ lldb/test/API/python_api/value/change_values/TestChangeValueAPI.py
@@ -130,6 +130,41 @@
 self.assertEquals(a

[Lldb-commits] [PATCH] D105470: [lldb] Clear children of ValueObject on value update

[Andy Yankovsky via Phabricator via lldb-commits]

werat added a comment.

Thanks for the explanation! But at this point I feel I'm a bit confused about 
how it all _supposed_ to work in the current design :)

If I understand correctly, there are four "types" of values from the user (API) 
perspective:

1. `ExpressionResult` -- value returned by `SBFrame::EvaluateExpression()`
2. `ExpressionPersistentVariable` -- value created by the expression via `auto 
$name = ...` syntax. Can be obtained by `SBFrame::FindValue("$name", 
lldb::eValueTypeConstResult)`.
3. "Const value" -- value created by `SBTarget::CreateValueFromData()` or 
`SBTarget::CreateValueFromAddress`
4. "Variable reference" -- value returned by `SBFrame::FindVariable()`

For which of these value the following test is supposed to work?

  struct Foo { int x; };
  Foo* f1 = { .x = 1}
  Foo* f2 = { .x = 2}  # pseudo-C for simplicity
  
  f1_ref = ...  # Get a value that holds the value of `f1` using one of the 
four methods described above
  print(f1_ref.GetChild(0))  # '1'
  f1_ref.SetValueFromCString(frame.FindVariable('f2').value)
  print(f1_ref.GetChild(0))  # '2'

My experiments show that it works for "variable references" and "const values" 
created by `CreateValueFromAddress` (but _not_ `CreateValueFromData`).
If I understand your comment correctly, you're saying it should work only for 
`ExpressionPersistentVariable` values (#2). Is that right?

I don't have the full picture about the internal implementation and all the use 
cases, but as a user I would expect it to work for at least #2, #3 and #4. 
Afaik there's no API to fully distinguish between these kinds of values, so I 
find it confusing why `SBValue::SetData()` would be allowed for some values and 
not allowed for others. If I can create a value using `CreateValueFromData` and 
then there's a method `SetValueFromCString`, then I don't see why it should not 
be allowed (apart from implementation complexity/consistency reasons).

What do you think? How should we proceed with this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105470/new/

https://reviews.llvm.org/D105470

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