[PATCH] Store TypeUnits in a SmallVector<DWARFUnitSection> instead of a single DWARFUnitSection.
David Blaikie
dblaikie at gmail.com
Wed Sep 24 10:31:10 PDT 2014
Looks fine to me - some /totally optional/ comments, just if you think they're improvements.
================
Comment at: lib/DebugInfo/DWARFContext.cpp:87
@@ -86,3 +86,3 @@
if ((DumpType == DIDT_All || DumpType == DIDT_Types) && getNumTypeUnits()) {
OS << "\n.debug_types contents:\n";
----------------
getNumTypeUnits is now going to be potentially misleading - it'll specify the number of type unit sections, not the number of type units.
That happens to be the same for type units (for well constructed DWARF - but again, we might be dealing with bad DWARF when using this tool, so it's interesting to consider the possibility of present though empty type_units sections, etc) but isn't for DWO type units (which, when I first implemented them, went in separate sections - but then I later fixed it per the DWARF Fission spec, to put them all in the same section).
Might be a place where the 'empty(range)' template in that patch I showed previously could be handy - then there's no need for the extra getNum* functions, simply size(dwo_type_unit_sections()) - same behavior, but not quite as misleading.
Honestly though, I'm not sure how much value the "getNum*" calls provide here, just printing the section header seems pretty benign/possibly desired.
Some food for thought.
================
Comment at: lib/DebugInfo/DWARFContext.cpp:383
@@ -378,1 +382,3 @@
DataExtractor(I.second.Data, isLittleEndian(), 0);
+ DWOTUs.push_back(DWARFUnitSection<DWARFTypeUnit>());
+ auto &TUS = DWOTUs.back();
----------------
I'm sure I've used this idiom before - I'm not sure if it's nicer than something more like:
Foo f;
...
v.push_back(std::move(f));
(which I'm sure I've used as well)
================
Comment at: lib/DebugInfo/DWARFContext.h:33
@@ -32,3 +32,3 @@
DWARFUnitSection<DWARFCompileUnit> CUs;
- DWARFUnitSection<DWARFTypeUnit> TUs;
+ SmallVector<DWARFUnitSection<DWARFTypeUnit>,1> TUs;
std::unique_ptr<DWARFDebugAbbrev> Abbrev;
----------------
If it's going to have a small size of 1, maybe just use std::vector?
(granted I'm not quite as averse to the standard containers as some other LLVM contributors/owners - so opinions certainly differ)
================
Comment at: lib/DebugInfo/DWARFContext.h:41
@@ -40,3 +40,3 @@
DWARFUnitSection<DWARFCompileUnit> DWOCUs;
- DWARFUnitSection<DWARFTypeUnit> DWOTUs;
+ SmallVector<DWARFUnitSection<DWARFTypeUnit>,1> DWOTUs;
std::unique_ptr<DWARFDebugAbbrev> AbbrevDWO;
----------------
same here
http://reviews.llvm.org/D5482
More information about the llvm-commits
mailing list