[PATCH] Attributes with enum arguments

Aaron Ballman aaron at aaronballman.com
Wed Sep 11 12:51:01 PDT 2013


Thanks!  I've rectified the indentation issues and committed in r190545.

~Aaron

On Wed, Sep 11, 2013 at 3:02 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 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
>> >
>> >
>> >
>
>



More information about the cfe-commits mailing list