jingham created this revision.
jingham added reviewers: JDevlieghere, clayborg, labath, jasonmolenda.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
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.
Repository:
rG LLVM Github Monorepo
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();
}
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();
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
