[PATCH] D25620: DebugInfo: introduce DIAlignment type

[David Blaikie via llvm-commits]

(ah, I just assumed from the number of files changed/the description this
was more than just adding a typedef - *shrug* I'm less fussed about it, not
sure it's worth having a separate typedef for it, but not enough to have
any opinion on this code review)

On Fri, Oct 14, 2016 at 1:51 PM Adrian Prantl <aprantl at apple.com> wrote:

>
> > 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161014/3ac3601f/attachment.html>