[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
Sun Dec 1 12:16:38 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