[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

mib added a comment. @labath Thanks for reporting that crash. D83327 should fix the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79554/new/ https://reviews.llvm.org/D79554

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

labath added inline comments. Comment at: lldb/source/Core/ValueObject.cpp:690-694 + if (!valobj && synthetic_array_member) +valobj = GetSyntheticValue() + ->GetChildAtIndex(synthetic_index, synthetic_array_member) + .get(); +

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

This revision was automatically updated to reflect the committed changes. mib marked 4 inline comments as done. Closed by commit rG0eba9de71e2a: [lldb/Dataformatter] Add support to CF{Dictionary,Set}Ref types (authored by mib). Changed prior to commit:

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

labath added inline comments. Comment at: lldb/source/Core/ValueObject.cpp:2839-2861 +// In case of C opaque pointers, use the pointee type and try to +// recreate a new ValueObjectChild using it. +if (!m_deref_valobj) { + if

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. IMHO this looks good now. Maybe add a comment to the commit that this adds support for incomplete types in ValueObjects. I think @jingham should also take a look at this to make sure

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

mib updated this revision to Diff 264038. mib edited the summary of this revision. mib added a comment. Instead of creating a new empty struct type, this new implementation will use the opaque pointer's pointee type to create the new `ValueObjectChild`. This makes the previously introduced

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

labath added a comment. In D79554#2026999 , @teemperor wrote: > I don't see a huge problem with things like `__lldb_autogen_nspair` as it's a > single obviously generated type hardcoded into LLDB. But this really generic > approach here can generate all

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. I don't see a huge problem with things like `__lldb_autogen_nspair` as it's a single obviously generated type hardcoded into LLDB. But this really generic approach here can

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

mib updated this revision to Diff 262823. mib edited the summary of this revision. mib added a comment. - Added a virtual method to create an empty struct type `TypeSystem::GetEmptyStructType` (if someone can think of a better name, I'm open to suggestions) - Refactored

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

jingham added inline comments. Comment at: lldb/source/Core/ValueObject.cpp:2846 + if (llvm::isa(compiler_type.GetTypeSystem())) { +if (HasSyntheticValue()) { + TargetSP target_sp = GetTargetSP(); mib wrote: > friss wrote: > > I am

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

mib marked 4 inline comments as done. mib added inline comments. Comment at: lldb/source/Core/ValueObject.cpp:52 +#include "Plugins/TypeSystem/Clang/TypeSystemClang.h" + friss wrote: > You don't want to introduce a dependency between Core and a plugin. What

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

jingham added a comment. The way the ValueObject code works w.r.t. Synthetic child providers is that you first look up and make a ValueObject for the actual value the user requested, (for instance a ValueObjectVariable or a ValueObjectChild or a ValueObjectConstResult for expressions, etc.),

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

shafik added a comment. I have to agree w/ Pavel here that I am not crazy about creating creating an empty struct here, it is not clear why there is no other way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79554/new/

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

mib marked 3 inline comments as done. mib added inline comments. Comment at: lldb/source/Core/ValueObject.cpp:665 bool child_is_deref_of_parent = false; + CompilerType compiler_type = GetCompilerType(); uint64_t language_flags = 0; teemperor wrote: > This

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

teemperor added a comment. I don't really know that ValueObject code, but I wish we could just pretend that the dereferenced type of X is a type named Y or something like that and then just map *Ref classes to their non-opaque bridged types when dereferenced. No idea if that's possible though.

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

labath added a reviewer: teemperor. labath added a comment. The ValueObject change looks very worrying to me. The ability to pretty-print incomplete types seems pretty useful, but the implementation of that looks a bit... scary. I mean you're not even checking whether the incomplete type is a

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

friss added inline comments. Comment at: lldb/source/Core/ValueObject.cpp:52 +#include "Plugins/TypeSystem/Clang/TypeSystemClang.h" + You don't want to introduce a dependency between Core and a plugin. What you need here might need to be introduced as a new

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

mib created this revision. mib added reviewers: friss, davide, jingham. mib added a project: LLDB. Herald added a subscriber: lldb-commits. This patch improves data formatting for CFDictionaryRef and CFSetRef. It uses the same data-formatter as NSCFDictionaries and NSCFSets introduced previously