Skip to content

Commit 9a172d1

Browse files
adrian-prantladrian-prantl
committed
Convert ValueObject::Dump() to return llvm::Error() (NFCish)
This change by itself has no measurable effect on the LLDB testsuite. I'm making it in preparation for threading through more errors in the Swift language plugin. (cherry picked from commit d1bc75c) Conflicts: lldb/source/API/SBValue.cpp lldb/source/Core/FormatEntity.cpp lldb/source/Core/ValueObject.cpp lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp
1 parent ed54117 commit 9a172d1

19 files changed

+107
-43
lines changed

‎lldb/include/lldb/Core/ValueObject.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -657,9 +657,9 @@ class ValueObject {
657657

658658
virtual SymbolContextScope *GetSymbolContextScope();
659659

660-
void Dump(Stream &s);
660+
llvm::Error Dump(Stream &s);
661661

662-
void Dump(Stream &s, const DumpValueObjectOptions &options);
662+
llvm::Error Dump(Stream &s, const DumpValueObjectOptions &options);
663663

664664
static lldb::ValueObjectSP
665665
CreateValueObjectFromExpression(llvm::StringRef name,

‎lldb/include/lldb/DataFormatters/ValueObjectPrinter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class ValueObjectPrinter {
3434

3535
~ValueObjectPrinter() = default;
3636

37-
bool PrintValueObject();
37+
llvm::Error PrintValueObject();
3838

3939
protected:
4040
typedef std::set<uint64_t> InstancePointersSet;

‎lldb/source/API/SBValue.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,11 +1210,14 @@ bool SBValue::GetDescription(SBStream &description) {
12101210

12111211
ValueLocker locker;
12121212
lldb::ValueObjectSP value_sp(GetSP(locker));
1213-
if (value_sp)
1214-
value_sp->Dump(strm);
1215-
else
1213+
if (value_sp) {
1214+
if (llvm::Error error = value_sp->Dump(strm)) {
1215+
strm << "error: " << toString(std::move(error));
1216+
return false;
1217+
}
1218+
} else {
12161219
strm.PutCString("No value");
1217-
1220+
}
12181221
return true;
12191222
}
12201223

‎lldb/source/Breakpoint/Watchpoint.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,9 @@ bool Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const {
301301
.SetHideRootType(true)
302302
.SetHideRootName(true)
303303
.SetHideName(true);
304-
m_old_value_sp->Dump(strm, options);
304+
if (llvm::Error error = m_old_value_sp->Dump(strm, options))
305+
strm << "error: " << toString(std::move(error));
306+
305307
if (strm.GetData())
306308
values_ss.Printf("old value: %s", strm.GetData());
307309
}
@@ -324,7 +326,9 @@ bool Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const {
324326
.SetHideRootType(true)
325327
.SetHideRootName(true)
326328
.SetHideName(true);
327-
m_new_value_sp->Dump(strm, options);
329+
if (llvm::Error error = m_new_value_sp->Dump(strm, options))
330+
strm << "error: " << toString(std::move(error));
331+
328332
if (strm.GetData())
329333
values_ss.Printf("new value: %s", strm.GetData());
330334
}

‎lldb/source/Commands/CommandObjectDWIMPrint.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,17 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
142142
auto dump_val_object = [&](ValueObject &valobj) {
143143
if (is_po) {
144144
StreamString temp_result_stream;
145-
valobj.Dump(temp_result_stream, dump_options);
145+
if (llvm::Error error = valobj.Dump(temp_result_stream, dump_options)) {
146+
result.AppendError(toString(std::move(error)));
147+
return;
148+
}
146149
llvm::StringRef output = temp_result_stream.GetString();
147150
maybe_add_hint(output);
148151
result.GetOutputStream() << output;
149152
} else {
150-
valobj.Dump(result.GetOutputStream(), dump_options);
153+
if (llvm::Error error =
154+
valobj.Dump(result.GetOutputStream(), dump_options))
155+
result.AppendError(toString(std::move(error)));
151156
}
152157
};
153158

‎lldb/source/Commands/CommandObjectExpression.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,11 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
497497
options.SetVariableFormatDisplayLanguage(
498498
result_valobj_sp->GetPreferredDisplayLanguage());
499499

500-
result_valobj_sp->Dump(output_stream, options);
500+
if (llvm::Error error =
501+
result_valobj_sp->Dump(output_stream, options)) {
502+
result.AppendError(toString(std::move(error)));
503+
return false;
504+
}
501505

502506
if (suppress_result)
503507
if (auto result_var_sp =

‎lldb/source/Commands/CommandObjectFrame.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,8 @@ class CommandObjectFrameDiagnose : public CommandObjectParsed {
182182
assert(valobj_sp.get() && "Must have a valid ValueObject to print");
183183
ValueObjectPrinter printer(*valobj_sp, &result.GetOutputStream(),
184184
options);
185-
printer.PrintValueObject();
185+
if (llvm::Error error = printer.PrintValueObject())
186+
result.AppendError(toString(std::move(error)));
186187
}
187188

188189
CommandOptions m_options;
@@ -609,7 +610,9 @@ may even involve JITing and running code in the target program.)");
609610
show_module))
610611
s.PutCString(": ");
611612
}
612-
valobj_sp->Dump(result.GetOutputStream(), options);
613+
auto &strm = result.GetOutputStream();
614+
if (llvm::Error error = valobj_sp->Dump(strm, options))
615+
result.AppendError(toString(std::move(error)));
613616
}
614617
}
615618
} else {
@@ -651,7 +654,8 @@ may even involve JITing and running code in the target program.)");
651654
Stream &output_stream = result.GetOutputStream();
652655
options.SetRootValueObjectName(
653656
valobj_sp->GetParent() ? entry.c_str() : nullptr);
654-
valobj_sp->Dump(output_stream, options);
657+
if (llvm::Error error = valobj_sp->Dump(output_stream, options))
658+
result.AppendError(toString(std::move(error)));
655659
} else {
656660
if (auto error_cstr = error.AsCString(nullptr))
657661
result.AppendError(error_cstr);
@@ -702,7 +706,9 @@ may even involve JITing and running code in the target program.)");
702706
valobj_sp->GetPreferredDisplayLanguage());
703707
options.SetRootValueObjectName(
704708
var_sp ? var_sp->GetName().AsCString() : nullptr);
705-
valobj_sp->Dump(result.GetOutputStream(), options);
709+
if (llvm::Error error =
710+
valobj_sp->Dump(result.GetOutputStream(), options))
711+
result.AppendError(toString(std::move(error)));
706712
}
707713
}
708714
}
@@ -723,7 +729,9 @@ may even involve JITing and running code in the target program.)");
723729
options.SetVariableFormatDisplayLanguage(
724730
rec_value_sp->GetPreferredDisplayLanguage());
725731
options.SetRootValueObjectName(rec_value_sp->GetName().AsCString());
726-
rec_value_sp->Dump(result.GetOutputStream(), options);
732+
if (llvm::Error error =
733+
rec_value_sp->Dump(result.GetOutputStream(), options))
734+
result.AppendError(toString(std::move(error)));
727735
}
728736
}
729737
}

