Skip to content

Commit

Permalink
An SBValue whose underlying ValueObject has no valid value, but does
Browse files Browse the repository at this point in the history
hold an error should:

(a) return false for IsValid, since that's the current behavior and is
    a convenient way to check "should I get the value for this".
(b) preserve the error when an SBValue is made from it, and print the
    error in the ValueObjectPrinter.

Make that happen.

Differential Revision: https://reviews.llvm.org/D144664
  • Loading branch information
jimingham committed Mar 1, 2023
1 parent c2d6cc9 commit e8a2fd5
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
11 changes: 10 additions & 1 deletion lldb/source/API/SBValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ class ValueImpl {
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();

Expand Down Expand Up @@ -1047,7 +1051,12 @@ lldb::SBFrame SBValue::GetFrame() {
}

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();
}
Expand Down
11 changes: 11 additions & 0 deletions lldb/source/Core/ValueObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,15 @@ 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());

Expand Down Expand Up @@ -1374,6 +1383,8 @@ bool ValueObject::DumpPrintableRepresentation(
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());
Expand Down
12 changes: 12 additions & 0 deletions lldb/source/DataFormatters/ValueObjectPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,18 @@ void ValueObjectPrinter::Init(
}

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;

Expand Down

0 comments on commit e8a2fd5

Please sign in to comment.