labath added a comment.

Isn't all of this dead code? These functions seem to be only called from 
`DWARFDebugInfoEntry::Dump`, and I can find no callers of the Dump method.

If that's the case, what I'd do is delete all of this stuff, and then if we 
ever need to dump debug info entries again, implement a llvm-style `dump` 
method on the `DWARFDIE` class. This will move us closer to aligning lldb and 
llvm versions of these two classes.

In D62211#1510974 <https://reviews.llvm.org/D62211#1510974>, @clayborg wrote:

> In D62211#1510903 <https://reviews.llvm.org/D62211#1510903>, @clayborg wrote:
>
> > We really shouldn't have any DWARFDIE references inside 
> > DWARFDebugInfoEntry.h. I noticed there are a lot now which is wrong.
>
>
> I take this back. Many uses are good inside of DWARFDebugInfoEntry as I ok'ed 
> patches when I looked at them. Initially this file was intended to be the 
> level below DWARFDIE, but that has changed, and for good reason: make sure we 
> never use the wrong DWARFUnit with a DWARFDebugInfoEntry. But my initial 
> comment about moving the GetName and AppendTypeName still stand. If we aren't 
> using these for forward references when we don't have all of the 
> DWARFDebugInfoEntry objects parsed already, then they should be moved to 
> DWARFDIE and removed from here.


I think it is still possible (and desirable) to maintain (or restore) the 
"DWARFDIE >> DWARFDebugInfoEntry" invariant, while still making sure that users 
don't have to remember which entry goes with which compile unit. The way to 
achieve that is by making that nobody (outside the DWARFDIE class, and maybe 
some other low-level stuff) deals with DWARFDebugInfoEntries directly). That's 
pretty much what was done in the llvm's version of the parser where 
DWARFDebugInfoEntry is an extremely dumb class with just 5 simple accessor 
functions, and all of the interesting stuff happens in `DWARFDie`.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62211



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

Reply via email to