[PATCH] PR17337: Retained language linkage

[Richard Smith]

On Tue, Oct 22, 2013 at 2:47 PM, Alp Toker <alp at nuanti.com> wrote:

>
> On 22/10/2013 20:26, Richard Smith wrote:
>
>  +def ext_retained_language_linkage : Extension<
> +  "friend function %0 retaining previous language linkage is an
> extension">,
> +  InGroup<DiagGroup<"retained-language-linkage">>;
>
>  I'd like this diagnostic to be clearer that
>
>    extern "C" void f();
>   struct S {
>     friend void f();
>   };
>
>  is fine but
>
>    extern "C" void f();
>   extern "C++" {
>     struct S {
>       friend void f();
>     };
>   }
>
>  is the extension. Maybe add a note pointing at the innermost surrounding
> linkage specification that we're ignoring?
>
>
> I don't think that's the right way to look at this change.
>
> 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:
>
> [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).
>
>
This wording was vague and has been fixed; you need a more recent draft of
the standard =) Here's what the latest draft says.

"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 *declared within* the
linkage-specification."

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...)

On the other hand, highlighting the selected outer linkage spec _would_ be
> nice to do when diagnosing linkage specs in general.
>
> 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.
>

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.

 +      // The friend object kind isn't yet complete so check IDNS directly.
> +      if (New->getIdentifierNamespace() & Decl::IDNS_OrdinaryFriend) {
>
>  We don't care whether it's FOK_Declared or FOK_Undeclared, so why not:
>
>    if (New->getFriendObjectKind() != FOK_None) {
>
>  ?
>
>
> Sure, let's do that.
>
> So, I'm looking to land this patch with the getFriendObjectKind() change
> and then contining with general improvement of linkage spec diagnostics.
>
> Let me know if you feel strongly enough about the diagnostic to object to
> landing this in the next day or so.
>

If you want to land the diagnostic improvement separately, that's fine by
me.


> On Tue, Oct 22, 2013 at 10:46 AM, Alp Toker <alp at nuanti.com> wrote:
>
> Hello Richard,
>>
>> With this patch, friend function declarations will retain the language
>> linkage specified for previous declarations instead of emitting an error
>> diagnostic.
>>
>> The feature is known to be compatible with GCC and MSVC and permits a
>> language to be specified indirectly where it cannot otherwise be written
>> directly in class scope.
>>
>> Further to the previous patch, this feature is now a clang extension so
>> warnings can be enabled/disabled with a -Wretained-language-linkage flag
>> while we seek clarifications to the language standard. Tests have been
>> moved to SemaCXX/linkage-spec.cpp.
>>
>> Alp.
>>
>> --
>> http://www.nuanti.com
>> the browser experts
>>
>>
>
> -- http://www.nuanti.com
> the browser experts
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131022/cffdfef8/attachment.html>