[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 14:43:29 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/7] [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/7] 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/7] [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/7] 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/7] 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
>From b067627b1d018d06c2b886848f7ebd5b0d5e032f Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Wed, 6 Nov 2024 23:57:27 +0200
Subject: [PATCH 6/7] prevent default argument instantiation in non-defining
friend declarations
---
clang/lib/Sema/SemaExpr.cpp | 7 +++++++
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 11 -----------
clang/test/CXX/temp/temp.res/p4.cpp | 14 ++++----------
3 files changed, 11 insertions(+), 21 deletions(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 49fdb5b5ab43da..ecae052c87dfc3 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6018,6 +6018,13 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,
} else {
assert(Param && "can't use default arguments without a known callee");
+ // C++ [dcl.fct.default]p4
+ // If a friend declaration D specifies a default argument expression,
+ // that declaration shall be a definition.
+ if (FDecl->getFriendObjectKind() != Decl::FOK_None &&
+ FDecl->getMemberSpecializationInfo() == nullptr)
+ return true;
+
ExprResult ArgExpr = BuildCXXDefaultArgExpr(CallLoc, FDecl, Param);
if (ArgExpr.isInvalid())
return true;
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 04d55c7ec20ef1..5a001843e2ba46 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4694,17 +4694,6 @@ 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.
- //
- // 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;
-
// 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 da22516773607e..0d28b77541e93c 100644
--- a/clang/test/CXX/temp/temp.res/p4.cpp
+++ b/clang/test/CXX/temp/temp.res/p4.cpp
@@ -192,20 +192,14 @@ template <typename = int> struct ct {
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<class T> using alias = int;
+template<typename T> struct C {
+ friend void f3(C, int a = alias<T&>(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'}}
+ f3(C<void>());
}
} // namespace GH113324
>From 263e151ba08cab5d1b09fdedf8c850c9e22533c8 Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Thu, 7 Nov 2024 00:42:55 +0200
Subject: [PATCH 7/7] add comments
---
clang/lib/Sema/SemaExpr.cpp | 8 ++++++--
clang/test/CXX/temp/temp.res/p4.cpp | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ecae052c87dfc3..df8f025030e2b1 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6018,8 +6018,12 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,
} else {
assert(Param && "can't use default arguments without a known callee");
- // C++ [dcl.fct.default]p4
- // If a friend declaration D specifies a default argument expression,
+ // 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 (FDecl->getFriendObjectKind() != Decl::FOK_None &&
FDecl->getMemberSpecializationInfo() == nullptr)
diff --git a/clang/test/CXX/temp/temp.res/p4.cpp b/clang/test/CXX/temp/temp.res/p4.cpp
index 0d28b77541e93c..743ffed14d81a8 100644
--- a/clang/test/CXX/temp/temp.res/p4.cpp
+++ b/clang/test/CXX/temp/temp.res/p4.cpp
@@ -194,6 +194,7 @@ template <typename = int> struct ct {
template<class T> using alias = int;
template<typename T> struct C {
+ // FIXME: We miss diagnosing the default argument instantiation failure (forming reference to void)
friend void f3(C, int a = alias<T&>(1)); // expected-error {{friend declaration specifying a default argument must be a definition}}
};
More information about the cfe-commits
mailing list