[PATCH] D25620: DebugInfo: introduce DIAlignment type

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 14 12:02:06 PDT 2016


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

> 
> -- adrian
> 
>>  
>> 
>> 
>> 
>> -- adrian
>> 
> 



More information about the llvm-commits mailing list