labath marked 4 inline comments as done. labath added inline comments.
================ Comment at: source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:153 + return true; + bool looking_for_methods = name_type_mask & eFunctionNameTypeMethod; + return looking_for_methods == die.IsMethod(); ---------------- clayborg wrote: > move up to line 150 and use this variable in the if statement instead of > repeating "name_type_mask & eFunctionNameTypeMethod" twice? Done. I've created variables for both enum values, as having just one of them in the if statement looked weird. ================ Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:222 + return true; + return GetReferencedDIE(DW_AT_specification) + .GetParent() ---------------- 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. https://reviews.llvm.org/D47470 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
