[PATCH] D25620: DebugInfo: introduce DIAlignment type
Adrian Prantl via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 14 13:51:08 PDT 2016
> On Oct 14, 2016, at 12:02 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
>>
>> On 2016-Oct-14, at 10:16, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> Victor - was there any particular benefit/motivation for adding a new type here that you had in mind/had discovered in implementing it? (or precedent you were inspired by - I admit to not having all the DI* hierarchy paged in these days, so perhaps I've missed some obvious prior art)
>>
>> On Fri, Oct 14, 2016 at 10:15 AM Adrian Prantl <aprantl at apple.com> wrote:
>>> On Oct 14, 2016, at 10:13 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>
>>>
>>> On Fri, Oct 14, 2016 at 9:59 AM Adrian Prantl <aprantl at apple.com> wrote:
>>>
>>>
>>>> On Oct 14, 2016, at 9:48 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>>
>>>
>>>> Could someone point me to where the discussion for adding this type came out of? I didn't spot it at a cursory glance of the previous/existing threads.
>>>
>>>>
>>>
>>>
>>>
>>> This came out of the review thread in:
>>>
>>>
>>>
>>> D25073: [DebugInfo]: preparation to implement DW_AT_alignment
>>>
>>> https://reviews.llvm.org/D25073
>>>
>>>
>>>
>>> where I argued that we should not be using a full uint64_t for the alignment fields in the DI.* metadata nodes. I don't think the concrete solution of introducing a new alignment type has been discussed here before.
>>>
>>> Any particular benefits of introducing a new type? We don't I think have any wrapper types for otherwise singular numeric values, do we? (except maybe DIFlags?)
>>
>> I don't have a strong opinion about adding a new type. If we also had a separate type for sizes, we could potentially make the interface more typesafe to avoid accidentally confusing size and alignment, but that's about it.
>
> Haven't looked at the patch, so forgive me if I'm confused, but I thought you said this was just a typedef. In that case, how is it providing any type safety?
You are right, not as a typedef. We would have to wrap it in a class to benefit from the type safety.
>>
>> -- adrian
>>
>>>
>>>
>>>
>>>
>>> -- adrian
More information about the llvm-commits
mailing list