[Lldb-commits] [PATCH] D97205: [lldb][NFC] Don't inherit from UserID in ValueObject

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: LLDB.
teemperor added a project: LLDB.
Herald added a subscriber: JDevlieghere.
teemperor requested review of this revision.

ValueObject inherits from UserID which is just a bad idea:

- The inheritance gives ValueObject some member functions that are at best misleading (such as `Clear()` which doesn't clear any value beside `id`).
- It allows passing ValueObject to the overloaded operators for UserID (such as `==` or `<<` which won't actually compare or print anything in the ValueObject).
- It exposes the `SetID` and `Clear` which both allow users to change the internal id value.

Similar to D91699 <https://reviews.llvm.org/D91699> which did the same for Process


https://reviews.llvm.org/D97205

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


Index: lldb/source/Core/ValueObject.cpp
===================================================================
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -76,11 +76,11 @@
 
 // ValueObject constructor
 ValueObject::ValueObject(ValueObject &parent)
-    : UserID(++g_value_obj_uid), // Unique identifier for every value object
-      m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
+    : m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
       m_manager(parent.GetManager()),
       m_is_synthetic_children_generated(
-          parent.m_is_synthetic_children_generated) {
+          parent.m_is_synthetic_children_generated),
+      m_id(++g_value_obj_uid) {
   InitHelper();
   m_data.SetByteOrder(parent.GetDataExtractor().GetByteOrder());
   m_data.SetAddressByteSize(parent.GetDataExtractor().GetAddressByteSize());
@@ -91,9 +91,9 @@
 ValueObject::ValueObject(ExecutionContextScope *exe_scope,
                          ValueObjectManager &manager,
                          AddressType child_ptr_or_ref_addr_type)
-    : UserID(++g_value_obj_uid), // Unique identifier for every value object
-      m_update_point(exe_scope), m_manager(&manager),
-      m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type) {
+    : m_update_point(exe_scope), m_manager(&manager),
+      m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type),
+      m_id(++g_value_obj_uid) {
   InitHelper();
   if (exe_scope) {
     TargetSP target_sp(exe_scope->CalculateTarget());
Index: lldb/include/lldb/Core/ValueObject.h
===================================================================
--- lldb/include/lldb/Core/ValueObject.h
+++ lldb/include/lldb/Core/ValueObject.h
@@ -102,7 +102,7 @@
 /// Shared Pointer to the contained ValueObject,
 /// just do so by calling GetSP() on the contained object.
 
-class ValueObject : public UserID {
+class ValueObject {
 public:
   enum GetExpressionPathFormat {
     eGetExpressionPathFormatDereferencePointers = 1,
@@ -455,6 +455,9 @@
 
   ConstString GetName() const;
 
+  /// Returns a unique uid for this ValueObject.
+  lldb::user_id_t GetID() const { return m_id.GetID(); }
+
   virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx, bool can_create);
 
   // this will always create the children if necessary
@@ -881,6 +884,9 @@
 
   uint64_t m_language_flags = 0;
 
+  /// Unique identifier for every value object.
+  UserID m_id;
+
   bool m_value_is_valid : 1, m_value_did_change : 1, m_children_count_valid : 1,
       m_old_value_valid : 1, m_is_deref_of_parent : 1,
       m_is_array_item_for_pointer : 1, m_is_bitfield_for_scalar : 1,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97205.325472.patch
Type: text/x-patch
Size: 2651 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20210222/bcf66e6d/attachment.bin>