[clang] 29bd78b - [Clang][Sema] Diagnose friend function specialization definitions (#72863)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 11 06:36:05 PST 2023
Author: Krystian Stasiowski
Date: 2023-12-11T06:35:57-08:00
New Revision: 29bd78b2f61e638f6c26bc417ae476754b91e985
URL: https://github.com/llvm/llvm-project/commit/29bd78b2f61e638f6c26bc417ae476754b91e985
DIFF: https://github.com/llvm/llvm-project/commit/29bd78b2f61e638f6c26bc417ae476754b91e985.diff
LOG: [Clang][Sema] Diagnose friend function specialization definitions (#72863)
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.
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/Decl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/class.access/class.friend/p6.cpp
clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
clang/test/SemaCXX/friend.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b4b5352a306c1c..bc3acf165f0759 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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
----------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 28d95ca9b13893..0b53c6dce810f8 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 c5c2edf1bfe3ab..527ea6042daa03 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 c6218a491aecec..36e53c684ac4dc 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/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
diff --git a/clang/test/CXX/class.access/class.friend/p6.cpp b/clang/test/CXX/class.access/class.friend/p6.cpp
index 2fe20fe77fc8f2..47104e29dc6b3c 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 ab1b9f7a73eec8..1cf9e1c9f9c0fa 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
diff --git a/clang/test/SemaCXX/friend.cpp b/clang/test/SemaCXX/friend.cpp
index 367d6a6c1807c9..53e6bbfcf42a8e 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