[Lldb-commits] [PATCH] D59217: Fix/unify SBType comparison

[Greg Clayton via lldb-commits]

The main reason for this that I can see is if we ever want to provide a lldb::user_id_t for a given SBType. We will lose that ability if we remove, but I am ok with this because we can force the TypeSystem to be able to remember this in metadata if we ever do need it. Why? In DWARF that most compilers produce, they produce many copies of a type, up to one in each compile unit within an symbol file. If you somehow get ahold of a type via the lldb::SB API right now, there is no way to figure out which one it picked from those many copies. So not a big deal if we remove this as we have not exposed "lldb::user_id_t lldb::SBType::GetID()" in the API yet, nor do we seem to have a need for it. You might keep the constructor that takes a type_sp just to keep the diffs down though, that ctor will extract the compiler type form the type_sp and not store it.

Greg

> On Mar 11, 2019, at 4:32 PM, Jim Ingham via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> Ah, I see.  This doesn't seem terribly confusing,  TypePair manages the TypeSP & CompilerType and keeps them in sync, so it just allows you to provide a higher quality representation if you have it.  A lot of SBValues come from debug information so they will have TypeSP's around when they get made.
> 
> But if you want to try simplifying things, that's also good.  Since this is caching the TypeSP we need to include "doesn't slow down debugging" to the things that don't break, but we don't have a good way to measure that right now.
> 
> Jim
> 
> 
>> On Mar 11, 2019, at 4:17 PM, Zachary Turner <zturner at google.com> wrote:
>> 
>> 
>> 
>> On Mon, Mar 11, 2019 at 4:15 PM Jim Ingham via lldb-commits <lldb-commits at lists.llvm.org> wrote:
>> Just to be precise: TypeImpl stores a TypePair for the static type and a CompilerType for the dynamic type.  These two have different meanings.  There's no assumption about the relationship between the static and dynamic type.  In ObjC, the dynamic type need not even be in the same class hierarchy as the static type.  That's there so that if an SBValue hands out a type, it can represent both the static and dynamic types of the value it comes from.
>> 
>> I'm not sure why the static type is a TypePair and the dynamic type is a CompilerType, however.
>> 
>> The TypePair stores a TypeSP and a CompilerType that are supposed to be the same type.  It doesn't look like there is any way for those two to get out of sync, but I'm not entirely sure why it helps to have both in the same object.  Presumably it's caching?
>> 
>> This was what i meant.  It seems that in the case of SBType, nothing depends on the type_sp member of the pair, only the CompilerType.
>> 
>> I think the reason why both are in the same object is so that if you initialize it with a TypeSP you have a superset of functionality available than if you initialize it from a CompilerType.  But, if nobody actually requires this, then it simplifies the interface and makes it easier to reason about to just store a CompilerType for the static type.
>> 
>> Like Greg said though, it should be easy to see if it breaks anything by just changing the static type from TypePair to CompilerType and then fixing up the code and seeing if anything breaks.
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits