Extract information related to a "unit header" from DWARFUnit into a new
DWARFUnitHeader class, and have DWARFUnit inherit it. This is one step
in the direction of recognizing type units in the .debug_info section.
Details
Diff Detail
Event Timeline
llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h | ||
---|---|---|
238 | Any chance of composition rather than inheritance? (I realize that'll be a bunch more churn (changing "getLength" to "getHeader().getLength", etc, presumably) - but it seems like the inheritance here isn't accurate - a DWARFUnit is not a DWARFUnitHeader (in the "isa" kind of phrasing/test about inheritance)) Guess that could be done in a separate patch to make it clearer. | |
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | ||
278 | Rather than testing the section inside DWARFUnitHeader::extract - perhaps you could pass down UnitType here? |
llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h | ||
---|---|---|
238 | It should be easy as each constructor was taking a "const DWARFUnitHeader &Header" parameter already, so instead of constructing DWARFUnitHeader with it, we could copy or move it into the m_header ivar. |
llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h | ||
---|---|---|
238 | Yeah, I wondered about composition versus inheritance. If I throw in DWARFUnit methods that forward to the Header member then it would be less churn. | |
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | ||
278 | Not 100% sure what you're suggesting--yes in this path we do already have the UnitType, but in the non-verifier path we don't, so passing in the UnitType feels like adding unnecessary complexity to extract(). |
llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp | ||
---|---|---|
115–117 | Not sure this comment makes sense - you can't look at the unit DIE, right? Because you have to parse the header before you parse the DIEs, so that doesn't seem like a relevant suggestion/approach here? |
llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp | ||
---|---|---|
115–117 | Fair. Changed the comment to avoid the suggestion. |
Any chance of composition rather than inheritance? (I realize that'll be a bunch more churn (changing "getLength" to "getHeader().getLength", etc, presumably) - but it seems like the inheritance here isn't accurate - a DWARFUnit is not a DWARFUnitHeader (in the "isa" kind of phrasing/test about inheritance))
Guess that could be done in a separate patch to make it clearer.