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

[Kal]

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)


>
> 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
>>
>>
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140113/0d4e9712/attachment.html>