[cfe-dev] [PATCH] New syntax and functionality for __has_attribute

[Aaron Ballman]

On Mon, Jan 13, 2014 at 8:39 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Mon, Jan 13, 2014 at 5:31 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> On Mon, Jan 13, 2014 at 8:20 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>> > On Sun, Jan 12, 2014 at 4:49 PM, Aaron Ballman <aaron at aaronballman.com>
>> > wrote:
>> >>
>> >> On Sun, Jan 12, 2014 at 7:38 PM, Sean Silva <silvas at purdue.edu> wrote:
>> >> >
>> >> >
>> >> >
>> >> > On Sun, Jan 12, 2014 at 6:53 PM, Aaron Ballman
>> >> > <aaron at aaronballman.com>
>> >> > wrote:
>> >> >>
>> >> >> > That's good news -- thanks for confirming.
>> >> >> >
>> >> >> > The feature detection macro itself will still need to have a
>> >> >> > different
>> >> >> > name
>> >> >> > (or some other mechanism) so it can be used compatibly with
>> >> >> > existing
>> >> >> > clang
>> >> >> > deployments, because _has_attribute() currently emits a parse
>> >> >> > error
>> >> >> > instead
>> >> >> > of gracefully returning 0 when passed the new argument syntax:
>> >> >> >
>> >> >> > tmp/attr2.cpp:1:5: error: builtin feature check macro requires a
>> >> >> > parenthesized identifier
>> >> >> > #if __has_attribute(__attribute__((weakref)))
>> >> >> >     ^
>> >> >>
>> >> >> Good catch. Unfortunately, __has_attribute is really the best
>> >> >> identifier for the macro, so I am loathe to let it go.
>> >> >>
>> >> >> Due to the current design of __has_attribute, we can't get away with
>> >> >> "
>> >> >> magic" by expanding the non-function-like form into a value that
>> >> >> could
>> >> >> be tested. So we would really have to pick a new name if we are
>> >> >> worried about this.
>> >> >>
>> >> >> Suggestions on the name are welcome.
>> >> >
>> >> >
>> >> > Ok, I'll bite:
>> >> >
>> >> > __has_attribute_written_as([[foo]])
>> >> > __has_attribute_syntax([[foo]])
>> >> > __has_attribute_spelling([[foo]])
>> >>
>> >> I kind of like __has_attribute_syntax, truth be told.
>> >
>> >
>> > Have we given up on using the name __has_attribute too soon? Users of
>> > the
>> > new syntax could write:
>> >
>> > // Probably already in project's porting header
>> > #ifndef __has_feature
>> > #define __has_feature(x) 0
>> > #endif
>> >
>> > #if __has_feature(__has_attribute_syntax)
>> > #define MY_HAS_ATTRIBUTE(...) __has_attribute(__VA_ARGS__)
>> > #else
>> > #define MY_HAS_ATTRIBUTE(...) 0
>> > #endif
>> >
>> > If it's given a different name, they instead would write something like:
>> >
>> > #ifdef __has_attribute_syntax
>> > #define MY_HAS_ATTRIBUTE(...) __has_attribute_syntax(__VA_ARGS__)
>> > #else
>> > #define MY_HAS_ATTRIBUTE(...) 0
>> > #endif
>> >
>> > So I don't think the change-in-syntax argument holds water.
>>
>> So are you proposing that we would have a different name for the
>> purposes of the __has_feature macro? Eg)
>> __has_feature(__has_attribute_syntax) is 1 for the proposed
>> functionality, and 0 otherwise?
>
>
> It's a possibility. It could be that a new name is a better approach, but
> both directions seem to be feasible.

I'll ponder; I rather like keeping the existing name.

>>
>> > Also, supporting arguments in the attributes is useful in some cases --
>> > it's
>> > not true that they don't make sense in a feature-checking facility. For
>> > instance:
>> >
>> >   __has_attribute( __attribute__((format)) )
>> >
>> > ... doesn't tell me whether __attribute__((format, gnu_scanf, 1, 2) will
>> > work (and I'd expect that the format attribute will gain additional
>> > archetypes in future).
>>
>> That's true, but the example also demonstrates why it's kind of
>> nonsensical. What do the 1, 2 represent for the purposes of
>> __has_attribute?
>
>
> They represent themselves. Suppose we added support for a format attribute
> with negative indices, or with three indices, or something -- this syntax
> would allow the user to determine if that syntax is available.
>
>> Can they be elided? If so, can we come up with
>> declarative rules as to when they can be elided?
>
>
> If you could omit them, how would you tell whether an attribute could be
> used without arguments?
>
> Again, I'm not saying we should go in this direction, but I don't think we
> should dismiss it without consideration -- we probably don't want to find we
> need a third form of __has_attribute later =)

That's one of the reasons Alp's suggestion for forwards compatibility
is so nice -- if implemented properly, we could add parameter support
at a later date (presuming we stick with the attribute syntax style
approach).

I'd like to avoid attempting to preprocess parameters for this patch,
but had intended to leave the door open for the future. So it wasn't
entirely without consideration. ;-)

~Aaron