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

[Aaron Ballman]

On Sun, Jan 12, 2014 at 11:43 PM, Kal <b17c0de at gmail.com> wrote:
> Am 13.01.14 03:43, schrieb Aaron Ballman:
>
> New patch attached with some suggestions implemented. Changes include:
>
> 1) Updated the cmake file because the name of the tablegen file was
> updated (this should have been in the initial patch)
> 2) Lexing unknown syntax now attempts better recovery. This allows us
> to be a bit more future-proof
> 3) Unknown syntax is now handled as a warning instead of an error.
> However, malformed __has_attribute is still treated as an error (for
> instance, __has_attribute[haha] will be an error, but
> __has_attribute([haha]) will be a warning). The warning may warrant
> its own diagnostic group instead of piggy-backing on UnknownAttribute.
> 4) C++11 language option checking was moved into tablegen instead of
> the preprocessor
> 5) Updated test cases for the new warning, future-proofing and the
> movement of C++11 language option checking
>
> What this patch does not address is:
>
> * It's still using __has_attribute while we discuss an improved name
> * The default __has_attribute(ident) functionality still checks for
> all attribute names (changing this would be a separate patch,
> regardless)
>
> One other thing to consider with regards to naming -- another approach
> we could take would be to not encode the syntax directly in the
> feature macro, but instead use separate macros (with its analogous
> currently-proposed syntax in comments):
>
> __has_gnu_attribute(ident)  // __has_attribute(__attribute__((ident)))
> __has_declspec_attribute(ident)  // __has_attribute(__declspec(ident))
> __has_generalized_attribute(ident) // __has_attribute([[ident]])
> __has_keyword_attribute(ident)  // __has_attribute(ident)
>
>
> What about the following instead (could also swap the order of these
> arguments):
>
> __has_attribute(gnu,         ident) //
> __has_attribute(__attribute__((ident)))
> __has_attribute(declspec,    ident) // __has_attribute(__declspec(ident))
> __has_attribute(generalized, ident) // __has_attribute([[ident]])
> __has_attribute(keyword,     ident) // __has_attribute(ident)
>

Thank you for the input!

Unfortunately, it would cause backwards compatibility problems as the
current implementation has no fallback mode for syntaxes it doesn't
understand (so this would break existing code, and have no migration
path forward). At the very least, the name would have to change.
Personally, if we are using a single identifier for the feature test
macro, I would prefer to key off the distinct attribute syntax instead
of another contextualized identifier. Users already know how to write
the attribute, so the syntax is something already known. A new
contextual identifier is one more thing to learn and remember.

~Aaron