[lldb] Fix a regression in SBValue::GetObjectDescription()

[adrian-prantl]

The old behavior was to return a null string in the error case,when refactoring the error handling I thought it would be a good idea to print the error in the description, but that breaks clients that try to print a description first and then do something else in the error case. The API is not great but it's clear that in-band errors are also not a good idea.

rdar://133956263

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)

Changes

The old behavior was to return a null string in the error case,when refactoring the error handling I thought it would be a good idea to print the error in the description, but that breaks clients that try to print a description first and then do something else in the error case. The API is not great but it's clear that in-band errors are also not a good idea.

rdar://133956263

---

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

2 Files Affected:

• (modified) lldb/source/API/SBValue.cpp (+4-2)
• (modified) lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py (+6-1)

diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index b35c82250d6ba1..a707b9aa7589c7 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -380,8 +380,10 @@ const char *SBValue::GetObjectDescription() {
     return nullptr;
 
   llvm::Expected<std::string> str = value_sp->GetObjectDescription();
-  if (!str)
-    return ConstString("error: " + toString(str.takeError())).AsCString();
+  if (!str) {
+    llvm::consumeError(str.takeError());
+    return nullptr;
+  }
   return ConstString(*str).AsCString();
 }
 
diff --git a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index b9b5bffb87e817..a68e921ef02dba 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -194,7 +194,12 @@ def test_error_type(self):
         frame = thread.GetFrameAtIndex(0)
         value = frame.EvaluateExpression('#error("I am error.")')
         error = value.GetError()
-        self.assertEqual(error.GetType(), lldb.eErrorTypeExpression)
+        self.assertEqual(error.GetType(), lldb.eErrorTypeExpression);
+        value = frame.FindVariable('f')
+        self.assertTrue(value.IsValid());
+        desc = value.GetObjectDescription()
+        self.assertEqual(desc, None);
+
 
     def test_command_expr_sbdata(self):
         """Test the structured diagnostics data"""

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[jimingham]

You're right, that isn't a great API. But at least this way you can tell whether to use the object description w/o having to detect an error string in the output.
At some point we should add a better API. I think just an overload that takes an SBError would be fine. But that's not necessary for this patch.