[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