<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
On 22/10/2013 23:04, Richard Smith wrote:<br>
<blockquote
cite="mid:CAOfiQqky=0ueQYDT7kawYtp3ZN+TtjoPaUpFYo84+xMSCFHQrw@mail.gmail.com"
type="cite">
<div dir="ltr">On Tue, Oct 22, 2013 at 2:47 PM, Alp Toker <span
dir="ltr"><<a moz-do-not-send="true"
href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span>
wrote:<br>
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div class="im"> <br>
<div>On 22/10/2013 20:26, Richard Smith wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div>
<div>+def ext_retained_language_linkage :
Extension<</div>
<div>+ "friend function %0 retaining previous
language linkage is an extension">,</div>
<div>+
InGroup<DiagGroup<"retained-language-linkage">>;</div>
</div>
<div><br>
</div>
<div>I'd like this diagnostic to be clearer that</div>
<div><br>
</div>
<div> extern "C" void f();</div>
<div> struct S {</div>
<div> friend void f();</div>
<div> };</div>
<div><br>
</div>
<div>is fine but</div>
<div><br>
</div>
<div> extern "C" void f();</div>
<div> extern "C++" {</div>
<div> struct S {</div>
<div> friend void f();</div>
<div> };</div>
<div> }</div>
<div><br>
</div>
<div>is the extension. Maybe add a note pointing
at the innermost surrounding linkage
specification that we're ignoring?</div>
</div>
</blockquote>
<br>
</div>
I don't think that's the right way to look at this
change.<br>
<br>
My interpretation of the spec is very much that friend
function declarations don't have a linkage spec in the
first place because they don't introduce a function
name:<br>
<br>
<pre>[dcl.link]p4:
In a linkage-specification,
the specified language linkage applies to the function types of all
function declarators, function names, and variable names **introduced** by
the declaration(s).</pre>
</div>
</blockquote>
<div><br>
</div>
<div>This wording was vague and has been fixed; you need a
more recent draft of the standard =) Here's what the
latest draft says.</div>
<div><br>
</div>
<div>
<div>"In a linkage-specification, the specified language
linkage applies to the function types of all function
declarators, function names with external linkage, and
variable names with external linkage <b>declared within</b>
the linkage-specification."</div>
</div>
<div><br>
</div>
<div>Even if the linkage-specification didn't apply to the
function name, it would apply to the function type and
again make the code ill-formed (but we don't implement
that rule...)</div>
<div>
<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> On the other hand,
highlighting the selected outer linkage spec _would_ be
nice to do when diagnosing linkage specs in general.<br>
<br>
I have some patches back from when the linkage spec work
started in 2010 to improve diags. Thinking to get back
to it, but either way, digging up and passing around the
LinkageSpecDecl belongs in a separate patch.</div>
</blockquote>
<div><br>
</div>
<div>I don't think the current diagnostic is very clear.
Friend functions retaining their previous language linkage
is standard; overriding an explicit linkage specification
with that of an earlier declaration is not. The diagnostic
suggests that the rule is something other than what it
actually is. For obscure rules like these, people use our
diagnostics to learn how the language works, and we do
them a disservice if we mislead them.</div>
</div>
</div>
</div>
</blockquote>
<br>
Alright Richard, you've made a compelling argument :-)<br>
<br>
I'll give you that diagnostic but it'll still come in an additional
commit. I'd like to extract the linkage spec SourceLocations
separately in a way that's generally useful.<br>
<br>
<blockquote
cite="mid:CAOfiQqky=0ueQYDT7kawYtp3ZN+TtjoPaUpFYo84+xMSCFHQrw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div class="im">
<blockquote type="cite">
<div dir="ltr">
<div>+ // The friend object kind isn't yet
complete so check IDNS directly.</div>
<div>+ if (New->getIdentifierNamespace()
& Decl::IDNS_OrdinaryFriend) {</div>
<div><br>
</div>
<div>We don't care whether it's FOK_Declared or
FOK_Undeclared, so why not:</div>
<div><br>
</div>
<div> if (New->getFriendObjectKind() !=
FOK_None) {</div>
<div><br>
</div>
<div>?</div>
</div>
</blockquote>
<br>
</div>
Sure, let's do that.<br>
<br>
So, I'm looking to land this patch with the
getFriendObjectKind() change and then contining with
general improvement of linkage spec diagnostics.<br>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
<br>
Just as we've decided where to go with the diagnostic, I've now had
second thoughts about your suggested IDNS change..<br>
<br>
Looking back, I chose<br>
<br>
<code>if (New->getIdentifierNamespace() &
Decl::IDNS_OrdinaryFriend)</code><br>
<br>
intentionally instead of<br>
<br>
<code>if (New->getFriendObjectKind() != FOK_None)</code><br>
<br>
This was to make it absolutely clear that we only retain the linkage
of _function declarations_ and not of any other friend kind. If the
getFriendObjectKind() version gets folded back into
haveIncompatibleLanguageLinkages() and applied to VarDecls or
potentially other Decl kinds (as it was in the original patch)
there's a risk it'll introduce a bug.<br>
<br>
So while we're splitting hairs, what do you think, stick with IDNS
or go to getFriendObjectKind()?<br>
<br>
<br>
<blockquote
cite="mid:CAOfiQqky=0ueQYDT7kawYtp3ZN+TtjoPaUpFYo84+xMSCFHQrw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> <br>
Let me know if you feel strongly enough about the
diagnostic to object to landing this in the next day or
so.</div>
</blockquote>
<div><br>
</div>
<div>If you want to land the diagnostic improvement
separately, that's fine by me.</div>
</div>
</div>
</div>
</blockquote>
<br>
Thanks for reviewing, it is appreciated.<br>
<br>
Alp.<br>
<br>
<br>
<blockquote
cite="mid:CAOfiQqky=0ueQYDT7kawYtp3ZN+TtjoPaUpFYo84+xMSCFHQrw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span
style="color:rgb(80,0,80)">On Tue, Oct 22, 2013 at
10:46 AM, Alp Toker </span><span dir="ltr"
style="color:rgb(80,0,80)"><<a
moz-do-not-send="true" href="mailto:alp@nuanti.com"
target="_blank">alp@nuanti.com</a>></span><span
style="color:rgb(80,0,80)"> wrote:</span>
<div class="im">
<blockquote type="cite">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hello
Richard,<br>
<br>
With this patch, friend function declarations
will retain the language<br>
linkage specified for previous declarations
instead of emitting an error<br>
diagnostic.<br>
<br>
The feature is known to be compatible with GCC
and MSVC and permits a<br>
language to be specified indirectly where it
cannot otherwise be written<br>
directly in class scope.<br>
<br>
Further to the previous patch, this feature is
now a clang extension so<br>
warnings can be enabled/disabled with a
-Wretained-language-linkage flag<br>
while we seek clarifications to the language
standard. Tests have been<br>
moved to SemaCXX/linkage-spec.cpp.<br>
<span><font color="#888888"><br>
Alp.<br>
<br>
--<br>
<a moz-do-not-send="true"
href="http://www.nuanti.com"
target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</font></span></blockquote>
</div>
<br>
</div>
</blockquote>
<br>
<pre cols="72">--
<a moz-do-not-send="true" href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a>
the browser experts
</pre>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<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>