[PATCH] Implement DR136

[Richard Smith]

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

Please add this as a testcase, then LGTM.