[clang] Implement provisional wording for CWG2398 regarding packs #90820
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling A template template parameter containing no packs should be more specialized than one that does. Given the following example: template<class T2> struct A;
template<template<class ...T3s> class TT1, class T4> struct A<TT1<T4>>; #<!-- -->1
template<template<class T5 > class TT2, class T6> struct A<TT2<T6>>; #<!-- -->2
template<class T1> struct B;
template struct A<B<char>>;Prior to P0522, #2 would be the only viable candidate. After P0522, #1 is also viable, and neither is considered more specialized, so this becomes ambiguous. The problem we solve here is that the specialization rules in Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here. Full diff: https://github.com/llvm/llvm-project/pull/90820.diff 4 Files Affected:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a80ac6dbc76137..2bea8732f61fcd 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9238,7 +9238,7 @@ class Sema final : public SemaBase {
CheckTemplateArgumentKind CTAK);
bool CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
TemplateParameterList *Params,
- TemplateArgumentLoc &Arg);
+ TemplateArgumentLoc &Arg, bool IsDeduced);
void NoteTemplateLocation(const NamedDecl &Decl,
std::optional<SourceRange> ParamRange = {});
@@ -9708,7 +9708,8 @@ class Sema final : public SemaBase {
sema::TemplateDeductionInfo &Info);
bool isTemplateTemplateParameterAtLeastAsSpecializedAs(
- TemplateParameterList *PParam, TemplateDecl *AArg, SourceLocation Loc);
+ TemplateParameterList *PParam, TemplateDecl *AArg, SourceLocation Loc,
+ bool IsDeduced);
void MarkUsedTemplateParameters(const Expr *E, bool OnlyDeduced,
unsigned Depth, llvm::SmallBitVector &Used);
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 989f3995ca5991..2c921d4365db0a 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -6384,7 +6384,8 @@ bool Sema::CheckTemplateArgument(
case TemplateArgument::Template:
case TemplateArgument::TemplateExpansion:
- if (CheckTemplateTemplateArgument(TempParm, Params, Arg))
+ if (CheckTemplateTemplateArgument(TempParm, Params, Arg,
+ /*IsDeduced=*/CTAK != CTAK_Specified))
return true;
SugaredConverted.push_back(Arg.getArgument());
@@ -8296,7 +8297,8 @@ static void DiagnoseTemplateParameterListArityMismatch(
/// It returns true if an error occurred, and false otherwise.
bool Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
TemplateParameterList *Params,
- TemplateArgumentLoc &Arg) {
+ TemplateArgumentLoc &Arg,
+ bool IsDeduced) {
TemplateName Name = Arg.getArgument().getAsTemplateOrTemplatePattern();
TemplateDecl *Template = Name.getAsTemplateDecl();
if (!Template) {
@@ -8348,8 +8350,8 @@ bool Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
!Template->hasAssociatedConstraints())
return false;
- if (isTemplateTemplateParameterAtLeastAsSpecializedAs(Params, Template,
- Arg.getLocation())) {
+ if (isTemplateTemplateParameterAtLeastAsSpecializedAs(
+ Params, Template, Arg.getLocation(), IsDeduced)) {
// P2113
// C++20[temp.func.order]p2
// [...] If both deductions succeed, the partial ordering selects the
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 9f9e4422827173..4fbaecd7dba960 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -145,7 +145,7 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
ArrayRef<TemplateArgument> As,
TemplateDeductionInfo &Info,
SmallVectorImpl<DeducedTemplateArgument> &Deduced,
- bool NumberOfArgumentsMustMatch);
+ bool NumberOfArgumentsMustMatch, bool Swapped);
static void MarkUsedTemplateParameters(ASTContext &Ctx,
const TemplateArgument &TemplateArg,
@@ -703,9 +703,9 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList *TemplateParams,
// Perform template argument deduction on each template
// argument. Ignore any missing/extra arguments, since they could be
// filled in by default arguments.
- return DeduceTemplateArguments(S, TemplateParams, PResolved,
- SA->template_arguments(), Info, Deduced,
- /*NumberOfArgumentsMustMatch=*/false);
+ return DeduceTemplateArguments(
+ S, TemplateParams, PResolved, SA->template_arguments(), Info, Deduced,
+ /*NumberOfArgumentsMustMatch=*/false, /*Swapped=*/false);
}
// If the argument type is a class template specialization, we
@@ -731,7 +731,8 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList *TemplateParams,
// Perform template argument deduction for the template arguments.
return DeduceTemplateArguments(S, TemplateParams, PResolved,
SA->getTemplateArgs().asArray(), Info, Deduced,
- /*NumberOfArgumentsMustMatch=*/true);
+ /*NumberOfArgumentsMustMatch=*/true,
+ /*Swapped=*/false);
}
static bool IsPossiblyOpaquelyQualifiedTypeInternal(const Type *T) {
@@ -2552,7 +2553,7 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
ArrayRef<TemplateArgument> As,
TemplateDeductionInfo &Info,
SmallVectorImpl<DeducedTemplateArgument> &Deduced,
- bool NumberOfArgumentsMustMatch) {
+ bool NumberOfArgumentsMustMatch, bool Swapped) {
// C++0x [temp.deduct.type]p9:
// If the template argument list of P contains a pack expansion that is not
// the last template argument, the entire template argument list is a
@@ -2583,8 +2584,11 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
return TemplateDeductionResult::MiscellaneousDeductionFailure;
// Perform deduction for this Pi/Ai pair.
- if (auto Result = DeduceTemplateArguments(S, TemplateParams, P,
- As[ArgIdx], Info, Deduced);
+ TemplateArgument Pi = P, Ai = As[ArgIdx];
+ if (Swapped)
+ std::swap(Pi, Ai);
+ if (auto Result =
+ DeduceTemplateArguments(S, TemplateParams, Pi, Ai, Info, Deduced);
Result != TemplateDeductionResult::Success)
return Result;
@@ -2611,9 +2615,12 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
for (; hasTemplateArgumentForDeduction(As, ArgIdx) &&
PackScope.hasNextElement();
++ArgIdx) {
+ TemplateArgument Pi = Pattern, Ai = As[ArgIdx];
+ if (Swapped)
+ std::swap(Pi, Ai);
// Deduce template arguments from the pattern.
- if (auto Result = DeduceTemplateArguments(S, TemplateParams, Pattern,
- As[ArgIdx], Info, Deduced);
+ if (auto Result =
+ DeduceTemplateArguments(S, TemplateParams, Pi, Ai, Info, Deduced);
Result != TemplateDeductionResult::Success)
return Result;
@@ -2636,7 +2643,8 @@ TemplateDeductionResult Sema::DeduceTemplateArguments(
SmallVectorImpl<DeducedTemplateArgument> &Deduced,
bool NumberOfArgumentsMustMatch) {
return ::DeduceTemplateArguments(*this, TemplateParams, Ps, As, Info, Deduced,
- NumberOfArgumentsMustMatch);
+ NumberOfArgumentsMustMatch,
+ /*Swapped=*/false);
}
/// Determine whether two template arguments are the same.
@@ -3272,7 +3280,7 @@ DeduceTemplateArguments(Sema &S, T *Partial,
if (TemplateDeductionResult Result = ::DeduceTemplateArguments(
S, Partial->getTemplateParameters(),
Partial->getTemplateArgs().asArray(), TemplateArgs, Info, Deduced,
- /*NumberOfArgumentsMustMatch=*/false);
+ /*NumberOfArgumentsMustMatch=*/false, /*Swapped=*/false);
Result != TemplateDeductionResult::Success)
return Result;
@@ -6187,7 +6195,8 @@ bool Sema::isMoreSpecializedThanPrimary(
}
bool Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
- TemplateParameterList *P, TemplateDecl *AArg, SourceLocation Loc) {
+ TemplateParameterList *P, TemplateDecl *AArg, SourceLocation Loc,
+ bool IsDeduced) {
// C++1z [temp.arg.template]p4: (DR 150)
// A template template-parameter P is at least as specialized as a
// template template-argument A if, given the following rewrite to two
@@ -6197,11 +6206,10 @@ bool Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
// equivalent partial ordering by performing deduction directly on
// the template parameter lists of the template template parameters.
//
- // Given an invented class template X with the template parameter list of
- // A (including default arguments):
- TemplateName X = Context.getCanonicalTemplateName(TemplateName(AArg));
TemplateParameterList *A = AArg->getTemplateParameters();
+ // Given an invented class template X with the template parameter list of
+ // A (including default arguments):
// - Each function template has a single function parameter whose type is
// a specialization of X with template arguments corresponding to the
// template parameters from the respective function template
@@ -6242,14 +6250,45 @@ bool Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
return false;
}
- QualType AType = Context.getCanonicalTemplateSpecializationType(X, AArgs);
- QualType PType = Context.getCanonicalTemplateSpecializationType(X, PArgs);
+ // Determine whether P1 is at least as specialized as P2.
+ TemplateDeductionInfo Info(Loc, A->getDepth());
+ SmallVector<DeducedTemplateArgument, 4> Deduced;
+ Deduced.resize(A->size());
// ... the function template corresponding to P is at least as specialized
// as the function template corresponding to A according to the partial
// ordering rules for function templates.
- TemplateDeductionInfo Info(Loc, A->getDepth());
- return isAtLeastAsSpecializedAs(*this, PType, AType, AArg, Info);
+
+ // Provisional resolution for CWG2398: Regarding temp.arg.template]p4, when
+ // applying the partial ordering rules for function templates on
+ // the rewritten template template parameters:
+ // - In a deduced context, the specific rules in [temp.deduct.type]p9
+ // regarding how the argument lists themselves are matched is swapped
+ // between P and A.
+ // Note: The roles of the elements themselves are not swapped.
+ ArrayRef<TemplateArgument> Ps = AArgs, As = PArgs;
+ if (IsDeduced)
+ std::swap(Ps, As);
+ if (::DeduceTemplateArguments(*this, A, Ps, As, Info, Deduced,
+ /*NumberOfArgumentsMustMatch=*/false,
+ /*Swapped=*/IsDeduced) !=
+ TemplateDeductionResult::Success)
+ return false;
+
+ SmallVector<TemplateArgument, 4> DeducedArgs(Deduced.begin(), Deduced.end());
+ Sema::InstantiatingTemplate Inst(*this, Info.getLocation(), AArg, DeducedArgs,
+ Info);
+ if (Inst.isInvalid())
+ return false;
+
+ bool AtLeastAsSpecialized;
+ runWithSufficientStackSpace(Info.getLocation(), [&] {
+ AtLeastAsSpecialized =
+ ::FinishTemplateArgumentDeduction(
+ *this, AArg, /*IsPartialOrdering=*/true, PArgs, Deduced, Info) ==
+ TemplateDeductionResult::Success;
+ });
+ return AtLeastAsSpecialized;
}
namespace {
diff --git a/clang/test/SemaTemplate/cwg2398.cpp b/clang/test/SemaTemplate/cwg2398.cpp
index a20155486b123d..9f1c6baa0a5a1f 100644
--- a/clang/test/SemaTemplate/cwg2398.cpp
+++ b/clang/test/SemaTemplate/cwg2398.cpp
@@ -62,25 +62,19 @@ namespace templ {
namespace type_pack1 {
template<class T2> struct A;
template<template<class ...T3s> class TT1, class T4> struct A<TT1<T4>> ;
- // new-note@-1 {{partial specialization matches}}
template<template<class T5 > class TT2, class T6> struct A<TT2<T6>> {};
- // new-note@-1 {{partial specialization matches}}
template<class T1> struct B;
template struct A<B<char>>;
- // new-error@-1 {{ambiguous partial specialization}}
} // namespace type_pack1
namespace type_pack2 {
template<class T2> struct A;
template<template<class ...T3s> class TT1, class ...T4> struct A<TT1<T4...>> ;
- // new-note@-1 {{partial specialization matches}}
template<template<class T5 > class TT2, class ...T6> struct A<TT2<T6...>> {};
- // new-note@-1 {{partial specialization matches}}
template<class T1> struct B;
template struct A<B<char>>;
- // new-error@-1 {{ambiguous partial specialization}}
} // namespace type_pack2
namespace type_pack3 {
|
Endilll
reviewed
May 2, 2024
h-vetinari
reviewed
May 2, 2024
There was a problem hiding this comment.
Despite CWG2398 not being voted in yet, the status in
llvm-project/clang/www/cxx_dr_status.html
Lines 14195 to 14200 in e3f42b0
should be updated.
cor3ntin
reviewed
May 8, 2024
mizvekov
force-pushed
the
users/mizvekov/clang-cwg2398-disambiguate-packs
branch
from
b7df9a3
to
c4b72af
Compare
This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. When performing template argument deduction, a template template parameter containing no packs should be more specialized than one that does. Given the following example: ```C++ template<class T2> struct A; template<template<class ...T3s> class TT1, class T4> struct A<TT1<T4>>; // #1 template<template<class T5 > class TT2, class T6> struct A<TT2<T6>>; // #2 template<class T1> struct B; template struct A<B<char>>; ``` Prior to P0522, candidate #2 would be more specialized. After P0522, neither is more specialized, so this becomes ambiguous. With this change, #2 becomes more specialized again, maintaining compatibility with pre-P0522 implementations. The problem is that in P0522, candidates are at least as specialized when matching packs to fixed-size lists both ways, whereas before, a fixed-size list is more specialized. This patch keeps the original behavior when checking template arguments outside deduction, but restores this aspect of pre-P0522 matching during deduction. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
mizvekov
force-pushed
the
users/mizvekov/clang-cwg2398-disambiguate-packs
branch
from
c4b72af
to
39e0af9
Compare
The only way to update it is to add a test to |
cor3ntin
approved these changes
May 16, 2024
erichkeane
approved these changes
May 16, 2024
zygoloid
reviewed
May 16, 2024
|
Seems like a bot is broken: https://lab.llvm.org/buildbot/#/builders/271/builds/7701 ; can you check? |
|
That test was merged after the last time pre-commit CI was run on this MR. The change looks like a consequence of my refactoring, we now preserve the type sugar from the injected arguments. |
|
So are we reverting here or do you have quick fix available? |
|
The quick fix would be to change the expectations of the test, I can do it for you. |
|
Weirdly enough the test passes on my machine, latest MacOS. Maybe the test is not constrained on target, and this is causing differences between machines? |
|
Yep, I confirm the behavior happens if I add |
|
I just pushed a fix. |
|
Great, thanks for the quick fix! |
|
I just double checked, the issue is present on main before this PR was merged, it's completely unrelated. |
|
You're right, it's visible on the link I posted, the build was already broken! Somehow I fat-fingered and didn't hit the first red build but the third one! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This solves some ambuguity introduced in P0522 regarding how
template template parameters are partially ordered, and should reduce
the negative impact of enabling
-frelaxed-template-template-argsby default.
When performing template argument deduction, a template template parameter
containing no packs should be more specialized than one that does.
Given the following example:
Prior to P0522, candidate
#2would be more specialized.After P0522, neither is more specialized, so this becomes ambiguous.
With this change,
#2becomes more specialized again,maintaining compatibility with pre-P0522 implementations.
The problem is that in P0522, candidates are at least as specialized
when matching packs to fixed-size lists both ways, whereas before,
a fixed-size list is more specialized.
This patch keeps the original behavior when checking template arguments
outside deduction, but restores this aspect of pre-P0522 matching
during deduction.
Since this changes provisional implementation of CWG2398 which has
not been released yet, and already contains a changelog entry,
we don't provide a changelog entry here.