[Clang][Sema] Diagnose friend function specialization definitions #72863
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Krystian Stasiowski (sdkrystian) ChangesPer [[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 trips an assert in Full diff: https://github.com/llvm/llvm-project/pull/72863.diff 6 Files Affected:
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) {}
};
|
|
Ping @AaronBallman @erichkeane |
| @@ -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}} | |||
There was a problem hiding this comment.
We probably shouldn't be double-erroring on all of these. In this case, the existing error seems preferential.
There was a problem hiding this comment.
@erichkeane I suppose we don't want double-erroring for any of these https://godbolt.org/z/Wdo7bqe1v... if that is the case, perhaps we should postpone diagnosing friend function definitions until after we call ActOnFunctionDecl?
| @@ -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}} | |||
There was a problem hiding this comment.
These already error, and doing a double-error here is not the right way to go. In this case, it seems to make sense that the 2nd error doesn't happen, same with the next one.
There was a problem hiding this comment.
Per my other comment, I think the right thing to do here is to diagnose friend function definitions after calling ActOnFunctionDecl... in this case we would emit the "no candidate function template was found for dependent friend function template specialization" diagnostic, but not "friend function specialization cannot be defined".
|
@erichkeane the most recent commit results in the following: template<typename T> void f(T);
void g() { }
template<typename T>
struct A {
// error: cannot declare an explicit specialization in a friend
template<> friend void f<>(T) { }
// error: friend function specialization cannot be defined
friend void f<>(T) { }
// error: cannot declare an explicit specialization in a friend
template<> friend void ::g() { }
// error: friend function definition cannot be qualified with '::'
friend void ::g() { }
};
void h() {
void i();
struct B {
// error: templates can only be declared in namespace or class scope
// error: cannot declare an explicit specialization in a friend
template<> friend void i() { }
// error: friend function cannot be defined in a local class
friend void i() { }
};
}Unrelated, but it also seems that I also added a release note as requested. |
|
sdkrystian
force-pushed
the
friend-specialization-def
branch
from
2dc6567
to
b219a5b
Compare
|
Ping @erichkeane |
sdkrystian
force-pushed
the
friend-specialization-def
branch
from
b219a5b
to
7b26c6e
Compare
|
@erichkeane Rebased and squashed. Will need you (or someone else with write access) to merge. |
Per [class.friend]p6 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 in
FunctionDecl::setFunctionTemplateSpecialization. This happens because we do not set theTemplateSpecializationKindof theFunctionDeclcreated by template argument deduction toTSK_ExplicitSpecializationfor friend functions here. I changed the assert condition because I believe this is the correct behavior.