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

Mon Feb 24 08:19:48 PST 2014

On Sun, Feb 23, 2014 at 10:08 PM, Alp Toker <alp at nuanti.com> wrote:

> Hi Aaron,
>
> As a fix for PR15853 this is looking OK and almost there.
>
> It's quirky to put a synthesis of the attribute parse rules in the
> preprocessor but your approach meets needs that the other solutions don't
> so it's cool.
>
> However the addition of new semantics to the existing __has_feature()
> macro is problematic given that even ToT clang will error out the
> preprocessor whenever new-style arguments are encountered. That's bad news
> for a feature check macro that needs to work on a range of clang versions.
>
> I know you've since added __has_feature(__has_attribute_syntax) to work
> around the problem but it's not discoverable and I suspect the feature
> check won't get used correctly in the wild.
>
> So how about just naming the new check __has_attribute_syntax()?
>

Why would this be more discoverable than
__has_feature(__has_attribute_syntax)?

>
> That way the widely-recognised feature detection fallback pattern will
> just work and you can drop the  __has_feature workaround entirely. Users
> will be able to use the familiar:
>
> #ifndef __has_attribute_syntax
> #define __has_attribute_syntax(...)
> #endif
>
> Doing this also means we can remove the old, ambiguous identifier-only
> form of the check from the new version of the check. With this change to
> your proposed patch we can ensure safe and compatible usage which is
> important with vendor extensions like this.
>
> Alp.
>
>
>
> On 23/02/2014 16:24, Aaron Ballman wrote:
>
>> Ping
>>
>> ~Aaron
>>
>> On Wed, Feb 19, 2014 at 1:05 PM, Aaron Ballman <aaron at aaronballman.com>
>> wrote:
>>
>>> This is the latest version of this patch -- with some additional
>>> features as requested. Specifically, this is using the existing
>>> __has_attribute syntax in an extended style, and implements a
>>> feature-test macro (__has_feature(__has_attribute_syntax)) which
>>> allows you to determine whether the extended syntax is supported or
>>> not. It adds some additional documentation explaining this.
>>>
>>> ~Aaron
>>>
>>> On Thu, Jan 23, 2014 at 10:23 AM, Aaron Ballman <aaron at aaronballman.com>
>>> wrote:
>>>
>>>> Before I start in on this again, I wanted to make sure that there was
>>>> a consensus that this functionality was desirable. I believe the
>>>> answer (based on some conversations in IRC and here on the lists) was
>>>> tentatively "yes."
>>>>
>>>> As far as I can tell, the work left to be done on this is to add a
>>>> feature test for __has_attribute_syntax, write the documentation for
>>>> it and that's about it? The syntax-based form is the preferable
>>>> nomenclature because it leaves the door open for testing parameters at
>>>> some point in the future, and with the __has_attribute_syntax feature
>>>> test, it is both backwards and forwards compatible.
>>>>
>>>> ~Aaron
>>>>
>>>> On Mon, Jan 13, 2014 at 8:49 PM, Richard Smith <richard at metafoo.co.uk>
>>>> wrote:
>>>>
>>>>> On Mon, Jan 13, 2014 at 5:44 PM, Aaron Ballman <aaron at aaronballman.com
>>>>> >
>>>>> wrote:
>>>>>
>>>>>> 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.
>>>>>>
>>>>>
>>>>> By the same argument, it's possible to add extra arguments to
>>>>> __has_attribute, if we have a __has_feature check for the new syntax.
>>>>>
>>>>>  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. ;-)
>>>>>>
>>>>>
>>>>> =) OK then!
>>>>>
>>>>
> --
> http://www.nuanti.com
> the browser experts
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140224/97c9955b/attachment.html>