[clang] [Clang] Ensure default arguments in friend declarations are only allowed in defining declarations to prevent multiple reachable declarations (PR #113777)

Oleksandr T. via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 6 09:51:02 PST 2024


https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/113777

>From 78019b9d9dc138f38bb5b32576b621351ae6427c Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Sun, 27 Oct 2024 01:07:57 +0300
Subject: [PATCH 1/5] [Clang] prevent assertion failure from an invalid
 template instantiation pattern when adding instantiated params to the scope
 in friend functions with defaulted params

---
 clang/docs/ReleaseNotes.rst                | 2 ++
 clang/lib/Sema/SemaTemplateInstantiate.cpp | 8 ++++----
 clang/test/CXX/temp/temp.res/p4.cpp        | 7 +++++++
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6a95337815174b..428ec8c87a432e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -561,6 +561,8 @@ Bug Fixes to C++ Support
   const-default-constructible even if a union member has a default member initializer.
   (#GH95854).
 - Fixed an assertion failure when evaluating an invalid expression in an array initializer (#GH112140)
+- Fixed an assertion failure caused by an invalid template instantiation pattern
+  for adding instantiated parameters to the scope in friend functions with defaulted parameters. (#GH113324).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 6a55861fe5af3b..cddfcc48312042 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3435,10 +3435,10 @@ bool Sema::SubstDefaultArgument(
       //   template<typename T> void f(T a, int = decltype(a)());
       //   void g() { f(0); }
       LIS = std::make_unique<LocalInstantiationScope>(*this);
-      FunctionDecl *PatternFD = FD->getTemplateInstantiationPattern(
-          /*ForDefinition*/ false);
-      if (addInstantiatedParametersToScope(FD, PatternFD, *LIS, TemplateArgs))
-        return true;
+      if (FunctionDecl *PatternFD =
+              FD->getTemplateInstantiationPattern(/*ForDefinition*/ false))
+        if (addInstantiatedParametersToScope(FD, PatternFD, *LIS, TemplateArgs))
+          return true;
     }
 
     runWithSufficientStackSpace(Loc, [&] {
diff --git a/clang/test/CXX/temp/temp.res/p4.cpp b/clang/test/CXX/temp/temp.res/p4.cpp
index f54d8649f5da88..62bd766e7e1140 100644
--- a/clang/test/CXX/temp/temp.res/p4.cpp
+++ b/clang/test/CXX/temp/temp.res/p4.cpp
@@ -185,3 +185,10 @@ template<typename T> struct S {
   friend void X::f(T::type);
 };
 }
+
+namespace GH113324 {
+template <typename = int> struct ct {
+  friend void f(ct, int = 0); // expected-error {{friend declaration specifying a default argument must be a definition}}
+};
+void test() { f(ct<>{}); }
+}

>From 89bbd61af02e7bada9bf9482f3fba8e28a475cbe Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Fri, 1 Nov 2024 18:22:34 +0200
Subject: [PATCH 2/5] eliminate nested conditions

---
 clang/lib/Sema/SemaTemplateInstantiate.cpp | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index cddfcc48312042..046ca63e26dbc0 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3428,17 +3428,19 @@ bool Sema::SubstDefaultArgument(
     ContextRAII SavedContext(*this, FD);
     std::unique_ptr<LocalInstantiationScope> LIS;
 
-    if (ForCallExpr) {
+    FunctionDecl *PatternFD =
+        ForCallExpr
+            ? FD->getTemplateInstantiationPattern(/*ForDefinition*/ false)
+            : nullptr;
+    if (PatternFD) {
       // When instantiating a default argument due to use in a call expression,
       // an instantiation scope that includes the parameters of the callee is
       // required to satisfy references from the default argument. For example:
       //   template<typename T> void f(T a, int = decltype(a)());
       //   void g() { f(0); }
       LIS = std::make_unique<LocalInstantiationScope>(*this);
-      if (FunctionDecl *PatternFD =
-              FD->getTemplateInstantiationPattern(/*ForDefinition*/ false))
-        if (addInstantiatedParametersToScope(FD, PatternFD, *LIS, TemplateArgs))
-          return true;
+      if (addInstantiatedParametersToScope(FD, PatternFD, *LIS, TemplateArgs))
+        return true;
     }
 
     runWithSufficientStackSpace(Loc, [&] {

>From c1c985e1b32908813c1f1c236b7f807b33ea427b Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Wed, 6 Nov 2024 15:53:22 +0200
Subject: [PATCH 3/5] [Clang] Ensure default arguments in friend declarations
 are only allowed in defining declarations to prevent multiple reachable
 declarations

---
 clang/docs/ReleaseNotes.rst                    | 4 ++--
 clang/lib/Sema/SemaTemplateInstantiate.cpp     | 8 +++-----
 clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 9 +++++++++
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 979f043eb7ee5c..4d706d552e256e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -590,8 +590,8 @@ Bug Fixes to C++ Support
 - Clang now correctly ignores previous partial specializations of member templates explicitly specialized for
   an implicitly instantiated class template specialization. (#GH51051)
 - Fixed an assertion failure caused by invalid enum forward declarations. (#GH112208)
-- Fixed an assertion failure caused by an invalid template instantiation pattern
-  for adding instantiated parameters to the scope in friend functions with defaulted parameters. (#GH113324).
+- 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/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 678406f9ededc6..b63063813f1b56 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3430,17 +3430,15 @@ bool Sema::SubstDefaultArgument(
     ContextRAII SavedContext(*this, FD);
     std::unique_ptr<LocalInstantiationScope> LIS;
 
-    FunctionDecl *PatternFD =
-        ForCallExpr
-            ? FD->getTemplateInstantiationPattern(/*ForDefinition*/ false)
-            : nullptr;
-    if (PatternFD) {
+    if (ForCallExpr) {
       // When instantiating a default argument due to use in a call expression,
       // an instantiation scope that includes the parameters of the callee is
       // required to satisfy references from the default argument. For example:
       //   template<typename T> void f(T a, int = decltype(a)());
       //   void g() { f(0); }
       LIS = std::make_unique<LocalInstantiationScope>(*this);
+      FunctionDecl *PatternFD = FD->getTemplateInstantiationPattern(
+          /*ForDefinition*/ false);
       if (addInstantiatedParametersToScope(FD, PatternFD, *LIS, TemplateArgs))
         return true;
     }
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 3e948232057afe..47f1ecea97e379 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4689,6 +4689,15 @@ bool Sema::InstantiateDefaultArgument(SourceLocation CallLoc, FunctionDecl *FD,
                                       ParmVarDecl *Param) {
   assert(Param->hasUninstantiatedDefaultArg());
 
+  // C++ [dcl.fct.default]p4
+  //   If a friend declaration D specifies a default argument expression, that
+  //   declaration shall be a definition and there shall be no other declaration
+  //   of the function or function template which is reachable from D or from
+  //   which D is reachable.
+  if (FD->getFriendObjectKind() != Decl::FOK_None &&
+      FD->getTemplateInstantiationPattern() == nullptr)
+    return true;
+
   NamedDecl *Pattern = FD;
   std::optional<ArrayRef<TemplateArgument>> Innermost;
 

>From 4fb421b30da57b78c3e41d369ccae0bef48c36bd Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Wed, 6 Nov 2024 16:01:44 +0200
Subject: [PATCH 4/5] add additional tests

---
 clang/test/CXX/temp/temp.res/p4.cpp | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/clang/test/CXX/temp/temp.res/p4.cpp b/clang/test/CXX/temp/temp.res/p4.cpp
index 62bd766e7e1140..c03920913694ff 100644
--- a/clang/test/CXX/temp/temp.res/p4.cpp
+++ b/clang/test/CXX/temp/temp.res/p4.cpp
@@ -188,7 +188,11 @@ template<typename T> struct S {
 
 namespace GH113324 {
 template <typename = int> struct ct {
-  friend void f(ct, int = 0); // expected-error {{friend declaration specifying a default argument must be a definition}}
+  friend void f1(ct, int = 0);               // expected-error {{friend declaration specifying a default argument must be a definition}}
+  friend void f2(ct a, ct = decltype(a){ }); // expected-error {{friend declaration specifying a default argument must be a definition}}
 };
-void test() { f(ct<>{}); }
+void test() {
+  f1(ct<>{});
+  f2(ct<>{});
 }
+} // namespace GH113324

>From 611d4102ac082355d2822b7323969f09f4ecba6d Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Wed, 6 Nov 2024 19:50:13 +0200
Subject: [PATCH 5/5] add fixme comment and additional test

---
 clang/lib/Sema/SemaTemplateInstantiateDecl.cpp |  8 +++++---
 clang/test/CXX/temp/temp.res/p4.cpp            | 13 +++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 566ea232575aae..04d55c7ec20ef1 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4696,9 +4696,11 @@ bool Sema::InstantiateDefaultArgument(SourceLocation CallLoc, FunctionDecl *FD,
 
   // C++ [dcl.fct.default]p4
   //   If a friend declaration D specifies a default argument expression, that
-  //   declaration shall be a definition and there shall be no other declaration
-  //   of the function or function template which is reachable from D or from
-  //   which D is reachable.
+  //   declaration shall be a definition.
+  //
+  // FIXME: The check for FD->getTemplateInstantiationPattern() might be a
+  // costly way to check for a function definition. Revisit this in the future
+  // to improve its efficiency.
   if (FD->getFriendObjectKind() != Decl::FOK_None &&
       FD->getTemplateInstantiationPattern() == nullptr)
     return true;
diff --git a/clang/test/CXX/temp/temp.res/p4.cpp b/clang/test/CXX/temp/temp.res/p4.cpp
index c03920913694ff..da22516773607e 100644
--- a/clang/test/CXX/temp/temp.res/p4.cpp
+++ b/clang/test/CXX/temp/temp.res/p4.cpp
@@ -191,8 +191,21 @@ template <typename = int> struct ct {
   friend void f1(ct, int = 0);               // expected-error {{friend declaration specifying a default argument must be a definition}}
   friend void f2(ct a, ct = decltype(a){ }); // expected-error {{friend declaration specifying a default argument must be a definition}}
 };
+
+template<typename T>
+class C {
+public:
+  friend void foo(T a = 1); // expected-error {{friend declaration specifying a default argument must be a definition}}
+};
+
+template<typename T>
+void foo(T a) { } // expected-note {{candidate function template not viable: requires single argument 'a', but no arguments were provided}}
+
 void test() {
   f1(ct<>{});
   f2(ct<>{});
+
+  C<int> c;
+  foo(); // expected-error {{no matching function for call to 'foo'}}
 }
 } // namespace GH113324



More information about the cfe-commits mailing list