<div dir="ltr">On Tue, Oct 22, 2013 at 2:47 PM, Alp Toker <span dir="ltr"><<a 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><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>
    <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><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 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 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 href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a>
the browser experts
</pre>
  </div></div>

</blockquote></div><br></div></div>