<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, Feb 23, 2014 at 10:08 PM, 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">Hi Aaron,<br>
<br>
As a fix for PR15853 this is looking OK and almost there.<br>
<br>
It's quirky to put a synthesis of the attribute parse rules in the preprocessor but your approach meets needs that the other solutions don't so it's cool.<br>
<br>
However the addition of new semantics to the existing __has_feature() macro is problematic given that even ToT clang will error out the preprocessor whenever new-style arguments are encountered. That's bad news for a feature check macro that needs to work on a range of clang versions.<br>

<br>
I know you've since added __has_feature(__has_attribute_<u></u>syntax) to work around the problem but it's not discoverable and I suspect the feature check won't get used correctly in the wild.<br>
<br>
So how about just naming the new check __has_attribute_syntax()?<br></blockquote><div><br></div><div>Why would this be more discoverable than __has_feature(__has_attribute_syntax)?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
That way the widely-recognised feature detection fallback pattern will just work and you can drop the  __has_feature workaround entirely. Users will be able to use the familiar:<br>
<br>
#ifndef __has_attribute_syntax<br>
#define __has_attribute_syntax(...)<br>
#endif<br>
<br>
Doing this also means we can remove the old, ambiguous identifier-only form of the check from the new version of the check. With this change to your proposed patch we can ensure safe and compatible usage which is important with vendor extensions like this.<span class="HOEnZb"><font color="#888888"><br>

<br>
Alp.</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
<br>
On 23/02/2014 16:24, Aaron Ballman wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Ping<br>
<br>
~Aaron<br>
<br>
On Wed, Feb 19, 2014 at 1:05 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This is the latest version of this patch -- with some additional<br>
features as requested. Specifically, this is using the existing<br>
__has_attribute syntax in an extended style, and implements a<br>
feature-test macro (__has_feature(__has_<u></u>attribute_syntax)) which<br>
allows you to determine whether the extended syntax is supported or<br>
not. It adds some additional documentation explaining this.<br>
<br>
~Aaron<br>
<br>
On Thu, Jan 23, 2014 at 10:23 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Before I start in on this again, I wanted to make sure that there was<br>
a consensus that this functionality was desirable. I believe the<br>
answer (based on some conversations in IRC and here on the lists) was<br>
tentatively "yes."<br>
<br>
As far as I can tell, the work left to be done on this is to add a<br>
feature test for __has_attribute_syntax, write the documentation for<br>
it and that's about it? The syntax-based form is the preferable<br>
nomenclature because it leaves the door open for testing parameters at<br>
some point in the future, and with the __has_attribute_syntax feature<br>
test, it is both backwards and forwards compatible.<br>
<br>
~Aaron<br>
<br>
On Mon, Jan 13, 2014 at 8:49 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Mon, Jan 13, 2014 at 5:44 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>><br>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Mon, Jan 13, 2014 at 8:39 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>><br>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Mon, Jan 13, 2014 at 5:31 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>><br>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Mon, Jan 13, 2014 at 8:20 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>><br>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Sun, Jan 12, 2014 at 4:49 PM, Aaron Ballman<br>
<<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>><br>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Sun, Jan 12, 2014 at 7:38 PM, Sean Silva <<a href="mailto:silvas@purdue.edu" target="_blank">silvas@purdue.edu</a>><br>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
On Sun, Jan 12, 2014 at 6:53 PM, Aaron Ballman<br>
<<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>><br>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
That's good news -- thanks for confirming.<br>
<br>
The feature detection macro itself will still need to have a<br>
different<br>
name<br>
(or some other mechanism) so it can be used compatibly with<br>
existing<br>
clang<br>
deployments, because _has_attribute() currently emits a parse<br>
error<br>
instead<br>
of gracefully returning 0 when passed the new argument syntax:<br>
<br>
tmp/attr2.cpp:1:5: error: builtin feature check macro requires<br>
a<br>
parenthesized identifier<br>
#if __has_attribute(__attribute__(<u></u>(weakref)))<br>
     ^<br>