‎lldb/source/Commands/CommandObjectMemory.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,10 @@ class CommandObjectMemoryRead : public CommandObjectParsed {
815815
DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions(
816816
eLanguageRuntimeDescriptionDisplayVerbosityFull, format));
817817

818-
valobj_sp->Dump(*output_stream_p, options);
818+
if (llvm::Error error = valobj_sp->Dump(*output_stream_p, options)) {
819+
result.AppendError(toString(std::move(error)));
820+
return;
821+
}
819822
} else {
820823
result.AppendErrorWithFormat(
821824
"failed to create a value object for: (%s) %s\n",

‎lldb/source/Commands/CommandObjectTarget.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -812,7 +812,8 @@ class CommandObjectTargetVariable : public CommandObjectParsed {
812812

813813
options.SetRootValueObjectName(root_name);
814814

815-
valobj_sp->Dump(s, options);
815+
if (llvm::Error error = valobj_sp->Dump(s, options))
816+
s << "error: " << toString(std::move(error));
816817
}
817818

818819
static size_t GetVariableCallback(void *baton, const char *name,

‎lldb/source/Commands/CommandObjectThread.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,7 +1367,10 @@ class CommandObjectThreadException : public CommandObjectIterateOverThreads {
13671367
Stream &strm = result.GetOutputStream();
13681368
ValueObjectSP exception_object_sp = thread_sp->GetCurrentException();
13691369
if (exception_object_sp) {
1370-
exception_object_sp->Dump(strm);
1370+
if (llvm::Error error = exception_object_sp->Dump(strm)) {
1371+
result.AppendError(toString(std::move(error)));
1372+
return false;
1373+
}
13711374
}
13721375

13731376
ThreadSP exception_thread_sp = thread_sp->GetCurrentExceptionBacktrace();
@@ -1419,9 +1422,12 @@ class CommandObjectThreadSiginfo : public CommandObjectIterateOverThreads {
14191422
return false;
14201423
}
14211424
ValueObjectSP exception_object_sp = thread_sp->GetSiginfoValue();
1422-
if (exception_object_sp)
1423-
exception_object_sp->Dump(strm);
1424-
else
1425+
if (exception_object_sp) {
1426+
if (llvm::Error error = exception_object_sp->Dump(strm)) {
1427+
result.AppendError(toString(std::move(error)));
1428+
return false;
1429+
}
1430+
} else
14251431
strm.Printf("(no siginfo)\n");
14261432
strm.PutChar('\n');
14271433

‎lldb/source/Core/DumpRegisterValue.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ static void dump_type_value(lldb_private::CompilerType &fields_type, T value,
5454
};
5555
dump_options.SetChildPrintingDecider(decider).SetHideRootType(true);
5656

57-
vobj_sp->Dump(strm, dump_options);
57+
if (llvm::Error error = vobj_sp->Dump(strm, dump_options))
58+
strm << "error: " << toString(std::move(error));
5859
}
5960

6061
void lldb_private::DumpRegisterValue(const RegisterValue &reg_val, Stream &s,

‎lldb/source/Core/FormatEntity.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1357,6 +1357,10 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
13571357
}
13581358
// END SWIFT
13591359
if (return_valobj_sp) {
1360+
if (llvm::Error error = return_valobj_sp->Dump(s)) {
1361+
s << "error: " << toString(std::move(error));
1362+
return false;
1363+
}
13601364
return true;
13611365
}
13621366
}
@@ -1373,7 +1377,11 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
13731377
ExpressionVariableSP expression_var_sp =
13741378
StopInfo::GetExpressionVariable(stop_info_sp);
13751379
if (expression_var_sp && expression_var_sp->GetValueObject()) {
1376-
expression_var_sp->GetValueObject()->Dump(s);
1380+
if (llvm::Error error =
1381+
expression_var_sp->GetValueObject()->Dump(s)) {
1382+
s << "error: " << toString(std::move(error));
1383+
return false;
1384+
}
13771385
return true;
13781386
}
13791387
}

‎lldb/source/Core/ValueObject.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2655,14 +2655,17 @@ ValueObjectSP ValueObject::GetValueForExpressionPath_Impl(
26552655
}
26562656
}
26572657

