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

[Alp Toker]

On 11/01/2014 20:15, Aaron Ballman wrote:
> No, I think I'm just a bit more dense today than usual. I blame it on
> the weather. ;-)
>
> I tried:
>
> #define __has_attribute(x)  0
>
> #if __has_attribute([[clang::fallthrough]])
> sdfsdfsfsd
> #endif
>
> #if __has_attribute(__attribute__((weakref)))
> dsfdfsdf
> #endif
>
> #if __has_attribute(__declspec(dllexport))
> sdfsdf
> #endif
>
> This compiles without errors or warnings in MSVC 2013, 2012 and 2010.
> Same for gcc 4.8.1.

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)))||
    ||     ^|


For forward compatibility your new version should warn, rather than 
emitting an error, upon encountering invalid attribute syntaxes that may 
be valid in future standards or other language dialects (e.g. 
single-braced C++/CLI [] attributes should return 0, indicating 
"unsupported" rather than triggering a parse error).

What's clear is that _has_attribute() isn't fit for purpose in its 
current form in ToT -- David Blaikie made some general arguments against 
the overall value of attribute feature detection. Assuming however that 
it's a desirable feature to keep around in any form I think your 
proposal is the strongest contender for PR15853.

Alp.


>
> Great question (once I understood it), btw!
>
> ~Aaron
>
> On Sat, Jan 11, 2014 at 3:02 PM, Alp Toker <alp at nuanti.com> wrote:
>> On 11/01/2014 19:41, Aaron Ballman wrote:
>>> On Sat, Jan 11, 2014 at 2:31 PM, Alp Toker <alp at nuanti.com> wrote:
>>>> On 10/01/2014 22:07, Aaron Ballman wrote:
>>>>> The __has_attribute feature macro is fantastic in many respects, but
>>>>> is lacking the ability to determine whether a specific attribute
>>>>> syntax is available or not. Instead, it currently checks whether the
>>>>> attribute is known within the compilation target, and nothing more.
>>>>> This can cause problems because not all attributes are applied in the
>>>>> same way.
>>>>>
>>>>> Consider dllexport as a contrived example:
>>>>>
>>>>> #if __has_attribute(dllexport)
>>>>>      void foo(void) __attribute__((dllexport));
>>>>> #endif
>>>>>
>>>>> This code looks fine, but is actually broken because clang only
>>>>> supports __declspec(dllexport) and not __attribute__((dllexport)), and
>>>>> __declspec must precede the declaration.
>>>>>
>>>>> The attached patch implements new syntax for __has_attribute while
>>>>> retaining backwards compatibility. It allows you to specify exactly
>>>>> which attribute syntax you desire. If no specific syntax is specified,
>>>>> it behaves as it always has.
>>>>>
>>>>> The supported forms are:
>>>>>
>>>>> __has_attribute(__attribute__((ident))) // GNU-style
>>>>> __has_attribute(__declspec(ident)) // MS-style
>>>>> __has_attribute([[ident]])  // C++11-style
>>>>> __has_attribute([[scope::ident]]) // C++11-style
>>>>> __has_attribute(ident) // Keywords, or "don't care"
>>>>>
>>>>> Note that attribute arguments are not supported by design -- they
>>>>> really don't make any sense in the context of a feature macro.
>>>>
>>>> Hi Aaron,
>>>>
>>>> This is a step forward with some long-standing problems so certainly
>>>> would
>>>> be a step forward. The syntax is unconventional but not unreasonable.
>>>>
>>>> Have you confirmed that the new __has_attribute() syntax can still be
>>>> defined to an empty expansion? That pattern is important to provide
>>>> source
>>>> compatibility with gcc / MSVC. The latter in particular has fairly
>>>> different
>>>> expansion rules to watch out for -- I've got a feeling it'll be OK as
>>>> long
>>>> as no commas appear in the argument list (which was a problem with the
>>>> other
>>>> proposed "cxx, ..." syntax) but it's worth double checking.
>>> There's currently a test in the test suite which I think covers this case:
>>>
>>> // CHECK: has_has_attribute
>>> #ifdef __has_attribute
>>> int has_has_attribute();
>>> #endif
>>>
>>> Is that what you are referring to? If so, then yes, this patch does
>>> still meet that need.
>>
>> Sorry, I wasn't particularly clear :-)
>>
>> The ifdef/undef ability for __has_attribute in clang itself is clearly
>> fine..
>>
>> I was referring rather to the compatibility, in practice, of the following
>> pattern (as seen in LLVM's Compiler.h):
>>
>> #ifndef __has_attribute
>> # define __has_attribute(x) 0
>> #endif
>>
>> This would potentially have been a problem with syntax (2) proposed by
>> Richard Smith in PR15853 given that commas are macro argument delimiters,
>> and compounded by the fact that Microsoft has a different take on variadic
>> macros. That would have made it difficult or impossible to provide an empty
>> macro definition fallback for non-clang compilers.
>>
>> What I'm wondering is whether your proposed __has_attribute syntax has any
>> similar expansion problems, taking into account the kinds of arguments it
>> accepts.
>>
>> I'm hopeful that there won't be a problem because you've already
>> intentionally excluded attribute arguments from the syntax, therefore commas
>> can't appear -- but it's still worth checking this to see if the various
>> attribute token sequences work in practice with third-party preprocessors.
>>
>> Alp.
>>
>>
>>
>>> Thanks!
>>>
>>> ~Aaron
>>
>> --
>> http://www.nuanti.com
>> the browser experts
>>

-- 
http://www.nuanti.com
the browser experts

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140112/55a2bca2/attachment.html>