[lldb] Scalar::GetValue() should take a Stream by reference

[bulbazord]

This function always expects the pointer to be valid, a reference seems more appropriate.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)

Changes

This function always expects the pointer to be valid, a reference seems more appropriate.

---

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

5 Files Affected:

• (modified) lldb/include/lldb/Utility/Scalar.h (+1-1)
• (modified) lldb/source/Core/Value.cpp (+3-1)
• (modified) lldb/source/Interpreter/CommandInterpreter.cpp (+1-1)
• (modified) lldb/source/Utility/Scalar.cpp (+5-5)
• (modified) lldb/unittests/Utility/ScalarTest.cpp (+1-1)

diff --git a/lldb/include/lldb/Utility/Scalar.h b/lldb/include/lldb/Utility/Scalar.h
index 34c2111ae0ac6ee..8e087a5ddeb8552 100644
--- a/lldb/include/lldb/Utility/Scalar.h
+++ b/lldb/include/lldb/Utility/Scalar.h
@@ -101,7 +101,7 @@ class Scalar {
 
   const char *GetTypeAsCString() const { return GetValueTypeAsCString(m_type); }
 
-  void GetValue(Stream *s, bool show_type) const;
+  void GetValue(Stream &s, bool show_type) const;
 
   bool IsValid() const { return (m_type >= e_int) && (m_type <= e_float); }
 
diff --git a/lldb/source/Core/Value.cpp b/lldb/source/Core/Value.cpp
index 8efcfd3b4a1adac..995cc934c82044a 100644
--- a/lldb/source/Core/Value.cpp
+++ b/lldb/source/Core/Value.cpp
@@ -98,7 +98,9 @@ void Value::AppendBytes(const void *bytes, int len) {
 }
 
 void Value::Dump(Stream *strm) {
-  m_value.GetValue(strm, true);
+  if (!strm)
+    return;
+  m_value.GetValue(*strm, true);
   strm->Printf(", value_type = %s, context = %p, context_type = %s",
                Value::GetValueTypeAsCString(m_value_type), m_context,
                Value::GetContextTypeAsCString(m_context_type));
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index dcff53ff843f328..e1275ce711fc172 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1773,7 +1773,7 @@ CommandInterpreter::PreprocessToken(std::string &expr_str) {
 
       StreamString value_strm;
       const bool show_type = false;
-      scalar.GetValue(&value_strm, show_type);
+      scalar.GetValue(value_strm, show_type);
       size_t value_string_size = value_strm.GetSize();
       if (value_string_size) {
         expr_str = value_strm.GetData();
diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 791c0fb74352913..5ad68065bce1b76 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -153,20 +153,20 @@ bool Scalar::IsZero() const {
   return false;
 }
 
-void Scalar::GetValue(Stream *s, bool show_type) const {
+void Scalar::GetValue(Stream &s, bool show_type) const {
   if (show_type)
-    s->Printf("(%s) ", GetTypeAsCString());
+    s.Printf("(%s) ", GetTypeAsCString());
 
   switch (m_type) {
   case e_void:
     break;
   case e_int:
-    s->PutCString(llvm::toString(m_integer, 10));
+    s.PutCString(llvm::toString(m_integer, 10));
     break;
   case e_float:
     llvm::SmallString<24> string;
     m_float.toString(string);
-    s->PutCString(string);
+    s.PutCString(string);
     break;
   }
 }
@@ -894,6 +894,6 @@ bool Scalar::SetBit(uint32_t bit) {
 
 llvm::raw_ostream &lldb_private::operator<<(llvm::raw_ostream &os, const Scalar &scalar) {
   StreamString s;
-  scalar.GetValue(&s, /*show_type*/ true);
+  scalar.GetValue(s, /*show_type*/ true);
   return os << s.GetString();
 }
diff --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp
index 17dfc689dd4e87b..29a4bcd356f1135 100644
--- a/lldb/unittests/Utility/ScalarTest.cpp
+++ b/lldb/unittests/Utility/ScalarTest.cpp
@@ -241,7 +241,7 @@ TEST(ScalarTest, ExtractBitfield) {
 
 template <typename T> static std::string ScalarGetValue(T value) {
   StreamString stream;
-  Scalar(value).GetValue(&stream, false);
+  Scalar(value).GetValue(stream, false);
   return std::string(stream.GetString());
 }
 

[medismailben]

LGTM!

[clayborg]

There are probably a lot of functions that could be converted from a "Stream *" to "Stream &".

[bulbazord]

There are probably a lot of functions that could be converted from a "Stream *" to "Stream &".

Yes, when I have a spare 5-10 minutes I pick a function and convert it. I don't want to end up with a huge diff that is difficult to merge downstream so I do it piecemeal.