2658-
void ValueObject::Dump(Stream &s) { Dump(s, DumpValueObjectOptions(*this)); }
2658+
llvm::Error ValueObject::Dump(Stream &s) {
2659+
return Dump(s, DumpValueObjectOptions(*this));
2660+
}
26592661

2660-
void ValueObject::Dump(Stream &s, const DumpValueObjectOptions &options) {
2662+
llvm::Error ValueObject::Dump(Stream &s,
2663+
const DumpValueObjectOptions &options) {
26612664
#ifdef LLDB_ENABLE_SWIFT
26622665
auto swift_scratch_ctx_lock = SwiftScratchContextLock(GetSwiftExeCtx(*this));
26632666
#endif // LLDB_ENABLE_SWIFT
26642667
ValueObjectPrinter printer(*this, &s, options);
2665-
printer.PrintValueObject();
2668+
return printer.PrintValueObject();
26662669
}
26672670

26682671
ValueObjectSP ValueObject::CreateConstantValue(ConstString name) {

‎lldb/source/DataFormatters/ValueObjectPrinter.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,13 @@ void ValueObjectPrinter::Init(
6767
SetupMostSpecializedValue();
6868
}
6969

70-
bool ValueObjectPrinter::PrintValueObject() {
70+
llvm::Error ValueObjectPrinter::PrintValueObject() {
7171
// If the incoming ValueObject is in an error state, the best we're going to
7272
// get out of it is its type. But if we don't even have that, just print
7373
// the error and exit early.
7474
if (m_orig_valobj.GetError().Fail() &&
75-
!m_orig_valobj.GetCompilerType().IsValid()) {
76-
m_stream->Printf("Error: '%s'", m_orig_valobj.GetError().AsCString());
77-
return true;
78-
}
75+
!m_orig_valobj.GetCompilerType().IsValid())
76+
return m_orig_valobj.GetError().ToError();
7977

8078
if (ShouldPrintValueObject()) {
8179
PrintLocationIfNeeded();
@@ -95,7 +93,7 @@ bool ValueObjectPrinter::PrintValueObject() {
9593
else
9694
m_stream->EOL();
9795

98-
return true;
96+
return llvm::Error::success();
9997
}
10098

10199
ValueObject &ValueObjectPrinter::GetMostSpecializedValue() {
@@ -639,7 +637,13 @@ void ValueObjectPrinter::PrintChild(
639637
ValueObjectPrinter child_printer(*(child_sp.get()), m_stream, child_options,
640638
ptr_depth, m_curr_depth + 1,
641639
m_printed_instance_pointers);
642-
child_printer.PrintValueObject();
640+
llvm::Error error = child_printer.PrintValueObject();
641+
if (error) {
642+
if (m_stream)
643+
*m_stream << "error: " << toString(std::move(error));
644+
else
645+
llvm::consumeError(std::move(error));
646+
}
643647
}
644648
}
645649

‎lldb/source/Plugins/REPL/Clang/ClangREPL.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ bool ClangREPL::PrintOneVariable(Debugger &debugger,
9595
if (m_implicit_expr_result_regex.Execute(var->GetName().GetStringRef()))
9696
return true;
9797
}
98-
valobj_sp->Dump(*output_sp);
98+
if (llvm::Error error = valobj_sp->Dump(*output_sp))
99+
*output_sp << "error: " << toString(std::move(error));
100+
99101
return true;
100102
}
101103

‎lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ def test_nested_values(self):
122122
"""Test dwim-print with nested values (structs, etc)."""
123123
self.build()
124124
lldbutil.run_to_source_breakpoint(
125-
self, "// break here", lldb.SBFileSpec("main.c")
125+
self, "break here", lldb.SBFileSpec("main.c")
126126
)
127127
self.runCmd("settings set auto-one-line-summaries false")
128128
self._expect_cmd(f"dwim-print s", "frame variable")
@@ -132,7 +132,7 @@ def test_summary_strings(self):
132132
"""Test dwim-print with nested values (structs, etc)."""
133133
self.build()
134134
lldbutil.run_to_source_breakpoint(
135-
self, "// break here", lldb.SBFileSpec("main.c")
135+
self, "break here", lldb.SBFileSpec("main.c")
136136
)
137137
self.runCmd("settings set auto-one-line-summaries false")
138138
self.runCmd("type summary add -e -s 'stub summary' Structure")
@@ -143,18 +143,26 @@ def test_void_result(self):
143143
"""Test dwim-print does not surface an error message for void expressions."""
144144
self.build()
145145
lldbutil.run_to_source_breakpoint(
146-
self, "// break here", lldb.SBFileSpec("main.c")
146+
self, "break here", lldb.SBFileSpec("main.c")
147147
)
148148
self.expect("dwim-print (void)15", matching=False, patterns=["(?i)error"])
149149

150150
def test_preserves_persistent_variables(self):
151151
"""Test dwim-print does not delete persistent variables."""
152152
self.build()
153153
lldbutil.run_to_source_breakpoint(
154-
self, "// break here", lldb.SBFileSpec("main.c")
154+
self, "break here", lldb.SBFileSpec("main.c")
155155
)
156156
self.expect("dwim-print int $i = 15")
157157
# Run the same expression twice and verify success. This ensures the
158158
# first command does not delete the persistent variable.
159159
for _ in range(2):
160160
self.expect("dwim-print $i", startstr="(int) 15")
161+
162+
def test_missing_type(self):
163+
"""The expected output of po opaque is its address (no error)"""
164+
self.build()
165+
lldbutil.run_to_source_breakpoint(
166+
self, "break here", lldb.SBFileSpec("main.c")
167+
)
168+
self.expect("dwim-print -O -- opaque", substrs=['0x'])

‎lldb/test/API/commands/dwim-print/main.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@ struct Structure {
22
int number;
33
};
44

5+
struct Opaque;
6+
int puts(const char *s);
7+
58
int main(int argc, char **argv) {
69
struct Structure s;
710
s.number = 30;
8-
// break here
11+
struct Opaque *opaque = &s;
12+
puts("break here");
913
return 0;
1014
}

‎lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
# RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj -o %t
55
# RUN: %lldb %t -o "target variable x" -o exit 2>&1 | FileCheck %s
66

7-
# CHECK: 'Unable to determine byte size.'
7+
# CHECK: Unable to determine byte size.
88

99
# This tests a fix for a crash. If things are working we don't get a segfault.
1010

‎lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
# RUN: ld.lld %t.o -o %t
55
# RUN: %lldb %t -o "target variable e" -b | FileCheck %s
66

7-
# CHECK: Error: 'Unable to determine byte size.'
7+
# CHECK: error: Unable to determine byte size.
88

99
.type e,@object # @e
1010
.section .rodata,"a",@progbits

0 commit comments

Comments
 (0)