[Lldb-commits] [PATCH] D47275: 1/3: DWARFDIE split out to DWARFBasicDIE

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In https://reviews.llvm.org/D47275#1111285, @labath wrote:

> In https://reviews.llvm.org/D47275#1111254, @clayborg wrote:
>
> > In https://reviews.llvm.org/D47275#1110772, @labath wrote:
> >
> > > I don't think a name like `DWARFUnitDIE` is a good one bacause it would make a weird `is-a` relationship (a DWARFDIE represetning a DW_TAG_variable is certainly **not** a "unit DIE" yet you could assign it to a  `DWARFUnitDIE&`). We could have a DWARFUnitDIE type if we wanted to, but that would have to be a special type in addition to DWARFBasicDIE. However, I think that would be overkill.
> >
> >
> > Yeah, we just need to be able to tell the difference between the top level DIE we hand out with no children and the one that has all the abilities.
>
>
> Yes, but this is not what this patch is doing. The class inheritance makes is such that any "die with all abilities" we hand out, also is-a "top level die with no children". This is where my problem comes from.


Yeah, I see where the name doesn't fit now. DWARFBaseDIE maybe?

> If I rephrase your comment a bit
> 
>> we just need to be able to tell the difference between a **DIE that has no ability to access children** and the one that has all the abilities.
> 
> then it is fine because a "die which has all abilities" is also a "die which has only a certain smaller set of abilities". However in this case a name like `DWARFUnitDIE` is not appropriate.
> 
> If we wanted to have a type called DwarfUnitDIE die then it should be a special type inheriting from DWARFBasicDIE (or whatever it's called). Then again it would be fine, because then it would be a "DIE that has no ability to access children **and** is a root DIE", which is a refinement of "DIE that has no ability to access children".

DWARFBaseDIE might work then. Name doesn't violate any of the assumptions you mention above. All of your issues revolve around the naming right?


https://reviews.llvm.org/D47275