</blockquote>
Good catch. Unfortunately, __has_attribute is really the best<br>
identifier for the macro, so I am loathe to let it go.<br>
<br>
Due to the current design of __has_attribute, we can't get away<br>
with<br>
"<br>
magic" by expanding the non-function-like form into a value that<br>
could<br>
be tested. So we would really have to pick a new name if we are<br>
worried about this.<br>
<br>
Suggestions on the name are welcome.<br>
</blockquote>
<br>
Ok, I'll bite:<br>
<br>
__has_attribute_written_as([[<u></u>foo]])<br>
__has_attribute_syntax([[foo]]<u></u>)<br>
__has_attribute_spelling([[<u></u>foo]])<br>
</blockquote>
I kind of like __has_attribute_syntax, truth be told.<br>
</blockquote>
<br>
Have we given up on using the name __has_attribute too soon? Users of<br>
the<br>
new syntax could write:<br>
<br>
// Probably already in project's porting header<br>
#ifndef __has_feature<br>
#define __has_feature(x) 0<br>
#endif<br>
<br>
#if __has_feature(__has_attribute_<u></u>syntax)<br>
#define MY_HAS_ATTRIBUTE(...) __has_attribute(__VA_ARGS__)<br>
#else<br>
#define MY_HAS_ATTRIBUTE(...) 0<br>
#endif<br>
<br>
If it's given a different name, they instead would write something<br>
like:<br>
<br>
#ifdef __has_attribute_syntax<br>
#define MY_HAS_ATTRIBUTE(...) __has_attribute_syntax(__VA_<u></u>ARGS__)<br>
#else<br>
#define MY_HAS_ATTRIBUTE(...) 0<br>
#endif<br>
<br>
So I don't think the change-in-syntax argument holds water.<br>
</blockquote>
So are you proposing that we would have a different name for the<br>
purposes of the __has_feature macro? Eg)<br>
__has_feature(__has_attribute_<u></u>syntax) is 1 for the proposed<br>
functionality, and 0 otherwise?<br>
</blockquote>
<br>
It's a possibility. It could be that a new name is a better approach,<br>
but<br>
both directions seem to be feasible.<br>
</blockquote>
I'll ponder; I rather like keeping the existing name.<br>
</blockquote>
<br>
By the same argument, it's possible to add extra arguments to<br>
__has_attribute, if we have a __has_feature check for the new syntax.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Also, supporting arguments in the attributes is useful in some cases<br>
--<br>
it's<br>
not true that they don't make sense in a feature-checking facility.<br>
For<br>
instance:<br>
<br>
   __has_attribute( __attribute__((format)) )<br>
<br>
... doesn't tell me whether __attribute__((format, gnu_scanf, 1, 2)<br>
will<br>
work (and I'd expect that the format attribute will gain additional<br>
archetypes in future).<br>
</blockquote>
That's true, but the example also demonstrates why it's kind of<br>
nonsensical. What do the 1, 2 represent for the purposes of<br>
__has_attribute?<br>
</blockquote>
<br>
They represent themselves. Suppose we added support for a format<br>
attribute<br>
with negative indices, or with three indices, or something -- this<br>
syntax<br>
would allow the user to determine if that syntax is available.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Can they be elided? If so, can we come up with<br>
declarative rules as to when they can be elided?<br>
</blockquote>
<br>
If you could omit them, how would you tell whether an attribute could be<br>
used without arguments?<br>
<br>
Again, I'm not saying we should go in this direction, but I don't think<br>
we<br>
should dismiss it without consideration -- we probably don't want to<br>
find we<br>
need a third form of __has_attribute later =)<br>
</blockquote>
That's one of the reasons Alp's suggestion for forwards compatibility<br>
is so nice -- if implemented properly, we could add parameter support<br>
at a later date (presuming we stick with the attribute syntax style<br>
approach).<br>
<br>
I'd like to avoid attempting to preprocess parameters for this patch,<br>
but had intended to leave the door open for the future. So it wasn't<br>
entirely without consideration. ;-)<br>
</blockquote>
<br>
=) OK then!<br>
</blockquote></blockquote></blockquote></blockquote>
<br></div></div><div class="HOEnZb"><div class="h5">
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</div></div></blockquote></div><br></div></div>