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

[Alp Toker]

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