-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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] Implement CWG 2628 "Implicit deduction guides should propagate constraints" #111143
Conversation
…e constraints"
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesCloses #98592 Full diff: https://github.com/llvm/llvm-project/pull/111143.diff 5 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6e7a5fb76b602b..a88c00ea038091 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -215,6 +215,9 @@ Resolutions to C++ Defect Reports
- Clang now allows trailing requires clause on explicit deduction guides.
(`CWG2707: Deduction guides cannot have a trailing requires-clause <https://cplusplus.github.io/CWG/issues/2707.html>`_).
+- Respect constructor constraints during CTAD.
+ (`CWG2628: Implicit deduction guides should propagate constraints <https://cplusplus.github.io/CWG/issues/2628.html>`_).
+
C Language Changes
------------------
diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
index 545da21183c3c4..dc9eb1b83b03fe 100644
--- a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
+++ b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
@@ -195,12 +195,14 @@ class ExtractTypeForDeductionGuide
// A deduction guide can be either a template or a non-template function
// declaration. If \p TemplateParams is null, a non-template function
// declaration will be created.
-NamedDecl *buildDeductionGuide(
- Sema &SemaRef, TemplateDecl *OriginalTemplate,
- TemplateParameterList *TemplateParams, CXXConstructorDecl *Ctor,
- ExplicitSpecifier ES, TypeSourceInfo *TInfo, SourceLocation LocStart,
- SourceLocation Loc, SourceLocation LocEnd, bool IsImplicit,
- llvm::ArrayRef<TypedefNameDecl *> MaterializedTypedefs = {}) {
+NamedDecl *
+buildDeductionGuide(Sema &SemaRef, TemplateDecl *OriginalTemplate,
+ TemplateParameterList *TemplateParams,
+ CXXConstructorDecl *Ctor, ExplicitSpecifier ES,
+ TypeSourceInfo *TInfo, SourceLocation LocStart,
+ SourceLocation Loc, SourceLocation LocEnd, bool IsImplicit,
+ llvm::ArrayRef<TypedefNameDecl *> MaterializedTypedefs = {},
+ Expr *FunctionTrailingRC = nullptr) {
DeclContext *DC = OriginalTemplate->getDeclContext();
auto DeductionGuideName =
SemaRef.Context.DeclarationNames.getCXXDeductionGuideName(
@@ -211,9 +213,9 @@ NamedDecl *buildDeductionGuide(
TInfo->getTypeLoc().castAs<FunctionProtoTypeLoc>().getParams();
// Build the implicit deduction guide template.
- auto *Guide =
- CXXDeductionGuideDecl::Create(SemaRef.Context, DC, LocStart, ES, Name,
- TInfo->getType(), TInfo, LocEnd, Ctor);
+ auto *Guide = CXXDeductionGuideDecl::Create(
+ SemaRef.Context, DC, LocStart, ES, Name, TInfo->getType(), TInfo, LocEnd,
+ Ctor, DeductionCandidate::Normal, FunctionTrailingRC);
Guide->setImplicit(IsImplicit);
Guide->setParams(Params);
@@ -355,10 +357,11 @@ struct ConvertConstructorToDeductionGuideTransform {
// template arguments) of the constructor, if any.
TemplateParameterList *TemplateParams =
SemaRef.GetTemplateParameterList(Template);
+ SmallVector<TemplateArgument, 16> Depth1Args;
+ Expr *OuterRC = TemplateParams->getRequiresClause();
if (FTD) {
TemplateParameterList *InnerParams = FTD->getTemplateParameters();
SmallVector<NamedDecl *, 16> AllParams;
- SmallVector<TemplateArgument, 16> Depth1Args;
AllParams.reserve(TemplateParams->size() + InnerParams->size());
AllParams.insert(AllParams.begin(), TemplateParams->begin(),
TemplateParams->end());
@@ -391,7 +394,7 @@ struct ConvertConstructorToDeductionGuideTransform {
/*EvaluateConstraint=*/false);
}
- assert(NewParam->getTemplateDepth() == 0 &&
+ assert(getDepthAndIndex(NewParam).first == 0 &&
"Unexpected template parameter depth");
AllParams.push_back(NewParam);
@@ -407,10 +410,11 @@ struct ConvertConstructorToDeductionGuideTransform {
Args.addOuterRetainedLevel();
if (NestedPattern)
Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
- ExprResult E = SemaRef.SubstExpr(InnerRC, Args);
- if (E.isInvalid())
+ ExprResult E =
+ SemaRef.SubstConstraintExprWithoutSatisfaction(InnerRC, Args);
+ if (!E.isUsable())
return nullptr;
- RequiresClause = E.getAs<Expr>();
+ RequiresClause = E.get();
}
TemplateParams = TemplateParameterList::Create(
@@ -446,10 +450,46 @@ struct ConvertConstructorToDeductionGuideTransform {
return nullptr;
TypeSourceInfo *NewTInfo = TLB.getTypeSourceInfo(SemaRef.Context, NewType);
+ // At this point, the function parameters are already 'instantiated' in the
+ // current scope. Substitute into the constructor's trailing
+ // requires-clause, if any.
+ Expr *FunctionTrailingRC = nullptr;
+ if (Expr *RC = CD->getTrailingRequiresClause()) {
+ MultiLevelTemplateArgumentList Args;
+ Args.setKind(TemplateSubstitutionKind::Rewrite);
+ Args.addOuterTemplateArguments(Depth1Args);
+ Args.addOuterRetainedLevel();
+ if (NestedPattern)
+ Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
+ ExprResult E = SemaRef.SubstConstraintExprWithoutSatisfaction(RC, Args);
+ if (!E.isUsable())
+ return nullptr;
+ FunctionTrailingRC = E.get();
+ }
+
+ // C++ [over.match.class.deduct]p1:
+ // If C is defined, for each constructor of C, a function template with
+ // the following properties:
+ // [...]
+ // - The associated constraints are the conjunction of the associated
+ // constraints of C and the associated constraints of the constructor, if
+ // any.
+ if (OuterRC) {
+ // The outer template parameters are not transformed, so their
+ // associated constraints don't need substitution.
+ if (!FunctionTrailingRC)
+ FunctionTrailingRC = OuterRC;
+ else
+ FunctionTrailingRC = BinaryOperator::Create(
+ SemaRef.Context, /*lhs=*/OuterRC, /*rhs=*/FunctionTrailingRC,
+ BO_LAnd, SemaRef.Context.BoolTy, VK_PRValue, OK_Ordinary,
+ TemplateParams->getTemplateLoc(), FPOptionsOverride());
+ }
+
return buildDeductionGuide(
SemaRef, Template, TemplateParams, CD, CD->getExplicitSpecifier(),
NewTInfo, CD->getBeginLoc(), CD->getLocation(), CD->getEndLoc(),
- /*IsImplicit=*/true, MaterializedTypedefs);
+ /*IsImplicit=*/true, MaterializedTypedefs, FunctionTrailingRC);
}
/// Build a deduction guide with the specified parameter types.
diff --git a/clang/test/CXX/drs/cwg26xx.cpp b/clang/test/CXX/drs/cwg26xx.cpp
index 63a954c803b77a..0d297b33e88bd9 100644
--- a/clang/test/CXX/drs/cwg26xx.cpp
+++ b/clang/test/CXX/drs/cwg26xx.cpp
@@ -132,27 +132,18 @@ struct E {
#endif
} // namespace cwg2627
-namespace cwg2628 { // cwg2628: no
- // this was reverted for the 16.x release
- // due to regressions, see the issue for more details:
- // https://github.com/llvm/llvm-project/issues/60777
+namespace cwg2628 { // cwg2628: 20
#if __cplusplus >= 202002L
template <bool A = false, bool B = false>
struct foo {
- // The expected notes below should be removed when cwg2628 is fully implemented again
- constexpr foo() requires (!A && !B) = delete; // #cwg2628-ctor-1
- constexpr foo() requires (A || B) = delete; // #cwg2628-ctor-2
+ constexpr foo() requires (!A && !B) = delete; // #cwg2628-ctor
+ constexpr foo() requires (A || B) = delete;
};
void f() {
- // The FIXME's below should be the expected errors when cwg2628 is
- // fully implemented again.
foo fooable; // #cwg2628-fooable
- // since-cxx20-error@-1 {{ambiguous deduction for template arguments of 'foo'}}
- // since-cxx20-note@#cwg2628-ctor-1 {{candidate function [with A = false, B = false]}}
- // since-cxx20-note@#cwg2628-ctor-2 {{candidate function [with A = false, B = false]}}
- // FIXME-since-cxx20-error@#cwg2628-fooable {{call to deleted}}
- // FIXME-since-cxx20-note@#cwg2628-ctor {{marked deleted here}}
+ // since-cxx20-error@#cwg2628-fooable {{call to deleted}}
+ // since-cxx20-note@#cwg2628-ctor {{marked deleted here}}
}
#endif
}
diff --git a/clang/test/SemaTemplate/deduction-guide.cpp b/clang/test/SemaTemplate/deduction-guide.cpp
index d03c783313dd71..1e7b4d80be53f1 100644
--- a/clang/test/SemaTemplate/deduction-guide.cpp
+++ b/clang/test/SemaTemplate/deduction-guide.cpp
@@ -478,3 +478,166 @@ A a{.f1 = {1}};
// CHECK-NEXT: `-DeclRefExpr {{.+}} <col:10> 'int' NonTypeTemplateParm {{.+}} 'N' 'int'
} // namespace GH83368
+
+namespace GH60777 {
+
+template <typename... Ts> constexpr bool True() { return true; }
+
+template <typename T>
+ requires(sizeof(T) == 4)
+struct A {
+ template <typename... Ts>
+ requires(sizeof...(Ts) == 0)
+ A(T val, Ts... tail)
+ requires(True<Ts...>())
+ {}
+};
+
+A a(42);
+
+// `requires (sizeof(T) == 4)` goes into the deduction guide together with
+// `requires (True<Ts...>())`, while `requires(sizeof...(Ts) == 0)` goes into
+// the template parameter list of the synthesized declaration.
+
+// CHECK-LABEL: Dumping GH60777::<deduction guide for A>:
+// CHECK-NEXT: FunctionTemplateDecl 0x{{.+}} <{{.+}}> {{.+}} implicit <deduction guide for A>
+// CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}} <{{.+}}> col:20 referenced typename depth 0 index 0 T
+// CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}} <{{.+}}> col:25 typename depth 0 index 1 ... Ts
+// CHECK-NEXT: |-ParenExpr 0x{{.+}} <{{.+}}> 'bool'
+// CHECK-NEXT: | `-BinaryOperator 0x{{.+}} <{{.+}}> 'bool' '=='
+// CHECK-NEXT: | |-SizeOfPackExpr 0x{{.+}} <{{.+}}> 'unsigned long' 0x{{.+}} Ts
+// CHECK-NEXT: | | `-TemplateArgument type 'Ts...':'type-parameter-0-1...'
+// CHECK-NEXT: | | `-PackExpansionType 0x{{.+}} 'Ts...' dependent
+// CHECK-NEXT: | | `-TemplateTypeParmType 0x{{.+}} 'Ts' dependent contains_unexpanded_pack depth 0 index 1 pack
+// CHECK-NEXT: | | `-TemplateTypeParm 0x{{.+}} 'Ts'
+// CHECK-NEXT: | `-ImplicitCastExpr 0x{{.+}} <{{.+}}> 'unsigned long' <IntegralCast>
+// CHECK-NEXT: | `-IntegerLiteral 0x{{.+}} <{{.+}}> 'int' 0
+// CHECK-NEXT: |-CXXDeductionGuideDecl 0x{{.+}} <{{.+}}> line:{{.+}} implicit <deduction guide for A> 'auto (T, Ts...) -> A<T>'
+// CHECK-NEXT: | |-ParmVarDecl 0x{{.+}} <{{.+}}> col:{{.+}} val 'T'
+// CHECK-NEXT: | |-ParmVarDecl 0x{{.+}} <{{.+}}> col:{{.+}} tail 'Ts...' pack
+// CHECK-NEXT: | `-BinaryOperator 0x{{.+}} <{{.+}}> 'bool' '&&'
+// CHECK-NEXT: | |-ParenExpr 0x{{.+}} <{{.+}}> 'bool'
+// CHECK-NEXT: | | `-BinaryOperator 0x{{.+}} <{{.+}}> 'bool' '=='
+// CHECK-NEXT: | | |-UnaryExprOrTypeTraitExpr 0x{{.+}} <{{.+}}> 'unsigned long' sizeof 'T'
+// CHECK-NEXT: | | `-ImplicitCastExpr 0x{{.+}} <{{.+}}> 'unsigned long' <IntegralCast>
+// CHECK-NEXT: | | `-IntegerLiteral 0x{{.+}} <{{.+}}> 'int' 4
+// CHECK-NEXT: | `-ParenExpr 0x{{.+}} <{{.+}}> '<dependent type>'
+// CHECK-NEXT: | `-CallExpr 0x{{.+}} <{{.+}}> '<dependent type>'
+// CHECK-NEXT: | `-UnresolvedLookupExpr 0x{{.+}} <col:14, col:24> '<dependent type>' lvalue (ADL) = 'True' 0x{{.+}}
+// CHECK-NEXT: | `-TemplateArgument type 'Ts...':'type-parameter-0-1...'
+// CHECK-NEXT: | `-PackExpansionType 0x{{.+}} 'Ts...' dependent
+// CHECK-NEXT: | `-TemplateTypeParmType 0x{{.+}} 'Ts' dependent contains_unexpanded_pack depth 0 index 1 pack
+// CHECK-NEXT: | `-TemplateTypeParm 0x{{.+}} 'Ts'
+
+template <typename T>
+struct B {
+ template <typename... Ts>
+ B(T val, Ts... tail)
+ requires(True<tail...>())
+ {}
+};
+
+B b(42, 43);
+// expected-error@-1 {{no viable constructor}} \
+// expected-note@-6 {{constraints not satisfied}} \
+// expected-note@-5 {{because substituted constraint expression is ill-formed}} \
+// expected-note@-6 {{implicit deduction guide declared as 'template <typename T, typename ...Ts> B(T val, Ts ...tail) -> B<T> requires (True<tail...>())'}} \
+// expected-note@-8 {{function template not viable}} \
+// expected-note@-8 {{implicit deduction guide declared as 'template <typename T> B(B<T>) -> B<T>'}}
+
+} // namespace GH60777
+
+// Examples from @hokein.
+namespace GH98592 {
+
+template <class T> concept True = true;
+double arr3[3];
+
+template <class T>
+struct X {
+ const int size;
+ template <class U>
+ constexpr X(T, U(&)[3]) requires True<T> : size(sizeof(T)) {}
+};
+
+template <typename T, typename U>
+X(T, U (&)[3]) -> X<U>;
+
+constexpr X x(3, arr3);
+
+// The synthesized deduction guide is more constrained than the explicit one.
+static_assert(x.size == 4);
+
+// CHECK-LABEL: Dumping GH98592::<deduction guide for X>:
+// CHECK-NEXT: FunctionTemplateDecl 0x{{.+}} <{{.+}}> col:13 implicit <deduction guide for X>
+// CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}} <{{.+}}> col:17 referenced class depth 0 index 0 T
+// CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}} <{{.+}}> col:19 class depth 0 index 1 U
+// CHECK-NEXT: |-CXXDeductionGuideDecl 0x{{.+}} <{{.+}}> col:13 implicit <deduction guide for X> 'auto (T, U (&)[3]) -> X<T>'
+// CHECK-NEXT: | |-ParmVarDecl 0x{{.+}} <col:15> col:16 'T'
+// CHECK-NEXT: | |-ParmVarDecl 0x{{.+}} <col:18, col:24> col:21 'U (&)[3]'
+// CHECK-NEXT: | `-ConceptSpecializationExpr 0x{{.+}} <col:36, col:42> 'bool' Concept 0x{{.+}} 'True'
+// CHECK-NEXT: | |-ImplicitConceptSpecializationDecl 0x{{.+}} <{{.+}}> col:28
+// CHECK-NEXT: | | `-TemplateArgument type 'type-parameter-0-0'
+// CHECK-NEXT: | | `-TemplateTypeParmType 0x{{.+}} 'type-parameter-0-0' dependent depth 0 index 0
+// CHECK-NEXT: | `-TemplateArgument <{{.+}}> type 'T':'type-parameter-0-0'
+// CHECK-NEXT: | `-TemplateTypeParmType 0x{{.+}} 'T' dependent depth 0 index 0
+// CHECK-NEXT: | `-TemplateTypeParm 0x{{.+}} 'T'
+// CHECK-NEXT: `-CXXDeductionGuideDecl 0x{{.+}} <col:3, col:63> col:13 implicit used <deduction guide for X> 'auto (int, double (&)[3]) -> GH98592::X<int>' implicit_instantiation
+// CHECK-NEXT: |-TemplateArgument type 'int'
+// CHECK-NEXT: | `-BuiltinType 0x{{.+}} 'int'
+// CHECK-NEXT: |-TemplateArgument type 'double'
+// CHECK-NEXT: | `-BuiltinType 0x{{.+}} 'double'
+// CHECK-NEXT: |-ParmVarDecl 0x{{.+}} <col:15> col:16 'int'
+// CHECK-NEXT: |-ParmVarDecl 0x{{.+}} <col:18, col:24> col:21 'double (&)[3]'
+// CHECK-NEXT: `-ConceptSpecializationExpr 0x{{.+}} <col:36, col:42> 'bool' Concept 0x{{.+}} 'True'
+// CHECK-NEXT: |-ImplicitConceptSpecializationDecl 0x{{.+}} <{{.+}}> col:28
+// CHECK-NEXT: | `-TemplateArgument type 'type-parameter-0-0'
+// CHECK-NEXT: | `-TemplateTypeParmType 0x{{.+}} 'type-parameter-0-0' dependent depth 0 index 0
+// CHECK-NEXT: `-TemplateArgument <{{.+}}> type 'T':'type-parameter-0-0'
+// CHECK-NEXT: `-TemplateTypeParmType 0x{{.+}} 'T' dependent depth 0 index 0
+// CHECK-NEXT: `-TemplateTypeParm 0x{{.+}} 'T'
+
+template <class T> requires True<T> struct Y {
+ const int size;
+ template <class U>
+ constexpr Y(T, U(&)[3]) : size(sizeof(T)) {}
+};
+
+template <typename T, typename U> Y(T, U (&)[3]) -> Y<U>;
+
+constexpr Y y(3, arr3);
+
+// Likewise, the synthesized deduction guide should be preferred
+// according to [over.match.class.deduct]p1.
+static_assert(y.size == 4);
+
+// Dumping GH98592::<deduction guide for Y>:
+// FunctionTemplateDecl 0x{{.+}} <{{.+}}> col:13 implicit <deduction guide for Y>
+// |-TemplateTypeParmDecl 0x{{.+}} <{{.+}}> col:17 referenced class depth 0 index 0 T
+// |-TemplateTypeParmDecl 0x{{.+}} <{{.+}}> col:19 class depth 0 index 1 U
+// |-CXXDeductionGuideDecl 0x{{.+}} <{{.+}}> col:13 implicit <deduction guide for Y> 'auto (T, U (&)[3]) -> Y<T>'
+// | |-ParmVarDecl 0x{{.+}} <col:15> col:16 'T'
+// | |-ParmVarDecl 0x{{.+}} <col:18, col:24> col:21 'U (&)[3]'
+// | `-ConceptSpecializationExpr 0x{{.+}} <{{.+}}> 'bool' Concept 0x{{.+}} 'True'
+// | |-ImplicitConceptSpecializationDecl 0x{{.+}} <{{.+}}> col:28
+// | | `-TemplateArgument type 'type-parameter-0-0'
+// | | `-TemplateTypeParmType 0x{{.+}} 'type-parameter-0-0' dependent depth 0 index 0
+// | `-TemplateArgument <{{.+}}> type 'T':'type-parameter-0-0'
+// | `-TemplateTypeParmType 0x{{.+}} 'T' dependent depth 0 index 0
+// | `-TemplateTypeParm 0x{{.+}} 'T'
+// `-CXXDeductionGuideDecl 0x{{.+}} <{{.+}}> col:13 implicit used <deduction guide for Y> 'auto (int, double (&)[3]) -> GH98592::Y<int>' implicit_instantiation
+// |-TemplateArgument type 'int'
+// | `-BuiltinType 0x{{.+}} 'int'
+// |-TemplateArgument type 'double'
+// | `-BuiltinType 0x{{.+}} 'double'
+// |-ParmVarDecl 0x{{.+}} <col:15> col:16 'int'
+// |-ParmVarDecl 0x{{.+}} <col:18, col:24> col:21 'double (&)[3]'
+// `-ConceptSpecializationExpr 0x{{.+}} <{{.+}}> 'bool' Concept 0x{{.+}} 'True'
+// |-ImplicitConceptSpecializationDecl 0x{{.+}} <{{.+}}> col:28
+// | `-TemplateArgument type 'type-parameter-0-0'
+// | `-TemplateTypeParmType 0x{{.+}} 'type-parameter-0-0' dependent depth 0 index 0
+// `-TemplateArgument <{{.+}}> type 'T':'type-parameter-0-0'
+// `-TemplateTypeParmType 0x{{.+}} 'T' dependent depth 0 index 0
+// `-TemplateTypeParm 0x{{.+}} 'T'
+
+} // namespce GH98592
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index ba63106ccc3875..185fb6a94f55a6 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -15615,7 +15615,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2628.html">2628</a></td>
<td>DRWP</td>
<td>Implicit deduction guides should propagate constraints</td>
- <td class="none" align="center">No</td>
+ <td class="unreleased" align="center">Clang 20</td>
</tr>
<tr id="2629">
<td><a href="https://cplusplus.github.io/CWG/issues/2629.html">2629</a></td>
|
template <class T> requires True<T> struct Y { | ||
const int size; | ||
template <class U> | ||
constexpr Y(T, U(&)[3]) : size(sizeof(T)) {} | ||
}; | ||
|
||
template <typename T, typename U> Y(T, U (&)[3]) -> Y<U>; | ||
|
||
constexpr Y y(3, arr3); | ||
|
||
// Likewise, the synthesized deduction guide should be preferred | ||
// according to [over.match.class.deduct]p1. | ||
static_assert(y.size == 4); |
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.
Though been discussed in #98592, it's still not so clear to me as to whether this case should be accepted:
While the wording from [over.match.class.deduct]p1.1.2 suggests the constraint of the synthesized CTAD is the conjunction of that of C and that of the constructor, the note there also seems to imply the constraint from C should be checked before the checking of the constraint of constructor, so this example should not compile that way. (The overload resolution would pick up the explicit CTAD guide if C's required-clause were not propagated into the implicit CTAD guide.)
I still build up a && operator for the literal "conjunction" currently, but I think I can remove that if our reading is wrong.
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'm trying to figure if the order of operation is observable - I don't think so, so I am confused by the note @erichkeane
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.
This looks ambiguous to me FWIW. They USE conjunction, but my reading doesn't make it mean the same as elsewhere (for the purposes of subsumption). I THINK it is just supposed to mean "all of them are checked for this deduction guide", but I'd like to hear from CWG/see what a CWG reflector conversation says.
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.
(Also tagging @zygoloid who added that note https://cplusplus.github.io/CWG/issues/2714.html)
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 filed cplusplus/draft#7300 and hopefully we can get an answer there.
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 filed cplusplus/draft#7300 and hopefully we can get an answer there.
As has been explained there, the use of "conjunction" indicates a conjunction form of the constraints and therefore it should participate in overload resolution.
if (!FunctionTrailingRC) | ||
FunctionTrailingRC = OuterRC; | ||
else | ||
FunctionTrailingRC = BinaryOperator::Create( |
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.
This has a certain elegance to it, but I'm not super thrilled with having a BinaryOperator that doesn't have a valid source location. I wonder if we should instead store a collection and check them that way. It makes checking them a little more difficult down the road, but keeps source fidelity.
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 think this would make everything way too complicated when checking subsumption etc (and afaik we do that in a few other places)
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.
Hmm, yeah, source fidelity.
But what’s also noteworthy is that we have already checked the class template’s constraints prior to checking the CTAD, which is required by the standard.
The binary operator actually changes the constraint-ness in the overload resolution against the explicit deduction guides. Though this might not be the intention of the DR, see my comments on the test.
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.
Changes to C++ DR tests LGTM.
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 nit/question
@@ -445,10 +449,46 @@ struct ConvertConstructorToDeductionGuideTransform { | |||
return nullptr; | |||
TypeSourceInfo *NewTInfo = TLB.getTypeSourceInfo(SemaRef.Context, NewType); | |||
|
|||
// At this point, the function parameters are already 'instantiated' in the |
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.
Why is 'instantiated' in quotes?
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.
Oops... I think I was going to say "the transformed function parameters are added to the mapping so the constraint substitution could see them". They're not actually instantiated though.
@@ -390,7 +393,7 @@ struct ConvertConstructorToDeductionGuideTransform { | |||
/*EvaluateConstraint=*/false); | |||
} | |||
|
|||
assert(NewParam->getTemplateDepth() == 0 && | |||
assert(getDepthAndIndex(NewParam).first == 0 && |
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.
A heads-up: we're encountering clang crashes on this assertion. I'm working on a minimal reproducer.
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.
A heads-up: we're encountering clang crashes on this assertion. I'm working on a minimal reproducer.
Filed #128691.
Closes #98592