On 11/01/2014 19:41, Aaron Ballman wrote:
On Sat, Jan 11, 2014 at 2:31 PM, Alp Toker <[email protected]> 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

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to