[PATCH] Implement DR136

[David Majnemer]

Committed in r184889.

-- 
David Majnemer


On Tue, Jun 25, 2013 at 3:31 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> 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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130625/68bcc177/attachment.html>