<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jan 13, 2014 at 1:12 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Jan 13, 2014 at 1:03 PM, Sean Silva <<a href="mailto:silvas@purdue.edu">silvas@purdue.edu</a>> wrote:<br>

><br>
><br>
><br>
> On Mon, Jan 13, 2014 at 9:09 AM, Alp Toker <<a href="mailto:alp@nuanti.com">alp@nuanti.com</a>> wrote:<br>
>><br>
>><br>
>> On 13/01/2014 13:23, Aaron Ballman wrote:<br>
>><br>
>> On Sun, Jan 12, 2014 at 11:43 PM, Kal <<a href="mailto:b17c0de@gmail.com">b17c0de@gmail.com</a>> wrote:<br>
>><br>
>> Am 13.01.14 03:43, schrieb Aaron Ballman:<br>
>><br>
>> New patch attached with some suggestions implemented. Changes include:<br>
>><br>
>> 1) Updated the cmake file because the name of the tablegen file was<br>
>> updated (this should have been in the initial patch)<br>
>> 2) Lexing unknown syntax now attempts better recovery. This allows us<br>
>> to be a bit more future-proof<br>
>> 3) Unknown syntax is now handled as a warning instead of an error.<br>
>> However, malformed __has_attribute is still treated as an error (for<br>
>> instance, __has_attribute[haha] will be an error, but<br>
>> __has_attribute([haha]) will be a warning). The warning may warrant<br>
>> its own diagnostic group instead of piggy-backing on UnknownAttribute.<br>
>> 4) C++11 language option checking was moved into tablegen instead of<br>
>> the preprocessor<br>
>> 5) Updated test cases for the new warning, future-proofing and the<br>
>> movement of C++11 language option checking<br>
>><br>
>> What this patch does not address is:<br>
>><br>
>> * It's still using __has_attribute while we discuss an improved name<br>
>> * The default __has_attribute(ident) functionality still checks for<br>
>> all attribute names (changing this would be a separate patch,<br>
>> regardless)<br>
>><br>
>> One other thing to consider with regards to naming -- another approach<br>
>> we could take would be to not encode the syntax directly in the<br>
>> feature macro, but instead use separate macros (with its analogous<br>
>> currently-proposed syntax in comments):<br>
>><br>
>> __has_gnu_attribute(ident)  // __has_attribute(__attribute__((ident)))<br>
>> __has_declspec_attribute(ident)  // __has_attribute(__declspec(ident))<br>
>> __has_generalized_attribute(ident) // __has_attribute([[ident]])<br>
>> __has_keyword_attribute(ident)  // __has_attribute(ident)<br>
>><br>
>><br>
>> What about the following instead (could also swap the order of these<br>
>> arguments):<br>
>><br>
>> __has_attribute(gnu,         ident) //<br>
>> __has_attribute(__attribute__((ident)))<br>
>> __has_attribute(declspec,    ident) // __has_attribute(__declspec(ident))<br>
>> __has_attribute(generalized, ident) // __has_attribute([[ident]])<br>
>> __has_attribute(keyword,     ident) // __has_attribute(ident)<br>
>><br>
>> Thank you for the input!<br>
>><br>
>> Unfortunately, it would cause backwards compatibility problems as the<br>
>> current implementation has no fallback mode for syntaxes it doesn't<br>
>> understand (so this would break existing code, and have no migration<br>
>> path forward). At the very least, the name would have to change.<br>
>> Personally, if we are using a single identifier for the feature test<br>
>> macro, I would prefer to key off the distinct attribute syntax instead<br>
>> of another contextualized identifier. Users already know how to write<br>
>> the attribute, so the syntax is something already known. A new<br>
>> contextual identifier is one more thing to learn and remember.<br>
>><br>
>><br>
>> Yes, for the alternative proposal (3) it only makes sense to name the<br>
>> macros individually for each attribute type. There's no value in expecting a<br>
>> name in the first macro argument and the comma form would be incompatible.<br>
>><br>
>> As a tweak to this latest alternative proposal, there's also no longer a<br>
>> need for the spelling to be written because each macro can test for the<br>
>> attribute name precisely.<br>
>><br>
>> Also instead of 'ident', it's strictly speaking a 'name' in the case of<br>
>> C++11 generalized attributes given that they can be namespaced (gnu, omp?).<br>
>> These are the forms and names I'd suggest:<br>
>><br>
>> __has_attribute(deprecated) // Test for GNU attributes only, macro always<br>
>> defined (minor tightening of semantics)<br>
>> __has_declspec(deprecated) // Macro only defined in MSVC extension mode<br>
>> __has_generalized_attribute(gnu::deprecated) // Macro only defined in<br>
>> C++11 mode<br>
>> __has_simple_attribute(alignas) // Macro always defined, but mostly useful<br>
>> for C++11 keyword attributes like 'alignas'<br>
>><br>
>><br>
>> Overall I think it'll be either this or Aaron's proposal to have a single<br>
>> unified feature macro that uses the spellings.<br>
><br>
><br>
> (Playing devil's advocate here to try to get things off the fence)<br>
><br>
> Is it possible for there to be a situation where e.g.<br>
><br>
> gcc 4.8 supports __attribute__((foo(bar)))<br>
> gcc 4.9 supports __attribute__((foo(bar))) and __attribute__((foo(bar,3)))<br>
><br>
> In that case, something like __has_gnu_attribute(foo) wouldn't tell the<br>
> whole story, but "__has_attribute_syntax" would be able to detect the<br>
> presence.<br>
<br>
</div></div>The syntax-based approach leaves the door open to testing parameter<br>
forms, but I'm not certain we'd ever want to go down that path in<br>
practice because so many attribute parameters require contextual<br>
information, and it seems like it would open the door to all sorts of<br>
impossible-to answer questions.<br>
<div class="im"><br>
> Btw, one point of confusion for the name `__has_attribute_syntax` is that it<br>
> could be interpreted as asking "do we support generalized vs. gnu vs.<br>
> declspec syntax" (independently of the actual attribute itself), rather than<br>
> "do we support this attribute when written with this syntax". I.e.<br>
> `__has_attribute_syntax([[foo]])` could be misinterpreted as being true when<br>
> we support spelling attributes with generalized syntax (the "foo" is then<br>
> just a placeholder), and false otherwise (this would be a terrible way to<br>
> actually implement exposing that check, but that's not necessarily obvious<br>
> to users).<br>
<br>
</div>True, but the user would likely catch that mistake pretty quickly (I<br>
would hope) and not make it again.<br>
<div class="im"><br>
> `__has_attribute_spelling([[foo]])` is slightly better in this regard, but<br>
> still could be misinterpreted in that way.<br>
> `__has_attribute_spelled([[foo]])` or<br>
> `__has_attribute_with_spelling([[foo]])` or<br>
<br>
</div>Spelling for attributes is a bit different than the syntax. For<br>
instance, you can have the same attribute "spelled" different ways.<br>
Eg) AlignedAttr is spelled aligned, align, alignas and _Alignas (with<br>
some differing syntaxes mixed in for good measure). So I'd want to<br>
avoid using "spelling" in the name as that could also cause confusion.<br>
<div class="im"><br>
> `__has_attribute_written([[foo]])` appear to avoid that misinterpretation.<br>
<br>
</div>I'd be amenable to that, but still slightly prefer _syntax since that<br>
form follows function a bit more closely.<br></blockquote><div><br></div><div>Cool. Just wanted to bring up this possible misinterpretation. I have no opinion on the right direction.</div><div><br></div><div>-- Sean Silva</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span></blockquote></div><br></div></div>