[Clang][Sema] Diagnose friend function specialization definitions (#7…

…2863)

Per [[class.friend]p6](http://eel.is/c++draft/class.friend#6) a friend
function shall not be defined if its name isn't unqualified. A
_template-id_ is not a name, meaning that a friend function
specialization does not meet this criteria and cannot be defined.

GCC, MSVC, and EDG all consider friend function specialization
definitions to be invalid de facto explicit specializations and diagnose
them as such.

Instantiating a dependent friend function specialization definition
[currently sets off an assert](https://godbolt.org/z/Krqdq95hY) in
`FunctionDecl::setFunctionTemplateSpecialization`. This happens because
we do not set the `TemplateSpecializationKind` of the `FunctionDecl`
created by template argument deduction to `TSK_ExplicitSpecialization`
for friend functions
[here](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaTemplate.cpp#L9600).
I changed the assert condition because I believe this is the correct
behavior.

clang/docs/ReleaseNotes.rst

@@ -513,6 +513,7 @@ Improvements to Clang's diagnostics
        48 | static_assert(1 << 4 == 15);
           |               ~~~~~~~^~~~~
 
+- Clang now diagnoses definitions of friend function specializations, e.g. ``friend void f<>(int) {}``.
 
 Improvements to Clang's time-trace
 ----------------------------------

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -1669,6 +1669,8 @@ def err_qualified_friend_def : Error<
   "friend function definition cannot be qualified with '%0'">;
 def err_friend_def_in_local_class : Error<
   "friend function cannot be defined in a local class">;
+def err_friend_specialization_def : Error<
+  "friend function specialization cannot be defined">;
 def err_friend_not_first_in_declaration : Error<
   "'friend' must appear first in a non-function declaration">;
 def err_using_decl_friend : Error<

clang/lib/AST/Decl.cpp

@@ -4150,6 +4150,7 @@ FunctionDecl::setFunctionTemplateSpecialization(ASTContext &C,
   assert(TSK != TSK_Undeclared &&
          "Must specify the type of function template specialization");
   assert((TemplateOrSpecialization.isNull() ||
+          getFriendObjectKind() != FOK_None ||
           TSK == TSK_ExplicitSpecialization) &&
          "Member specialization must be an explicit specialization");
   FunctionTemplateSpecializationInfo *Info =

clang/lib/Sema/SemaDeclCXX.cpp

@@ -17879,6 +17879,8 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
   LookupResult Previous(*this, NameInfo, LookupOrdinaryName,
                         ForExternalRedeclaration);
 
+  bool isTemplateId = D.getName().getKind() == UnqualifiedIdKind::IK_TemplateId;
+
   // There are five cases here.
   //   - There's no scope specifier and we're in a local class. Only look
   //     for functions declared in the immediately-enclosing block scope.
@@ -17916,14 +17918,6 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
     }
     adjustContextForLocalExternDecl(DC);
 
-    // C++ [class.friend]p6:
-    //   A function can be defined in a friend declaration of a class if and
-    //   only if the class is a non-local class (9.8), the function name is
-    //   unqualified, and the function has namespace scope.
-    if (D.isFunctionDefinition()) {
-      Diag(NameInfo.getBeginLoc(), diag::err_friend_def_in_local_class);
-    }
-
   //   - There's no scope specifier, in which case we just go to the
   //     appropriate scope and look for a function or function template
   //     there as appropriate.
@@ -17934,8 +17928,6 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
     //   elaborated-type-specifier, the lookup to determine whether
     //   the entity has been previously declared shall not consider
     //   any scopes outside the innermost enclosing namespace.
-    bool isTemplateId =
-        D.getName().getKind() == UnqualifiedIdKind::IK_TemplateId;
 
     // Find the appropriate context according to the above.
     DC = CurContext;
@@ -17988,39 +17980,12 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
              diag::warn_cxx98_compat_friend_is_member :
              diag::err_friend_is_member);
 
-    if (D.isFunctionDefinition()) {
-      // C++ [class.friend]p6:
-      //   A function can be defined in a friend declaration of a class if and
-      //   only if the class is a non-local class (9.8), the function name is
-      //   unqualified, and the function has namespace scope.
-      //
-      // FIXME: We should only do this if the scope specifier names the
-      // innermost enclosing namespace; otherwise the fixit changes the
-      // meaning of the code.
-      SemaDiagnosticBuilder DB
-        = Diag(SS.getRange().getBegin(), diag::err_qualified_friend_def);
-
-      DB << SS.getScopeRep();
-      if (DC->isFileContext())
-        DB << FixItHint::CreateRemoval(SS.getRange());
-      SS.clear();
-    }
-
   //   - There's a scope specifier that does not match any template
   //     parameter lists, in which case we use some arbitrary context,
   //     create a method or method template, and wait for instantiation.
   //   - There's a scope specifier that does match some template
   //     parameter lists, which we don't handle right now.
   } else {
-    if (D.isFunctionDefinition()) {
-      // C++ [class.friend]p6:
-      //   A function can be defined in a friend declaration of a class if and
-      //   only if the class is a non-local class (9.8), the function name is
-      //   unqualified, and the function has namespace scope.
-      Diag(SS.getRange().getBegin(), diag::err_qualified_friend_def)
-        << SS.getScopeRep();
-    }
-
     DC = CurContext;
     assert(isa<CXXRecordDecl>(DC) && "friend declaration not in class?");
   }
@@ -18105,6 +18070,38 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
     else
       FD = cast<FunctionDecl>(ND);
 
+    // C++ [class.friend]p6:
+    //   A function may be defined in a friend declaration of a class if and
+    //   only if the class is a non-local class, and the function name is
+    //   unqualified.
+    if (D.isFunctionDefinition()) {
+      // Qualified friend function definition.
+      if (SS.isNotEmpty()) {
+        // FIXME: We should only do this if the scope specifier names the
+        // innermost enclosing namespace; otherwise the fixit changes the
+        // meaning of the code.
+        SemaDiagnosticBuilder DB =
+            Diag(SS.getRange().getBegin(), diag::err_qualified_friend_def);
+
+        DB << SS.getScopeRep();
+        if (DC->isFileContext())
+          DB << FixItHint::CreateRemoval(SS.getRange());
+
+        // Friend function defined in a local class.
+      } else if (FunctionContainingLocalClass) {
+        Diag(NameInfo.getBeginLoc(), diag::err_friend_def_in_local_class);
+
+        // Per [basic.pre]p4, a template-id is not a name. Therefore, if we have
+        // a template-id, the function name is not unqualified because these is
+        // no name. While the wording requires some reading in-between the
+        // lines, GCC, MSVC, and EDG all consider a friend function
+        // specialization definitions // to be de facto explicit specialization
+        // and diagnose them as such.
+      } else if (isTemplateId) {
+        Diag(NameInfo.getBeginLoc(), diag::err_friend_specialization_def);
+      }
+    }
+
     // 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

clang/test/CXX/class.access/class.friend/p6.cpp

@@ -22,3 +22,16 @@ void local() {
     friend void f() { } // expected-error{{friend function cannot be defined in a local class}}
   };
 }
