https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/81314
>From 1d24f118373af64a212893366051b7da1b02d791 Mon Sep 17 00:00:00 2001 From: Jim Ingham <jing...@apple.com> Date: Fri, 9 Feb 2024 12:09:27 -0800 Subject: [PATCH 1/2] It makes no sense to make a ValueObjectPrinter with a null ValueObject, make that not possible. Also, be more principled about ensuring that the ValueObject we choose to print from the original ValueObject can't get set to null. --- .../lldb/DataFormatters/ValueObjectPrinter.h | 34 ++- lldb/source/Commands/CommandObjectFrame.cpp | 5 +- lldb/source/Core/ValueObject.cpp | 2 +- lldb/source/DataFormatters/TypeSummary.cpp | 5 +- .../DataFormatters/ValueObjectPrinter.cpp | 222 +++++++++--------- 5 files changed, 146 insertions(+), 122 deletions(-) diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h index 2b3936eaa707f2..91963b4d126eba 100644 --- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h +++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h @@ -22,9 +22,9 @@ namespace lldb_private { class ValueObjectPrinter { public: - ValueObjectPrinter(ValueObject *valobj, Stream *s); + ValueObjectPrinter(ValueObject &valobj, Stream *s); - ValueObjectPrinter(ValueObject *valobj, Stream *s, + ValueObjectPrinter(ValueObject &valobj, Stream *s, const DumpValueObjectOptions &options); ~ValueObjectPrinter() = default; @@ -39,7 +39,7 @@ class ValueObjectPrinter { // only this class (and subclasses, if any) should ever be concerned with the // depth mechanism - ValueObjectPrinter(ValueObject *valobj, Stream *s, + ValueObjectPrinter(ValueObject &valobj, Stream *s, const DumpValueObjectOptions &options, const DumpValueObjectOptions::PointerDepth &ptr_depth, uint32_t curr_depth, @@ -47,13 +47,27 @@ class ValueObjectPrinter { // we should actually be using delegating constructors here but some versions // of GCC still have trouble with those - void Init(ValueObject *valobj, Stream *s, + void Init(ValueObject &valobj, Stream *s, const DumpValueObjectOptions &options, const DumpValueObjectOptions::PointerDepth &ptr_depth, uint32_t curr_depth, InstancePointersSetSP printed_instance_pointers); - bool GetMostSpecializedValue(); + /// Cache the ValueObject we are actually going to print. If this + /// ValueObject has a Dynamic type, we return that, if either the original + /// ValueObject or its Dynamic type has a Synthetic provider, return that. + /// This will never return an empty ValueObject, since we use the ValueObject + /// to carry errors. + /// Note, this gets called when making the printer object, and uses the + /// use dynamic and use synthetic settings of the ValueObject being printed, + /// so changes made to these settings won't affect already made + /// ValueObjectPrinters. SetupMostSpecializedValue(); + + /// Access the cached "most specialized value" - that is the one to use for + /// printing the value object's value. However, be sure to use + /// GetValueForChildGeneration when you are generating the children of this + /// value. + ValueObject &GetMostSpecializedValue(); const char *GetDescriptionForDisplay(); @@ -95,13 +109,13 @@ class ValueObjectPrinter { bool ShouldExpandEmptyAggregates(); - ValueObject *GetValueObjectForChildrenGeneration(); + ValueObject &GetValueObjectForChildrenGeneration(); void PrintChildrenPreamble(bool value_printed, bool summary_printed); void PrintChildrenPostamble(bool print_dotdotdot); - lldb::ValueObjectSP GenerateChild(ValueObject *synth_valobj, size_t idx); + lldb::ValueObjectSP GenerateChild(ValueObject &synth_valobj, size_t idx); void PrintChild(lldb::ValueObjectSP child_sp, const DumpValueObjectOptions::PointerDepth &curr_ptr_depth); @@ -121,8 +135,10 @@ class ValueObjectPrinter { private: bool ShouldShowName() const; - ValueObject *m_orig_valobj; - ValueObject *m_valobj; + ValueObject &m_orig_valobj; + ValueObject *m_cached_valobj; /// Cache the current "most specialized" value. + /// Don't use this directly, use + /// GetMostSpecializedValue. Stream *m_stream; DumpValueObjectOptions m_options; Flags m_type_flags; diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index 17a7e858b24e58..53886140fd2d6c 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -177,7 +177,10 @@ class CommandObjectFrameDiagnose : public CommandObjectParsed { DumpValueObjectOptions options; options.SetDeclPrintingHelper(helper); - ValueObjectPrinter printer(valobj_sp.get(), &result.GetOutputStream(), + // We've already handled the case where the value object sp is null, so + // this is just to make sure future changes don't skip that: + assert(valobj_sp.get() && "Must have a valid ValueObject to print"); + ValueObjectPrinter printer(*(valobj_sp.get()), &result.GetOutputStream(), options); printer.PrintValueObject(); } diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp index 9208047be36668..e80042826f7d64 100644 --- a/lldb/source/Core/ValueObject.cpp +++ b/lldb/source/Core/ValueObject.cpp @@ -2525,7 +2525,7 @@ ValueObjectSP ValueObject::GetValueForExpressionPath_Impl( void ValueObject::Dump(Stream &s) { Dump(s, DumpValueObjectOptions(*this)); } void ValueObject::Dump(Stream &s, const DumpValueObjectOptions &options) { - ValueObjectPrinter printer(this, &s, options); + ValueObjectPrinter printer(*this, &s, options); printer.PrintValueObject(); } diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp index c09ed31d0338fd..3707d2d879d33a 100644 --- a/lldb/source/DataFormatters/TypeSummary.cpp +++ b/lldb/source/DataFormatters/TypeSummary.cpp @@ -80,7 +80,10 @@ bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval, sc = frame->GetSymbolContext(lldb::eSymbolContextEverything); if (IsOneLiner()) { - ValueObjectPrinter printer(valobj, &s, DumpValueObjectOptions()); + // We've already checked the case of a NULL valobj above. Let's put in an + // assert here to make sure someone doesn't take that out: + assert(valobj && "Must have a valid ValueObject to summarize"); + ValueObjectPrinter printer(*valobj, &s, DumpValueObjectOptions()); printer.PrintChildrenOneLiner(HideNames(valobj)); retval = std::string(s.GetString()); return true; diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp index 074d0b648e6fa9..46e50a8d421a71 100644 --- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp +++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp @@ -18,39 +18,35 @@ using namespace lldb; using namespace lldb_private; -ValueObjectPrinter::ValueObjectPrinter(ValueObject *valobj, Stream *s) { - if (valobj) { - DumpValueObjectOptions options(*valobj); - Init(valobj, s, options, m_options.m_max_ptr_depth, 0, nullptr); - } else { - DumpValueObjectOptions options; - Init(valobj, s, options, m_options.m_max_ptr_depth, 0, nullptr); - } +ValueObjectPrinter::ValueObjectPrinter(ValueObject &valobj, Stream *s) + : m_orig_valobj(valobj) { + DumpValueObjectOptions options(valobj); + Init(valobj, s, options, m_options.m_max_ptr_depth, 0, nullptr); } -ValueObjectPrinter::ValueObjectPrinter(ValueObject *valobj, Stream *s, - const DumpValueObjectOptions &options) { +ValueObjectPrinter::ValueObjectPrinter(ValueObject &valobj, Stream *s, + const DumpValueObjectOptions &options) + : m_orig_valobj(valobj) { Init(valobj, s, options, m_options.m_max_ptr_depth, 0, nullptr); } ValueObjectPrinter::ValueObjectPrinter( - ValueObject *valobj, Stream *s, const DumpValueObjectOptions &options, + ValueObject &valobj, Stream *s, const DumpValueObjectOptions &options, const DumpValueObjectOptions::PointerDepth &ptr_depth, uint32_t curr_depth, - InstancePointersSetSP printed_instance_pointers) { + InstancePointersSetSP printed_instance_pointers) + : m_orig_valobj(valobj) { Init(valobj, s, options, ptr_depth, curr_depth, printed_instance_pointers); } void ValueObjectPrinter::Init( - ValueObject *valobj, Stream *s, const DumpValueObjectOptions &options, + ValueObject &valobj, Stream *s, const DumpValueObjectOptions &options, const DumpValueObjectOptions::PointerDepth &ptr_depth, uint32_t curr_depth, InstancePointersSetSP printed_instance_pointers) { - m_orig_valobj = valobj; - m_valobj = nullptr; + m_cached_valobj = nullptr; m_stream = s; m_options = options; m_ptr_depth = ptr_depth; m_curr_depth = curr_depth; - assert(m_orig_valobj && "cannot print a NULL ValueObject"); assert(m_stream && "cannot print to a NULL Stream"); m_should_print = eLazyBoolCalculate; m_is_nil = eLazyBoolCalculate; @@ -68,23 +64,18 @@ void ValueObjectPrinter::Init( printed_instance_pointers ? printed_instance_pointers : InstancePointersSetSP(new InstancePointersSet()); + SetupMostSpecializedValue(); } 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()); + 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(); @@ -107,66 +98,68 @@ bool ValueObjectPrinter::PrintValueObject() { return true; } -bool ValueObjectPrinter::GetMostSpecializedValue() { - if (m_valobj) - return true; - bool update_success = m_orig_valobj->UpdateValueIfNeeded(true); - if (!update_success) { - m_valobj = m_orig_valobj; - } else { - if (m_orig_valobj->IsDynamic()) { +ValueObject &ValueObjectPrinter::GetMostSpecializedValue() { + assert(m_cached_valobj && "ValueObjectPrinter must have a valid ValueObject"); + return *m_cached_valobj; +} + +void ValueObjectPrinter::SetupMostSpecializedValue() { + bool update_success = m_orig_valobj.UpdateValueIfNeeded(true); + // If we can't find anything better, we'll fall back on the original + // ValueObject. + m_cached_valobj = &m_orig_valobj; + if (update_success) { + if (m_orig_valobj.IsDynamic()) { if (m_options.m_use_dynamic == eNoDynamicValues) { - ValueObject *static_value = m_orig_valobj->GetStaticValue().get(); + ValueObject *static_value = m_orig_valobj.GetStaticValue().get(); if (static_value) - m_valobj = static_value; - else - m_valobj = m_orig_valobj; - } else - m_valobj = m_orig_valobj; + m_cached_valobj = static_value; + } } else { if (m_options.m_use_dynamic != eNoDynamicValues) { ValueObject *dynamic_value = - m_orig_valobj->GetDynamicValue(m_options.m_use_dynamic).get(); + m_orig_valobj.GetDynamicValue(m_options.m_use_dynamic).get(); if (dynamic_value) - m_valobj = dynamic_value; - else - m_valobj = m_orig_valobj; - } else - m_valobj = m_orig_valobj; + m_cached_valobj = dynamic_value; + } } - if (m_valobj->IsSynthetic()) { + if (m_cached_valobj->IsSynthetic()) { if (!m_options.m_use_synthetic) { - ValueObject *non_synthetic = m_valobj->GetNonSyntheticValue().get(); + ValueObject *non_synthetic = + m_cached_valobj->GetNonSyntheticValue().get(); if (non_synthetic) - m_valobj = non_synthetic; + m_cached_valobj = non_synthetic; } } else { if (m_options.m_use_synthetic) { - ValueObject *synthetic = m_valobj->GetSyntheticValue().get(); + ValueObject *synthetic = m_cached_valobj->GetSyntheticValue().get(); if (synthetic) - m_valobj = synthetic; + m_cached_valobj = synthetic; } } } - m_compiler_type = m_valobj->GetCompilerType(); + m_compiler_type = m_cached_valobj->GetCompilerType(); m_type_flags = m_compiler_type.GetTypeInfo(); - return true; + assert(m_cached_valobj && + "SetupMostSpecialized value must compute a valid ValueObject"); } const char *ValueObjectPrinter::GetDescriptionForDisplay() { - const char *str = m_valobj->GetObjectDescription(); + ValueObject &valobj = GetMostSpecializedValue(); + const char *str = valobj.GetObjectDescription(); if (!str) - str = m_valobj->GetSummaryAsCString(); + str = valobj.GetSummaryAsCString(); if (!str) - str = m_valobj->GetValueAsCString(); + str = valobj.GetValueAsCString(); return str; } const char *ValueObjectPrinter::GetRootNameForDisplay() { - const char *root_valobj_name = m_options.m_root_valobj_name.empty() - ? m_valobj->GetName().AsCString() - : m_options.m_root_valobj_name.c_str(); + const char *root_valobj_name = + m_options.m_root_valobj_name.empty() + ? GetMostSpecializedValue().GetName().AsCString() + : m_options.m_root_valobj_name.c_str(); return root_valobj_name ? root_valobj_name : ""; } @@ -181,14 +174,16 @@ bool ValueObjectPrinter::ShouldPrintValueObject() { bool ValueObjectPrinter::IsNil() { if (m_is_nil == eLazyBoolCalculate) - m_is_nil = m_valobj->IsNilReference() ? eLazyBoolYes : eLazyBoolNo; + m_is_nil = + GetMostSpecializedValue().IsNilReference() ? eLazyBoolYes : eLazyBoolNo; return m_is_nil == eLazyBoolYes; } bool ValueObjectPrinter::IsUninitialized() { if (m_is_uninit == eLazyBoolCalculate) - m_is_uninit = - m_valobj->IsUninitializedReference() ? eLazyBoolYes : eLazyBoolNo; + m_is_uninit = GetMostSpecializedValue().IsUninitializedReference() + ? eLazyBoolYes + : eLazyBoolNo; return m_is_uninit == eLazyBoolYes; } @@ -213,19 +208,20 @@ bool ValueObjectPrinter::IsAggregate() { bool ValueObjectPrinter::IsInstancePointer() { // you need to do this check on the value's clang type + ValueObject &valobj = GetMostSpecializedValue(); if (m_is_instance_ptr == eLazyBoolCalculate) - m_is_instance_ptr = (m_valobj->GetValue().GetCompilerType().GetTypeInfo() & + m_is_instance_ptr = (valobj.GetValue().GetCompilerType().GetTypeInfo() & eTypeInstanceIsPointer) != 0 ? eLazyBoolYes : eLazyBoolNo; - if ((eLazyBoolYes == m_is_instance_ptr) && m_valobj->IsBaseClass()) + if ((eLazyBoolYes == m_is_instance_ptr) && valobj.IsBaseClass()) m_is_instance_ptr = eLazyBoolNo; return m_is_instance_ptr == eLazyBoolYes; } bool ValueObjectPrinter::PrintLocationIfNeeded() { if (m_options.m_show_location) { - m_stream->Printf("%s: ", m_valobj->GetLocationAsCString()); + m_stream->Printf("%s: ", GetMostSpecializedValue().GetLocationAsCString()); return true; } return false; @@ -244,6 +240,8 @@ void ValueObjectPrinter::PrintDecl() { (m_curr_depth == 0 && !m_options.m_flat_output); StreamString typeName; + // Figure out which ValueObject we're acting on + ValueObject &valobj = GetMostSpecializedValue(); // always show the type at the root level if it is invalid if (show_type) { @@ -252,8 +250,8 @@ void ValueObjectPrinter::PrintDecl() { ConstString type_name; if (m_compiler_type.IsValid()) { type_name = m_options.m_use_type_display_name - ? m_valobj->GetDisplayTypeName() - : m_valobj->GetQualifiedTypeName(); + ? valobj.GetDisplayTypeName() + : valobj.GetQualifiedTypeName(); } else { // only show an invalid type name if the user explicitly triggered // show_type @@ -277,7 +275,7 @@ void ValueObjectPrinter::PrintDecl() { if (ShouldShowName()) { if (m_options.m_flat_output) - m_valobj->GetExpressionPath(varName); + valobj.GetExpressionPath(varName); else varName << GetRootNameForDisplay(); } @@ -289,7 +287,7 @@ void ValueObjectPrinter::PrintDecl() { // one for the ValueObject lldb::LanguageType lang_type = (m_options.m_varformat_language == lldb::eLanguageTypeUnknown) - ? m_valobj->GetPreferredDisplayLanguage() + ? valobj.GetPreferredDisplayLanguage() : m_options.m_varformat_language; if (Language *lang_plugin = Language::FindPlugin(lang_type)) { m_options.m_decl_printing_helper = lang_plugin->GetDeclPrintingHelper(); @@ -327,14 +325,15 @@ void ValueObjectPrinter::PrintDecl() { bool ValueObjectPrinter::CheckScopeIfNeeded() { if (m_options.m_scope_already_checked) return true; - return m_valobj->IsInScope(); + return GetMostSpecializedValue().IsInScope(); } TypeSummaryImpl *ValueObjectPrinter::GetSummaryFormatter(bool null_if_omitted) { if (!m_summary_formatter.second) { - TypeSummaryImpl *entry = m_options.m_summary_sp - ? m_options.m_summary_sp.get() - : m_valobj->GetSummaryFormat().get(); + TypeSummaryImpl *entry = + m_options.m_summary_sp + ? m_options.m_summary_sp.get() + : GetMostSpecializedValue().GetSummaryFormat().get(); if (m_options.m_omit_summary_depth > 0) entry = nullptr; @@ -357,18 +356,19 @@ void ValueObjectPrinter::GetValueSummaryError(std::string &value, std::string &summary, std::string &error) { lldb::Format format = m_options.m_format; + ValueObject &valobj = GetMostSpecializedValue(); // if I am printing synthetized elements, apply the format to those elements // only if (m_options.m_pointer_as_array) - m_valobj->GetValueAsCString(lldb::eFormatDefault, value); - else if (format != eFormatDefault && format != m_valobj->GetFormat()) - m_valobj->GetValueAsCString(format, value); + valobj.GetValueAsCString(lldb::eFormatDefault, value); + else if (format != eFormatDefault && format != valobj.GetFormat()) + valobj.GetValueAsCString(format, value); else { - const char *val_cstr = m_valobj->GetValueAsCString(); + const char *val_cstr = valobj.GetValueAsCString(); if (val_cstr) value.assign(val_cstr); } - const char *err_cstr = m_valobj->GetError().AsCString(); + const char *err_cstr = valobj.GetError().AsCString(); if (err_cstr) error.assign(err_cstr); @@ -378,7 +378,7 @@ void ValueObjectPrinter::GetValueSummaryError(std::string &value, if (IsNil()) { lldb::LanguageType lang_type = (m_options.m_varformat_language == lldb::eLanguageTypeUnknown) - ? m_valobj->GetPreferredDisplayLanguage() + ? valobj.GetPreferredDisplayLanguage() : m_options.m_varformat_language; if (Language *lang_plugin = Language::FindPlugin(lang_type)) { summary.assign(lang_plugin->GetNilReferenceSummaryString().str()); @@ -392,11 +392,11 @@ void ValueObjectPrinter::GetValueSummaryError(std::string &value, } else if (m_options.m_omit_summary_depth == 0) { TypeSummaryImpl *entry = GetSummaryFormatter(); if (entry) { - m_valobj->GetSummaryAsCString(entry, summary, - m_options.m_varformat_language); + valobj.GetSummaryAsCString(entry, summary, + m_options.m_varformat_language); } else { const char *sum_cstr = - m_valobj->GetSummaryAsCString(m_options.m_varformat_language); + valobj.GetSummaryAsCString(m_options.m_varformat_language); if (sum_cstr) summary.assign(sum_cstr); } @@ -431,16 +431,17 @@ bool ValueObjectPrinter::PrintValueAndSummaryIfNeeded(bool &value_printed, // this thing is nil (but show the value if the user passes a format // explicitly) TypeSummaryImpl *entry = GetSummaryFormatter(); + ValueObject &valobj = GetMostSpecializedValue(); const bool has_nil_or_uninitialized_summary = (IsNil() || IsUninitialized()) && !m_summary.empty(); if (!has_nil_or_uninitialized_summary && !m_value.empty() && (entry == nullptr || - (entry->DoesPrintValue(m_valobj) || + (entry->DoesPrintValue(&valobj) || m_options.m_format != eFormatDefault) || m_summary.empty()) && !m_options.m_hide_value) { if (m_options.m_hide_pointer_value && - IsPointerValue(m_valobj->GetCompilerType())) { + IsPointerValue(valobj.GetCompilerType())) { } else { if (ShouldShowName()) m_stream->PutChar(' '); @@ -470,7 +471,7 @@ bool ValueObjectPrinter::PrintObjectDescriptionIfNeeded(bool value_printed, m_stream->Printf(" "); const char *object_desc = nullptr; if (value_printed || summary_printed) - object_desc = m_valobj->GetObjectDescription(); + object_desc = GetMostSpecializedValue().GetObjectDescription(); else object_desc = GetDescriptionForDisplay(); if (object_desc && *object_desc) { @@ -523,8 +524,9 @@ bool ValueObjectPrinter::ShouldPrintChildren( return false; bool print_children = true; + ValueObject &valobj = GetMostSpecializedValue(); if (TypeSummaryImpl *type_summary = GetSummaryFormatter()) - print_children = type_summary->DoesPrintChildren(m_valobj); + print_children = type_summary->DoesPrintChildren(&valobj); // We will show children for all concrete types. We won't show pointer // contents unless a pointer depth has been specified. We won't reference @@ -537,7 +539,7 @@ bool ValueObjectPrinter::ShouldPrintChildren( // We have a pointer or reference whose value is an address. Make sure // that address is not NULL AddressType ptr_address_type; - if (m_valobj->GetPointerValue(&ptr_address_type) == 0) + if (valobj.GetPointerValue(&ptr_address_type) == 0) return false; const bool is_root_level = m_curr_depth == 0; @@ -565,8 +567,8 @@ bool ValueObjectPrinter::ShouldExpandEmptyAggregates() { return entry->DoesPrintEmptyAggregates(); } -ValueObject *ValueObjectPrinter::GetValueObjectForChildrenGeneration() { - return m_valobj; +ValueObject &ValueObjectPrinter::GetValueObjectForChildrenGeneration() { + return GetMostSpecializedValue(); } void ValueObjectPrinter::PrintChildrenPreamble(bool value_printed, @@ -612,7 +614,7 @@ void ValueObjectPrinter::PrintChild( if (does_consume_ptr_depth) ptr_depth = curr_ptr_depth.Decremented(); - ValueObjectPrinter child_printer(child_sp.get(), m_stream, child_options, + ValueObjectPrinter child_printer(*(child_sp.get()), m_stream, child_options, ptr_depth, m_curr_depth + 1, m_printed_instance_pointers); child_printer.PrintValueObject(); @@ -620,16 +622,17 @@ void ValueObjectPrinter::PrintChild( } uint32_t ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) { - ValueObject *synth_m_valobj = GetValueObjectForChildrenGeneration(); + ValueObject &synth_valobj = GetValueObjectForChildrenGeneration(); if (m_options.m_pointer_as_array) return m_options.m_pointer_as_array.m_element_count; - size_t num_children = synth_m_valobj->GetNumChildren(); + size_t num_children = synth_valobj.GetNumChildren(); print_dotdotdot = false; if (num_children) { - const size_t max_num_children = - m_valobj->GetTargetSP()->GetMaximumNumberOfChildrenToDisplay(); + const size_t max_num_children = GetMostSpecializedValue() + .GetTargetSP() + ->GetMaximumNumberOfChildrenToDisplay(); if (num_children > max_num_children && !m_options.m_ignore_cap) { print_dotdotdot = true; @@ -642,7 +645,8 @@ uint32_t ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) { void ValueObjectPrinter::PrintChildrenPostamble(bool print_dotdotdot) { if (!m_options.m_flat_output) { if (print_dotdotdot) { - m_valobj->GetTargetSP() + GetMostSpecializedValue() + .GetTargetSP() ->GetDebugger() .GetCommandInterpreter() .ChildrenTruncated(); @@ -655,7 +659,7 @@ void ValueObjectPrinter::PrintChildrenPostamble(bool print_dotdotdot) { bool ValueObjectPrinter::ShouldPrintEmptyBrackets(bool value_printed, bool summary_printed) { - ValueObject *synth_m_valobj = GetValueObjectForChildrenGeneration(); + ValueObject &synth_valobj = GetValueObjectForChildrenGeneration(); if (!IsAggregate()) return false; @@ -665,7 +669,7 @@ bool ValueObjectPrinter::ShouldPrintEmptyBrackets(bool value_printed, return false; } - if (synth_m_valobj->MightHaveChildren()) + if (synth_valobj.MightHaveChildren()) return true; if (m_val_summary_ok) @@ -679,25 +683,25 @@ static constexpr size_t PhysicalIndexForLogicalIndex(size_t base, size_t stride, return base + logical * stride; } -ValueObjectSP ValueObjectPrinter::GenerateChild(ValueObject *synth_valobj, +ValueObjectSP ValueObjectPrinter::GenerateChild(ValueObject &synth_valobj, size_t idx) { if (m_options.m_pointer_as_array) { // if generating pointer-as-array children, use GetSyntheticArrayMember - return synth_valobj->GetSyntheticArrayMember( + return synth_valobj.GetSyntheticArrayMember( PhysicalIndexForLogicalIndex( m_options.m_pointer_as_array.m_base_element, m_options.m_pointer_as_array.m_stride, idx), true); } else { // otherwise, do the usual thing - return synth_valobj->GetChildAtIndex(idx); + return synth_valobj.GetChildAtIndex(idx); } } void ValueObjectPrinter::PrintChildren( bool value_printed, bool summary_printed, const DumpValueObjectOptions::PointerDepth &curr_ptr_depth) { - ValueObject *synth_m_valobj = GetValueObjectForChildrenGeneration(); + ValueObject &synth_valobj = GetValueObjectForChildrenGeneration(); bool print_dotdotdot = false; size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot); @@ -705,7 +709,7 @@ void ValueObjectPrinter::PrintChildren( bool any_children_printed = false; for (size_t idx = 0; idx < num_children; ++idx) { - if (ValueObjectSP child_sp = GenerateChild(synth_m_valobj, idx)) { + if (ValueObjectSP child_sp = GenerateChild(synth_valobj, idx)) { if (m_options.m_child_printing_decider && !m_options.m_child_printing_decider(child_sp->GetName())) continue; @@ -733,7 +737,7 @@ void ValueObjectPrinter::PrintChildren( if (ShouldPrintValueObject()) { // if it has a synthetic value, then don't print {}, the synthetic // children are probably only being used to vend a value - if (m_valobj->DoesProvideSyntheticValue() || + if (GetMostSpecializedValue().DoesProvideSyntheticValue() || !ShouldExpandEmptyAggregates()) m_stream->PutCString("\n"); else @@ -746,10 +750,7 @@ void ValueObjectPrinter::PrintChildren( } bool ValueObjectPrinter::PrintChildrenOneLiner(bool hide_names) { - if (!GetMostSpecializedValue() || m_valobj == nullptr) - return false; - - ValueObject *synth_m_valobj = GetValueObjectForChildrenGeneration(); + ValueObject &synth_valobj = GetValueObjectForChildrenGeneration(); bool print_dotdotdot = false; size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot); @@ -759,7 +760,7 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool hide_names) { bool did_print_children = false; for (uint32_t idx = 0; idx < num_children; ++idx) { - lldb::ValueObjectSP child_sp(synth_m_valobj->GetChildAtIndex(idx)); + lldb::ValueObjectSP child_sp(synth_valobj.GetChildAtIndex(idx)); if (child_sp) child_sp = child_sp->GetQualifiedRepresentationIfAvailable( m_options.m_use_dynamic, m_options.m_use_synthetic); @@ -795,6 +796,7 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool hide_names) { void ValueObjectPrinter::PrintChildrenIfNeeded(bool value_printed, bool summary_printed) { PrintObjectDescriptionIfNeeded(value_printed, summary_printed); + ValueObject &valobj = GetMostSpecializedValue(); DumpValueObjectOptions::PointerDepth curr_ptr_depth = m_ptr_depth; const bool print_children = ShouldPrintChildren(curr_ptr_depth); @@ -803,9 +805,9 @@ void ValueObjectPrinter::PrintChildrenIfNeeded(bool value_printed, !m_options.m_allow_oneliner_mode || m_options.m_flat_output || (m_options.m_pointer_as_array) || m_options.m_show_location) ? false - : DataVisualization::ShouldPrintAsOneLiner(*m_valobj); + : DataVisualization::ShouldPrintAsOneLiner(valobj); if (print_children && IsInstancePointer()) { - uint64_t instance_ptr_value = m_valobj->GetValueAsUnsigned(0); + uint64_t instance_ptr_value = valobj.GetValueAsUnsigned(0); if (m_printed_instance_pointers->count(instance_ptr_value)) { // We already printed this instance-is-pointer thing, so don't expand it. m_stream->PutCString(" {...}\n"); @@ -831,7 +833,7 @@ void ValueObjectPrinter::PrintChildrenIfNeeded(bool value_printed, // the user. The warning tells the user that the limit has been reached, but // more importantly tells them how to expand the limit if desired. if (m_options.m_max_depth_is_default) - m_valobj->GetTargetSP() + valobj.GetTargetSP() ->GetDebugger() .GetCommandInterpreter() .SetReachedMaximumDepth(); >From dc2c387e357c7cdd6837614f20095d2f4540f9d8 Mon Sep 17 00:00:00 2001 From: Jim Ingham <jing...@apple.com> Date: Fri, 9 Feb 2024 14:18:32 -0800 Subject: [PATCH 2/2] Change to use the shared ptr "*" operator. Also don't fumble-finger delete the declaration of SetupMostSpecializedValue. --- lldb/include/lldb/DataFormatters/ValueObjectPrinter.h | 2 ++ lldb/source/Commands/CommandObjectFrame.cpp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h index 91963b4d126eba..29264bc8d9c509 100644 --- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h +++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h @@ -69,6 +69,8 @@ class ValueObjectPrinter { /// value. ValueObject &GetMostSpecializedValue(); + void SetupMostSpecializedValue(); + const char *GetDescriptionForDisplay(); const char *GetRootNameForDisplay(); diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index 53886140fd2d6c..a4d3fb66e8b552 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -180,7 +180,7 @@ class CommandObjectFrameDiagnose : public CommandObjectParsed { // We've already handled the case where the value object sp is null, so // this is just to make sure future changes don't skip that: assert(valobj_sp.get() && "Must have a valid ValueObject to print"); - ValueObjectPrinter printer(*(valobj_sp.get()), &result.GetOutputStream(), + ValueObjectPrinter printer(*valobj_sp, &result.GetOutputStream(), options); printer.PrintValueObject(); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits