<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jan 13, 2014 at 9:09 AM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><div><div class="h5">
<br>
<div>On 13/01/2014 13:23, Aaron Ballman
wrote:<br>
</div>
<blockquote type="cite">
<pre>On Sun, Jan 12, 2014 at 11:43 PM, Kal <a href="mailto:b17c0de@gmail.com" target="_blank"><b17c0de@gmail.com></a> wrote:
</pre>
<blockquote type="cite">
<pre>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)
</pre>
</blockquote>
<pre>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.</pre>
</blockquote>
<br></div></div>
Yes, for the alternative proposal (3) it only makes sense to name
the macros individually for each attribute type. There's no value in
expecting a 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 need for the spelling to be written because each macro can
test for the attribute name precisely.<br>
<br>
Also instead of 'ident', it's strictly speaking a 'name' in the case
of C++11 generalized attributes given that they can be namespaced
(gnu, omp?). These are the forms and names I'd suggest:<br>
<br>
<blockquote><code>__has_attribute(deprecated) // Test for GNU
attributes only, macro always defined (minor tightening of
semantics)</code><code></code><br>
<code></code><code>__has_declspec(deprecated) // Macro only
defined in MSVC extension mode</code><code></code><br>
<code></code><code>__has_generalized_attribute(gnu::deprecated)</code><code>
// Macro only defined in C++11 mode</code><code></code><br>
<code></code><code>__has_simple_attribute(alignas) // Macro always
defined, but mostly useful for C++11 keyword attributes like
'alignas'</code><br>
</blockquote>
<br>
Overall I think it'll be either this or Aaron's proposal to have a
single unified feature macro that uses the spellings.<br></div></blockquote><div><br></div><div>(Playing devil's advocate here to try to get things off the fence)</div><div><br></div><div>Is it possible for there to be a situation where e.g.</div>
<div><br></div><div>gcc 4.8 supports __attribute__((foo(bar)))</div><div>gcc 4.9 supports __attribute__((foo(bar))) and __attribute__((foo(bar,3)))</div><div><br></div><div>In that case, something like __has_gnu_attribute(foo) wouldn't tell the whole story, but "__has_attribute_syntax" would be able to detect the presence.</div>
<div><br></div><div><br></div><div>Btw, one point of confusion for the name `__has_attribute_syntax` is that it could be interpreted as asking "do we support generalized vs. gnu vs. declspec syntax" (independently of the actual attribute itself), rather than "do we support this attribute when written with this syntax". I.e. `__has_attribute_syntax([[foo]])` could be misinterpreted as being true when we support spelling attributes with generalized syntax (the "foo" is then just a placeholder), and false otherwise (this would be a terrible way to actually implement exposing that check, but that's not necessarily obvious to users).</div>
<div><br></div><div>`__has_attribute_spelling([[foo]])` is slightly better in this regard, but still could be misinterpreted in that way. `__has_attribute_spelled([[foo]])` or `__has_attribute_with_spelling([[foo]])` or `__has_attribute_written([[foo]])` appear to avoid that misinterpretation.</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"><div bgcolor="#FFFFFF" text="#000000">
<br>
This one has the benefit of simpler parse rules in the preprocessor
where we don't have access to balanced token and recovery
facilities, so is less "magical".<br>
<br>
On the other hand Aaron's original proposal has the benefit of being
visually closer to the attribute as written.<br>
<br>
Both this one and Aaron's patch are now forward compatible and well
specified. Overall I'm on the fence as to which is preferable,
slightly favouring Aaron's original proposal.<span class="HOEnZb"><font color="#888888"><br>
<br>
Alp.</font></span><div class="im"><br>
<br>
<br>
<blockquote type="cite">
<pre>~Aaron
_______________________________________________
cfe-dev mailing list
<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a>
</pre>
</blockquote>
<br>
</div><div class="im"><pre cols="72">--
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a>
the browser experts
</pre>
</div></div>
<br>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br></div></div>