labath added a comment.

It's been a while since we had that discussion, but the way I'm remembering it, 
sizeof(DWARFDIE) was not my biggest concern with this stuff (though I may have 
been echoing the concerns of some other people). What I was trying to prevent 
is diverging even more from the llvm dwarf implementation. In that sense, 
changing lldb's DWARFDIE without a matching change to the llvm counterpart is 
equally bad. I don't think the first two options are ever going to make it into 
llvm. The third might, but even that may be a hard sell -- a slightly more 
tractable option might be to reuse the low-order bits in the DWARFDIE via 
PointerIntPair. We might be able to get those eight bits that way.
But all of that seems peripheral to me. The main thing I was trying to achieve 
was to have the low-level DWARF code (DWARFUnit, DWARFDIE, ... -- basically the 
things that are in the llvm dwarf parser right now) not require access to 
anything outside of what is available to them. If there's a thing which 
requires more info, it should be built on top of this lower layer, not trying 
to reach out from inside it. Once it's outside, a lot of options become 
available. If there is a lldb_private::CompileUnit in the vicinity of this 
code, it could just fetch the language from there. Or, we can have some sort of 
a SuperDWARFDIE, which includes the language, or the main compile unit (either 
lldb or dwarf) to pass around as a single entity (using PointerIntPair would be 
an optimization of that). Or, instead of SuperDWARFDIE we could use a DIERef, a 
user_id_t, or some of the other reference kinds we have already..

As for this patch, I don't see any big problem with changing the representation 
in this case, but given that it's motivation is to able to  later change 
DWARFDIE, I don't think its really useful/needed.

The test changes are cool, and I'd encourage you to split them off into a 
separate patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73206/new/

https://reviews.llvm.org/D73206



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to