[clang] [Clang][Sema] Diagnose friend function specialization definitions (PR #72863)

Krystian Stasiowski via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 27 05:00:44 PST 2023


https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/72863

>From 3e191f245325742338eba223a3f98b6bb5ad414b Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Mon, 20 Nov 2023 07:11:41 -0500
Subject: [PATCH 1/2] [Clang][Sema] Diagnose friend function specialization
 definitions

---
 clang/include/clang/Basic/DiagnosticSemaKinds.td   |  2 ++
 clang/lib/AST/Decl.cpp                             |  1 +
 clang/lib/Sema/SemaDeclCXX.cpp                     | 14 ++++++++++++++
 clang/test/CXX/class.access/class.friend/p6.cpp    | 13 +++++++++++++
 clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp  |  8 +++++---
 .../CXX/temp/temp.spec/temp.expl.spec/p20-2.cpp    |  3 ++-
 6 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3f6ca64baf23ae6..35acbb40a4b4f38 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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<
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index c5c2edf1bfe3aba..527ea6042daa034 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/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 =
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 3688192e6cbe5c5..2bfb02da08bde54 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17960,6 +17960,20 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
 
     DCScope = getScopeForDeclContext(S, DC);
 
+    // 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.
+    //
+    // 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.
+    if (D.isFunctionDefinition() && isTemplateId) {
+      Diag(NameInfo.getBeginLoc(), diag::err_friend_specialization_def);
+    }
+
   //   - There's a non-dependent scope specifier, in which case we
   //     compute it and do a previous lookup there for a function
   //     or function template.
diff --git a/clang/test/CXX/class.access/class.friend/p6.cpp b/clang/test/CXX/class.access/class.friend/p6.cpp
index 2fe20fe77fc8f21..47104e29dc6b3c7 100644
--- a/clang/test/CXX/class.access/class.friend/p6.cpp
+++ b/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::'}}
+};
diff --git a/clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp b/clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
index ab1b9f7a73eec89..974b080f2783934 100644
--- a/clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
+++ b/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
@@ -373,6 +373,7 @@ template <class T> void foo() {} // expected-note{{candidate ignored: not a memb
 }
 using ns::foo;
 template <class T> struct A {
+  // expected-error at +1{{friend function specialization cannot be defined}}
   friend void foo<T>() {} // expected-error{{no candidate function template was found for dependent friend function template specialization}}
 };
 }
@@ -384,6 +385,7 @@ using ns1::foo; // expected-note {{found by name lookup}}
 using ns2::foo; // expected-note {{found by name lookup}}
 
 template <class T> class A {
+  // expected-error at +1{{friend function specialization cannot be defined}}
   friend void foo<T>() {} // expected-error {{ambiguous}} expected-error{{no candidate function template was found for dependent friend function template specialization}}
 };
 }
diff --git a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p20-2.cpp b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p20-2.cpp
index 7bc5f13cd7375bd..8fdc7d454828a88 100644
--- a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p20-2.cpp
+++ b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p20-2.cpp
@@ -4,7 +4,8 @@ void f(T);
 
 template<typename T>
 struct A {
-  // expected-error at +1{{cannot declare an explicit specialization in a friend}}
+  // expected-error at +2{{cannot declare an explicit specialization in a friend}}
+  // expected-error at +1{{friend function specialization cannot be defined}}
   template <> friend void f<>(int) {}
 };
 

>From 2dc6567a7054feb755351b8d403b228f3d4b91ea Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Mon, 27 Nov 2023 07:59:31 -0500
Subject: [PATCH 2/2] [FOLD]

---
 clang/docs/ReleaseNotes.rst                   |  1 +
 clang/lib/Sema/SemaDeclCXX.cpp                | 85 ++++++++-----------
 .../CXX/temp/temp.decls/temp.friend/p1.cpp    |  2 -
 .../temp/temp.spec/temp.expl.spec/p20-2.cpp   |  3 +-
 clang/test/SemaCXX/friend.cpp                 |  2 +-
 5 files changed, 37 insertions(+), 56 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 52c9d6eb69617b0..1f03895b58af72e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -451,6 +451,7 @@ Improvements to Clang's diagnostics
   with GCC.
 - Clang will warn on deprecated specializations used in system headers when their instantiation
   is caused by user code.
+- Clang now diagnoses definitions of friend function specializations, e.g. ``friend void f<>(int) {}``.
 
 Improvements to Clang's time-trace
 ----------------------------------
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 2bfb02da08bde54..9dbfd92de7f369f 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17870,6 +17870,9 @@ 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.
@@ -17907,14 +17910,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.
@@ -17925,8 +17920,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;
@@ -17960,20 +17953,6 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
 
     DCScope = getScopeForDeclContext(S, DC);
 
-    // 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.
-    //
-    // 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.
-    if (D.isFunctionDefinition() && isTemplateId) {
-      Diag(NameInfo.getBeginLoc(), diag::err_friend_specialization_def);
-    }
-
   //   - There's a non-dependent scope specifier, in which case we
   //     compute it and do a previous lookup there for a function
   //     or function template.
@@ -17993,39 +17972,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?");
   }
@@ -18110,6 +18062,37 @@ 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
diff --git a/clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp b/clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
index 974b080f2783934..1cf9e1c9f9c0fa3 100644
--- a/clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
+++ b/clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
@@ -373,7 +373,6 @@ template <class T> void foo() {} // expected-note{{candidate ignored: not a memb
 }
 using ns::foo;
 template <class T> struct A {
-  // expected-error at +1{{friend function specialization cannot be defined}}
   friend void foo<T>() {} // expected-error{{no candidate function template was found for dependent friend function template specialization}}
 };
 }
@@ -385,7 +384,6 @@ using ns1::foo; // expected-note {{found by name lookup}}
 using ns2::foo; // expected-note {{found by name lookup}}
 
 template <class T> class A {
-  // expected-error at +1{{friend function specialization cannot be defined}}
   friend void foo<T>() {} // expected-error {{ambiguous}} expected-error{{no candidate function template was found for dependent friend function template specialization}}
 };
 }
diff --git a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p20-2.cpp b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p20-2.cpp
index 8fdc7d454828a88..7bc5f13cd7375bd 100644
--- a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p20-2.cpp
+++ b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p20-2.cpp
@@ -4,8 +4,7 @@ void f(T);
 
 template<typename T>
 struct A {
-  // expected-error at +2{{cannot declare an explicit specialization in a friend}}
-  // expected-error at +1{{friend function specialization cannot be defined}}
+  // expected-error at +1{{cannot declare an explicit specialization in a friend}}
   template <> friend void f<>(int) {}
 };
 
diff --git a/clang/test/SemaCXX/friend.cpp b/clang/test/SemaCXX/friend.cpp
index 367d6a6c1807c92..53e6bbfcf42a8ed 100644
--- a/clang/test/SemaCXX/friend.cpp
+++ b/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'}}
   };
 }
 



More information about the cfe-commits mailing list