rsmith added inline comments. ================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1153 @@ +1152,3 @@ +def warn_non_template_friend : Warning<"friend declaration %q0 depends on " + "template parameter but is not a template function">, + InGroup<NonTemplateFriend>; ---------------- template function -> function template
================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1155-1157 @@ +1154,5 @@ + InGroup<NonTemplateFriend>; +def note_non_template_friend : Note<"if this is intentional, add function " + "declaration to suppress the warning, if not - make sure the function " + "template has already been declared and use '<>'">; +def note_befriend_template : Note<"to befriend a template specialization, " ---------------- Rather than this long conditional note, how about producing two notes here: note: declare function outside class template to suppress this warning note: use '<>' to befriend a template specialization ================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1158-1159 @@ -1152,1 +1157,4 @@ + "template has already been declared and use '<>'">; +def note_befriend_template : Note<"to befriend a template specialization, " + "use '<>'">; ---------------- This note should also point out that you need to declare the function template prior to the class template. ================ Comment at: include/clang/Sema/Sema.h:9342 @@ +9341,3 @@ + /// of proper templates, but they are needed for checks. + llvm::DenseSet<FunctionDecl *> FriendsOfTemplates; + ---------------- This will produce diagnostics in a nondeterministic order, and you have no need of a set because there can never be any duplicates. Just use a `SmallVector`? ================ Comment at: include/clang/Sema/Sema.h:9342 @@ +9341,3 @@ + /// of proper templates, but they are needed for checks. + llvm::DenseSet<FunctionDecl *> FriendsOfTemplates; + ---------------- rsmith wrote: > This will produce diagnostics in a nondeterministic order, and you have no > need of a set because there can never be any duplicates. Just use a > `SmallVector`? You're missing serialization code for this for the PCH / preamble case. ================ Comment at: include/clang/Sema/Sema.h:9344-9345 @@ +9343,4 @@ + + /// \brief Check dependent friends functions for misinterpretation as template + /// ones. + void CheckDependentFriends(); ---------------- friends -> friend template ones -> function templates ================ Comment at: lib/Sema/SemaChecking.cpp:10468 @@ +10467,3 @@ +/// function types. +class FunctionMatcher : public TypeVisitor<FunctionMatcher, bool> { + const Type *InstType; ---------------- This is a huge amount of complexity and maintenance burden for this check, and it still seems to some important cases wrong. Is there a simpler way we can get a largely-equivalent result? Or maybe we can just unconditionally produce both notes? ================ Comment at: lib/Sema/SemaChecking.cpp:10700 @@ +10699,2 @@ + } +} ---------------- `FriendsOfTemplates.clear()` here? ================ Comment at: lib/Sema/SemaDecl.cpp:8502-8507 @@ -8501,1 +8501,8 @@ + if (isFriend && !NewFD->isInvalidDecl()) { + if (TemplateParamLists.empty() && !DC->isRecord() && + !isFunctionTemplateSpecialization) { + FriendsOfTemplates.insert(NewFD); + } + } + ---------------- This should also be conditioned on the function having a dependent type and this declaration not being a definition -- this will add all non-template friends to `FriendsOfTemplates`, whether they're friends of a class template or not. http://reviews.llvm.org/D16579 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits