[Lldb-commits] [PATCH] D144664: Preserve SBValue's that only have an Error state

[Jim Ingham via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe8a2fd5e7be2: An SBValue whose underlying ValueObject has no valid value, but does (authored by jingham).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144664/new/

https://reviews.llvm.org/D144664

Files:
  lldb/source/API/SBValue.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp


Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===================================================================
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -71,6 +71,18 @@
 }
 
 bool ValueObjectPrinter::PrintValueObject() {
+  if (!m_orig_valobj)
+    return false;
+
+  // If the incoming ValueObject is in an error state, the best we're going to 
+  // get out of it is its type.  But if we don't even have that, just print
+  // the error and exit early.
+  if (m_orig_valobj->GetError().Fail() 
+      && !m_orig_valobj->GetCompilerType().IsValid()) {
+    m_stream->Printf("Error: '%s'", m_orig_valobj->GetError().AsCString());
+    return true;
+  }
+   
   if (!GetMostSpecializedValue() || m_valobj == nullptr)
     return false;
 
Index: lldb/source/Core/ValueObject.cpp
===================================================================
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -1173,6 +1173,15 @@
     Stream &s, ValueObjectRepresentationStyle val_obj_display,
     Format custom_format, PrintableRepresentationSpecialCases special,
     bool do_dump_error) {
+    
+  // If the ValueObject has an error, we might end up dumping the type, which
+  // is useful, but if we don't even have a type, then don't examine the object
+  // further as that's not meaningful, only the error is.
+  if (m_error.Fail() && !GetCompilerType().IsValid()) {
+    if (do_dump_error)
+      s.Printf("<%s>", m_error.AsCString());
+    return false;
+  }
 
   Flags flags(GetTypeInfo());
 
@@ -1374,6 +1383,8 @@
     if (!str.empty())
       s << str;
     else {
+      // We checked for errors at the start, but do it again here in case
+      // realizing the value for dumping produced an error.
       if (m_error.Fail()) {
         if (do_dump_error)
           s.Printf("<%s>", m_error.AsCString());
Index: lldb/source/API/SBValue.cpp
===================================================================
--- lldb/source/API/SBValue.cpp
+++ lldb/source/API/SBValue.cpp
@@ -114,6 +114,10 @@
     lldb::ValueObjectSP value_sp = m_valobj_sp;
 
     Target *target = value_sp->GetTargetSP().get();
+    // If this ValueObject holds an error, then it is valuable for that.
+    if (value_sp->GetError().Fail()) 
+      return value_sp;
+
     if (!target)
       return ValueObjectSP();
 
@@ -1047,7 +1051,12 @@
 }
 
 lldb::ValueObjectSP SBValue::GetSP(ValueLocker &locker) const {
-  if (!m_opaque_sp || !m_opaque_sp->IsValid()) {
+  // IsValid means that the SBValue has a value in it.  But that's not the
+  // only time that ValueObjects are useful.  We also want to return the value
+  // if there's an error state in it.
+  if (!m_opaque_sp || (!m_opaque_sp->IsValid() 
+      && (m_opaque_sp->GetRootSP() 
+          && !m_opaque_sp->GetRootSP()->GetError().Fail()))) {
     locker.GetError().SetErrorString("No value");
     return ValueObjectSP();
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144664.501332.patch
Type: text/x-patch
Size: 2971 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230301/62bba620/attachment.bin>