[clang] Reland [Clang] skip default argument instantiation for non-defining friend declarations to meet [dcl.fct.default] p4 (PR #115487)
Oleksandr T. via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 27 10:17:08 PST 2024
https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/115487
>From 5e24d212f797b5fa1b6da1526c807046373d3c21 Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Fri, 8 Nov 2024 16:13:17 +0200
Subject: [PATCH 1/6] [Clang] skip default argument instantiation for
non-defining friend declarations to meet [dcl.fct.default] p4
---
clang/docs/ReleaseNotes.rst | 2 +
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 4 ++
clang/test/CXX/temp/temp.res/p4.cpp | 43 +++++++++++++++++++
3 files changed, 49 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d0c43ff11f7bae..e8cf6fc50a1290 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -636,6 +636,8 @@ Bug Fixes to C++ Support
an implicitly instantiated class template specialization. (#GH51051)
- Fixed an assertion failure caused by invalid enum forward declarations. (#GH112208)
- Name independent data members were not correctly initialized from default member initializers. (#GH114069)
+- Fixed an assertion failure caused by invalid default argument substitutions in non-defining
+ friend declarations. (#GH113324).
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 5a001843e2ba46..200519c71c57ab 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4694,6 +4694,10 @@ bool Sema::InstantiateDefaultArgument(SourceLocation CallLoc, FunctionDecl *FD,
ParmVarDecl *Param) {
assert(Param->hasUninstantiatedDefaultArg());
+ if (FD->getFriendObjectKind() != Decl::FOK_None &&
+ !FD->getTemplateInstantiationPattern())
+ return true;
+
// Instantiate the expression.
//
// FIXME: Pass in a correct Pattern argument, otherwise
diff --git a/clang/test/CXX/temp/temp.res/p4.cpp b/clang/test/CXX/temp/temp.res/p4.cpp
index f54d8649f5da88..cf6c45b4c351c5 100644
--- a/clang/test/CXX/temp/temp.res/p4.cpp
+++ b/clang/test/CXX/temp/temp.res/p4.cpp
@@ -185,3 +185,46 @@ template<typename T> struct S {
friend void X::f(T::type);
};
}
+
+namespace GH113324 {
+template <typename = int> struct S1 {
+ friend void f1(S1, int = 0); // expected-error {{friend declaration specifying a default argument must be a definition}}
+ friend void f2(S1 a, S1 = decltype(a){}); // expected-error {{friend declaration specifying a default argument must be a definition}}
+};
+
+template <class T> using alias = int;
+template <typename T> struct S2 {
+ // FIXME: We miss diagnosing the default argument instantiation failure
+ // (forming reference to void)
+ friend void f3(S2, int a = alias<T &>(1)); // expected-error {{friend declaration specifying a default argument must be a definition}}
+};
+
+struct S3 {
+ friend void f4(S3, int = 42) { }
+};
+
+template <bool, class> using __enable_if_t = int;
+template <int v> struct S4 {
+ static const int value = v;
+};
+struct S5 {
+ template <__enable_if_t<S4<true>::value, int> = 0>
+ S5(const char *);
+};
+struct S6 {
+ template <typename a, typename b>
+ friend void f5(int, S6, a, b, S5 = "") { }
+};
+
+void test() {
+ f1(S1<>{});
+ f2(S1<>{});
+ f3(S2<void>());
+
+ S3 s3;
+ f4(s3);
+
+ S6 s6;
+ auto result = f5(0, s6, [] {}, [] {}); // expected-error {{variable has incomplete type 'void}}
+}
+} // namespace GH113324
>From 3ad3b6c5f35730be32f4f6ba2dc8d19f53be0442 Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Fri, 8 Nov 2024 16:53:39 +0200
Subject: [PATCH 2/6] update comments
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 200519c71c57ab..0bbab95001ad8e 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4694,6 +4694,13 @@ bool Sema::InstantiateDefaultArgument(SourceLocation CallLoc, FunctionDecl *FD,
ParmVarDecl *Param) {
assert(Param->hasUninstantiatedDefaultArg());
+ // FIXME: We don't track member specialization info for non-defining
+ // friend declarations, so we will not be able to later find the function
+ // pattern. As a workaround, don't instantiate the default argument in this
+ // case. This is correct per wording and only an error recovery issue, as per
+ // [dcl.fct.default]p4:
+ // if a friend declaration D specifies a default argument expression,
+ // that declaration shall be a definition.
if (FD->getFriendObjectKind() != Decl::FOK_None &&
!FD->getTemplateInstantiationPattern())
return true;
>From 09215dea0212368ef54956d8464788cc4b88cc02 Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Thu, 21 Nov 2024 16:56:11 +0200
Subject: [PATCH 3/6] move cases that cause code generation failures to the
appropriate folder
---
clang/test/CXX/temp/temp.res/p4.cpp | 23 -------------------
clang/test/CodeGenCXX/default-arguments.cpp | 25 +++++++++++++++++++++
2 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/clang/test/CXX/temp/temp.res/p4.cpp b/clang/test/CXX/temp/temp.res/p4.cpp
index cf6c45b4c351c5..9dbdd235e925d1 100644
--- a/clang/test/CXX/temp/temp.res/p4.cpp
+++ b/clang/test/CXX/temp/temp.res/p4.cpp
@@ -199,32 +199,9 @@ template <typename T> struct S2 {
friend void f3(S2, int a = alias<T &>(1)); // expected-error {{friend declaration specifying a default argument must be a definition}}
};
-struct S3 {
- friend void f4(S3, int = 42) { }
-};
-
-template <bool, class> using __enable_if_t = int;
-template <int v> struct S4 {
- static const int value = v;
-};
-struct S5 {
- template <__enable_if_t<S4<true>::value, int> = 0>
- S5(const char *);
-};
-struct S6 {
- template <typename a, typename b>
- friend void f5(int, S6, a, b, S5 = "") { }
-};
-
void test() {
f1(S1<>{});
f2(S1<>{});
f3(S2<void>());
-
- S3 s3;
- f4(s3);
-
- S6 s6;
- auto result = f5(0, s6, [] {}, [] {}); // expected-error {{variable has incomplete type 'void}}
}
} // namespace GH113324
diff --git a/clang/test/CodeGenCXX/default-arguments.cpp b/clang/test/CodeGenCXX/default-arguments.cpp
index 215bcd882e9625..a8207807a1cc1f 100644
--- a/clang/test/CodeGenCXX/default-arguments.cpp
+++ b/clang/test/CodeGenCXX/default-arguments.cpp
@@ -12,6 +12,31 @@ void g() {
}
}
+namespace GH113324 {
+struct S1 {
+ friend void f1(S1, int = 42) {}
+};
+
+template <bool, class> using __enable_if_t = int;
+template <int v> struct S2 {
+ static const int value = v;
+};
+struct S3 {
+ template <__enable_if_t<S2<true>::value, int> = 0> S3(const char *);
+};
+struct S4 {
+ template <typename a, typename b> friend void f2(int, S4, a, b, S3 = "") {}
+};
+
+void test() {
+ S1 s1;
+ f1(s1);
+
+ S4 s4;
+ f2(0, s4, [] {}, [] {});
+}
+}
+
struct A1 {
A1();
~A1();
>From dbb2c1d0760050548f70f78311405e3dc6ed0ddd Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Mon, 25 Nov 2024 15:03:09 +0200
Subject: [PATCH 4/6] remove unnecessary tests
---
clang/test/CodeGenCXX/default-arguments.cpp | 32 ++-------------------
1 file changed, 2 insertions(+), 30 deletions(-)
diff --git a/clang/test/CodeGenCXX/default-arguments.cpp b/clang/test/CodeGenCXX/default-arguments.cpp
index a8207807a1cc1f..0399f313759288 100644
--- a/clang/test/CodeGenCXX/default-arguments.cpp
+++ b/clang/test/CodeGenCXX/default-arguments.cpp
@@ -14,41 +14,13 @@ void g() {
namespace GH113324 {
struct S1 {
- friend void f1(S1, int = 42) {}
-};
-
-template <bool, class> using __enable_if_t = int;
-template <int v> struct S2 {
- static const int value = v;
-};
-struct S3 {
- template <__enable_if_t<S2<true>::value, int> = 0> S3(const char *);
-};
-struct S4 {
- template <typename a, typename b> friend void f2(int, S4, a, b, S3 = "") {}
+ friend void f(S1, int = 42) {}
};
void test() {
S1 s1;
- f1(s1);
-
- S4 s4;
- f2(0, s4, [] {}, [] {});
-}
+ f(s1);
}
-
-struct A1 {
- A1();
- ~A1();
-};
-
-struct A2 {
- A2();
- ~A2();
-};
-
-struct B {
- B(const A1& = A1(), const A2& = A2());
};
// CHECK-LABEL: define{{.*}} void @_Z2f1v()
>From a0dbd9e794fe9f2e900cf0a2af46d00fb1ab0834 Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Mon, 25 Nov 2024 16:52:43 +0200
Subject: [PATCH 5/6] revert required tests
---
clang/test/CodeGenCXX/default-arguments.cpp | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/clang/test/CodeGenCXX/default-arguments.cpp b/clang/test/CodeGenCXX/default-arguments.cpp
index 0399f313759288..2459ef1ad41fcd 100644
--- a/clang/test/CodeGenCXX/default-arguments.cpp
+++ b/clang/test/CodeGenCXX/default-arguments.cpp
@@ -23,6 +23,20 @@ void test() {
}
};
+struct A1 {
+ A1();
+ ~A1();
+};
+
+struct A2 {
+ A2();
+ ~A2();
+};
+
+struct B {
+ B(const A1& = A1(), const A2& = A2());
+};
+
// CHECK-LABEL: define{{.*}} void @_Z2f1v()
void f1() {
>From 9f0c3ed810c2ac7c370e097bdda104e3c2cadb03 Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Tue, 26 Nov 2024 17:05:45 +0200
Subject: [PATCH 6/6] update comments
---
clang/docs/ReleaseNotes.rst | 2 +-
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index be9b1a1a3e1506..a797478e6e9d0e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -718,7 +718,7 @@ Bug Fixes to C++ Support
missing placeholder return type. (#GH78694)
- Fixed a bug where bounds of partially expanded pack indexing expressions were checked too early. (#GH116105)
- Fixed an assertion failure caused by invalid default argument substitutions in non-defining
- friend declarations. (#GH113324).
+ friend declarations. (#GH113324)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 4042cdecc08187..f5eea01fc6a2be 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4695,8 +4695,8 @@ bool Sema::InstantiateDefaultArgument(SourceLocation CallLoc, FunctionDecl *FD,
// FIXME: We don't track member specialization info for non-defining
// friend declarations, so we will not be able to later find the function
// pattern. As a workaround, don't instantiate the default argument in this
- // case. This is correct per wording and only an error recovery issue, as per
- // [dcl.fct.default]p4:
+ // case. This is correct per the standard and only an issue for recovery
+ // purposes. [dcl.fct.default]p4:
// if a friend declaration D specifies a default argument expression,
// that declaration shall be a definition.
if (FD->getFriendObjectKind() != Decl::FOK_None &&
More information about the cfe-commits
mailing list