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

David Blaikie dblaikie at gmail.com
Wed Oct 8 10:48:54 PDT 2014


>>! 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)?

> I have the feeling that this commit gets us one step closer to that.
> 
> But I can see your point though, won't argue if you reject the change.

http://reviews.llvm.org/D5659






More information about the llvm-commits mailing list