-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[clang][Sema] Populate function template depth at AddTemplateOverloadCandidate #80395
Conversation
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesThis is yet another one-line patch to fix crashes on constraint substitution. template <class, class> struct formatter;
template <class, class> struct basic_format_context {};
template <typename CharType>
concept has_format_function = format(basic_format_context<CharType, CharType>());
template <typename ValueType, typename CharType>
requires has_format_function<CharType>
struct formatter<ValueType, CharType> {
template <typename OutputIt>
CharType format(basic_format_context<OutputIt, CharType>);
}; In this case, we would build up a
Before we emit an error, we still want to recover the most viable functions. This goes downhill to deducing template parameters against its arguments, where we would collect the argument type with the same depth as the parameter type into a Deduced set. The size of the set is presumed to be that of function template parameters, which is 1 in this case. However, since we haven't yet properly set the template depth before the dance, we'll end up putting the type for The bug seems to appear since clang 12.0. This should fix #58548. Full diff: https://github.com/llvm/llvm-project/pull/80395.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 53040aa0f9074..028afba2de6cd 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -186,6 +186,9 @@ Bug Fixes to C++ Support
- Fix for crash when using a erroneous type in a return statement.
Fixes (`#63244 <https://github.com/llvm/llvm-project/issues/63244>`_)
and (`#79745 <https://github.com/llvm/llvm-project/issues/79745>`_)
+- Fixed an out-of-bounds error caused by building a recovery expression for ill-formed
+ function calls while substituting into constraints.
+ (`#58548 <https://github.com/llvm/llvm-project/issues/58548>`_)
- Fix incorrect code generation caused by the object argument of ``static operator()`` and ``static operator[]`` calls not being evaluated.
Fixes (`#67976 <https://github.com/llvm/llvm-project/issues/67976>`_)
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 940bcccb9e261..6a04d68b4f041 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -7623,7 +7623,8 @@ void Sema::AddTemplateOverloadCandidate(
// functions. In such a case, the candidate functions generated from each
// function template are combined with the set of non-template candidate
// functions.
- TemplateDeductionInfo Info(CandidateSet.getLocation());
+ TemplateDeductionInfo Info(CandidateSet.getLocation(),
+ FunctionTemplate->getTemplateDepth());
FunctionDecl *Specialization = nullptr;
ConversionSequenceList Conversions;
if (TemplateDeductionResult Result = DeduceTemplateArguments(
diff --git a/clang/test/SemaTemplate/concepts-recovery-expr.cpp b/clang/test/SemaTemplate/concepts-recovery-expr.cpp
index 2f9d432ebac0e..b338f3bc271bf 100644
--- a/clang/test/SemaTemplate/concepts-recovery-expr.cpp
+++ b/clang/test/SemaTemplate/concepts-recovery-expr.cpp
@@ -180,3 +180,30 @@ void StaticMemOVCUse() {
// expected-note@#SMEMOVC3 {{candidate template ignored: constraints not satisfied}}
// expected-note@#SMEMOVC3REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
}
+
+namespace GH58548 {
+
+template <class, class> struct formatter; // #primary-template
+template <class, class> struct basic_format_context {};
+
+template <typename CharType>
+concept has_format_function =
+ format(basic_format_context<CharType, CharType>());
+
+template <typename ValueType, typename CharType>
+ requires has_format_function<CharType>
+struct formatter<ValueType, CharType> {
+ template <typename OutputIt>
+ CharType format(basic_format_context<OutputIt, CharType>);
+};
+
+template <class Ctx> int handle_replacement_field(Ctx arg) {
+ formatter<decltype(arg), int> ctx; // expected-error {{implicit instantiation of undefined template}}
+ return 0;
+}
+
+int x = handle_replacement_field(0);
+// expected-note@-1 {{template specialization 'GH58548::handle_replacement_field<int>' requested here}}
+// expected-note@#primary-template {{is declared here}}
+
+} // GH58548
|
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.
Looks great, Thanks!
if that's impacting std::format we might want to backport @AaronBallman @erichkeane |
Thanks for the quick review! |
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.
I don't see much value in a backport, particularly since this affects back to Clang 12. Else, LGTM.
…Candidate (llvm#80395) This is yet another one-line patch to fix crashes on constraint substitution. ```cpp template <class, class> struct formatter; template <class, class> struct basic_format_context {}; template <typename CharType> concept has_format_function = format(basic_format_context<CharType, CharType>()); template <typename ValueType, typename CharType> requires has_format_function<CharType> struct formatter<ValueType, CharType> { template <typename OutputIt> CharType format(basic_format_context<OutputIt, CharType>); }; ``` In this case, we would build up a `RecoveryExpr` for a call within a constraint expression due to the absence of viable functions. The heuristic algorithm attempted to find such a function inside of a ClassTemplatePartialSpecialization, from which we started to substitute its requires-expression, and it succeeded with a FunctionTemplate such that 1) It has only one parameter, which is dependent. 2) The only one parameter depends on two template parameters. They are, in canonical form, `<template-parameter-1-0>` and `<template-parameter-0-1>` respectively. Before we emit an error, we still want to recover the most viable functions. This goes downhill to deducing template parameters against its arguments, where we would collect the argument type with the same depth as the parameter type into a Deduced set. The size of the set is presumed to be that of function template parameters, which is 1 in this case. However, since we haven't yet properly set the template depth before the dance, we'll end up putting the type for `<template-parameter-0-1>` to the second position of Deduced set, which is unfortunately an access violation! The bug seems to appear since clang 12.0. This fixes [the case](llvm#58548 (comment)).
This is yet another one-line patch to fix crashes on constraint substitution.
In this case, we would build up a
RecoveryExpr
for a call within a constraint expression due to the absence of viable functions. The heuristic algorithm attempted to find such a function inside of a ClassTemplatePartialSpecialization, from which we started to substitute its requires-expression, and it succeeded with a FunctionTemplate such that<template-parameter-1-0>
and<template-parameter-0-1>
respectively.Before we emit an error, we still want to recover the most viable functions. This goes downhill to deducing template parameters against its arguments, where we would collect the argument type with the same depth as the parameter type into a Deduced set. The size of the set is presumed to be that of function template parameters, which is 1 in this case. However, since we haven't yet properly set the template depth before the dance, we'll end up putting the type for
<template-parameter-0-1>
to the second position of Deduced set, which is unfortunately an access violation!The bug seems to appear since clang 12.0.
This fixes the case.