<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 25/02/2014 15:13, Alp Toker wrote:<br>
</div>
<blockquote cite="mid:530CB325.9010208@nuanti.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<br>
<div class="moz-cite-prefix">On 25/02/2014 13:45, Aaron Ballman
wrote:<br>
</div>
<blockquote
cite="mid:CAAt6xTvyik3a5gtyJ0CircQAQGPvS3KFEw+CW0cD8Txsyk9avw@mail.gmail.com"
type="cite">
<pre wrap="">On Mon, Feb 24, 2014 at 6:46 PM, Alp Toker <a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:alp@nuanti.com"><alp@nuanti.com></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On 24/02/2014 16:19, David Blaikie wrote:
</pre>
<blockquote type="cite">
<pre wrap="">
On Sun, Feb 23, 2014 at 10:08 PM, Alp Toker <<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:alp@nuanti.com">alp@nuanti.com</a>
<a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:alp@nuanti.com"><mailto:alp@nuanti.com></a>> wrote:
Hi Aaron,
As a fix for PR15853 this is looking OK and almost there.
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.
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.
I know you've since added __has_feature(__has_attribute_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.
So how about just naming the new check __has_attribute_syntax()?
Why would this be more discoverable than
__has_feature(__has_attribute_syntax)?
</pre>
</blockquote>
<pre wrap="">
If a user writes __has_attribute_syntax() and it doesn't work on some other
compiler, the natural thing to do is to check and conditionalize on whether
the macro exists with ifdef.
#ifndef __has_attribute_syntax
#define __has_attribute_syntax(...)
#endif
That's how the existing feature check macros work and users have been
successfully conditionalizing the macros in this way.
The trouble is that __has_attribute() already exists so changing the
semantics will break current versions of clang:
#ifndef __has_attribute
#define __has_attribute([[deprecated]]) // no good, preprocessor error on
clang ToT
#endif
</pre>
</blockquote>
<pre wrap="">This same problem exists with __has_attribute_syntax.
</pre>
<blockquote type="cite">
<pre wrap="">It's non-obvious for the user to then make the leap that this
__has_attribute() check needs to be conditionalized against a second level
__has_feature(__has_attribute_syntax) because the user has to learn two
other features -- '__has_feature' and '__has_attribute_syntax' which have
little connection to '__has_attribute' in order to safely use
'__has_attribute' with the new semantics.
Adding the functionality with a new name makes it usable without resorting
to the compiler documentation.
</pre>
</blockquote>
<pre wrap="">I disagree -- you still have to refer to the documentation because you
have no idea __has_attribute_syntax exists in the first place (esp
when __has_attribute has been around for a while).
Richard had pointed out a while back that
__has_feature(__has_attribute_syntax) is morally equivalent to using
__has_attribute_syntax as a new spelling for __has_attribute, and I
agree. I would rather use __has_attribute as </pre>
</blockquote>
<br>
<br>
<blockquote
cite="mid:CAAt6xTvyik3a5gtyJ0CircQAQGPvS3KFEw+CW0cD8Txsyk9avw@mail.gmail.com"
type="cite">
<pre wrap="">1) it's already an
established API that we can extend, </pre>
</blockquote>
<br>
Well, my take on this is that it's an established API that wasn't
designed to be extensible :-)<br>
<br>
<blockquote
cite="mid:CAAt6xTvyik3a5gtyJ0CircQAQGPvS3KFEw+CW0cD8Txsyk9avw@mail.gmail.com"
type="cite">
<pre wrap="">2) it's a less awkward spelling</pre>
</blockquote>
<br>
Granted, but that needs to be weighed up against the cost to users
of breaking backward compatibility.<br>
<br>
<blockquote
cite="mid:CAAt6xTvyik3a5gtyJ0CircQAQGPvS3KFEw+CW0cD8Txsyk9avw@mail.gmail.com"
type="cite">
<pre wrap="">for the functionality, and 3) It's one less API for users to have to
keep in their brain. </pre>
</blockquote>
<br>
Not really, it's three things to learn (bold emphasis on the
points that need to be learnt below):<br>
<br>
<code>#if defined(<b>__has_feature</b>) && __has_feature(<b>__has_attribute_syntax</b>)
&& <b>__has_attribute</b>([[deprecated]])</code><code><br>
</code><code> </code><br>
Instead of one thing to learn:<br>
<br>
<code>#if defined(<b>__has_attribute_syntax</b>) &&
__has_attribute_syntax([[deprecated]])</code><br>
<br>
I prefer the latter because it matches the prescribed usage for
all the other feature detection macros:<br>
<br>
<br>
<meta charset="utf-8">
<pre style="overflow-x: auto; overflow-y: hidden; border: thin dotted rgb(12, 55, 98); margin: 0px 0px 12px; padding: 0.8em; background-color: rgb(240, 240, 240); color: rgb(51, 51, 51); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">#ifndef __has_builtin // Optional of course.
#define __has_builtin(x) 0 // Compatibility with non-clang compilers.
#endif</pre>
<br>
<meta charset="utf-8">
<pre style="overflow-x: auto; overflow-y: hidden; border: thin dotted rgb(12, 55, 98); margin: 0px 0px 12px; padding: 0.8em; background-color: rgb(240, 240, 240); color: rgb(51, 51, 51); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">#ifndef __has_feature // Optional of course.
#define __has_feature(x) 0 // Compatibility with non-clang compilers.
#endif</pre>
<meta charset="utf-8">
<meta charset="utf-8">
<pre style="overflow-x: auto; overflow-y: hidden; border: thin dotted rgb(12, 55, 98); margin: 0px 0px 12px; padding: 0.8em; background-color: rgb(240, 240, 240); color: rgb(51, 51, 51); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span class="cp" style="color: rgb(0, 112, 32);">#if defined(__has_include)</span>
<span class="cp" style="color: rgb(0, 112, 32);">#if __has_include("myinclude.h")</span>
<span class="cp" style="color: rgb(0, 112, 32);"># include "myinclude.h"</span>
<span class="cp" style="color: rgb(0, 112, 32);">#endif</span>
<span class="cp" style="color: rgb(0, 112, 32);">#endif</span></pre>
etc.<br>
<br>
Is the old name really that precious that we'd be willing to make
__has_attribute() a special case that's inconsistent with the
other feature macros?<br>
<br>
<blockquote
cite="mid:CAAt6xTvyik3a5gtyJ0CircQAQGPvS3KFEw+CW0cD8Txsyk9avw@mail.gmail.com"
type="cite">
<pre wrap="">Also, remember that the "older versions of clang"
problem will go away with time, but the semi-duplicated syntax is
something we'd have forever.</pre>
</blockquote>
<br>
Changing the semantics means pain for our users until those
compilers go away, and compilers have a tendency to stick around
:-/<br>
<br>
Also we've been trying to clear up __has_feature() (see my
proposal on C++ type trait detection) so it grates that we'd be
adding another non-language-standard check while at the same time
trying to deprecate the existing ones:
<meta charset="utf-8">
<span style="color: rgb(51, 51, 51); font-family: 'DejaVu Sans',
Arial, Helvetica, sans-serif; font-size: 14px; font-style:
normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: 21.600000381469727px;
orphans: auto; text-align: justify; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); display: inline
!important; float: none;"><span class="Apple-converted-space"><br>
<br>
"</span></span><tt class="docutils literal"
style="background-color: rgb(226, 226, 226); font-size: 1em;
font-family: monospace; color: rgb(51, 51, 51); font-style:
normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; orphans: auto; text-align: justify;
text-indent: 0px; text-transform: none; white-space: normal;
widows: auto; word-spacing: 0px; -webkit-text-stroke-width:
0px;"><span class="pre">__has_feature</span></tt><span
style="color: rgb(51, 51, 51); font-family: 'DejaVu Sans',
Arial, Helvetica, sans-serif; font-size: 14px; font-style:
normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: 21.600000381469727px;
orphans: auto; text-align: justify; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); display: inline
!important; float: none;"><span class="Apple-converted-space"> </span>evaluates
to 1 if the feature is <b>both</b> supported by Clang and <b><i>standardized
in the current language standard</i></b> or 0 if not. </span>
<meta charset="utf-8">
<span style="color: rgb(51, 51, 51); font-family: 'DejaVu Sans',
Arial, Helvetica, sans-serif; font-size: 14px; font-style:
normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: 21.600000381469727px;
orphans: auto; text-align: justify; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); display: inline
!important; float: none;">For <b><i>backwards compatibility</i></b>
reasons,<span class="Apple-converted-space"> </span></span><tt
class="docutils literal" style="background-color: rgb(226, 226,
226); font-size: 1em; font-family: monospace; color: rgb(51, 51,
51); font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; orphans: auto; text-align:
justify; text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;"><span class="pre">__has_feature</span></tt><span
style="color: rgb(51, 51, 51); font-family: 'DejaVu Sans',
Arial, Helvetica, sans-serif; font-size: 14px; font-style:
normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: 21.600000381469727px;
orphans: auto; text-align: justify; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); display: inline
!important; float: none;"><span class="Apple-converted-space"> </span>can
also be used to test for support for non-standardized features,
i.e. features not prefixed<span class="Apple-converted-space"> </span></span><tt
class="docutils literal" style="background-color: rgb(226, 226,
226); font-size: 1em; font-family: monospace; color: rgb(51, 51,
51); font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; orphans: auto; text-align:
justify; text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;"><span class="pre">c_</span></tt><span
style="color: rgb(51, 51, 51); font-family: 'DejaVu Sans',
Arial, Helvetica, sans-serif; font-size: 14px; font-style:
normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: 21.600000381469727px;
orphans: auto; text-align: justify; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); display: inline
!important; float: none;">,<span class="Apple-converted-space"> </span></span><tt
class="docutils literal" style="background-color: rgb(226, 226,
226); font-size: 1em; font-family: monospace; color: rgb(51, 51,
51); font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; orphans: auto; text-align:
justify; text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;"><span class="pre">cxx_</span></tt><span
style="color: rgb(51, 51, 51); font-family: 'DejaVu Sans',
Arial, Helvetica, sans-serif; font-size: 14px; font-style:
normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: 21.600000381469727px;
orphans: auto; text-align: justify; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); display: inline
!important; float: none;"><span class="Apple-converted-space"> </span>or</span><tt
class="docutils literal" style="background-color: rgb(226, 226,
226); font-size: 1em; font-family: monospace; color: rgb(51, 51,
51); font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; orphans: auto; text-align:
justify; text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;"><span class="pre">objc_</span></tt><span
style="color: rgb(51, 51, 51); font-family: 'DejaVu Sans',
Arial, Helvetica, sans-serif; font-size: 14px; font-style:
normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: 21.600000381469727px;
orphans: auto; text-align: justify; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); display: inline
!important; float: none;">."</span><br>
</blockquote>
<br>
And I forgot to mention, changing the semantics of __has_attribute()
then recommending users guard it with
__has_feature(__has_attribute_syntax) makes it difficult for other
vendors to implement the proposed feature independently without also
supporting a clang-style __has_feature() detection macro. This is
secondary but still worth keeping in mind.<br>
<br>
<blockquote cite="mid:530CB325.9010208@nuanti.com" type="cite"> <br>
I don't mean to be contrary but I did want to write out my
justification in full here from a language design perspective. If
you know all this and want to go ahead, it's your module :-)<br>
</blockquote>
<br>
It may be in practice that the points raised don't matter much and
I'd like to avoid confirmation bias -- perhaps Richard can
tie-break?<br>
<br>
Alp.<br>
<br>
<br>
<pre class="moz-signature" cols="72">--
<a class="moz-txt-link-freetext" href="http://www.nuanti.com">http://www.nuanti.com</a>
the browser experts
</pre>
</body>
</html>