-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Reland [Clang] skip default argument instantiation for non-defining friend declarations to meet [dcl.fct.default] p4 #115487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…eclarations to meet [dcl.fct.default] p4
…113324-follow_up
…113324-follow_up
@llvm/pr-subscribers-clang Author: Oleksandr T. (a-tarasyuk) ChangesFixes #113324#113777 (comment) Full diff: https://github.com/llvm/llvm-project/pull/115487.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d4bf05651a63eb..a3bce0e77581fb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -641,6 +641,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..0bbab95001ad8e 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4694,6 +4694,17 @@ 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;
+
// 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo 1 nit.
@mizvekov are you happy with it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this LGTM, thanks again.
This fixes a crash when instantiating default arguments for templated friend function declarations which lack a definition.
There are implementation limits which prevents us from finding the pattern for such functions, and this causes difficulties
setting up the instantiation scope for the function parameters.
This patch skips instantiating the default argument in these cases, which causes a minor regression in error recovery, but otherwise avoids the crash.
The previous attempt #113777 accidentally skipped all default argument constructions, causing some regressions. This patch resolves that by moving the guard to InstantiateDefaultArgument() where the handling of templates takes place.
Fixes #113324