[Lldb-commits] [PATCH] D62211: Simplify `GetName`+`AppendTypeName` by `DWARFDIE`

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed May 22 00:04:21 PDT 2019


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





More information about the lldb-commits mailing list