labath marked 4 inline comments as done. labath added inline comments.
================ Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:222 + return true; + return GetReferencedDIE(DW_AT_specification) + .GetParent() ---------------- JDevlieghere wrote: > labath wrote: > > clayborg wrote: > > > labath wrote: > > > > clayborg wrote: > > > > > I can never remember when a DW_AT_abstract_origin might be used. > > > > > Might be nice to have a DWARFDIE method: > > > > > > > > > > ``` > > > > > DWARFDIE DWARFDIE::GetAbstractOriginOrSpecification(); > > > > > ``` > > > > > this would return either the the DW_AT_specification or the > > > > > DW_AT_abstract_origin. If we go that route the this coce would become: > > > > > > > > > > ``` > > > > > GetAbstractOriginOrSpecification().GetParent().IsStructUnionOrClass(); > > > > > ``` > > > > > > > > > How would this method know which DIE to return? In case of inlined > > > > methods you can have a chain of DIEs: > > > > `DIE1 --- DW_AT_abstract_origin --> DIE2 --- DW_AT_specification --> > > > > DIE3` > > > > Each of these dies will have a different parent. > > > > > > > > The current function will check for the parent of DIE1 and DIE3 (this > > > > is what the manual index does) though to be fully correct maybe we > > > > should check all three of them (?) Or do you think checking the last > > > > DIE in that list should be always enough? That would seem to be the > > > > case for the dwarf I've seen, but I'm not sure if that is always > > > > correct.. > > > Yeah, unfortunately I can't elaborate too much without seeing a bad > > > example. If anything in the chain has a parent that is a struct/union or > > > class might be enough, so we could ask each DIE in the specification or > > > abstract origin chain if its parent is a struct/union/class and just > > > return true if so? > > I've reimplemented the function to do a proper recursive search for the > > parent, including a infinite-recursion guard. > Looks good, we have almost the exact same implementation in `findRecursively` > in llvm. It should be. I copied it from there. :) https://reviews.llvm.org/D47470 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
