Stop calling ValueObject::SetName from synthetic child providers

[tberghammer]

Summary

Calling ValueObject::SetName from a sythetic child provider would change
the underying value object used for the non-synthetic child as well what
is clearly unintentional.

[jingham]

If SetName is the wrong thing to use for synthetic child providers, then we need to make Clone available to the SB API's as well. In any case where they want to return an extant ValueObject with a different name - as opposed to making one up out of whole cloth - won't they need the same operation?

Also, probably should document this requirement in the varformats.html.

include/lldb/Core/ValueObject.h

607–610 ↗

(On Diff #93058)

Spelling... "he" -> "the", "chane" -> "change", "actul" -> "actual"

[tberghammer]

SBValue::SetName is not part of the SB API (what is the right decision IMO as an SBValue should be mostly immutable) so this issue doesn't effect it. I looked through the code in examples/synthetic/gnu_libstdcpp.py and it is always using one of the SBValue::Create* method to produce new SBValue what will create a new value object one way or the other. Considering that nobody complained about the missing SetName method at the SB API level I don't see a big need for exposing the Clone method there. At the same line if SetName/Clone isn't part of the SB API then I think we shouldn't document it at the webpage.

(I will upload a fix for the spelling errors later)

[tberghammer]

Fix typos in the comments

[jingham]

Good.

lldb/trunk/include/lldb/Core/ValueObject.h

public:
   bool GetValueDidChange();
 
   bool UpdateValueIfNeeded(bool update_format = true);
 
   bool UpdateFormatsIfNeeded();
 
   lldb::ValueObjectSP GetSP() { return m_manager->GetSharedPointer(this); }
 
+  // Change the name of the current ValueObject. Should *not* be used from a
+  // synthetic child provider as it would change the name of the non synthetic
+  // child as well.
   void SetName(const ConstString &name);
 
   virtual lldb::addr_t GetAddressOf(bool scalar_is_load_address = true,
                                     AddressType *address_type = nullptr);
 
   lldb::addr_t GetPointerValue(AddressType *address_type = nullptr);
 
   lldb::ValueObjectSP GetSyntheticChild(const ConstString &key) const;
public:
   lldb::ValueObjectSP
   GetQualifiedRepresentationIfAvailable(lldb::DynamicValueType dynValue,
                                         bool synthValue);
 
   virtual lldb::ValueObjectSP CreateConstantValue(const ConstString &name);
 
   virtual lldb::ValueObjectSP Dereference(Error &error);
 
+  // Creates a copy of the ValueObject with a new name and setting the current
+  // ValueObject as its parent. It should be used when we want to change the
+  // name of a ValueObject without modifying the actual ValueObject itself
+  // (e.g. sythetic child provider).
+  virtual lldb::ValueObjectSP Clone(const ConstString &new_name);
+
   virtual lldb::ValueObjectSP AddressOf(Error &error);
 
   virtual lldb::addr_t GetLiveAddress() { return LLDB_INVALID_ADDRESS; }
 
   virtual void SetLiveAddress(lldb::addr_t addr = LLDB_INVALID_ADDRESS,
                               AddressType address_type = eAddressTypeLoad) {}
 
   // Find the address of the C++ vtable pointer

lldb/trunk/source/Core/ValueObject.cpp

ValueObjectSP ValueObject::AddressOf(Error &error) {
 
   return m_addr_of_valobj_sp;
 }
 
 ValueObjectSP ValueObject::Cast(const CompilerType &compiler_type) {
   return ValueObjectCast::Create(*this, GetName(), compiler_type);
 }
 
+lldb::ValueObjectSP ValueObject::Clone(const ConstString &new_name) {
+  return ValueObjectCast::Create(*this, new_name, GetCompilerType());
+}
+
 ValueObjectSP ValueObject::CastPointerType(const char *name,
                                            CompilerType &compiler_type) {
   ValueObjectSP valobj_sp;
   AddressType address_type;
   addr_t ptr_value = GetPointerValue(&address_type);
 
   if (ptr_value != LLDB_INVALID_ADDRESS) {
     Address ptr_addr(ptr_value);

lldb/trunk/source/DataFormatters/VectorType.cpp

public:
   ~VectorTypeSyntheticFrontEnd() override = default;
 
   size_t CalculateNumChildren() override { return m_num_children; }
 
   lldb::ValueObjectSP GetChildAtIndex(size_t idx) override {
     if (idx >= CalculateNumChildren())
       return lldb::ValueObjectSP();
     auto offset = idx * m_child_type.GetByteSize(nullptr);
-    ValueObjectSP child_sp(
-        m_backend.GetSyntheticChildAtOffset(offset, m_child_type, true));
-    if (!child_sp)
-      return child_sp;
-
     StreamString idx_name;
     idx_name.Printf("[%" PRIu64 "]", (uint64_t)idx);
-    child_sp->SetName(ConstString(idx_name.GetString()));
+    ValueObjectSP child_sp(m_backend.GetSyntheticChildAtOffset(
+        offset, m_child_type, true, ConstString(idx_name.GetString())));
+    if (!child_sp)
+      return child_sp;
 
     child_sp->SetFormat(m_item_format);
 
     return child_sp;
   }
 
   bool Update() override {
     m_parent_format = m_backend.GetFormat();

lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp

lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
   auto potential_child_sp = CreateValueObjectFromData(
       name.GetString(), data, m_backend.GetExecutionContextRef(),
       m_element_type);
   if (potential_child_sp) {
     switch (potential_child_sp->GetNumChildren()) {
     case 1: {
       auto child0_sp = potential_child_sp->GetChildAtIndex(0, true);
       if (child0_sp && child0_sp->GetName() == g___cc)
-        potential_child_sp = child0_sp;
+        potential_child_sp = child0_sp->Clone(ConstString(name.GetString()));
       break;
     }
     case 2: {
       auto child0_sp = potential_child_sp->GetChildAtIndex(0, true);
       auto child1_sp = potential_child_sp->GetChildAtIndex(1, true);
       if (child0_sp && child0_sp->GetName() == g___cc && child1_sp &&
           child1_sp->GetName() == g___nc)
-        potential_child_sp = child0_sp;
+        potential_child_sp = child0_sp->Clone(ConstString(name.GetString()));
       break;
     }
     }
-    potential_child_sp->SetName(ConstString(name.GetString()));
   }
   m_iterators[idx] = iterator;
   return potential_child_sp;
 }
 
 bool lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::Update() {
   static ConstString g___tree_("__tree_");
   static ConstString g___begin_node_("__begin_node_");

lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp

for (size_t i = 0; i < child_count; ++i) {
       if (name_str.startswith("std::_Tuple_impl<")) {
         next_child_sp = child_sp;
       } else if (name_str.startswith("std::_Head_base<")) {
         ValueObjectSP value_sp =
             child_sp->GetChildMemberWithName(ConstString("_M_head_impl"), true);
         if (value_sp) {
           StreamString name;
           name.Printf("[%zd]", m_members.size());
-          value_sp->SetName(ConstString(name.GetString()));
-
-          m_members.push_back(value_sp);
+          m_members.push_back(value_sp->Clone(ConstString(name.GetString())));
         }
       }
     }
   }
 
   return false;
 }
 

lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp

bool LibStdcppUniquePtrSyntheticFrontEnd::Update() {
   ValueObjectSP tuple_sp =
       valobj_sp->GetChildMemberWithName(ConstString("_M_t"), true);
   if (!tuple_sp)
     return false;
 
   std::unique_ptr<SyntheticChildrenFrontEnd> tuple_frontend(
       LibStdcppTupleSyntheticFrontEndCreator(nullptr, tuple_sp));
 
-  m_ptr_obj = tuple_frontend->GetChildAtIndex(0);
-  if (m_ptr_obj)
-    m_ptr_obj->SetName(ConstString("pointer"));
-
-  m_del_obj = tuple_frontend->GetChildAtIndex(1);
-  if (m_del_obj)
-    m_del_obj->SetName(ConstString("deleter"));
+  ValueObjectSP ptr_obj = tuple_frontend->GetChildAtIndex(0);
+  if (ptr_obj)
+    m_ptr_obj = ptr_obj->Clone(ConstString("pointer"));
+
+  ValueObjectSP del_obj = tuple_frontend->GetChildAtIndex(1);
+  if (del_obj)
+    m_del_obj = del_obj->Clone(ConstString("deleter"));
 
   if (m_ptr_obj) {
     Error error;
-    m_obj_obj = m_ptr_obj->Dereference(error);
+    ValueObjectSP obj_obj = m_ptr_obj->Dereference(error);
     if (error.Success()) {
-      m_obj_obj->SetName(ConstString("object"));
+      m_obj_obj = obj_obj->Clone(ConstString("object"));
     }
   }
 
   return false;
 }
 
 bool LibStdcppUniquePtrSyntheticFrontEnd::MightHaveChildren() { return true; }