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

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

This would still be future-proof regardless of new attribute syntaxes,
and would solve the naming question. The downside is that I think it's
less discoverable, and means we're using N identifiers instead of one
identifier for the feature test. But I figured it'd be best to bring
it up as an option in case it sways anyone's opinions.

~Aaron


On Sun, Jan 12, 2014 at 8:40 PM, Alp Toker <alp at nuanti.com> wrote:
>
> On 12/01/2014 23:57, Aaron Ballman wrote:
>>
>> Also, as I mentioned above, the same argument could be made for any of
>> the feature test macros. Saying "but there's documentation" is not a
>> compelling argument to me.
>
>
> Hi David,
>
> The point Aaron makes here is significant.
>
> The reality goes even further: feature detection macros are little more than
> "fun" opportunistic features intended to save the hassle of a configure-time
> feature check. Many of your criticisms are valid and apply to other
> detection macros but that doesn't stop them being useful in a lot of trivial
> use cases.
>
> Serious projects* with an expectation of full-featured portability and
> guarantees that a given language construct works as intended shouldn't be
> using these macros in the first place.
>
> So I'd suggest getting the recommended usage documented as a resolution.
>
> * LLVM's Compiler.h is an exception because it's biased and optimizes for
> bootstrap clang builds. Much of Compiler.h was simply disabled in non-clang
> builds until very recently without anyone noticing.
>
> Alp.
>
>
> --
> http://www.nuanti.com
> the browser experts
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: HasAttribute.patch
Type: application/octet-stream
Size: 21418 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140112/6809e55b/attachment.obj>