[PATCH] Simplify DWARFCompileUnit / DWARFTypeUnit constructors (NFC).
Frederic Riss
friss at apple.com
Wed Oct 8 08:09:12 PDT 2014
This looks like a nice improvement to me. Some comments inline just for discussion, nothing to change.
Thanks!
================
Comment at: lib/DebugInfo/DWARFUnit.cpp:29-37
@@ -32,9 +28,11 @@
-DWARFUnit::DWARFUnit(DWARFContext &DC, const DWARFSection &Section,
- const DWARFDebugAbbrev *DA, StringRef RS, StringRef SS,
- StringRef SOS, StringRef AOS, bool LE,
+DWARFUnit::DWARFUnit(DWARFContext &DC, const DWARFSection &Section, bool IsDWO,
const DWARFUnitSectionBase &UnitSection)
- : Context(DC), InfoSection(Section), Abbrev(DA), RangeSection(RS),
- StringSection(SS), StringOffsetSection(SOS), AddrOffsetSection(AOS),
- isLittleEndian(LE), UnitSection(UnitSection) {
+ : Context(DC), InfoSection(Section),
+ Abbrev(IsDWO ? DC.getDebugAbbrevDWO() : DC.getDebugAbbrev()),
+ RangeSection(IsDWO ? DC.getRangeDWOSection() : DC.getRangeSection()),
+ StringSection(IsDWO ? DC.getStringDWOSection() : DC.getStringSection()),
+ StringOffsetSection(IsDWO ? DC.getStringOffsetDWOSection() : StringRef()),
+ AddrOffsetSection(DC.getAddrSection()),
+ isLittleEndian(DC.isLittleEndian()), UnitSection(UnitSection) {
clear();
----------------
We talked at one point of using inheritance to distinguish the DWO Units from the others, do you already have a plan for that? I ask because in my branch I tried various approaches to avoid all these conditionals by having the DWO/standard section selection done using a virtual method, but I couldn't come up with something that I found nicer than this (at which level of the type hierarchy do you introduce the DWO attribute for example?).
================
Comment at: lib/DebugInfo/DWARFUnit.h:82-83
@@ -82,5 +81,4 @@
private:
- void parseImpl(DWARFContext &Context, const DWARFSection &Section,
- const DWARFDebugAbbrev *DA, StringRef RS, StringRef SS,
- StringRef SOS, StringRef AOS, bool LE) override {
+ void parseImpl(DWARFContext &Context, const DWARFSection &Section, bool IsDWO,
+ bool LE) override {
if (Parsed)
----------------
It's a pity that we can't get rid of the bool argument that is also a Context derivative, but it's already much nicer like this.
http://reviews.llvm.org/D5659
More information about the llvm-commits
mailing list