+
+template<typename T> void f3(T);
+
+namespace N {
+  template<typename T> void f4(T);
+}
+
+template<typename T> struct A {
+  friend void f3(T) {}
+  friend void f3<T>(T) {} // expected-error{{friend function specialization cannot be defined}}
+  friend void N::f4(T) {} // expected-error{{friend function definition cannot be qualified with 'N::'}}
+  friend void N::f4<T>(T) {} // expected-error{{friend function definition cannot be qualified with 'N::'}}
+};

clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp

@@ -17,7 +17,7 @@ template <typename T> struct Num {
       for (U count = n.count_; count; --count)
         x += a;
       return x;
-    } 
+    }
   };
 
   friend Num operator+(const Num &a, const Num &b) {
@@ -145,7 +145,7 @@ namespace test5 {
 
 namespace Dependent {
   template<typename T, typename Traits> class X;
-  template<typename T, typename Traits> 
+  template<typename T, typename Traits>
   X<T, Traits> operator+(const X<T, Traits>&, const T*);
 
   template<typename T, typename Traits> class X {
@@ -249,7 +249,7 @@ namespace test11 {
   };
 
   template struct Foo::IteratorImpl<int>;
-  template struct Foo::IteratorImpl<long>;  
+  template struct Foo::IteratorImpl<long>;
 }
 
 // PR6827

clang/test/SemaCXX/friend.cpp

@@ -162,7 +162,7 @@ namespace test9 {
   class C {
   };
   struct A {
-    friend void C::f(int, int, int) {}  // expected-error {{friend function definition cannot be qualified with 'C::'}}
+    friend void C::f(int, int, int) {}  // expected-error {{friend declaration of 'f' does not match any declaration in 'test9::C'}}
   };
 }