[PATCH] Simplify DWARFCompileUnit / DWARFTypeUnit constructors (NFC).
David Blaikie
dblaikie at gmail.com
Wed Oct 8 09:59:22 PDT 2014
>>! 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.
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.
http://reviews.llvm.org/D5659
More information about the llvm-commits
mailing list