[NFC][lldb] Document a few ivars on the value object system.

[augusto2112]

No description provided.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Augusto Noronha (augusto2112)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/124971.diff

3 Files Affected:

• (modified) lldb/include/lldb/Expression/ExpressionVariable.h (+7)
• (modified) lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h (+4)
• (modified) lldb/source/Expression/Materializer.cpp (+1)

diff --git a/lldb/include/lldb/Expression/ExpressionVariable.h b/lldb/include/lldb/Expression/ExpressionVariable.h
index fc36793b3a475c..a2175dee7dca9e 100644
--- a/lldb/include/lldb/Expression/ExpressionVariable.h
+++ b/lldb/include/lldb/Expression/ExpressionVariable.h
@@ -108,7 +108,14 @@ class ExpressionVariable
   FlagType m_flags; // takes elements of Flags
 
   // these 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;
 };
 
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..500b8c098f7ca8 100644
--- a/lldb/source/Expression/Materializer.cpp
+++ b/lldb/source/Expression/Materializer.cpp
@@ -1187,6 +1187,7 @@ class EntityResultVariable : public Materializer::Entity {
 
 private:
   CompilerType m_type;
+  /// If this result entity can (and should) track the value on inferior memory.
   bool m_is_program_reference;
   bool m_keep_in_memory;
 

[JDevlieghere]

LGTM with one solution since you're already improving the comments.

[JDevlieghere]

Suggested change

	// these should be private
	/// These members should be private.
	/// @{
	[...]
	lldb::ValueObjectSP m_live_sp;
	/// @}

[jimingham]

Other than that tiny quibble, these are accurate and helpful comments.

[augusto2112]

Re-requesting review as I added more comments.

[JDevlieghere]

LGTM with some formatting nits.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff bda19768de03a0322c4094c8d0e00ad033268309 87eca7770ba2a747db1843a8abfdceaa2f4debf8 --extensions cpp,h -- lldb/include/lldb/Core/Value.h lldb/include/lldb/Expression/ExpressionVariable.h lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h lldb/source/Expression/Materializer.cpp

View the diff from clang-format here.

diff --git a/lldb/include/lldb/Core/Value.h b/lldb/include/lldb/Core/Value.h
index 3714621b46..b1044eef5b 100644
--- a/lldb/include/lldb/Core/Value.h
+++ b/lldb/include/lldb/Core/Value.h
@@ -170,8 +170,8 @@ protected:
   // - 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.
+  //   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