[PATCH] Implement DR136

[David Majnemer]

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); };
>
> ?
>

The short and boring story is that I added this check while investigating
the crash in PR16423.
Sorry for letting it split into the patch :/

-- 
David Majnemer
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130625/eb112850/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dr136.diff
Type: application/octet-stream
Size: 6806 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130625/eb112850/attachment.obj>