<div dir="ltr">Committed in r184889.<div><br></div><div>-- </div><div style>David Majnemer</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jun 25, 2013 at 3:31 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Jun 25, 2013 at 3:24 PM, David Majnemer<br>
<<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:<br>
> On Tue, Jun 25, 2013 at 1:30 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
> wrote:<br>
>><br>
>> On Tue, Jun 25, 2013 at 12:59 AM, David Majnemer<br>
>> <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:<br>
>> > The attached patch implements DR136 [*], "Default arguments and friend<br>
>> > functions."<br>
>> ><br>
>> > The crux of this issue is that a function declaration that specifies a<br>
>> > default argument must define the function and be it's only declaration.<br>
>> ><br>
>> > [*] <a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#136" target="_blank">http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#136</a><br>
>><br>
>> --- lib/Sema/SemaDeclCXX.cpp    (revision 184825)<br>
>> +++ lib/Sema/SemaDeclCXX.cpp    (working copy)<br>
>> @@ -404,6 +404,15 @@<br>
>>    }<br>
>>  }<br>
>><br>
>> +static bool functionDeclHasDefaultArgument(const FunctionDecl *FD) {<br>
>> +  for (unsigned NumParams = FD->getNumParams(); NumParams > 0;<br>
>> --NumParams) {<br>
>> +    const ParmVarDecl *PVD = FD->getParamDecl(NumParams-1);<br>
>> +    if (PVD->hasDefaultArg() && !PVD->hasInheritedDefaultArg())<br>
>> +      return true;<br>
>> +  }<br>
>><br>
>> You can bail out early if you hit a parameter that doesn't have a<br>
>> default argument (since no earlier parameters can have a default<br>
>> argument).<br>
><br>
><br>
> Great point, implemented.<br>
><br>
>><br>
>><br>
>> @@ -578,6 +587,18 @@<br>
>>      Invalid = true;<br>
>>    }<br>
>><br>
>> +  // C++11 [dcl.fct.default]p4: If a friend declaration specifies a<br>
>> default<br>
>> +  // argument expression, that declaration shall be a definition and shall<br>
>> be<br>
>> +  // the only declaration of the function or function template in the<br>
>> +  // translation unit.<br>
>> +  if (!isa<CXXMethodDecl>(Old) &&<br>
>><br>
>> What is this check for?<br>
><br>
><br>
> See bellow.<br>
><br>
>><br>
>><br>
>><br>
>> +      Old->getFriendObjectKind() == Decl::FOK_Undeclared &&<br>
>> +      functionDeclHasDefaultArgument(Old)) {<br>
>> +      Diag(New->getLocation(),<br>
>> diag::err_friend_decl_with_def_arg_redeclared);<br>
>> +      Diag(Old->getLocation(), diag::note_previous_declaration);<br>
>> +      Invalid = true;<br>
>> +    }<br>
>><br>
>> Too much indentation for these 4 lines.<br>
><br>
><br>
> Fixed.<br>
><br>
>><br>
>><br>
>><br>
>> @@ -11377,6 +11398,18 @@<br>
>>      else<br>
>>        FD = cast<FunctionDecl>(ND);<br>
>><br>
>> +    // C++11 [dcl.fct.default]p4: If a friend declaration specifies a<br>
>> +    // default argument expression, that declaration shall be a<br>
>> definition<br>
>> +    // and shall be the only declaration of the function or function<br>
>> +    // template in the translation unit.<br>
>> +    if (!isa<CXXMethodDecl>(FD) && functionDeclHasDefaultArgument(FD)) {<br>
>><br>
>> This isa check looks suspicious. Doesn't it lead to us incorrectly<br>
>> accepting this:<br>
>><br>
>>   struct A { void f(int); };<br>
>>   struct B { friend void A::f(int = 0); };<br>
<br>
</div></div>Please add this as a testcase, then LGTM.<br>
</blockquote></div><br></div>