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

Frederic Riss friss at apple.com
Wed Oct 8 11:14:04 PDT 2014


>>! In D5659#11, @dblaikie wrote:
>>>! In D5659#10, @friss wrote:
>>>>! In D5659#9, @dblaikie wrote
>> :> 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.
>> 
>> It's not only about the number of arguments, I like the fact that the Unit choses the sections it needs. I find it more logical from an OO POV than having a helper in DWARFUnitSection make the choice.
> 
> Though DWARFUnitSection is currently dwo/o-agnostic. Unless we were to make it type-specific (which we could do, making the current DWARFUnitSection a CRTP base, perhaps... - but now we've got to pivot in two directions & end up with 4 classes. {DWO,O}x{TypeUnit,CompileUnit}... which seems awkward, though possible. (note that the debug info emission code (lib/CodeGen/AsmPrinter) currently only pivots, in the inheritance hierarchy, by type/compile unit, not by .o/.dwo - though similarly, it's not a perfect fit and there are certainly times I'd like the other axis rather than a lot of "isSplitDwarf" checks)
> 
>> Ideally, I'd like to get rid of parseDWO totally and have the DWOness be a Unit type attribute (that was the point of my comment regarding the type hierarchy).
> 
> I'm not quite sure what you're describing when you say "a unit type attribute" - could you sketch out the ideal end state you have in mind (in terms of inheritance, reuse, where decisions get made, etc)?

The idea would be to push the decision further down in the Unit type. This would remove the need for the parseDWO helper, the high level code would just call a generic parse method on the UnitSection. I tried to do something like that but I wasn't satisfied by the end result (I introduced some kind of DWO trait template argument to DWARFUnit to avoid duplicating the code, but then the iterator types got ugly and moreover I ended up with 2 virtual calls per call to parse). So I don't have a precise idea of the outcome, but Alexey was the first one coming up with the idea of specializing the Unit types to account for the DWO specificities thus I thought this patch was the first of his grand plan to make that happen :-)

http://reviews.llvm.org/D5659






More information about the llvm-commits mailing list