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