[PATCH] Simplify DWARFCompileUnit / DWARFTypeUnit constructors (NFC).

[Alexey Samsonov]

>>! In D5659#9, @dblaikie wrote:
>>>! In D5659#8, @friss wrote:
>>>>! In D5659#7, @dblaikie wrote:
>>> I'm not really sure I agree that this is an improvement.
>>> 
>>> I have a bit of an aversion to making something a dynamic choice (passing a bool down & then doing a bunch of conditionals) compared to a static choice (at the call site, we know which sections to retrieve).
>> 
>> I always disliked the huge list of acronym parameters that got passed to these constructor methods, and I stylistically prefer the proposed patch. But as I commented, the bunch of conditionals isn't that nice either and if someone can come up with a nice solution that combines the best of both worlds then I'm all for it.
>> 
>> Are you concerned about the performance implications of making this choice dynamic or just objecting that something that can be static should be static?
> 
> Not concerned about performance implications - mostly just that the static choice seemed to be made where the information needed for the choice existed - and to plumb that information down a few layers, unnecessarily deferring the choice (it's not like we already had a separate DWO/O abstraction - we actually had to push the choice down with the boolean flag) doesn't seem great.

Fair enough. I agree that it would be cleaner to first separate .dwo units from regular ones, and then proceed with this change, so that we don't have to pull conditionals into constructor code.


> 
> If the problem is just the number of arguments - we can wrap that in a struct and achieve the same terseness without pushing the choice down into layers that are otherwise agnostic of that detail.

================
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();
----------------
friss wrote:
> 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?).
I haven't actually tried to code it yet, so probably you're in a better position. I thought of introducing a DWARFDWOContext entity, that would contain .dwo counterparts of regular sections. DWARFDWOContext is contained inside a regular DWARFContext (to represent the case when .dwo sections are not yet moved away from the executable). On the other hand, we should be able to construct DWARFDWOContext directly from an object file (with .dwo extension) - this would be used in DWARFUnit::parseDWO() method, that locates and loads .dwo file for a given compilation unit.

We can make DWARFDWOContext implement the same set of methods as DWARFContext, e.g. DWARFDWOContext::getDebugAbbrev() will return .debug_abbrev.dwo section. Instead of calling  DWARFContext.getDebugAbbrevDWO() one may call DWARFContext.getDWOContext().getDebugAbbrev().

http://reviews.llvm.org/D5659