Preserve SBValue's that only have an Error state

[jingham]

Summary

SBValues carry their own errors with them (SBValue::GetError). That means that even if an SBValue was made in a situation where we couldn't actually make a value (e.g. you called lldb.frame.EvaluateExpression while the target is running) the error is still useful. We were using the IsValid check to decide whether to actually return the underlying ValueObject to the SBValue.

This could have been implemented two ways. One is to add "Has an error" to the conditions that would cause IsValid to return true. I chose not to do that because previously you could use IsValid to mean "has a valid Value" and so this would have been a semantic change and could break scripts.

Instead I just add the few "or has error" checks needed to preserve "error only SBValues" in the SBValue interface, and fixed up a couple places in ValueObject printing where we needed to print the error if the ValueObject had no other useful state.

This patch doesn't have a test. I have a follow-on commit which is what motivated this change and the test for that was what necessitated, and will verify, this change.

lldb/source/API/SBValue.cpp

lldb::ValueObjectSP GetSP(Process::StopLocker &stop_locker,
     if (!m_valobj_sp) {
       error.SetErrorString("invalid value object");
       return m_valobj_sp;
     }
 
     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();
 
     lock = std::unique_lock<std::recursive_mutex>(target->GetAPIMutex());
 
     ProcessSP process_sp(value_sp->GetProcessSP());
     if (process_sp && !stop_locker.TryLock(&process_sp->GetRunLock())) {
       // We don't allow people to play around with ValueObject if the process
if (m_opaque_sp) {
     frame_sp = m_opaque_sp->GetFrameSP();
     sb_frame.SetFrameSP(frame_sp);
   }
 
   return sb_frame;
 }
 
 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();
   }
   return locker.GetLockedSP(*m_opaque_sp.get());
 }
 
 lldb::ValueObjectSP SBValue::GetSP() const {
   LLDB_INSTRUMENT_VA(this);

lldb/source/Core/ValueObject.cpp

bool ValueObject::HasSpecialPrintableRepresentation(
   }
   return false;
 }
 
 bool ValueObject::DumpPrintableRepresentation(
     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());
 
   bool allow_special =
       (special == ValueObject::PrintableRepresentationSpecialCases::eAllow);
   const bool only_special = false;
 
   if (allow_special) {
     if (flags.AnySet(eTypeIsArray | eTypeIsPointer) &&
if (str.empty()) {
         } else
           str = GetValueAsCString();
       }
     }
 
     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());
         else
           return false;
       } else if (val_obj_display == eValueObjectRepresentationStyleSummary)
         s.PutCString("<no summary available>");
       else if (val_obj_display == eValueObjectRepresentationStyleValue)

lldb/source/DataFormatters/ValueObjectPrinter.cpp

void ValueObjectPrinter::Init(
   m_val_summary_ok = false;
   m_printed_instance_pointers =
       printed_instance_pointers
           ? printed_instance_pointers
           : InstancePointersSetSP(new InstancePointersSet());
 }
 
 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;
 
   if (ShouldPrintValueObject()) {
     PrintLocationIfNeeded();
     m_stream->Indent();
 
     PrintDecl();