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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to