[PATCH] Attributes with enum arguments

[Richard Smith]

On Wed, Sep 11, 2013 at 11:57 AM, Aaron Ballman <aaron at aaronballman.com>wrote:

> On Wed, Sep 11, 2013 at 1:32 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> > SemaDeclAttr:2450 is underindented
>
> That appears to be existing code which was hyperindented, but I've
> changed the surrounding code.
>

Err, sorry, I meant 2340:

+  // Complain about attempts to use protected visibility on targets
+  // (like Darwin) that don't support it.
+  if (type == VisibilityAttr::Protected &&
+    !S.Context.getTargetInfo().hasProtectedVisibility()) {


> > ClangAttrEmitter:988 is underindented
>
> Good catch!
>
> >
> > The writeConversion function you generate has undefined behavior if the
> > string is not a member of the enumeration (because -1 is not a
> representable
> > value in the enumeration type). How about using a
> > StringSwitch<Optional<EnumType> > ? (You could even return
> > Optional<EnumType> directly from ConvertStrToEnumType.)
>
> Also a good catch!  I tried returning an Optional<EnumType>, but it
> made the surrounding code a bit more gnarly to work with.  So I left
> the signature the same, but changed the implementation.
>

The 'if' statement in the generated code is overindented ;)

Otherwise, LGTM. Thanks!


> Thanks for the review!
>
> ~Aaron
> >
> >
> > On Tue, Sep 10, 2013 at 7:44 PM, Aaron Ballman <aaron at aaronballman.com>
> > wrote:
> >>
> >> Ping
> >>
> >> On Fri, Sep 6, 2013 at 2:31 PM, Aaron Ballman <aaron at aaronballman.com>
> >> wrote:
> >> > This patch causes tablegen to generate a StringSwitch for attributes
> >> > that contain enum args, mapping from the enumerant string to the
> >> > proper enumeration value.  It resolves inconsistencies between
> >> > diagnostics, and brings us one step closer to more generalized error
> >> > reporting for attributes.
> >> >
> >> > ~Aaron
> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130911/34fd8255/attachment.html>