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 <sdkryst...@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@+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@+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@+1{{cannot declare an explicit specialization in a friend}} + // expected-error@+2{{cannot declare an explicit specialization in a friend}} + // expected-error@+1{{friend function specialization cannot be defined}} template <> friend void f<>(int) {} }; >From b219a5b7a5ba4023dcb95e911aab5518b5d410d7 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski <sdkryst...@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..50b0653a89371e7 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -17870,6 +17870,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. @@ -17907,14 +17909,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 +17919,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 +17952,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 +17971,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 +18061,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 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@+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@+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@+2{{cannot declare an explicit specialization in a friend}} - // expected-error@+1{{friend function specialization cannot be defined}} + // expected-error@+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'}} }; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits