[Lldb-commits] [lldb] 6a05bee - [NFC][lldb] Document a few ivars on the value object system. (#124971)

[via lldb-commits]

Author: Augusto Noronha
Date: 2025-01-30T14:29:16-08:00
New Revision: 6a05beeb2bed366f7e6e0056a758c6f8d385fdde

URL: https://github.com/llvm/llvm-project/commit/6a05beeb2bed366f7e6e0056a758c6f8d385fdde
DIFF: https://github.com/llvm/llvm-project/commit/6a05beeb2bed366f7e6e0056a758c6f8d385fdde.diff

LOG: [NFC][lldb] Document a few ivars on the value object system. (#124971)

Co-authored-by: Jonas Devlieghere <jonas at devlieghere.com>

Added: 
    

Modified: 
    lldb/include/lldb/Core/Value.h
    lldb/include/lldb/Expression/ExpressionVariable.h
    lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h
    lldb/source/Expression/Materializer.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/Value.h b/lldb/include/lldb/Core/Value.h
index d0c338ffec0cd3..3714621b469ecf 100644
--- a/lldb/include/lldb/Core/Value.h
+++ b/lldb/include/lldb/Core/Value.h
@@ -109,8 +109,10 @@ class Value {
 
   Scalar &ResolveValue(ExecutionContext *exe_ctx, Module *module = nullptr);
 
+  /// See comment on m_scalar to understand what GetScalar returns.
   const Scalar &GetScalar() const { return m_value; }
 
+  /// See comment on m_scalar to understand what GetScalar returns.
   Scalar &GetScalar() { return m_value; }
 
   size_t ResizeData(size_t len);
@@ -148,6 +150,32 @@ class Value {
   static ValueType GetValueTypeFromAddressType(AddressType address_type);
 
 protected:
+  /// Represents a value, which can be a scalar, a load address, a file address,
+  /// or a host address.
+  ///
+  /// The interpretation of `m_value` depends on `m_value_type`:
+  /// - Scalar: `m_value` contains the scalar value.
+  /// - Load Address: `m_value` contains the load address.
+  /// - File Address: `m_value` contains the file address.
+  /// - Host Address: `m_value` contains a pointer to the start of the buffer in
+  ///    host memory.
+  ///   Currently, this can point to either:
+  ///     - The `m_data_buffer` of this Value instance (e.g., in DWARF
+  ///     computations).
+  ///     - The `m_data` of a Value Object containing this Value.
+  // TODO: the GetScalar() API relies on knowledge not codified by the type
+  // system, making it hard to understand and easy to misuse.
+  // - Separate the scalar from the variable that contains the address (be it a
+  //   load, file or host address).
+  // - Rename GetScalar() to something more indicative to what the scalar is,
+  //   like GetScalarOrAddress() for example.
+  // - Split GetScalar() into two functions, GetScalar() and GetAddress(), which
+  //   verify (or assert) what m_value_type is to make sure users of the class are
+  //   querying the right thing.
+  // TODO: It's confusing to point to multiple possible buffers when the
+  // ValueType is a host address. Value should probably always own its buffer.
+  // Perhaps as a shared pointer with a copy on write system if the same buffer
+  // can be shared by multiple classes.
   Scalar m_value;
   CompilerType m_compiler_type;
   void *m_context = nullptr;

diff  --git a/lldb/include/lldb/Expression/ExpressionVariable.h b/lldb/include/lldb/Expression/ExpressionVariable.h
index fc36793b3a475c..f5bd9389219668 100644
--- a/lldb/include/lldb/Expression/ExpressionVariable.h
+++ b/lldb/include/lldb/Expression/ExpressionVariable.h
@@ -107,9 +107,18 @@ class ExpressionVariable
 
   FlagType m_flags; // takes elements of Flags
 
-  // these should be private
+  /// These members should be private.
+  /// @{
+  /// A value object whose value's data lives in host (lldb's) memory.
   lldb::ValueObjectSP m_frozen_sp;
+  /// The ValueObject counterpart to m_frozen_sp that tracks the value in
+  /// inferior memory. This object may not always exist; its presence depends on
+  /// whether it is logical for the value to exist in the inferior memory. For
+  /// example, when evaluating a C++ expression that generates an r-value, such
+  /// as a single function call, there is no memory address in the inferior to
+  /// track.
   lldb::ValueObjectSP m_live_sp;
+  /// @}
 };
 
 /// \class ExpressionVariableList ExpressionVariable.h

diff  --git a/lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h b/lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h
index dbd68160acb4dc..5509886a8965da 100644
--- a/lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h
+++ b/lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h
@@ -66,6 +66,10 @@ class ValueObjectConstResultImpl {
 
 private:
   ValueObject *m_impl_backend;
+  /// The memory address in the inferior process that this ValueObject tracks.
+  /// This address is used to request additional memory when the actual data
+  /// size exceeds the initial local buffer size, such as when a dynamic type
+  /// resolution results in a type larger than its statically determined type.
   lldb::addr_t m_live_address;
   AddressType m_live_address_type;
   lldb::ValueObjectSP m_address_of_backend;

diff  --git a/lldb/source/Expression/Materializer.cpp b/lldb/source/Expression/Materializer.cpp
index 8cd050f9fdb7ef..13a72a9921e1d6 100644
--- a/lldb/source/Expression/Materializer.cpp
+++ b/lldb/source/Expression/Materializer.cpp
@@ -1187,6 +1187,9 @@ class EntityResultVariable : public Materializer::Entity {
 
 private:
   CompilerType m_type;
+  /// This is used both to control whether this result entity can (and should)
+  /// track the value in inferior memory, as well as to control whether LLDB
+  /// needs to allocate memory for the variable during materialization.
   bool m_is_program_reference;
   bool m_keep_in_memory;