[PATCH] D16989: Change interpretation of function definition in friend declaration of template class.

[Richard Smith via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

A couple of comment nits, then this LGTM. Thanks!



> SemaDecl.cpp:8659-8664
> +  //       void func();
> +  //       template<typename T> class C1 { friend void func() { } };
> +  //       template<typename T> class C2 { friend void func() { } };
> +  // and
> +  //       template<typename T> class C1 { friend void func(); };
> +  //       template<typename T> class C2 { friend int func(); };

We accept the second example without your change, and never get here (because we never find a `PrevDecl`). Just remove that one? The first example alone is enough.

> SemaDeclCXX.cpp:13770-13774
> +      if (!Previous.empty()) {
>          Diag(FD->getLocation(), diag::err_friend_decl_with_def_arg_redeclared);
> -        Diag(OldFD->getLocation(), diag::note_previous_declaration);
> +        Diag(Previous.getRepresentativeDecl()->getLocation(),
> +             diag::note_previous_declaration);
>        } else if (!D.isFunctionDefinition())

This is a bit of a weird thing to do, so please add a comment here explaining why we're not just looking at `getPreviousDecl()`. Something like:

"We can't look at FD->getPreviousDecl() because it may not have been set if we're in a dependent context. If we get this far with a non-empty Previous set, we must have a valid previous declaration of this function."

https://reviews.llvm.org/D16989